align caller-supplied buffer in value_stack::stack - #1193
Ramya-9353 wants to merge 1 commit into
Conversation
|
An automated preview of the documentation is available at https://1193.json.prtest2.cppalliance.org/libs/json/doc/html/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-09-27 14:58:43 UTC |
|
GCOVR code coverage report https://1193.json.prtest2.cppalliance.org/gcovr/index.html Build time: 2026-09-27 15:16:52 UTC |
|
|
86dfe9a to
37912e9
Compare
|
Rebased onto develop to pick up the GCC 16 CI fix (82398f9); the failing Drone stages were the null_resource.cpp -Wfree-nonheap-object error, not this change. No code changes. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1193 +/- ##
========================================
Coverage 93.72% 93.72%
========================================
Files 85 85
Lines 8981 8982 +1
========================================
+ Hits 8417 8418 +1
Misses 564 564
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
|
| // must cope with a buffer that is not aligned for one. Walk every | ||
| // misalignment; before the fix the odd offsets construct a `value` | ||
| // at a misaligned address (UBSan: misaligned constructor call). | ||
| alignas(value) unsigned char buf[4096 + alignof(value)]; |
There was a problem hiding this comment.
Such a large buffer is not needed. 1024 is more than enough.
There was a problem hiding this comment.
Done, buffer is 1024 now.
| // misalignment; before the fix the odd offsets construct a `value` | ||
| // at a misaligned address (UBSan: misaligned constructor call). | ||
| alignas(value) unsigned char buf[4096 + alignof(value)]; | ||
| for(std::size_t off = 0; off < alignof(value); ++off) |
There was a problem hiding this comment.
No need for a loop. The buffer is aligned for value. And offset of 1 will lead to an unalligned allocation.
There was a problem hiding this comment.
Right, dropped the loop; it's a single value-aligned buffer offset by 1, which is enough to make the allocation misaligned.
| storage_ptr(), buf + off, sizeof(buf) - off); | ||
| st.reset(); | ||
| st.push_int64(1); | ||
| st.push_int64(2); |
There was a problem hiding this comment.
No need to push more values and then turn them into an array. Pushing just a single number will already lead to a value construction.
There was a problem hiding this comment.
Done, it just pushes one int64 now, which is enough to trip the misaligned construction.
| sizeof(value)) | ||
| // the buffer stores `value`s, so it has to be aligned for one; | ||
| // align it up the same way static_resource does for its buffer | ||
| if(std::align( |
There was a problem hiding this comment.
Please move the call to std::align to a separate line, it will make the condition more readable.
There was a problem hiding this comment.
Done, pulled the std::align call out into its own line before the condition.
37912e9 to
7a32d8f
Compare
|
|
Repro: hand any of the buffer-taking parser / stream_parser / value_stack constructors a buffer that is not aligned for
value(buf + 1, a heap block, a sub-buffer) and parse a small array. UBSan reportsconstructor call on misaligned address ... for type boost::json::value, which requires 8 byte alignmentatdetail/value.hpp:229, viavalue_stack::stack::push.Cause:
value_stack::stackstoresvalueobjects in the caller-owned buffer but reinterprets it asvalue*without aligning the pointer.static_resource::do_allocateandmonotonic_resourceboth align a caller buffer throughstd::align; this constructor was the one that skipped it, and the buffer overloads document the parameter only as "a pointer to valid storage".Fix: align the buffer up to
alignof(value)withstd::align, keepingmin_size_ * sizeof(value)usable bytes, and fall back to the memory resource when the aligned region is too small.Regression test walks every misalignment of a value-sized buffer and parses
[1,2,3]; the odd offsets trip UBSan before the change and pass after.