bazel: fix hermetic ASan builds#10911
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the .bazelrc configuration for the ASan build to use hermetic LLVM's sanitizer setting and disables external_include_paths. The feedback highlights that disabling external_include_paths could cause compilation warnings in third-party headers to be treated as errors, potentially breaking the ASan build, and suggests adding -Wno-error to prevent this.
| # the toolchain resource directory. external_include_paths is incompatible | ||
| # with compiler-rt's internal LLVM header dependencies. | ||
| build:asan --@llvm//config:asan=true | ||
| build:asan --features=-external_include_paths |
There was a problem hiding this comment.
Disabling external_include_paths globally for the asan configuration means that third-party headers (such as Boost, Eigen, etc.) will no longer be treated as system headers (-isystem). Consequently, any warnings within these external headers will be emitted during compilation. Since the project treats several specific warnings as errors (lines 48-55), this is highly likely to break the ASan build. To prevent warning-induced build failures, consider adding -Wno-error to the ASan compiler options.
build:asan --features=-external_include_paths
build:asan --copt=-Wno-error
build:asan --host_copt=-Wno-error
There was a problem hiding this comment.
I ran the full debug build with this configuration: bazel build --config=asan -c dbg :openroad. It completed successfully after compiling the Boost, Eigen, and generated SWIG sources. The external headers do emit warnings, but none of them hit the projects warning-as-error set. I would rather keep the existing warning checks active than disable all of them for ASan.
There was a problem hiding this comment.
Are you saying it won't compile without this option or just that you prefer to see the warnings?
There was a problem hiding this comment.
It will not compile without disabling external_include_paths. With the feature enabled, compiler-rt fails while compiling its own sources because headers such as llvm/DebugInfo/Symbolize/DIPrinter.h are not found. The warnings from Boost and Eigen are a side effect of disabling the feature. I mentioned them only to confirm that the full build still passes without adding -Wno-error.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Naveen Venkat <archgen.guest@nyayanidhi.in>
392ff26 to
c17c515
Compare
|
I found one more issue while expanding the ASan test coverage. Python tests could build the instrumented extension modules, but importing them failed because the Python interpreter itself does not load compiler-rt. The branch now adds the hermetic libc++abi, libc++, and ASan libraries as test runfiles and preloads them only when the ASan config is active. There are no system library paths in the test setup.\n\nI tested all 18 OpenDB Python binding tests under ASan with leak detection disabled, and all 18 pass. I also ran the same 18 tests without ASan, and all 18 pass. |
Signed-off-by: Naveen Venkat <archgen.guest@nyayanidhi.in>
c17c515 to
f4643d9
Compare
Signed-off-by: Naveen Venkat <archgen.guest@nyayanidhi.in>
|
I found another gap while testing a full Python regression under ASan. The runtime preload was exported by the shell harness, so helper programs inherited the hermetic C++ libraries and could fail before Python started. The runtime set was also missing libunwind.\n\nThe latest commit resolves the runfiles to absolute paths, preloads them only for the Python command, and adds hermetic libunwind. I tested nine OpenDB Python binding targets plus the DRT ispd18 Python regression under ASan with leak detection disabled. All 10 pass. The same 10 targets also pass in the normal configuration. |
|
leaving this to @maliberty |
Fixes #7754.
Address sanitizer builds stopped linking after the hermetic LLVM migration because the raw compiler flags did not stage compiler-rt in the toolchain resource directory.
Use the hermetic toolchain ASan setting so it builds and stages the runtime libraries. Disable
external_include_pathsfor this config because that feature prevents compiler-rt from finding its internal LLVM headers.Tests run on Linux:
bazel build --config=asan -c dbg :openroadbazel test --config=asan //src/cts/test:cts_unittestbazel test --config=asan --test_env=ASAN_OPTIONS=detect_leaks=0 //src/cts/test:cts_unittest //src/cts/test:simple_test-tcl_test //src/rsz/test:TestBufRem1 //src/rsz/test:TestBufferRemoval2 //src/rsz/test:TestBufferRemoval3 //src/rsz/test:TestInsertBuffer //src/rsz/test:TestNestedJournal //src/rsz/test:TestResizer //src/rsz/test:TestResizerMtbazel test //src/cts/test:cts_unittest //src/cts/test:simple_test-tcl_testLeak detection was disabled in the broader run to exclude the existing process-lifetime allocations created by
sta::TimingArcSet::init().