Make the 32 bit build work again, and assert the endianness it assumes - #97
Merged
Conversation
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<uint8_t,1> 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 <[email protected]>
martinus
force-pushed
the
portability-32bit-endian
branch
from
August 5, 2026 03:49
2b9b968 to
11ea0d2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #87.
Two related holes in the portability of a design built on packing a pointer and a tag into the same bytes.
32 bit was claimed but broken
test/meson.buildhas carried a deliberate-m32accommodation for years. It had rotted into not configuring at all: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'smeson.override_dependency('fmt', fmt_dep), so the fallback came back NO and the setup failed outright rather than falling back. Asking the subproject forfmt_depdirectly says the same thing and keeps working.Nothing was wrong with svector itself — only the harness. With the fix,
-m32produces a real i386 binary and both test suites pass:The defining property holds at both word sizes:
svector<uint8_t,1>is 8 bytes holding 7 where a pointer is 8 bytes, and 4 bytes holding 3 where it is 4. No test needed changing — I checkedautomatic_capacity.cppandalignment.cppfor hardcoded 8s and they express what they mean in terms of the type, not the host.There is now a
linux-32bitCI leg, with ag++-multilibstep since the runner images are 64 bit only, so this cannot rot again silently.The little endian assumption was unguarded
is_direct()readsm_data[0] & 1whileset_indirect()memcpys a whole pointer acrossm_data[0..sizeof(void*)-1]. The flag only has a byte of its own on a little endian target, where the pointer's least significant byte lands inm_data[0]and alignment guarantees its low bit is 0. On big endianm_data[0]would be the pointer's most significant byte and the flag would read something unrelated — andset_indirect()'s ownif (is_direct()) throw std::bad_alloc()safety check would be inspecting the wrong byte.Nothing said so anywhere in the header, README,
doc/orCONTRIBUTING.md.No CI job can ever catch this, because every GitHub runner is little endian, so the fix is a
static_assertand a paragraph in the layout comment rather than an s390x-under-qemu job.__BYTE_ORDER__where the compiler provides it; MSVC does not define it but targets nothing big endian either. I verified the assert actually fires by compiling with__BYTE_ORDER__forced to__ORDER_BIG_ENDIAN__, and that ordinary builds are unaffected.The README now states the constraint, since it is a real requirement a user evaluating the library should see rather than discover.
Note
linux-32bitis not in main's required status checks — I only added the eleven that existed when protection was configured. Happy to add it once this is merged.🤖 Generated with Claude Code