On Wed, Jul 14, 2021 at 08:42:27PM +0200, Andrzej Hunt wrote: > My other question is: if we are adding a new job - should it really be just > a leak checking job? Leak checking is just a subset of ASAN (Address > Sanitizer). And as discussed at [1] it's possible to run ASAN and UBSAN > (Undefined Behaviour Sanitizer) in the same build. I feel like it's much > more useful to first add a combined ASAN+UBSAN job, followed by enabling > leak-checking as part of ASAN in those jobs for known leak-free tests - as > opposed to only adding leak checking. We currently disable Leak checking for > ASAN here [2], but that could be made conditional on the test ID (i.e. check > an allowlist to enable leak checking for some tests)? I do think it's worth having an ASan+UBSan job. In the CI we use for our custom fork of Git at GitHub, we run it for every pull request (and I do bring upstream any applicable fixes). It's kind of expensive compared to a regular "make test", but probably not nearly as bad as just running the regular test suite on Windows. And it's true that ASan can do leak-checking, too. In the long run, when we are leak-free, I think it may make sense to combine the jobs. But in the interim state where we can run the whole suite with ASan/UBSan, but not with LSan, I think it's simpler to just keep them separate. That lets us just entirely skip tests or scripts in the leak-checking run. I haven't measured, but I also expect that LSan is not much more expensive than a regular run, so combining the two isn't that big a win). So I do like your suggestion, but I think it just be orthogonal further to leak-checking. -Peff
