search

google / xls

XLS: Accelerated HW Synthesis
http://google.github.io/xls/
Apache License 2.0
1.17k stars 171 forks source link

Add --config=asan test in CI #1447

Open hzeller opened 3 months ago

hzeller commented 3 months ago

To make sure to not accidentally introduce memory leaks or invalid memory access issues, we should run the tests with AddressSanitizer in the CI (maybe as separate after successful 'regular' testing, as it can be quite a bit slower, and we should only run it after we know that the regular test execution passes).

The .bazelrc already has the configuration prepared for this.

Context: PR #1443 would benefit from an ASAN run.

hzeller commented 3 months ago

Survey: This is the subset of tests that seem to work currently with asan.

Some are suffering the #1449 compile issue, others are not working in the context of fuzzing, yet others don't work due to the use of cython. The remaining 1358 tests pass.

bazel test --config=asan --keep_going -- //xls/... -//xls/fuzzer/... -//xls/codegen/...  \
   -//xls/contrib/ice40:wrap_io_test -//xls/contrib/xlscc/unit_tests:translator_verilog_test \
  -//xls/simulation:module_simulator_codegen_test -//xls/simulation:module_simulator_test \
  -//xls/simulation:module_testbench_test -//xls/simulation:testbench_io_test \
  -//xls/simulation:verilog_simulator_test -//xls/simulation:verilog_test_base_test \
  -//xls/synthesis:synthesis_utils_test -//xls/synthesis:timing_characterization_client_test \
  -//xls/tools:simulate_module_main_test -//xls/contrib/xlscc/unit_tests:fuzz_fixed_10_test  \
  -//xls/contrib/xlscc/unit_tests:fuzz_int_10_test
hzeller commented 3 months ago

Do you think adding a asan build would fit the resources @proppy ?

proppy commented 2 months ago

@hzeller yep, do we want to run those continuously or nightly?

hzeller commented 2 months ago

I think on every github PR (but not the regular CI on head) and then once nightly.

proppy commented 2 months ago

I think on every github PR (but not the regular CI on head) and then once nightly.

that should be a problem we have big enough runner, and we can run the two build in two parallel jobs.

proppy commented 2 months ago

@hzeller I assume the build cache for the two config (w/ and w/o asan) would end up being different?

hzeller commented 2 months ago

Yes. And possibly asan a lot larger. mmh, that might bring us over the available cache edge, wouldn't it ?

proppy commented 2 months ago

I suspect that we could run the asan build without cache?

hzeller commented 2 months ago

Sounds good. I suspect it might run forever though. Maybe we can narrow the scope of tha asan test in a PR to only the targets affected (I guess some sort of collaboration between git diff and bazel query). That way we can build relevant targets and maybe bail out if it would be too many targets.

That way we could service most of the PRs, but also not hog down resources too much.

(and then, maybe nightly do a full asan ?)