From dbd5dbed468ed1c8977a5b7cb8d78bf39c26563b Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 5 Mar 2022 09:51:31 -0800 Subject: [PATCH 1/3] Fixed pointer tagging on 32-bit builds. --- .github/workflows/bazel_tests.yml | 1 + upb/def.c | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index c45348c43c..faf8c42523 100644 --- a/.github/workflows/bazel_tests.yml +++ b/.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: diff --git a/upb/def.c b/upb/def.c index 4d45551212..f5331d1599 100644 --- a/upb/def.c +++ b/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; From 283451a9f3f532141a8aa16994a4e0db5f950c82 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 5 Mar 2022 10:01:18 -0800 Subject: [PATCH 2/3] Run "apt update" before "apt install". --- .github/workflows/bazel_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index faf8c42523..3592710713 100644 --- a/.github/workflows/bazel_tests.yml +++ b/.github/workflows/bazel_tests.yml @@ -32,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 }} From c944638b9b17d19f5fb0e98ba95d98dc8a22da98 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 5 Mar 2022 10:19:56 -0800 Subject: [PATCH 3/3] Changed benchmarks:benchmark to cc_test(), so it can be run with "bazel test". --- benchmarks/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/BUILD b/benchmarks/BUILD index cce5cb11da..ae4b1995b2 100644 --- a/benchmarks/BUILD +++ b/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"],