Skip to content

Avoid forming misaligned typed pointers in (de)serialization - #888

Merged
lemire merged 1 commit into
masterfrom
fix_misaligned_typed_pointers_deserialize
Sep 22, 2026
Merged

lemire merged 1 commit into
masterfrom
fix_misaligned_typed_pointers_deserialize

Conversation

@lemire

@lemire lemire commented Sep 22, 2026

Copy link
Copy Markdown
Member

Summary

Two places formed misaligned typed pointers that were only ever passed to memcpy:

  • roaring64_bitmap_frozen_serialize accepts an unaligned output buffer (only roaring64_bitmap_frozen_view requires 64-byte alignment), but it carved the output into uint64_t * / uint16_t * / rle16_t * cursors aligned relative to the buffer start. With an unaligned base pointer (as check_frozen_serialization in roaring64_unit deliberately uses), those typed pointers are misaligned.
  • roaring_bitmap_deserialize and roaring_bitmap_deserialize_safe formed const uint32_t *elems at buf + 5 and computed elems + i before the memcpy. The comment already said the data may be unaligned; the pointer type just didn't match that intent.

Nothing faults at run time since the copies go through memcpy, but forming a misaligned pointer to a type is undefined behavior (C11 6.3.2.3p7). clang's -fsanitize=alignment checks typed memcpy arguments and reports both; GCC's UBSan (what ubuntu-sani-ub-ci uses) does not, which is why CI is green.

The fix keeps byte (char *) cursors and advances them by byte counts. container_get_frozen_size() already returns the byte count copied, so container_frozen_serialize now advances by it directly. Serialized output is byte-for-byte unchanged.

Test plan

  • clang + -DROARING_SANITIZE=ON -DROARING_SANITIZE_UNDEFINED=ON: 27/27 tests pass, zero runtime error lines in the ctest log (previously roaring64_unit reported a misaligned uint16_t * store in container_frozen_serialize, and toplevel_unit aborted in test_serialize at roaring.c:1687)
  • Release build: 27/27 tests pass
  • clang-format 17 clean

roaring64_bitmap_frozen_serialize accepts an unaligned output buffer (only
roaring64_bitmap_frozen_view requires alignment), but it carved the output
into uint64_t*/uint16_t*/rle16_t* cursors aligned relative to the buffer
start, so with an unaligned base pointer those typed pointers were
misaligned. Likewise roaring_bitmap_deserialize and
roaring_bitmap_deserialize_safe formed a const uint32_t* at buf + 5 and did
pointer arithmetic on it before reading through memcpy.

The data was always copied with memcpy, so nothing faults at run time, but
forming a misaligned pointer to a type is undefined behavior (C11 6.3.2.3p7)
and clang's -fsanitize=alignment reports it (GCC's UBSan, used in CI, does
not check memcpy arguments). Use byte cursors instead and advance them by
byte counts; the serialized bytes are unchanged.

With clang and -DROARING_SANITIZE_UNDEFINED=ON, roaring64_unit reported
"store to misaligned address ... for type 'uint16_t *'" in
container_frozen_serialize, and toplevel_unit aborted in test_serialize.
Both are clean now.
@Dr-Emann

Copy link
Copy Markdown
Member

It always seems crazy that just creating an unaligned pointer is UB in C, something that always catches me when going from rust to C/C++.

@lemire

lemire commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

@Dr-Emann I don't think it actually matters. I am being pedantic here. I would not fix it if not for the sanitizer warnings.

@lemire
lemire merged commit efce7fe into master Sep 22, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants