u70b3 opened a new pull request, #66857:
URL: https://github.com/apache/doris/pull/66857
### What problem does this PR solve?
Problem Summary:
Systematic analysis of the BE codebase for aarch64 (ARM64) hazards —
x86-only code paths, misaligned-access UB, and weak-memory-model races — done
on a 128-core ARM server (aarch64, Ubuntu 22.04). Several real bugs were found
and reproduced, plus a class of latent UB that is fragile on ARM. This PR fixes
the real bugs, hardens the latent ones with zero-cost changes, and adds
regression tests. A final commit fixes what prevents master from compiling at
all on a current aarch64 toolchain.
**Real bugs (reproduced on aarch64):**
1. **`common/signal_handler.h` — the crash-handler election CAS is
non-atomic on aarch64.** The fallback chain requires
`HAVE___SYNC_VAL_COMPARE_AND_SWAP` (never defined by this CMake build) or x86
inline asm, so aarch64 lands in a naive read-check-write that "has a race
condition" (per its own comment). `FailureSignalHandler()` uses it to elect the
single thread that dumps crash state. Reproduced with a 128-thread gtest:
**2–13 threads won the election per run** (should be exactly 1), which means
concurrent crashers corrupt each other's dumps or crash again inside the
handler. Fixed with `__atomic_compare_exchange_n` (available on every supported
arch).
2. **`service/doris_main.cpp` — AArch32 asm in the startup NEON check.**
`vadd.i32 q8,q8,q8` does not assemble on AArch64 once `__ARM_NEON__` is defined
(Clang defines it on some ARM targets); reproduced: `unknown mnemonic
'vadd.i32'`. Now picks `add v8.4s,...` on `__aarch64__` and keeps the AArch32
spelling on `__arm__`.
3. **`util/bfd_parser.cpp` — hardcoded
`bfd_set_default_target("elf64-x86-64")`** fails on aarch64 (verified: the x86
backend is not bundled in aarch64 libbfd), logging a spurious error at every BE
start. Selects the target per architecture.
4. **`core/column/columns_common.cpp` — `#if defined(__SSE2__) ||
defined(__aarch64__) && defined(__POPCNT__)`.** `&&` binds tighter than `||`
and ARM toolchains never define `__POPCNT__`, so the SIMD path of
`count_bytes_in_filter` was silently compiled out on aarch64. Parenthesized.
5. **`thirdparty/build-thirdparty.sh` — `[[ "${USE_AVX2}" -eq 0 ]]`** is
true for `USE_AVX2=ON` (bash arithmetic evaluates "ON" to 0), silently
disabling croaring AVX2 for users following the script's own help text. Changed
to a string compare.
**Misaligned-access UB hardening** (aarch64 scalar loads tolerate
misalignment so nothing crashes today, but these are C++ UB, trip UBSan, and
can miscompile under aggressive optimization; `unaligned_load`/`memcpy` compile
to the same single load instruction — zero cost):
`util/bit_packing.inline.h` (parquet/RLE decode hot path),
`util/bitmap_intersect.h` (serialize/deserialize), `storage/types.h`
(`get_cpp_type_value` on packed rows), `util/hash_util.hpp`
(`crc_hash`/`crc_hash64`/`murmur_hash2_64`), `ngram_bloom_filter.cpp` (`init` —
also fixes a latent `reserve()`-then-index misuse).
**Weak-memory-model hardening (aarch64 is weakly ordered; these are fine
under x86 TSO by luck):**
- `io/cache/block_file_cache_profile`: double-checked locking on a plain
`std::shared_ptr` → `atomic_shared_ptr` (lock-free fast path preserved).
- `storage/olap_common.h` `VersionWithTime`: `update_ts` made atomic;
version CAS uses release/acquire so readers never see "new version + stale/torn
timestamp".
- `bucketed_aggregation_source_operator`: relaxed load of the merge-target
index (whose value is then dereferenced cross-thread) → acquire, matching the
sibling use in the same file.
- `util/histogram.cpp`: CAS retry loops get `_mm_pause` (maps to `isb` via
sse2neon on aarch64) to avoid LL/SC contention storms on many-core ARM.
**Build fixes** (separate commit — master does not compile on aarch64 Ubuntu
22.04 with a current distro toolchain; needed to build/test any of the above):
clang-19 compat: `-Wshadow` kept non-fatal (newer clang flags namespace
shadowing by protobuf-generated enums), `std::powf`→`std::pow`,
`std::format`→`fmt::format`, `[[maybe_unused]]` for a function only used on the
x86_64 unwind path, UT `-Wno-deprecated-declarations` (libstdc++ ≥ 12), and a
`__res_nsearch`→`res_nsearch` forwarder in glibc-compatibility (glibc ≥ 2.34
exports it only as a non-default compat version, which breaks linking the
prebuilt krb5 archive).
### How verified (TDD on a 128-core aarch64 server)
- The CAS race test **fails before** the fix (2–13 winners/128 threads) and
**passes after**; same for the asm compile check and the bfd/bash behavior
checks.
- New unaligned-buffer tests pass with the hardening and serve as regression
guards (also runnable under `-fsanitize=alignment`).
- `SignalHandlerTest`, `NGramBloomFilterTest`, `BitPackingUnalignedTest`,
`BitmapIntersectTest`, `ColumnsCommonTest`, `HashUtilUnalignedTest`: 10/10 pass.
- 251 existing unit tests across the touched areas (bit packing, bitmap,
bloom filter, crc, hash util, histogram, columns): all pass.
- Full FE+BE build with these changes succeeds (`-Werror`), single-node
cluster smoke test (create/insert/filter/aggregate/md5) verified.
### Release note
Fix several aarch64 (ARM64) correctness and build issues in BE, including a
non-atomic CAS in the crash signal handler, and harden hot paths against
misaligned-access UB and weak-memory-ordering hazards.
### Check List (For Author)
- Test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- Built FE+BE on aarch64 Ubuntu 22.04 (clang 19.1.7,
`-march=armv8-a+crc`), started a single-node cluster, ran create/insert/select
smoke SQL successfully. New unit tests: `run-be-ut.sh --run
--filter='SignalHandlerTest.*:NGramBloomFilterTest.*:BitPackingUnalignedTest.*:BitmapIntersectTest.*:ColumnsCommonTest.*:HashUtilUnalignedTest.*'`.
- Behavior changed:
- [x] No. (Bug fixes only; no on-disk format, protocol, or default
behavior change. Hash results and serialized bytes are unchanged — the
hardening only changes *how* the same bytes are loaded.)
- Does this need documentation?
- [x] No.
--
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]
