Repository navigation
DAOS-18626 build: Add ASan/UBSan/TSan compiler and Docker build support - #18613
Conversation
|
Ticket title is 'Add ASan/UBSan/TSan sanitizer checks to the GitHub Actions unit-test CI' |
|
Test stage Test RPMs on EL 9.6 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18613/2/execution/node/1314/log |
de6bafa to
7b89b29
Compare
7b89b29 to
6f22e4c
Compare
Add the build-system plumbing needed to compile DAOS with UndefinedBehaviorSanitizer and ThreadSanitizer, in addition to the already-supported AddressSanitizer: - site_scons/site_tools/compiler_setup.py: link the UBSan/TSan runtime libraries (-lubsan/-ltsan) explicitly, including into CGO_LDFLAGS so that 'go build' picks them up when CGO-linking a sanitizer-compiled C archive. - src/control/SConscript: skip control man-page generation under a SANITIZERS build (not needed, avoids extra build cost). - src/gurt/misc.c: skip d_free()'s debug poisoning memset() under TSan, which otherwise turns a harmless zero-size-allocation address reuse into a false heap-use-after-free report. - utils/scripts/install-el9.sh: install compiler-rt, libubsan, and libtsan alongside the existing libasan dependency. - utils/docker/Dockerfile.el.9: thread a new SANITIZERS build-arg through to the scons invocation (empty/unset by default, so existing callers are unaffected). This is purely additive: no existing (non-sanitizer) build is affected, and SANITIZERS=address behavior is unchanged. Part of the DAOS-18626 ASan CI split (see ckochhof/dev/master/daos-18626/patch-001 for the original, unsplit commit). Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
6f22e4c to
ce9165c
Compare
…/daos-18626/patch-002
_f_get_alloc_size() was called unconditionally in d_free(), with its result discarded via `(void)msize;` under TSan to silence -Wunused-but-set-variable. Scoping only the call site inside #if !defined(__SANITIZE_THREAD__) leaves _f_get_alloc_size() with no caller left under TSan, tripping -Werror=unused-function instead. Guard the function definition with the same #if so it compiles out entirely under TSan rather than being computed and discarded. Verified with gcc -Wall -Wextra -Werror for plain, ASan, and TSan builds. Addresses PR #18613 review feedback from daltonbohning. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-18626/patch-002
Add a short code comment on why man-page generation is skipped when building with SANITIZERS: it runs the just-built (sanitizer-instrumented) binary as a build step, which can trip sanitizer false positives. Suggested-by: daltonbohning Link: #18613 (comment) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-18626/patch-002
|
@grom72, @daltonbohning and @phender could you have a look at this PR please. |
|
Test stage Functional on EL 9 completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18613/8/testReport/ |
kjacque
left a comment
There was a problem hiding this comment.
Basically looks OK, I think it's worthwhile to update the confusing name in the build scripts though.
…/daos-18626/patch-002
Reviewer feedback (kjacque) pointed out that the SANITIZERS value 'undefined' for enabling UBSan is confusing -- easily misread as value-not-set rather than as UBSan's own name (Undefined Behavior Sanitizer). Rename the public SANITIZERS token to the clearer 'undefined_behavior', translating it back to the compiler's fixed -fsanitize=undefined identifier at both points a -fsanitize=<name> flag is constructed from a SANITIZERS token (_base_setup() and _check_func(), which must stay symmetric: _check_func() strips the exact flag _base_setup() added, so both need the same translation or sanitizer flags stop being cleanly removed for CheckFunc() probes). Also update site_scons/components/__init__.py, which independently inspects SANITIZERS to pick Mercury's own CMAKE_BUILD_TYPE -- it must recognize the new token too, or Mercury silently falls back to RelWithDebInfo instead of Ubsan. Link: #18613 (comment) Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-18626/patch-002
PR #18613's Pylint check job failed with 2 wrong-spelling-in-comment (C0401) findings for the word "fsanitize", both in the explanatory comment added above SANITIZER_FLAG_NAMES in the previous commit (rename SANITIZERS UBSan token to undefined_behavior). Pylint's spell checker tokenizes "-fsanitize=" (stripping the leading '-' and trailing '=') into the bare word "fsanitize", which isn't a real English word and wasn't yet in utils/cq/words.dict. Fix by adding it to the dictionary, in its correct alphabetical position. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-18626/patch-002
…/daos-18626/patch-002
|
@daos-stack/daos-gatekeeper could you lend this PR with the following commit message:
|
Description
This is PR 1 of 10 in the split of #18557 (see
ckochhof/dev/master/daos-18626/patch-001for the original, unsplit commit). It adds the build-system plumbing needed to compile DAOS with UndefinedBehaviorSanitizer and ThreadSanitizer, in addition to the already-supported AddressSanitizer. It does not add any CI wiring or test-runner changes — those land in later PRs in this stack.What/why
site_scons/site_tools/compiler_setup.py: link the UBSan/TSan runtime libraries (-lubsan/-ltsan) explicitly, including intoCGO_LDFLAGSso thatgo buildpicks them up when CGO-linking a sanitizer-compiled C archive (otherwise Go binaries get "DSO missing from command line" forlibubsan.so.1).src/control/SConscript: skip control man-page generation under aSANITIZERSbuild (not needed, avoids extra build cost).src/gurt/misc.c: skipd_free()'s debug poisoningmemset()under TSan, which otherwise turns a harmless zero-size-allocation address reuse into a false heap-use-after-free report (confirmed via an isolated single-threaded reproducer to be a TSan tool limitation, not a DAOS bug).utils/scripts/install-el9.sh: installcompiler-rt,libubsan, andlibtsanalongside the existinglibasandependency.utils/docker/Dockerfile.el.9: thread a newSANITIZERSbuild-arg through to thesconsinvocation (empty/unset by default, so existing callers are unaffected).This is purely additive: no existing (non-sanitizer) build is affected, and
SANITIZERS=addressbehavior is unchanged.Steps for the author:
After all prior steps are complete: