LGTM

formatting

address comments

LGTM

revert unrelated changes

added an empty line

+ a comment in test

FYI @vlad.tsyrklevich
LGTM

LGTM

Looks good enough to me.

I've re-read the original problem. Since this is not custom cflags, but an LLVM cmake flag, I think we are obligated to handle it.
Removing -stdlib.* from the flags looks like an OK solution.
We could also fix clang to understand that in "-stdlib=libc++ -nodefaultlibs" there are no unused flags, like with the other flag groups that cancel each other.

Btw, I've pushed the RSS utility here:
https://github.com/google/sanitizers/blob/master/hwaddress-sanitizer/scan.cc
It depends on this change to find the shadow region. It is hwasan-specific, but can be easily adapted to other sanitizers if needed.
It's also really ugly, but I don't want to invest time in beautifying it.

In D57720#1386140, @pcc wrote:

The GetNamedMappingFd and DecorateMapping APIs seem a little awkwards to me. Everyone is using them as GetNamedMappingFd/mmap/DecorateMapping except for one place in hwasan. Would it make more sense to have a single function that does GetNamedMappingFd/mmap/DecorateMapping and continue using PR_SET_VMA_ANON_NAME in hwasan?

.

LGTM

LGTM

.

Does the problem appear when -stdlib is passed through custom CFLAGS? I think it is common to disable WERROR as well in that case, or add -Wno-unused-command-line-argument to custom CFLAGS.

.

I find the current code more readable than with this change.

LGTM

LGTM

Do we have a test that at most one __asan_handle_no_return call is inserted before a [[noreturn]] call?

LGTM
Doing less things while libc is not fully initialized is a step in the right direction.

LGTM assuming the plan is to add !nosanitize where !noreturn has been, at the original call site, in the follow-up change.

Sounds good.

LGTM

LGTM
Since the previous iteration of this was controversial, please wait for at least one more review.

ping

ping

LGTM

Maybe the frontend should insert __asan_handle_noreturn whenever ASan is enabled, and then ASan would not care about the attribute? I'd like to avoid having this logic in two places.

Wouldn’t it be preferable to unpoison the stack inside of maybe_longjmp, once the opaque condition can be checked?

Because "expect_noreturn" calls are allowed to return, the compiler must behave as they could. In particular, this means that unpoisoning the stack before expect_noreturn calls (given the current semantics) is premature.

That should not be necessary.
__asan_handle_noreturn is needed for functions that move SP without going through ASan epilogue, in order to maintain the requirement that stack below SP has clean shadow.
Ubsan-rt does nothing of the sort.

madvise the entire allocation range

LGTM

LGTM

This patch is missing tests for the new attribute in AsmParser / BitcodeReader / BitcodeWriter.

I think I could, but it would make the tests (more) painful to update. I guess I could drop the tests for abort in basic.ll in the first change and then bring them back in the second change, if that's ok?

More than half of this changelist is switching shadow addressing to getelementptr. Could you do it in a separate change?

This one is hard to test reliably for RSS change, because it only affect thread exit, and, generally, we don't know how much memory the platform thread support would release in that event, and it might even be a different amount for different threads (some kind of stack caching, etc).

.

a test case

LGTM with comments.

LGTM

LGTM

In D56624#1362458, @yln wrote:

How about sanitizer_noreturn or noreturn_for_sanitizer. It should convey the same meaning as noreturn but has a specific audience: sanitizers.

And we should probably skip this note in the case where it is obvious - for example, for an 8 byte, 8-aligned access!