Use AddressSanitizer when available

[Jules-Bertholet]

Compiles and runs the program and standard library with AddressSanitizer on targets that support it. This sanitizer "is not expected to give false positive reports".

[RalfJung]

That sounds like a great idea, thanks!

Two things:

• I don't understand how know are used via rustc, or what half of this code does. Please add comments explaining that. Assume that the reader knows what a sanitizer is but never used one. A function like sanitizer with no comment at all is basically never okay. :) It needs a doc comment at least, and probably also some inner comments to explain why it is doing what.
• What is the performance effect of this? Would it make sense to make this controllable via some user-exposed flag?

[Jules-Bertholet]

I've added some comments and a command-line flag to disable sanitizing. According to clang docs, slowdown is ~2x

[thomcc]

I've used ASan often, and for a long time. I think this is a good idea, but that it needs to be optional (even if on by default).

Sadly, "false positives are rare[^1]" is not the same as "every program can use ASan without issues". There are a number of situations that are unsupported:

1. Programs with custom C/C++ allocators (e.g. which override malloc/free). ASan needs to override these too, and they can't both do it. Even on macOS where this could actually work because of the zone API, it will still cause issues[^2]
2. Programs that use non-standard system allocator APIs that are not overridden by Asan will often behave badly. Often this is in the form of some custom allocator function asan doesn't happen to have an override for, which is still compatible with free. I think this has gotten a lot better, but it seems unlikely to ever be totally fixed.
3. Asan will often reserve a crapload of virutal memory (several TB), which can cause problems on some systems.

[^1]: Honestly, the number of false positives from asan's leak checker is fairly high IME -- at least if you don't build everything with ASan on. But false positives for memory safety errors are low, excluding the issues I mention around allocators.

[^2]: For example, jemalloc will loop until it is not the default zone. Unfortunately, ASan interposes the malloc_default_zone function to always return its zone... which means this loop never terminates.

These are mainly cases that crop up with FFI, but a big reason to use cargo-careful over miri is because you have some code that does FFI.

Similarly, it may also be worth setting adding -fsanitize=address to CFLAGS and CXXFLAGS, so code built with the e.g. cc crate in a build script will get checked too. This may be way more trouble than its worth though, especially since ASan (unlike the other sanitizers) doesn't need it (although it helps the leak check).

[Jules-Bertholet]

I've addressed all the review comments and added a note to the readme.

[Jules-Bertholet]

It's done.

[RalfJung]

CI seems to be unhappy on macOS?

  = note: ld: library not found for -lrustc-nightly_rt.asan
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

[RalfJung]

@Jules-Bertholet any idea what is going on with the macOS CI failures here? Is that something you plan to look into?

As-is the PR clearly cannot land.

[Jules-Bertholet]

@RalfJung It turns out that in addition to the macOS issue (which I haven't looked into), there is another problem: the -Zsanitizer feature doesn't play well with proc macros. Until that rustc issue is fixed, sanitizing can't really be enabled by default. I could modify this PR to make sanitizing off-by-default, if you prefer.

[RalfJung]

Ah, yeah we don't have any proc macros in the test suite here I think.

An off-by-default flag sounds reasonable, with the docs saying it is currently experimental and not tested on CI and pointing to the rust issue about proc macros.

[Jules-Bertholet]

Ready for review

[RalfJung]

Looks good! Just some minor nits, and there are conflicts. Also please run cargo fmt.

[Jules-Bertholet]

Should be good