Merge pull request #537 from haberman/ptr-tagging-32

Fixed pointer tagging on 32-bit builds.

.github/workflows/bazel_tests.yml

@@ -24,6 +24,7 @@ jobs:
           - { CC: clang, os: ubuntu-20.04, flags: "--//:fasttable_enabled=true -- -cmake:test_generated_files" }
           - { CC: clang, os: ubuntu-20.04, flags: "--config=asan -c dbg -- -benchmarks:benchmark -python/..." }
           - { CC: clang, os: ubuntu-20.04, flags: "--config=ubsan -c dbg -- -benchmarks:benchmark -python/... -upb/bindings/lua/...", install: "libunwind-dev" }
+          - { CC: clang, os: ubuntu-20.04, flags: "--copt=-m32 --linkopt=-m32 -- -... benchmarks:benchmark ", install: "g++-multilib" }
           - { CC: clang, os: macos-11, flags: "" }
 
     steps:
@@ -31,7 +32,7 @@ jobs:
       - name: Setup Python venv
         run: rm -rf /tmp/venv && python3 -m venv /tmp/venv
       - name: Install dependencies
-        run: sudo apt install -y ${{ matrix.install }}
+        run: sudo apt update && sudo apt install -y ${{ matrix.install }}
         if: matrix.install != ''
       - name: Run tests
         run: cd ${{ github.workspace }} && PATH=/tmp/venv/bin:$PATH CC=${{ matrix.CC }} bazel test --test_output=errors ... ${{ matrix.flags }}

benchmarks/BUILD

@@ -73,7 +73,7 @@ cc_proto_library(
     deps = [":benchmark_descriptor_sv_proto"],
 )
 
-cc_binary(
+cc_test(
     name = "benchmark",
     testonly = 1,
     srcs = ["benchmark.cc"],

upb/def.c

@@ -86,6 +86,9 @@ struct upb_FieldDef {
   bool has_json_name_;
   upb_FieldType type_;
   upb_Label label_;
+#if UINTPTR_MAX == 0xffffffff
+  uint32_t padding;  // Increase size to a multiple of 8.
+#endif
 };
 
 struct upb_ExtensionRange {
@@ -123,6 +126,9 @@ struct upb_MessageDef {
   int nested_ext_count;
   bool in_message_set;
   upb_WellKnown well_known_type;
+#if UINTPTR_MAX == 0xffffffff
+  uint32_t padding;  // Increase size to a multiple of 8.
+#endif
 };
 
 struct upb_EnumDef {
@@ -136,6 +142,9 @@ struct upb_EnumDef {
   const upb_EnumValueDef* values;
   int value_count;
   int32_t defaultval;
+#if UINTPTR_MAX == 0xffffffff
+  uint32_t padding;  // Increase size to a multiple of 8.
+#endif
 };
 
 struct upb_EnumValueDef {
@@ -154,6 +163,9 @@ struct upb_OneofDef {
   const upb_FieldDef** fields;
   upb_strtable ntof;
   upb_inttable itof;
+#if UINTPTR_MAX == 0xffffffff
+  uint32_t padding;  // Increase size to a multiple of 8.
+#endif
 };
 
 struct upb_FileDef {
@@ -247,6 +259,20 @@ static const void* unpack_def(upb_value v, upb_deftype_t type) {
 }
 
 static upb_value pack_def(const void* ptr, upb_deftype_t type) {
+  // Our 3-bit pointer tagging requires all pointers to be multiples of 8.
+  // The arena will always yield 8-byte-aligned addresses, however we put
+  // the defs into arrays.  For each element in the array to be 8-byte-aligned,
+  // the sizes of each def type must also be a multiple of 8.
+  //
+  // If any of these asserts fail, we need to add or remove padding on 32-bit
+  // machines (64-bit machines will have 8-byte alignment already due to
+  // pointers, which all of these structs have).
+  UPB_ASSERT((sizeof(upb_FieldDef) & UPB_DEFTYPE_MASK) == 0);
+  UPB_ASSERT((sizeof(upb_MessageDef) & UPB_DEFTYPE_MASK) == 0);
+  UPB_ASSERT((sizeof(upb_EnumDef) & UPB_DEFTYPE_MASK) == 0);
+  UPB_ASSERT((sizeof(upb_EnumValueDef) & UPB_DEFTYPE_MASK) == 0);
+  UPB_ASSERT((sizeof(upb_ServiceDef) & UPB_DEFTYPE_MASK) == 0);
+  UPB_ASSERT((sizeof(upb_OneofDef) & UPB_DEFTYPE_MASK) == 0);
   uintptr_t num = (uintptr_t)ptr;
   UPB_ASSERT((num & UPB_DEFTYPE_MASK) == 0);
   num |= type;