carbon-lang
carbon-lang copied to clipboard
Fix ASAN poison bugs, add debugging infrastructure for finding them, and enable poisoning with ASAN by default
ASAN crashes do not include which test file was running. Normally we inject the command line which includes the file, and any diagnostics, but https://github.com/llvm/llvm-project/issues/138759 prevents that with ASAN crashes. So, when --threads=1 is given on the command line, we print each test name before running it in order to have some way to determine which test crashed.
ASAN use-after-poison crashes also do not contain a stack trace for where the poisoning occurred. So you know which pointer is invalid, but you don't know why. There's a long-standing bug about this here: https://github.com/google/sanitizers/issues/191.
We add two new command line flags for debugging use-after-poison:
--poison_verbosewhich logs each poison and unpoison event, giving each poison event a unique identifier that includes the type of the value store.--poison_abortwhich takes an identifier from the log, and will cause the test to abort when that poisoning event (or any later-numbered one in the same value store type) occurs, giving a stack trace for the poisoning event.
Together these flags allow for pretty quick debugging of why a pointer was invalidated.
However --poison_abort can be unreliable because the identifiers in the log from --poison_verbose are not entirely stable across runs. For most stores, where invalidations are infrequent, this is okay, and --poison_abort helps deal with this by aborting on any event after the specified one. But for inst store this may make debugging harder, and require multiple runs to get the correct poison stack. While https://github.com/google/sanitizers/issues/191 would be ideal here, this is at least something to help us debug issues.
The use-after-poison bugs that are currently surfaced by our test cases are fixed. There's only a few caught by our tests, though it's likely there's many more such bugs lurking, where our test cases just do not yet happen to cause an import to happen in the many places that allow for importing or that run eval. We hope that fuzzers will help find more until we can provide some more structural way to help avoid these bugs.
The file_test framework also needs to know about these new flags so that it can pass them along to the driver. The driver usually uses names like --poison-verbose with a hyphen, but absl flags can't use hyphen separators. Rather than using different flags in file_test and the compiler, we use underscore separators.