bazel: add debug/sanitizer configs and expand build documentation#3538
bazel: add debug/sanitizer configs and expand build documentation#3538napetrov wants to merge 5 commits intouxlfoundation:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to improve Bazel/Make parity for developer builds by adding debug/sanitizer Bazel configs, enforcing “full ISA” behavior for //:release, and expanding Bazel build documentation.
Changes:
- Adds
.bazelrcnamed configs for debug and common sanitizers (ASan/TSan/UBSan) and documents Make → Bazel flag mappings. - Introduces a Starlark transition to make
//:releasebuild all ISA variants by default (when--cpuis left atauto), and updates CI to build a single ISA. - Adds an ISA coverage shell test script and wires it into Bazel as a manual
sh_test.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
.bazelrc |
Adds Bazel named configs for dbg/sanitizers and documents custom flag usage |
dev/bazel/README.md |
Expands Bazel build documentation (debug/sanitizers, release, flag mapping) |
dev/bazel/release.bzl |
Adds a CPU build-setting transition for //:release library dependencies |
dev/bazel/tests/isa_coverage_test.sh |
New script to validate ISA-dispatch symbol presence in libonedal_core.so |
dev/bazel/tests/BUILD |
Exports Bazel test scripts from dev/bazel/tests package |
cpp/daal/BUILD |
Adds a manual sh_test target to run the ISA coverage script on :core_dynamic |
dev/bazel/dal.bzl |
Extends module macro to accept copts and sets visibility/export-related defines |
dev/bazel/daal.bzl |
Extends module macro to accept copts and sets visibility/export-related defines |
.github/workflows/docker-validation-nightly.yml |
CI builds :release for avx2 only for speed |
dev/bazel/tests/BUILD
Outdated
| "mkl_linkage_test.sh", | ||
| "isa_coverage_test.sh", | ||
| ]) |
There was a problem hiding this comment.
exports_files() lists mkl_linkage_test.sh, but this file does not exist under dev/bazel/tests/ in the PR. Bazel will fail to load this package. Either add the script or remove it from exports_files().
| "mkl_linkage_test.sh", | |
| "isa_coverage_test.sh", | |
| ]) | |
| "isa_coverage_test.sh", | |
| ]) |
dev/bazel/release.bzl
Outdated
| "include_prefix": attr.string_list(), | ||
| "include_skip_prefix": attr.string_list(), | ||
| "lib": attr.label_list(allow_files=True), | ||
| "lib": attr.label_list(allow_files=True, cfg=_release_cpu_all_transition), |
There was a problem hiding this comment.
This rule now uses a Starlark configuration transition (cfg=_release_cpu_all_transition), but the rule doesn’t declare the required _allowlist_function_transition attribute. On Bazel 9 this typically causes analysis to fail with a transition allowlist error; add the allowlist attr to _release’s attrs.
| "lib": attr.label_list(allow_files=True, cfg=_release_cpu_all_transition), | |
| "lib": attr.label_list(allow_files=True, cfg=_release_cpu_all_transition), | |
| "_allowlist_function_transition": attr.label( | |
| default = "@bazel_tools//tools/allowlists/function_transition_allowlist", | |
| ), |
dev/bazel/tests/isa_coverage_test.sh
Outdated
| count=$(nm -D "${LIB}" | grep -c "${cpu_type}" || true) | ||
| if [[ "${count}" -gt 0 ]]; then | ||
| echo " OK ${isa_name} (${cpu_type}): ${count} dispatch symbols" |
There was a problem hiding this comment.
nm failures can leave count empty (e.g., if nm isn’t available), and then [[ "${count}" -gt 0 ]] errors under set -e. Consider explicitly handling the nm-missing/error case (e.g., default count to 0 or check command -v nm).
dev/bazel/tests/isa_coverage_test.sh
Outdated
| echo " FAIL ${isa_name} (${cpu_type}): 0 dispatch symbols found" | ||
| echo " Build with --cpu=all or --config=release to include all ISAs" | ||
| FAIL=$((FAIL + 1)) |
There was a problem hiding this comment.
The failure hint mentions --config=release, but this repo doesn’t define a release config in .bazelrc. Suggest updating the message to point to the supported flags (--cpu=all or building //:release).
| ```sh | ||
| bazel build //:release --config=dbg | ||
| bazel test //cpp/oneapi/dal:tests --config=dbg | ||
| ``` |
There was a problem hiding this comment.
The guide recommends bazel test ... --config=dbg/asan/..., but .bazelrc only defines build:dbg, build:asan, etc. To ensure the configs apply to bazel test as documented, add matching test:<config> stanzas (or move these options under common:<config>).
dev/bazel/README.md
Outdated
| > Note: Not all compilers support all sanitizers. GCC will fail with | ||
| > `REQSAN=memory` by design. Use icpx or clang for memory sanitizer. |
There was a problem hiding this comment.
The note references Make’s REQSAN=memory, but this is the Bazel guide and there is no corresponding Bazel config/flag shown here. Consider rephrasing in terms of Bazel/clang flags (e.g., -fsanitize=memory) or explicitly documenting that msan isn’t currently wired as a --config.
| > Note: Not all compilers support all sanitizers. GCC will fail with | |
| > `REQSAN=memory` by design. Use icpx or clang for memory sanitizer. | |
| > Note: Not all compilers support all sanitizers. In the Make-based flow, | |
| > MemorySanitizer corresponds to `REQSAN=memory`, but there is currently no | |
| > Bazel `--config` preset for msan. To use MemorySanitizer with Bazel you must | |
| > use a compiler that supports it (such as icpx or clang) and pass the | |
| > appropriate flags (for example, `--copt=-fsanitize=memory` and related msan | |
| > options). GCC does not support MemorySanitizer and will fail by design. |
dev/bazel/README.md
Outdated
| The `//:release` target automatically compiles all ISA variants regardless of | ||
| the `--cpu` flag default. To restrict ISA coverage (e.g., for faster CI): | ||
|
|
There was a problem hiding this comment.
This claim is a bit stronger than what the transition implements: release.bzl only forces --cpu=all when the cpu build setting is auto; if a user sets --cpu=avx2 in a personal .bazelrc, //:release will not build all ISAs. Suggest updating the wording to reflect that it forces all ISAs only when --cpu is left as auto (unset).
.bazelrc
Outdated
| # Adds: -g debug symbols only | ||
| # bazel build //:release --config=dbg-symbols | ||
| build:dbg-symbols \ | ||
| --compilation_mode=dbg |
There was a problem hiding this comment.
build:dbg-symbols sets --compilation_mode=dbg, but this toolchain’s dbg feature already adds assertion-related defines (see dev/bazel/toolchains/cc_toolchain_lnx.bzl dbg compile flags). That makes dbg-symbols behave like “debug + asserts”, not “symbols only” as documented. Consider keeping -c opt and only adding -g (and avoiding --enable_assert) for the symbols-only config.
| --compilation_mode=dbg | |
| --copt=-g \ | |
| --cxxopt=-g \ | |
| --linkopt=-g |
.bazelrc
Outdated
| --linkopt=-fsanitize=address \ | ||
| --linkopt=-shared-libasan |
There was a problem hiding this comment.
build:asan unconditionally adds -shared-libasan. In Make, this flag is compiler-dependent and is intentionally empty for GCC (dev/make/compiler_definitions/gnu.mk), so using it unconditionally can break the documented CC=gcc bazel ... workflow. Consider dropping -shared-libasan (GCC defaults to shared) or gating it by compiler/toolchain.
| --linkopt=-fsanitize=address \ | |
| --linkopt=-shared-libasan | |
| --linkopt=-fsanitize=address |
1cecb48 to
92dc524
Compare
|
Addressed all Copilot review comments:
Comments on |
|
|
I've already cleaned up the |
dev/bazel/README.md
Outdated
|
|
||
| ## Custom Compiler and Linker Flags | ||
|
|
||
| Equivalent to Make `COPT` / `CXXFLAGS`. Pass via `--copt` and `--linkopt`: |
There was a problem hiding this comment.
Bazel docs mention to use cxxopt when they are C++ flags:

There was a problem hiding this comment.
Fixed in 04b6db8: updated README to use --cxxopt for C++-only flags (e.g. -D_GLIBCXX_DEBUG) and --copt for flags that apply to both C and C++. Added explicit distinction in both the section text and the ~/.bazelrc examples.
There was a problem hiding this comment.
Fixed in 04b6db8: README now distinguishes --copt (C+C++) from --cxxopt (C++-only) and updates examples accordingly.
dev/bazel/README.md
Outdated
| > `--config=msan` preset. To use MemorySanitizer, use a compiler that supports | ||
| > it (icpx or clang) and pass flags manually: | ||
| > ```sh | ||
| > bazel test //cpp/oneapi/dal:tests --copt=-fsanitize=memory --linkopt=-fsanitize=memory |
There was a problem hiding this comment.
Should also use -fsanitize-memory-track-origins.
There was a problem hiding this comment.
Added in a5f1714: msan config now includes -fsanitize-memory-track-origins.
| bazel build //:release --config=dbg-symbols | ||
| ``` | ||
|
|
||
| > Note: The `libonedal_dpc.so` library does not support debug mode due to |
There was a problem hiding this comment.
This will change in one of the PRs that is in progress.
There was a problem hiding this comment.
Acknowledged. I adjusted the wording to avoid hard dependency on that in-flight PR and kept the note generic.
dev/bazel/README.md
Outdated
|
|
||
| > Note: Not all compilers support all sanitizers. There is currently no | ||
| > `--config=msan` preset. To use MemorySanitizer, use a compiler that supports | ||
| > it (icpx or clang) and pass flags manually: |
There was a problem hiding this comment.
Some sanitizers also requiring linking with LLD.
There was a problem hiding this comment.
Added README note in a5f1714 that MSan/TSan with Clang may require --linkopt=-fuse-ld=lld.
|
Addressed the feedback:
|
| The `//:release` target automatically compiles all ISA variants when `--cpu` | ||
| is left at its default value (`auto`). If `--cpu` is set explicitly — e.g. in | ||
| a personal `~/.bazelrc` or passed in CI — the transition respects that value. |
There was a problem hiding this comment.
The README claims //:release “automatically compiles all ISA variants” when --cpu is left at its default (auto). In the current Bazel config, @config//:cpu defaults to auto, which expands to sse2 + the detected host ISA (not all ISAs), and there’s no transition in the repo that forces all for //:release. Please update this section to match the actual behavior (e.g., document --cpu=all for full ISA coverage) or add the missing transition if that’s the intent.
| | `OPTFLAG=O2` | `--copt=-O2` | Override optimization level | | ||
| | `COPT=-flag` | `--copt=-flag` | Arbitrary compiler flag | | ||
| | `--cpu=<isa>` (Make `PLAT`) | `--cpu=<isa>` | ISA selection | | ||
| | (Make default all ISAs) | `bazel build //:release` | Transition forces all ISAs | |
There was a problem hiding this comment.
In the Make→Bazel reference table, the row stating “Transition forces all ISAs” for bazel build //:release doesn’t match the current implementation (no ISA-forcing transition present; default --cpu=auto builds sse2 + host ISA). Please correct the note to avoid misleading users.
| | (Make default all ISAs) | `bazel build //:release` | Transition forces all ISAs | | |
| | (Make default all ISAs) | `bazel build //:release` | Default `--cpu=auto` builds `sse2` + host ISA | |
| > Note: The `libonedal_dpc.so` library does not support debug mode due to | ||
| > excessive debug information causing long link times. Use the static library | ||
| > variant for DPC++ debugging. |
There was a problem hiding this comment.
The note says libonedal_dpc.so “does not support debug mode”, but the Bazel release target selects :dynamic_dpc based only on release_dpc and does not gate it on dbg/opt. Unless there is an actual build-time restriction elsewhere, this reads as inaccurate. Consider rephrasing to something verifiable (e.g., “debug builds may be impractically slow/large for the DPC++ shared library”) or document the concrete limitation/condition.
| > Note: The `libonedal_dpc.so` library does not support debug mode due to | |
| > excessive debug information causing long link times. Use the static library | |
| > variant for DPC++ debugging. | |
| > Note: Debug builds of the `libonedal_dpc.so` shared library may produce | |
| > excessive debug information and result in very long link times. For | |
| > practical DPC++ debugging, it is recommended to use the static library | |
| > variant instead. |
| # The //:release target automatically builds all ISA variants when | ||
| # --cpu is unset (auto). Use --cpu=<isa> to restrict for CI speed. | ||
| # bazel build //:release # CPU only, all ISAs |
There was a problem hiding this comment.
These comments state that //:release builds “all ISA variants” when --cpu is unset/auto. In the current config, --cpu=auto expands to sse2 + the detected host ISA, not all ISAs. Please update the comment/examples to match reality (or switch examples to --cpu=all if the intent is full ISA coverage).
| # The //:release target automatically builds all ISA variants when | |
| # --cpu is unset (auto). Use --cpu=<isa> to restrict for CI speed. | |
| # bazel build //:release # CPU only, all ISAs | |
| # By default, the //:release target builds for SSE2 plus the detected host | |
| # ISA when --cpu is unset (auto). Use --cpu=all for all ISAs, or | |
| # --cpu=<isa> to restrict for CI speed. | |
| # bazel build //:release # CPU only, SSE2 + host ISA | |
| # bazel build //:release --cpu=all # CPU only, all ISAs |
| # bazel build //:release --config=release-dpc # with DPC++ libs | ||
| # bazel build //:release --cpu=avx2 # CI: avx2 only | ||
| build:release-dpc \ | ||
| --release_dpc=true |
There was a problem hiding this comment.
--release_dpc=true uses a different boolean spelling/casing than the other config flags in this repo (e.g., config settings compare against "True"). For consistency (and to avoid any ambiguity in bool parsing), use True/False here as well.
| --release_dpc=true | |
| --release_dpc=True |
| # Fast local development build — compiles only for the host machine's | ||
| # highest ISA (auto-detected). Use this to speed up iterative builds. | ||
| # NOT suitable for release/packaging. |
There was a problem hiding this comment.
The dev config description says it “compiles only for the host machine's highest ISA”, but --cpu=auto still includes the sse2 baseline in addition to the detected host ISA (per cpu_info implementation). Please adjust the wording so developers aren’t surprised by the extra ISA being built.
| # Fast local development build — compiles only for the host machine's | |
| # highest ISA (auto-detected). Use this to speed up iterative builds. | |
| # NOT suitable for release/packaging. | |
| # Fast local development build — compiles for the host machine's | |
| # highest ISA (auto-detected), plus the baseline sse2 variant. Use this | |
| # to speed up iterative builds. NOT suitable for release/packaging. |
BLOCKER 5: Build options parity with Make (REQDBG / REQSAN) - .bazelrc: --config=dbg (REQDBG=1: debug symbols + assertions) - .bazelrc: --config=dbg-symbols (REQDBG=symbols: debug symbols only) - .bazelrc: --config=asan, --config=asan-static (REQSAN=address/static) - .bazelrc: --config=tsan (REQSAN=thread) - .bazelrc: --config=ubsan (REQSAN=undefined) - Custom flags via --copt/--linkopt documented (equiv. to Make COPT/CXXFLAGS) BLOCKER 6: Bazel build documentation - dev/bazel/README.md: Debug and sanitizer builds section - dev/bazel/README.md: Release build section - dev/bazel/README.md: Custom compiler/linker flags section - dev/bazel/README.md: Make to Bazel flag reference table Part of uxlfoundation#3530.
- .bazelrc: dbg-symbols uses --copt=-g instead of --compilation_mode=dbg (avoids unwanted assertions and -O0 from dbg toolchain feature) - .bazelrc: asan drops -shared-libasan (breaks GCC; compiler defaults correctly) - .bazelrc: all debug/sanitizer configs use common: instead of build: so they apply uniformly to both 'bazel build' and 'bazel test' - README.md: msan note rephrased in Bazel terms (no Make REQSAN reference) - README.md: ISA transition wording clarified — forces all ISAs only when --cpu is auto (default); explicit --cpu is respected
86d3d4b to
9fa277e
Compare
Per david-cortes-intel review: use --cxxopt for C++-only flags (e.g. -D_GLIBCXX_DEBUG), --copt for flags that apply to both C and C++. Updated Standard Library Assertions section and Custom Flags section.
| # The //:release target automatically builds all ISA variants when | ||
| # --cpu is unset (auto). Use --cpu=<isa> to restrict for CI speed. | ||
| # bazel build //:release # CPU only, all ISAs | ||
| # bazel build //:release --config=release-dpc # with DPC++ libs | ||
| # bazel build //:release --cpu=avx2 # CI: avx2 only |
There was a problem hiding this comment.
The comments here say //:release builds all ISA variants when --cpu is left at the default (auto). In the current config system, @config//:cpu defaults to auto, which resolves to a single auto-detected ISA (not all). Consider updating the comment/examples to use --cpu=all (or --cpu=modern / an explicit list) for “all ISA variants” behavior, and reserve auto for single-ISA dev builds.
| # The //:release target automatically builds all ISA variants when | |
| # --cpu is unset (auto). Use --cpu=<isa> to restrict for CI speed. | |
| # bazel build //:release # CPU only, all ISAs | |
| # bazel build //:release --config=release-dpc # with DPC++ libs | |
| # bazel build //:release --cpu=avx2 # CI: avx2 only | |
| # The //:release target builds multiple ISA variants when | |
| # --cpu is set to 'all' (or another multi-ISA setting). By default, | |
| # @config//:cpu is 'auto', which selects a single host ISA. | |
| # bazel build //:release --cpu=all # CPU only, all ISAs | |
| # bazel build //:release --config=release-dpc --cpu=all # with DPC++ libs | |
| # bazel build //:release --cpu=avx2 # CI: avx2 only |
| common:dbg \ | ||
| --compilation_mode=dbg \ | ||
| --enable_assert=True | ||
|
|
||
| # Configuration: 'dbg-symbols' | ||
| # Debug symbols only — no assertion checking, no -O0. | ||
| # Equivalent to Make: REQDBG=symbols | ||
| # Adds -g to the default opt build; preserves -O2/-O3 optimization. | ||
| # bazel build //:release --config=dbg-symbols | ||
| common:dbg-symbols \ | ||
| --copt=-g \ | ||
| --cxxopt=-g |
There was a problem hiding this comment.
Using common:<config> means these options will also be applied to non-build commands when invoked with --config=... (e.g., bazel query), which can cause “unknown option” failures for flags like --compilation_mode, --copt, and --linkopt. To keep configs usable across Bazel subcommands, consider defining these as build:<config> and test:<config> (duplicated), rather than common:<config>.
| common:dbg \ | |
| --compilation_mode=dbg \ | |
| --enable_assert=True | |
| # Configuration: 'dbg-symbols' | |
| # Debug symbols only — no assertion checking, no -O0. | |
| # Equivalent to Make: REQDBG=symbols | |
| # Adds -g to the default opt build; preserves -O2/-O3 optimization. | |
| # bazel build //:release --config=dbg-symbols | |
| common:dbg-symbols \ | |
| --copt=-g \ | |
| --cxxopt=-g | |
| build:dbg \ | |
| --compilation_mode=dbg \ | |
| --enable_assert=True | |
| test:dbg \ | |
| --compilation_mode=dbg \ | |
| --enable_assert=True | |
| # Configuration: 'dbg-symbols' | |
| # Debug symbols only — no assertion checking, no -O0. | |
| # Equivalent to Make: REQDBG=symbols | |
| # Adds -g to the default opt build; preserves -O2/-O3 optimization. | |
| # bazel build //:release --config=dbg-symbols | |
| build:dbg-symbols \ | |
| --copt=-g \ | |
| --cxxopt=-g | |
| test:dbg-symbols \ | |
| --copt=-g \ | |
| --cxxopt=-g |
|
|
||
| # Configuration: 'type' | ||
| # Type sanitizer build. | ||
| common:type \ | ||
| --copt=-fsanitize=type \ | ||
| --copt=-fno-omit-frame-pointer \ | ||
| --linkopt=-fsanitize=type \ |
There was a problem hiding this comment.
--config=type uses -fsanitize=type, which is Clang-only (GCC doesn’t support TypeSanitizer). Consider documenting that this config requires Clang/ICPX (similar to the MSan note), so users on GCC don’t get a confusing compiler error.
| --- | ||
|
|
||
| ## Release Build | ||
|
|
||
| Build the full release artifact (all ISA variants: sse2, sse42, avx2, avx512): | ||
|
|
||
| ```sh | ||
| bazel build //:release | ||
| ``` | ||
|
|
||
| The `//:release` target automatically compiles all ISA variants when `--cpu` |
There was a problem hiding this comment.
This section says bazel build //:release builds “all ISA variants” and that the default --cpu=auto triggers that. In the current Bazel config, @config//:cpu defaults to auto, which selects a single auto-detected ISA. Please update the instructions to use --cpu=all (or modern / explicit ISA list) when you mean “all ISA variants”, and clarify what auto does.
| | `OPTFLAG=O2` | `--copt=-O2` | Override optimization level | | ||
| | `COPT=-flag` | `--copt=-flag` (C+C++) / `--cxxopt=-flag` (C++ only) | Arbitrary compiler flag | | ||
| | `--cpu=<isa>` (Make `PLAT`) | `--cpu=<isa>` | ISA selection | | ||
| | (Make default all ISAs) | `bazel build //:release` | Transition forces all ISAs | |
There was a problem hiding this comment.
The “Make default all ISAs” mapping here assumes bazel build //:release forces all ISA builds, but the current --cpu default (auto) is single-ISA. Update this table row to reflect the actual flag needed for all-ISA coverage (e.g., --cpu=all).
| | (Make default all ISAs) | `bazel build //:release` | Transition forces all ISAs | | |
| | (Make default all ISAs) | `bazel build //:release --cpu=all` | Transition forces all ISAs | |
| ``` | ||
|
|
||
| ### Type Sanitizer | ||
|
|
There was a problem hiding this comment.
The Type Sanitizer section doesn’t mention compiler requirements. -fsanitize=type is Clang-only; GCC users will see a hard failure. Consider adding a short note (like the MSan section) indicating this requires Clang/ICPX and may be unavailable depending on the toolchain.
| Requires Clang or ICPX. GCC does not support the Type Sanitizer and availability may depend on your toolchain. |
Summary
Addresses BLOCKER 5 and BLOCKER 6 from #3530.
BLOCKER 5: Build options parity with Make
Adds
.bazelrcconfigurations matching MakeREQDBG/REQSANoptions:REQDBG=1--config=dbgREQDBG=symbols--config=dbg-symbolsREQSAN=address--config=asanREQSAN=static--config=asan-staticREQSAN=thread--config=tsanREQSAN=undefined--config=ubsanCOPT=-flag--copt=-flagExample usage:
BLOCKER 6: Build documentation
Expands
dev/bazel/README.mdwith:Changes
.bazelrc: newdbg,dbg-symbols,asan,asan-static,tsan,ubsanconfigsdev/bazel/README.md: new sections appended to existing guideTesting
No functional code changes — configuration and documentation only.
--config=dbguses existing toolchaindbg_compile_flags(-galready wired incc_toolchain_lnx.bzl).enable_assertflag already connected toONEDAL_ENABLE_ASSERT/DEBUG_ASSERTdefines.Part of #3530.