From 11ea0d29ab757727b56f769901a94a37f0f62a33 Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Wed, 5 Aug 2026 05:35:35 +0200 Subject: [PATCH] Make the 32 bit build work again, and assert the endianness it assumes test/meson.build has carried a -m32 accommodation for years, and it had rotted into not configuring at all: ERROR: Dependency 'fmt' is required but not found. The accommodation asked for fmt with method: 'builtin', meaning "build it from source, the system one is the wrong word size". Restricting the method is also what stops meson consulting the wrap's override_dependency, so the fallback came back NO and the setup failed rather than falling back. Asking the subproject for fmt_dep directly says the same thing and keeps working. Nothing was wrong with svector itself. With the harness fixed, -m32 builds a real i386 binary and the whole suite passes, and the defining property holds at both word sizes: svector is 8 bytes holding 7 where a pointer is 8, and 4 holding 3 where it is 4. There is now a CI leg so it cannot rot again. The endianness assumption is the other half. is_direct() reads m_data[0] & 1 while set_indirect() memcpys a whole pointer across m_data[0..sizeof(void*)-1], so the flag only has a byte of its own on a little endian target; on big endian m_data[0] would be the pointer's most significant byte. Nothing said so, and no runner can ever catch it because they are all little endian, so this is a static_assert and a paragraph in the layout comment rather than an exotic CI job. Verified it fires by compiling with __BYTE_ORDER__ forced to big endian. See #87. Co-Authored-By: Claude Fable 5 --- .github/workflows/main.yml | 13 +++++++++++++ README.md | 9 +++++++-- include/ankerl/svector.h | 15 ++++++++++++++- test/meson.build | 17 +++++++++++------ 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 034c180..565ac47 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -113,6 +113,15 @@ jobs: -fstack-protector-strong -fstack-clash-protection -fcf-protection=full -Wformat -Wformat-security -Werror=format-security" -Dcpp_link_args="-Wl,-z,relro -Wl,-z,now -Wl,-z,noexecstack" + + # A tagged pointer packed into a byte array is exactly the design that a different + # word size can break, and test/meson.build has carried a -m32 accommodation for + # years that no job ever built. It had rotted into not configuring at all. + - name: linux-32bit + os: ubuntu-latest + cxx: c++ + multilib: true + setup_args: -Dcpp_args=-m32 -Dcpp_link_args=-m32 steps: - uses: actions/checkout@v7 - uses: hendrikmuhs/ccache-action@v1.2 @@ -129,6 +138,10 @@ jobs: path: ${{ env.MESON_PACKAGE_CACHE }} key: wraps-${{ hashFiles('subprojects/*.wrap') }} - run: pip install -r .github/workflows/requirements.txt + # The runner images carry a 64 bit only toolchain, so -m32 needs the 32 bit libstdc++. + - name: 32 bit toolchain + if: matrix.multilib + run: sudo apt-get update && sudo apt-get install -y g++-multilib # ccache does not support MSVC, sccache does. Without this the windows job had no # compiler cache at all and was by far the slowest. - uses: ilammy/msvc-dev-cmd@v1 diff --git a/README.md b/README.md index ffc3e82..db6609a 100644 --- a/README.md +++ b/README.md @@ -302,8 +302,13 @@ meson test `meson test` runs the unit tests — 108 cases and ~630k assertions, much of it comparing against `std::vector` operation by operation — and replays a 1651 entry fuzzing corpus. CI additionally builds on Linux, macOS and Windows, at C++20 as well as the default C++17 and at C++23 on Linux, under address+undefined sanitizers, and -with a distribution's hardening flags including `_GLIBCXX_ASSERTIONS`. It also compiles a small consumer -project against `CMakeLists.txt`, which is the only thing that exercises the CMake path. +with a distribution's hardening flags including `_GLIBCXX_ASSERTIONS`, and as a 32 bit build. It also compiles +a small consumer project against `CMakeLists.txt`, which is the only thing that exercises the CMake path. + +The one thing a build has to provide is a **little endian** target, because the direct/indirect flag shares a +byte with the low end of the indirect pointer. That is checked with a `static_assert` rather than left to be +discovered. Both 32 and 64 bit are fine: `svector` is 8 bytes holding 7 where a pointer is 8 bytes, +and 4 holding 3 where it is 4. Benchmarks are separate and want a release build. There are two of them: diff --git a/include/ankerl/svector.h b/include/ankerl/svector.h index 909d33b..c4e121b 100644 --- a/include/ankerl/svector.h +++ b/include/ankerl/svector.h @@ -593,10 +593,23 @@ class svector : private detail::allocator_holder { * Then 0-X bytes unused (padding), and then the actual inline T data. * indirect: * m_data[0] & 1: lowest bit is 0 for indirect mode - * m_data[0..7]: stores an uintptr_t, which points to the indirect data. + * m_data[0..sizeof(void*)-1]: stores an uintptr_t, which points to the indirect data. + * + * The mode flag and the pointer share m_data[0], which only holds on a little endian target. + * There the pointer's least significant byte lands in m_data[0] and alignment guarantees its + * low bit is 0, so the flag has a byte to live in. On a big endian target m_data[0] would be + * the pointer's most significant byte instead and the flag would read something unrelated. */ alignas(detail::alignment_of_svector()) std::array(MinInlineCapacity)> m_data; + // Nothing in CI can catch this: every runner is little endian. MSVC does not define + // __BYTE_ORDER__ but targets nothing big endian either, so check where a check is possible. +#if defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) + static_assert(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__, + "svector packs its direct/indirect flag into the low byte of the indirect pointer, " + "which requires a little endian target"); +#endif + /** * @brief Whether the whole inline buffer can stand in for the elements it holds. * diff --git a/test/meson.build b/test/meson.build index 8a96700..e29e5db 100644 --- a/test/meson.build +++ b/test/meson.build @@ -54,11 +54,16 @@ if compiler.has_argument('-Wno-stringop-overflow') cpp_args += '-Wno-stringop-overflow' # g++ error in fmtlib endif -fmt_method = 'auto' +# fmt is only used by the test harness. A system fmt is fine normally, but it is built for the +# host word size, so a -m32 build cannot link against it and has to build fmt from the wrap. +# +# method: 'builtin' used to say that and silently stopped meaning it: restricting the method +# keeps meson from consulting the wrap's override_dependency, so the fallback came back NO and +# a 32 bit setup failed outright rather than falling back. Ask for the subproject directly. if get_option('cpp_args').contains('-m32') - # use builtin so we can compile it for 32bit. - # Can't use it as a default or sanitizer doesn't work... - fmt_method = 'builtin' + fmt_dep = subproject('fmt').get_variable('fmt_dep') +else + fmt_dep = dependency('fmt') endif # boost and abseil are only used to compare against in the benchmarks and in @@ -81,7 +86,7 @@ test_exe = executable( # see what's in the [provide] sections for the dependency names dependency('doctest'), - dependency('fmt', method: fmt_method), + fmt_dep, ], ) @@ -120,7 +125,7 @@ if compiler.get_id() == 'clang' fuzz_deps = [ dependency('doctest'), dependency('threads'), - dependency('fmt', method: fmt_method), + fmt_dep, ] fuzz_cpp_args = [ '-DFUZZ',