Augment bazelrc to support asan & ubsan #2197
Conversation
A prior failure in the google3 internal codebase showed that we were not executing the sanitizers like we thought we were. This change augments bazelrc to do asan and ubsan via config. It leverages the default feature set offered by bazelrc. I tested the commit priot to 474caa8 to validate that the bug was exhibited.
There was a problem hiding this comment.
Very cool! A few high-level comments/questions:
- Consider adding a link to the documentation for sanitizers somewhere.
- Do we want
msanas well? - I thought we ran sanitizers in our CMake build? Any insight on why that didn't catch the issues?
- From the CI on this PR, it does look like this makes the bazel build a lot slower. Also, it seems to be producing some errors, is that expected?
| @@ -14,6 +14,11 @@ | |||
|
|
|||
| build --cxxopt=-std=c++17 | |||
| build --host_cxxopt=-std=c++17 | |||
| # llvm builds typically without rtti | |||
There was a problem hiding this comment.
Consider linking to a reference for this, would be helpful for context and in case we need to adjust this in the future.
.bazelrc
Outdated
| # turn on fission https://gcc.gnu.org/wiki/DebugFission | ||
| # this separates the debug information and can improve link times | ||
| build --fission=yes | ||
|
|
||
| build:dbg --compilation_mode=dbg | ||
| build:opt --compilation_mode=opt |
There was a problem hiding this comment.
These lines look like they belong in a separate change? Not a huge deal, but consider splitting this out into its own PR and/or calling this out in the commit message/PR description.
(--fission seems like general QOL, --opt seems unused, and --dbg is used, but I only see it used once below, so it could be inlined.)
There was a problem hiding this comment.
I removed them for now and turned on fission only for dbg.
This should bring the code size back down to normal?
| @@ -22,5 +22,5 @@ if [[ $# -ne 0 ]] ; then | |||
| fi | |||
|
|
|||
| # Build and Test StableHLO | |||
| bazel build --lockfile_mode=error //... | |||
| bazel test //... | |||
| bazel build --lockfile_mode=error //... --config=asan --config=ubsan | |||
There was a problem hiding this comment.
Consider creating a separate config that wraps these two, e.g. build:san --config=asan --config=ubsan and then you can do --config=san here.
I am looking into having it run non debug. |
|
If this builds and passes, I'll augment the README with a little section on how to build with Bazel & the sanitizers TODO remains to look at our CMake :S |
A prior failure in the google3 internal codebase showed that we were not executing the sanitizers like we thought we were.
This change augments bazelrc to do asan and ubsan via config. It leverages the default feature set offered by bazelrc.
I tested the commit priot to 474caa8 to validate that the bug was exhibited.
I've added the sanitizers to the default Bazel build which also builds debug builds.
This may come at an increase cost. If undesirable we can split the job out.