Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a9f0f8d
Hack around GCC issue with using winrt::impl::bind_in.
alvinhochun Dec 5, 2022
87d2dd0
Fix WINRT_IMPL_LINK weak symbol being discarded on GCC
alvinhochun Dec 5, 2022
244df2e
Work around GCC wanting move constructor with awaitable
alvinhochun Dec 5, 2022
e87b249
Work around GCC wanting copy constructor in thread_pool
alvinhochun Dec 5, 2022
1406d1f
cmake: Stop adding Clang-specific flag when using GCC
alvinhochun Dec 5, 2022
f323371
Fix custom_error.cpp tests on GCC
alvinhochun Dec 5, 2022
089ae3e
Disable code in weak.cpp test that doesn't build with GCC
alvinhochun Dec 8, 2022
47222ab
Fix `async, resume_on_signal` test
alvinhochun Dec 9, 2022
d6b56d6
GCC: Fix clock::from_time_t with libstdc++
alvinhochun Dec 9, 2022
36bb18a
cmake: Remove the update step for winmd
alvinhochun Dec 9, 2022
f3f838d
GCC: Work around out_params_bad.cpp miscompilation
alvinhochun Dec 9, 2022
f90f754
GCC: Work around multi_threaded_vector.cpp miscompilation
alvinhochun Dec 10, 2022
1f4edae
GCC: Disable test_old/agile_ref that randomly crashes
alvinhochun Dec 11, 2022
2b37b0a
Remove use-after-free in hstring test
alvinhochun Dec 11, 2022
5a5fc09
CI: Add GCC tests using MSYS2
alvinhochun Dec 8, 2022
8cdacaa
CI/cmake: Enable colour test output
alvinhochun Dec 8, 2022
3ea3663
CI: Enable CMake colour diagnostics
alvinhochun Dec 8, 2022
2ad00ef
CI: Update msys2 packages
alvinhochun Dec 9, 2022
bb78632
CI/msys2: Add mingw64 build
alvinhochun Dec 9, 2022
9c9627b
CI: Skip building large PCH for i686
alvinhochun Dec 10, 2022
e7d0d09
CI: Remove msys2 i686 Debug build
alvinhochun Dec 11, 2022
0f0a3ef
GCC: Fix clock::to_time_t with libstdc++
alvinhochun Dec 11, 2022
0129567
Include directxmath.h only if it exists
alvinhochun Dec 11, 2022
24cd40f
CI: Add cross-built tests with GCC
alvinhochun Dec 11, 2022
ed9d296
CI: Disable GCC cross build (mingw-w64 headers are still too old)
alvinhochun Dec 11, 2022
e2cd4df
test: Tag async_propagate_cancel test as mayfail
alvinhochun Dec 8, 2022
9645cf0
Revert "Include directxmath.h only if it exists"
alvinhochun Dec 12, 2022
cc37b3f
Revert "Work around GCC wanting copy constructor in thread_pool"
alvinhochun Dec 12, 2022
9d4583f
test: Disable thread_pool.cpp tests for GCC
alvinhochun Dec 12, 2022
75d5c6e
test: Remove redundant checks in test/custom_error.cpp
alvinhochun Dec 12, 2022
fb9d105
Fix typo
alvinhochun Dec 12, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 106 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,9 @@ jobs:
arch: [i686, x86_64]
config: [Debug, Release]
runs-on: windows-latest
env:
CMAKE_COLOR_DIAGNOSTICS: 1
CLICOLOR_FORCE: 1
steps:
- uses: actions/checkout@v3

Expand All @@ -277,6 +280,8 @@ jobs:
}
cmake ../ -G"MinGW Makefiles" -DCMAKE_BUILD_TYPE=${{ matrix.config }} `
-DDOWNLOAD_WINDOWSNUMERICS=TRUE `
-DUSE_ANSI_COLOR=TRUE `
-DCMAKE_CXX_FLAGS="-fansi-escape-codes" `
-DENABLE_TEST_SANITIZERS=$sanitizers
cmake --build . -j2 --target cppwinrt

Expand All @@ -303,12 +308,69 @@ jobs:
$env:UBSAN_OPTIONS = "print_stacktrace=1"
ctest --verbose

test-msys2-gcc-cppwinrt:
name: 'gcc/msys2: Build and test (${{ matrix.sys }}, ${{ matrix.config }})'
strategy:
fail-fast: false
matrix:
include:
- { sys: mingw32, arch: i686, config: Release }
- { sys: mingw64, arch: x86_64, config: Debug }
- { sys: mingw64, arch: x86_64, config: Release }
- { sys: ucrt64, arch: x86_64, config: Release }
runs-on: windows-latest
env:
CMAKE_COLOR_DIAGNOSTICS: 1
CLICOLOR_FORCE: 1
defaults:
run:
shell: msys2 {0}
steps:
- uses: msys2/setup-msys2@v2
with:
msystem: ${{matrix.sys}}
update: true
pacboy: >-
crt:p
gcc:p
binutils:p
cmake:p
ninja:p

- uses: actions/checkout@v3

- name: Build cppwinrt
run: |
mkdir build
cd build
if [[ "${{ matrix.arch }}" = "i686" ]]; then
skip_large_pch_arg="-DSKIP_LARGE_PCH=TRUE"
fi
cmake ../ -GNinja -DCMAKE_BUILD_TYPE=${{ matrix.config }} \
-DDOWNLOAD_WINDOWSNUMERICS=TRUE \
-DUSE_ANSI_COLOR=TRUE \
$skip_large_pch_arg
cmake --build . --target cppwinrt

- name: Build tests
run: |
cd build
cmake --build . -j2 --target test-vanilla test_cpp20 test_win7 test_old

- name: Run tests
run: |
cd build
ctest --verbose

build-linux-cross-cppwinrt:
name: 'cross: Cross-build from Linux'
strategy:
matrix:
arch: [i686, x86_64]
runs-on: ubuntu-22.04
env:
CMAKE_COLOR_DIAGNOSTICS: 1
CLICOLOR_FORCE: 1
steps:
- uses: actions/checkout@v3

Expand Down Expand Up @@ -367,7 +429,8 @@ jobs:
cd build
cmake ../test -G"MinGW Makefiles" -DCMAKE_BUILD_TYPE=Debug `
-DCPPWINRT_PROJECTION_INCLUDE_DIR="../.test/out" `
-DDOWNLOAD_WINDOWSNUMERICS=TRUE
-DDOWNLOAD_WINDOWSNUMERICS=TRUE `
-DUSE_ANSI_COLOR=TRUE
cmake --build . -j2

- name: Run tests
Expand All @@ -376,11 +439,35 @@ jobs:
ctest --verbose

build-linux-native-cppwinrt:
name: 'linux: GCC native build + llvm-mingw cross-build tests'
name: 'linux: GCC native build + mingw-w64 cross-build tests'
strategy:
fail-fast: false
matrix:
# TODO: Enable gcc build once Arch Linux gets more recent mingw-w64 headers (ver. 11 perhaps?)
# cross_toolchain: [gcc, llvm-mingw]
cross_toolchain: [llvm-mingw]
cross_arch: [i686, x86_64]
# include:
# - cross_toolchain: gcc
# container:
# image: archlinux:base-devel
runs-on: ubuntu-22.04
container: ${{ matrix.container }}
defaults:
run:
shell: bash
env:
CMAKE_COLOR_DIAGNOSTICS: 1
CLICOLOR_FORCE: 1
steps:
- uses: actions/checkout@v3

- name: Install build tools
if: matrix.cross_toolchain == 'gcc'
run: |
pacman --noconfirm -Suuy
pacman --needed --noconfirm -S cmake ninja git

- name: Build cppwinrt
run: |
cmake -S . -B build/native/ \
Expand All @@ -399,38 +486,49 @@ jobs:

- id: setup-llvm
name: Set up llvm-mingw
if: matrix.cross_toolchain == 'llvm-mingw'
uses: ./.github/actions/setup-llvm-mingw

- name: Install GCC cross compiler
if: matrix.cross_toolchain == 'gcc'
run: |
pacman --needed --noconfirm -S mingw-w64-gcc

- name: Cross-build tests using projection
run: |
cmake -S test -B build/cross-tests --toolchain "$PWD/cross-mingw-toolchain.cmake" \
-DCMAKE_SYSTEM_PROCESSOR=x86_64 \
-DCMAKE_SYSTEM_PROCESSOR=${{ matrix.cross_arch }} \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_CXX_FLAGS="-static" \
-DCPPWINRT_PROJECTION_INCLUDE_DIR=/tmp/cppwinrt \
-DDOWNLOAD_WINDOWSNUMERICS=TRUE
-DDOWNLOAD_WINDOWSNUMERICS=TRUE \
-DUSE_ANSI_COLOR=TRUE
cmake --build build/cross-tests -j2

- name: Upload built tests
uses: actions/upload-artifact@v3
with:
name: linux-native-cppwinrt-cross-build-tests-x86_64-bin
name: linux-native-cppwinrt-cross-build-tests-${{ matrix.cross_toolchain }}-${{ matrix.cross_arch }}-bin
path: build/cross-tests/*.exe

test-linux-native-cppwinrt-cross-tests:
name: 'linux: Run llvm-mingw cross-build tests'
needs: build-linux-native-cppwinrt
strategy:
fail-fast: false
matrix:
arch: [x86_64]
# TODO: Enable gcc build test when it is buildable
# cross_toolchain: [gcc, llvm-mingw]
cross_toolchain: [llvm-mingw]
cross_arch: [i686, x86_64]
runs-on: windows-latest
steps:
- uses: actions/checkout@v3

- name: Fetch test executables
uses: actions/download-artifact@v3
with:
name: linux-native-cppwinrt-cross-build-tests-${{ matrix.arch }}-bin
name: linux-native-cppwinrt-cross-build-tests-${{ matrix.cross_toolchain }}-${{ matrix.cross_arch }}-bin
path: ./

- name: Run tests
Expand All @@ -439,7 +537,7 @@ jobs:
$has_failed_tests = 0
foreach ($test_exe in $test_exes) {
echo "::group::Run '$test_exe'"
& .\$test_exe
& .\$test_exe --use-colour yes
echo "::endgroup::"
if ($LastExitCode -ne 0) {
echo "::error::Test '$test_exe' failed!"
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ ExternalProject_Add(winmd
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
INSTALL_COMMAND ""
UPDATE_COMMAND ""
)
add_dependencies(cppwinrt winmd)
ExternalProject_Get_Property(winmd SOURCE_DIR)
Expand Down
4 changes: 2 additions & 2 deletions strings/base_chrono.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ WINRT_EXPORT namespace winrt

static time_t to_time_t(time_point const& time) noexcept
{
return static_cast<time_t>(std::chrono::system_clock::to_time_t(to_sys(std::chrono::time_point_cast<std::chrono::system_clock::duration>(time))));
return static_cast<time_t>(std::chrono::system_clock::to_time_t(std::chrono::time_point_cast<std::chrono::system_clock::duration>(to_sys(time))));
}

static time_point from_time_t(time_t time) noexcept
{
return std::chrono::time_point_cast<duration>(from_sys(std::chrono::system_clock::from_time_t(time)));
return from_sys(std::chrono::time_point_cast<duration>(std::chrono::system_clock::from_time_t(time)));
}

static file_time to_file_time(time_point const& time) noexcept
Expand Down
28 changes: 28 additions & 0 deletions strings/base_coroutine_threadpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,19 @@ WINRT_EXPORT namespace winrt
{
}

#if defined(__GNUC__) && !defined(__clang__)
// HACK: GCC seems to require a move when calling operator co_await
// on the return value of resume_after.
// This might be related to upstream bug:
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99575
awaitable(awaitable &&other) noexcept :
m_timer{std::move(other.m_timer)},
m_duration{std::move(other.m_duration)},
m_handle{std::move(other.m_handle)},
m_state{other.m_state.load()}
{}
#endif

void enable_cancellation(cancellable_promise* promise)
{
promise->set_canceller([](void* context)
Expand Down Expand Up @@ -434,6 +447,21 @@ WINRT_EXPORT namespace winrt
m_handle(handle)
{}

#if defined(__GNUC__) && !defined(__clang__)
// HACK: GCC seems to require a move when calling operator co_await
// on the return value of resume_on_signal.
// This might be related to upstream bug:
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99575
awaitable(awaitable &&other) noexcept :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use = default to force the compiler to generate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the issue is that std::atomic<T> is unmovable (copy constructor is deleted).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's too bad.

m_wait{std::move(other.m_wait)},
m_timeout{std::move(other.m_timeout)},
m_handle{std::move(other.m_handle)},
m_result{std::move(other.m_result)},
m_resume{std::move(other.m_resume)},
m_state{other.m_state.load()}
{}
#endif

void enable_cancellation(cancellable_promise* promise)
{
promise->set_canceller([](void* context)
Expand Down
10 changes: 8 additions & 2 deletions strings/base_extern.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,15 @@ extern "C"
#endif
#elif defined(__GNUC__)
#if defined(__i386__)
#define WINRT_IMPL_LINK(function, count) __asm__(".weak _WINRT_IMPL_" #function "@" #count "\n.set _WINRT_IMPL_" #function "@" #count ", _" #function "@" #count);
#define WINRT_IMPL_LINK(function, count) __asm__( \
".globl _" #function "@" #count "\n\t" \
".weak _WINRT_IMPL_" #function "@" #count "\n\t" \
".set _WINRT_IMPL_" #function "@" #count ", _" #function "@" #count);
#else
#define WINRT_IMPL_LINK(function, count) __asm__(".weak WINRT_IMPL_" #function "\n.set WINRT_IMPL_" #function ", " #function);
#define WINRT_IMPL_LINK(function, count) __asm__( \
".globl " #function "\n\t" \
".weak WINRT_IMPL_" #function "\n\t" \
".set WINRT_IMPL_" #function ", " #function);
#endif
#endif

Expand Down
12 changes: 12 additions & 0 deletions strings/base_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,23 @@ namespace winrt::impl

T const& object;

#if !defined(__GNUC__) || defined(__clang__)
template <typename R>
operator R const& () const noexcept
{
return reinterpret_cast<R const&>(object);
}
#else
// HACK: GCC does not handle template deduction of const T& conversion
// function according to CWG issue 976. To make this compile on GCC,
// we have to intentionally drop the const qualifier.
// Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61663
template <typename R>
operator R& () const noexcept
{
return const_cast<R&>(reinterpret_cast<R const&>(object));
}
#endif
};

template <typename T>
Expand Down
10 changes: 10 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ if(ENABLE_TEST_SANITIZERS)
endif()


set(USE_ANSI_COLOR FALSE CACHE BOOL "Enable ANSI colour output for Catch2 test runner.")
if(USE_ANSI_COLOR)
add_compile_definitions(CATCH_CONFIG_COLOUR_ANSI)
set(TEST_COLOR_ARG "--use-colour yes")
endif()


set(SKIP_LARGE_PCH FALSE CACHE BOOL "Skip building large precompiled headers.")


add_subdirectory(test)
add_subdirectory(test_cpp20)
add_subdirectory(test_win7)
Expand Down
14 changes: 8 additions & 6 deletions test/old_tests/UnitTests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,17 @@ endforeach()
add_executable(test_old Main.cpp ${TEST_SRCS})
target_link_libraries(test_old runtimeobject synchronization)

target_precompile_headers(test_old PRIVATE pch.h)
set_source_files_properties(
conditional_implements_pure.cpp
PROPERTIES SKIP_PRECOMPILE_HEADERS true
)
if(NOT SKIP_LARGE_PCH)
target_precompile_headers(test_old PRIVATE pch.h)
set_source_files_properties(
conditional_implements_pure.cpp
PROPERTIES SKIP_PRECOMPILE_HEADERS true
)
endif()

add_dependencies(test_old build-cppwinrt-projection)

add_test(
NAME test_old
COMMAND "$<TARGET_FILE:test_old>"
COMMAND "$<TARGET_FILE:test_old>" ${TEST_COLOR_ARG}
)
3 changes: 3 additions & 0 deletions test/old_tests/UnitTests/agile_ref.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ IAsyncAction test_agile_ref()
#if defined(__clang__) && defined(_MSC_VER) && (defined(_M_IX86) || defined(__i386__))
// FIXME: Test is known to crash from calling invalid address on x86 when built with Clang.
TEST_CASE("agile_ref", "[.clang-crash]")
#elif defined(__GNUC__) && !defined(__clang__)
// FIXME: Test is known to randomly crash or abort when built with GCC (segfaults under appverifier).
TEST_CASE("agile_ref", "[.gcc-crash]")
#else
TEST_CASE("agile_ref")
#endif
Expand Down
6 changes: 4 additions & 2 deletions test/old_tests/UnitTests/async.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1566,10 +1566,12 @@ namespace
co_await resume_on_signal(signal); // should not suspend because already signaled
REQUIRE(caller == GetCurrentThreadId()); // still on calling thread

REQUIRE(false == co_await resume_on_signal(signal, 1us)); // should suspend but timeout
bool suspend_but_timeout_result = co_await resume_on_signal(signal, 1us);
REQUIRE(false == suspend_but_timeout_result); // should suspend but timeout
REQUIRE(caller != GetCurrentThreadId()); // now on background thread

REQUIRE(true == co_await resume_on_signal(signal, 1s)); // should eventually succeed
bool suspend_and_succeed_result = co_await resume_on_signal(signal, 1s);
REQUIRE(true == suspend_and_succeed_result); // should eventually succeed
}
}

Expand Down
1 change: 0 additions & 1 deletion test/old_tests/UnitTests/hstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ TEST_CASE("hstring,operator,std::wstring_view")
REQUIRE(L"abc" == ws);

hs.clear();
REQUIRE(L"abc" == ws);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what's the point of this test because this seems to be a use-after-free, and it crashes when run with Application Verifier enabled. So I removed it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

ws = hs;
REQUIRE(ws.empty());
}
Expand Down
Loading