Merge pull request #13656 from protocolbuffers/backport-tdp-msvc-32

Backport MSVC 32-bit fix
protocolbuffers · Aug 23, 2023 · 7b825e1 · 7b825e1
2 parents 57d1229 + 5626baf
commit 7b825e1
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 43 deletions.
diff --git 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

diff --git 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
@@ -579,7 +579,9 @@ class PROTOBUF_EXPORT TcParser final {
   template <typename T>
   static inline T& RefAt(void* x, size_t offset) {
     T* target = reinterpret_cast<T*>(static_cast<char*>(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<uintptr_t>(target) % alignof(T) != 0)) {
       AlignFail(std::integral_constant<size_t, alignof(T)>(),
@@ -593,18 +595,7 @@ class PROTOBUF_EXPORT TcParser final {
 
   template <typename T>
   static inline const T& RefAt(const void* x, size_t offset) {
-    const T* target =
-        reinterpret_cast<const T*>(static_cast<const char*>(x) + offset);
-#ifndef NDEBUG
-    if (PROTOBUF_PREDICT_FALSE(
-            reinterpret_cast<uintptr_t>(target) % alignof(T) != 0)) {
-      AlignFail(std::integral_constant<size_t, alignof(T)>(),
-                reinterpret_cast<uintptr_t>(target));
-      // Explicit abort to let compilers know this code-path does not return
-      abort();
-    }
-#endif
-    return *target;
+    return RefAt<T>(const_cast<void*>(x), offset);
   }
 
   template <typename T>

diff --git 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<int>::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";