From a478e190268bd7e842def5cdf97ea008c092030d Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 22 Aug 2023 12:32:51 -0700 Subject: [PATCH 1/3] Add windows 32-bit tests and re-enable 2022 PiperOrigin-RevId: 559192056 --- .github/workflows/test_cpp.yml | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index ed90e97833fd..ecc15ac93b48 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -348,16 +348,19 @@ jobs: -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dprotobuf_BUILD_EXAMPLES=ON vsversion: '2019' - # TODO(b/285566773) Re-enable this test. - # This is broken due to a github runner update. - # See https://github.com/actions/runner-images/issues/7662 for more details - #- name: Windows CMake 2022 - # os: windows-2022 - # flags: >- - # -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF - # -Dprotobuf_BUILD_SHARED_LIBS=OFF - # -Dprotobuf_BUILD_EXAMPLES=ON - # vsversion: '2022' + - name: Windows CMake 2022 + os: windows-2022 + flags: >- + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -Dprotobuf_BUILD_SHARED_LIBS=OFF + -Dprotobuf_BUILD_EXAMPLES=ON + vsversion: '2022' + - name: Windows CMake 32-bit + os: windows-2019 + flags: >- + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + vsversion: '2019' + windows-arch: 'win32' - name: Windows CMake Shared os: windows-2019 flags: >- @@ -386,6 +389,7 @@ jobs: with: cache-prefix: ${{ matrix.name }} vsversion: ${{ matrix.vsversion }} + windows-arch: ${{ matrix.windows-arch || 'win64' }} # Install phase. - name: Configure CMake for install From 01e1a5c28ae00cca0821a95e3c68447df4d1591c Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 23 Aug 2023 11:43:35 -0700 Subject: [PATCH 2/3] Fixes for 32-bit MSVC. Disable the alignment check in 32-bit msvc. This toolchain has a difference between "natural" and "required" alignment of types and it can have the alignment of members of type `T` to be smaller than `alignof(T)`. Also, disable AnyTest.TestPackFromSerializationExceedsSizeLimit there because it can't allocate that much memory. PiperOrigin-RevId: 559495544 --- src/google/protobuf/any_test.cc | 3 +++ .../protobuf/generated_message_tctable_impl.h | 17 ++++------------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/google/protobuf/any_test.cc b/src/google/protobuf/any_test.cc index 8b544d9edadc..b367b89a361b 100644 --- a/src/google/protobuf/any_test.cc +++ b/src/google/protobuf/any_test.cc @@ -63,6 +63,9 @@ TEST(AnyTest, TestPackAndUnpack) { } TEST(AnyTest, TestPackFromSerializationExceedsSizeLimit) { +#if defined(_MSC_VER) && defined(_M_IX86) + GTEST_SKIP() << "This toolchain can't allocate that much memory."; +#endif protobuf_unittest::TestAny submessage; submessage.mutable_text()->resize(INT_MAX, 'a'); protobuf_unittest::TestAny message; diff --git a/src/google/protobuf/generated_message_tctable_impl.h b/src/google/protobuf/generated_message_tctable_impl.h index 84c8e309eb90..019ff5078a4d 100644 --- a/src/google/protobuf/generated_message_tctable_impl.h +++ b/src/google/protobuf/generated_message_tctable_impl.h @@ -579,7 +579,9 @@ class PROTOBUF_EXPORT TcParser final { template static inline T& RefAt(void* x, size_t offset) { T* target = reinterpret_cast(static_cast(x) + offset); -#ifndef NDEBUG +#if !defined(NDEBUG) && !(defined(_MSC_VER) && defined(_M_IX86)) + // Check the alignment in debug mode, except in 32-bit msvc because it does + // not respect the alignment as expressed by `alignof(T)` if (PROTOBUF_PREDICT_FALSE( reinterpret_cast(target) % alignof(T) != 0)) { AlignFail(std::integral_constant(), @@ -593,18 +595,7 @@ class PROTOBUF_EXPORT TcParser final { template static inline const T& RefAt(const void* x, size_t offset) { - const T* target = - reinterpret_cast(static_cast(x) + offset); -#ifndef NDEBUG - if (PROTOBUF_PREDICT_FALSE( - reinterpret_cast(target) % alignof(T) != 0)) { - AlignFail(std::integral_constant(), - reinterpret_cast(target)); - // Explicit abort to let compilers know this code-path does not return - abort(); - } -#endif - return *target; + return RefAt(const_cast(x), offset); } template From 5626baf004c3bb39fd4b362434c3f4cc8cc170f0 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Mon, 21 Aug 2023 11:11:37 -0700 Subject: [PATCH 3/3] Remove a flaky test with very high memory requirements. PiperOrigin-RevId: 558844591 --- .../protobuf/io/zero_copy_stream_unittest.cc | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/google/protobuf/io/zero_copy_stream_unittest.cc b/src/google/protobuf/io/zero_copy_stream_unittest.cc index 4472d584606b..eb36621f3910 100644 --- a/src/google/protobuf/io/zero_copy_stream_unittest.cc +++ b/src/google/protobuf/io/zero_copy_stream_unittest.cc @@ -729,26 +729,6 @@ TEST_F(IoTest, StringIo) { } } -// Verifies that outputs up to kint32max can be created. -TEST_F(IoTest, LargeOutput) { - // Filter out this test on 32-bit architectures and builds where our test - // infrastructure can't handle it. - if(sizeof(void*) < 8) return; -#if !defined(THREAD_SANITIZER) && !defined(MEMORY_SANITIZER) && \ - !defined(_MSC_VER) - std::string str; - StringOutputStream output(&str); - void* unused_data; - int size; - // Repeatedly calling Next should eventually grow the buffer to kint32max. - do { - EXPECT_TRUE(output.Next(&unused_data, &size)); - } while (str.size() < std::numeric_limits::max()); - // Further increases should be possible. - output.Next(&unused_data, &size); - EXPECT_GT(size, 0); -#endif // !THREAD_SANITIZER && !MEMORY_SANITIZER -} TEST(DefaultReadCordTest, ReadSmallCord) { std::string source = "abcdefghijk";