gavinchou opened a new pull request, #67641:
URL: https://github.com/apache/doris/pull/67641
### What problem does this PR solve?
Problem Summary:
`get_txn_id_from_fdb_ts` decodes a character buffer through `int64_t*`
dereferences, including an overlapping read at offset 2. These reads violate
alignment and aliasing requirements.
- Copy the 10 input bytes into a byte array and use
`Versionstamp::version()` / `order()` to decode the big-endian fields in one
place.
- Preserve the transaction ID bit layout, sequence limit, error codes,
unchanged output on rejected input, and existing little-endian restriction.
This does not change the version range or overflow policy.
- Remove the `fmt` dependency from `versionstamp.h`, preserving its exact
20-character lowercase hexadecimal output. Add an explicit `byteswap.h` include
to the consumer that previously relied on the transitive include.
- Add differential tests against both the pre-Versionstamp memcpy
implementation and the original reinterpret-cast implementation. Each reference
covers 8 version boundaries, all 65,536 sequence values, and 8 input alignments
(4,194,304 combinations), plus malformed lengths and empty views. Also check
the complete decoded Versionstamp fields and compare hexadecimal formatting
against the previous fmt rule.
The unsafe historical load is retained only as a test oracle on
x86_64/AArch64, with its known alignment sanitizer violation locally
suppressed. Production code and the memcpy reference retain normal
instrumentation; matching the historical output does not make the old
implementation well-defined.
### Release note
None
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
Validation:
- Cloud ASAN build and compilation/linking of all Cloud unit-test targets
pass on Linux x86_64 with Clang 20.1.8.
- `doris_txn_test:TxnIdConvert.*`: 5 tests pass;
`versionstamp_test:VersionstampTest.*`: 5 tests pass. GTest XML reports zero
failures, errors, or skips.
- Nine isolated related test bodies pass under ASAN+UBSAN on local ARM64.
- A standalone program including only `versionstamp.h` compiles, links,
and runs without third-party include paths or fmt linkage.
- Changed-line clang-tidy, clang-format 16.0.6, and `git diff --check`
pass.
- External FDB/S3 integration tests were not run.
- Behavior changed:
- [x] No.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
