Skip to content

Replace strict-aliasing UB in BSON double serialization#44975

Open
metsw24-max wants to merge 2 commits into
envoyproxy:mainfrom
metsw24-max:fix-bson-double-aliasing-ub
Open

Replace strict-aliasing UB in BSON double serialization#44975
metsw24-max wants to merge 2 commits into
envoyproxy:mainfrom
metsw24-max:fix-bson-double-aliasing-ub

Conversation

@metsw24-max
Copy link
Copy Markdown

Replace reinterpret_cast-based type punning in BufferHelper::writeDouble with a standards-compliant std::memcpy bit copy.

The previous implementation reinterpreted a double as an int64_t through a pointer cast, which violates C++ strict-aliasing rules and may result in undefined behavior under compiler optimization.

This change preserves the exact serialized bit pattern while removing UB from the BSON double serialization path. A static_assert was also added to verify size compatibility between double and int64_t.

Risk Level:

Low

Testing:

  • Verified bit-pattern equivalence for representative edge cases:
    zero, signed zero, ±1, π, ±∞, NaN, denormals, and ±DBL_MAX
  • Verified round-trip correctness through writeDouble/removeDouble

Docs Changes:

None

@metsw24-max metsw24-max requested a review from mattklein123 as a code owner May 9, 2026 12:04
@metsw24-max metsw24-max had a problem deploying to external-contributors May 9, 2026 12:04 — with GitHub Actions Error
@repokitteh-read-only
Copy link
Copy Markdown

Hi @metsw24-max, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #44975 was opened by metsw24-max.

see: more, trace.

Signed-off-by: metsw24-max <metsw24@gmail.com>
@metsw24-max metsw24-max force-pushed the fix-bson-double-aliasing-ub branch from 71faaa8 to a9fc07e Compare May 9, 2026 12:05
@metsw24-max metsw24-max temporarily deployed to external-contributors May 9, 2026 12:05 — with GitHub Actions Inactive
@metsw24-max metsw24-max temporarily deployed to external-contributors May 12, 2026 18:11 — with GitHub Actions Inactive
Copy link
Copy Markdown
Member

@Mythra Mythra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual code itself looks fine to me, though the CI system retriggers are interesting, have you been able to get tests passing locally @metsw24-max (i'll be running them locally myself here)? I'm just a bit surprised this PR has had force pushes/ci retriggers so often.

@kyessenov
Copy link
Copy Markdown
Contributor

See the warning in CI:

ERROR: ./source/extensions/filters/network/mongo_proxy/bson_impl.cc:138: Don't call memcpy() directly; use safeMemcpy, safeMemcpyUnsafeSrc, safeMemcpyUnsafeDst or MemBlockBuilder instead.

@tyxia
Copy link
Copy Markdown
Member

tyxia commented May 22, 2026

@metsw24-max Please fix the presubmit failure

/wait

Signed-off-by: metsw24-max <metsw24@gmail.com>
@metsw24-max metsw24-max force-pushed the fix-bson-double-aliasing-ub branch from b678556 to 2f0c6ba Compare May 22, 2026 04:26
@metsw24-max metsw24-max requested a deployment to external-contributors May 22, 2026 04:26 — with GitHub Actions Waiting
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.

4 participants