Did you try BFD with more than one function? There was a fun bug where the "first" (for some definition of the word) input section of an output section was treated differently from the rest.

Sounds like a good idea.

Yes, and we put all globals in comdats, too. See AddressSanitizerModule::InstrumentGlobalsELF.

Looks great.

i.e. unless you've tested it on one of those systems, it might be safer to disable the test instead

OK, fine.
Just keep in mind that this test tries to exhaust the address space, but, without NORESERVE semantics, it would exhaust physical RAM instead and may DOS the entire system.

Two questions,

1. How does MSan work at all if MAP_NORESERVE is not implemented? Is it actually the default setting, and it's the opposite behavior that is not implemented?
2. If the flag is defined in the headers but ignored by the kernel, then what is the purpose of this patch?

more tests

LGTM

Nice!

What if there is no short read and the entire list is read in one syscall - does that mean that we've got a consistent thread list for some point of time in the past? If so, then iterating until the list stops changing and ignoring any attempts that ended in short reads should work.

Looks great, thank you!

That's a lot of code :(
But I don't see anything that could be thrown or factored out. We don't really need fixed (zero-based) stuff, but it's good to have it around for a while (for benchmarks and such).

LGTM

Thanks!

Add a test, please.

LGTM (+missing *-commits MLs)

LGTM (did not check the tests).

Could be too many parallel links, try -DLLVM_PARALLEL_LINK_JOBS=3.

LGTM, please add a comment though

Out of curiosity, how does access that far below SP happen in that test? Redzone? But that's not a leaf function.

Simply ninja check-all. Maybe the patch got applied incorrectly. Phabricator is really bad with file copies.

the tests fail with:
clang: /code/llvm-project/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1064: llvm::FunctionPass *llvm::createAddressSanitizerFunctionPass(bool, bool, bool): Assertion `!CompileKernel || Recover' failed.

Let's keep adding kernel- sanitizers for now.

Merged. Please don't forget to update tests next time.

I take that back, it's natural to expect there to be a no_sanitize("") attribute matching each possible -fsanitize= value.

You are right, it's not the best idea.
Is it too late to get rid of C/C++-level "kernel-address" attribute entirely? For the source code perspective, it's just ASan, but for the kernel. If that's an important distinction, one can always do #ifdef KERNEL or something.

Oh, right. And I removed the extra -msan flags before running the tests. It means you were testing msan^3 before :)

LGTM then. This naming is confusing.

This approach can not handle arrays - it would unpoison only the first element. It could be confusing for the user, but not really worse than the current state. Please mention this in the comment and/or the flag description.

This looks wrong. cfi_slowpath is defined in libclang_rt.cfi, which is linked to the main executable. It is not always dso-local. On Android it is defined in libdl.so and is never dso-local.

Actually, no. The test does not pass, not even close.
This check:

; CHECK-BASE-NOT: ret

is tripped by this line:

call void @__msan_warning_noreturn()

Sure. There is still a few extra -msan flags in the tests, I can remove them before commit.

I meant an integer flag with -1 as default value (for no match-all tag).

I too find these checks arbitrary and pointless. @kcc ?

Yes, this sounds fine.
You may want to provide a read-only page for reads and a write-only page for write to validate the use of this interface.

Please address Vitaly's comment.

Please add a separate flag, something like -hwasan-match-all-tag= (0 .. 0xFF, -1).

OK, sure, If you feel so strongly about this.

Looks fine.