Doris-Breakwater commented on issue #67445: URL: https://github.com/apache/doris/issues/67445#issuecomment-5509461482
## Initial assessment **Verdict: confirmed build-portability bug (high confidence).** The issue is sufficiently detailed for initial triage; no additional reporter information is required to establish the failure mechanism. ### Verified facts - At `ddbaaab1388`, `be/src/storage/index/snii/encoding/crc32c.cpp` defines `SNII_CRC32C_X86` only when `__x86_64__` or `_M_X64` is present, but evaluates it numerically at lines 105, 239, 253, and 268. - `be/CMakeLists.txt` enables both `-Wundef` and `-Werror`. `run-be-ut.sh` configures with `-DMAKE_TEST=ON`, and that configuration defines `BE_TEST`, so the guarded implementation is compiled for BE unit-test builds. - On aarch64, the x86 condition is false and `SNII_CRC32C_X86` remains undefined. Clang therefore emits the reported `[-Wundef]` diagnostic at each numeric `#if`, and `-Werror` makes the build fail before the existing portable slice-by-8 fallback can be used. - A minimal Clang preprocessor check reproduces the diagnostic exactly. I did not run a full arm64 Doris build during this analysis, but the failure follows deterministically from the verified source and compiler flags. - Production builds without `BE_TEST` are unaffected. The configuration surface is slightly broader than unit tests: non-x86 benchmark or file-cache microbenchmark builds that define `BE_TEST` can encounter the same compile error. - The issue is currently open, unassigned, and has no labels or linked PR. ### Recommended fix Prefer defining the feature macro on both branches, since the file intentionally treats it as a numeric feature flag and this matches existing Doris patterns such as `DORIS_HAS_X87_FMOD_FAST`: ```cpp #if defined(__x86_64__) || defined(_M_X64) #define SNII_CRC32C_X86 1 #include <cpuid.h> #include <nmmintrin.h> #else #define SNII_CRC32C_X86 0 #endif ``` Changing all four sites to `#if defined(SNII_CRC32C_X86)` would also resolve this specific failure, but the explicit `0/1` definition better preserves the current value-style contract. ### Validation and next steps 1. Apply the single preprocessor fix without changing CRC behavior. 2. On arm64, build with `BUILD_TYPE_UT=Debug ./run-be-ut.sh`; ideally also run `./run-be-ut.sh --run --filter='SniiCrc32cTest.*'` to verify that the software fallback remains byte-identical to the bundled CRC32C implementation. 3. Keep or add x86 coverage for `SniiCrc32cTest.*` so the SSE4.2 path remains covered. 4. Consider a narrow arm64 `BE_TEST` compile check in CI, because the current macOS arm64 workflow explicitly builds with `MAKE_TEST=OFF` and cannot detect test-only portability failures. 5. Add the repository's applicable bug and BE/build/arm64 labels; the issue is currently unlabeled. Breakwater-GitHub-Analysis-Slot: slot_eeef596d7d73 -- 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]
