LGTM

In D54951#1325058, @vitalybuka wrote:

I don't see it in D54904

FYI, regarding -fsanitize-minimal-runtime it's also used for Scudo to link the lib that doesn't include RTUBsan & the symbolizer & stacktraces RT.

In D53528#1272945, @kubamracek wrote:

So, it's okay to land this?

In D53528#1272592, @kubamracek wrote:

The problem I'm seeing is when real_clock_gettime does exist, but it's not yet initialized, see the backtrace posted earlier. I'm not sure how performance sensitive this area is: Is it used elsewhere other than the memory allocator?

In D53528#1272587, @kubamracek wrote:

Yes, but it would need to be on the if (CanUseVDSO) line otherwise we still get a crash. I'll update the patch.

There is a check for real_clock_gettime (as an extern "C", not part of the interception namespace) in MonotonicNanoTime, as defined by sanitizer_common_interceptors.inc, can this check be moved there? eg: if real_clock_gettime exists & the interception function is not null?

Thanks!

Alright let's go with this one for the sake of HWasan.

LGTM
For reference, see comments in https://reviews.llvm.org/D52279#1246222

Or maybe replace the CHECK there with a Trap?

In D52279#1246222, @mstorsjo wrote:

Should we revert this again, or readd the attribute for windows builds only?

I am actually gonna have to work on the numbers again.
I ran into some issues with the Quarantine. If the Quarantine is low or off, then the numbers are good because we keep reusing the recently freed chunks.
But when the Quarantine is enabled, then have a low amount of cached pointers is detrimental.

Do not update base_ & size_ to reflect the fact that the reserved range
remains unchanged. Adding a comment to clarify that partial unmapping still
leaves the memory reserved.

Correct a comment to reflect that it is the destruction of the vmar that is
responsible for the unmapping.

Skip the UnmapOrDieVmar call when unmapping the whole mapping, as
vmar_destroy will take care of this. We still have to do some bookkeeping
via DecreaseTotalMmap.

Some before & after numbers for one of the benchmarks involved:

1552377     207294    1121659    2214385    1550481 nanoseconds              N/A Thread/CreateAndJoin
451883      35547     374475     984192     448331 nanoseconds              N/A Thread/CreateAndJoin

Reverting the initial patch with D51451.
Abandoning this.

In D51364#1218027, @eugenis wrote:

I wonder if ProtectMemoryRange in msan_linux.cc needs updating?

In D51364#1217985, @eugenis wrote:

Something's broken.

Could I please get a LGTM and/or other options so that I can try and fix the aarch64 bots?

Link to test source: https://github.com/llvm-mirror/compiler-rt/blob/master/test/msan/mmap.cc#L78
Last test output: 0xf00000000
Link to msan mmap interceptor: https://github.com/llvm-mirror/compiler-rt/blob/master/lib/msan/msan_interceptors.cc#L939

LGTM with a nit.
Additional question but that doesn't require changes to the CL: if you have a ByteMap it means you are using the SizeClassAllocator32 in 64-bit mode (it's gated by a define, look for SANITIZER_CAN_USE_ALLOCATOR64).
Have you tried using the SizeClassAllocator64?

Ping!

You probably can add as well the couple others that default to true that could be set to false as well:

• DeallocationTypeMismatch
• DeleteSizeMismatch

Updated proposal: make Mmap*NoAccess return nullptr on failure (like the
other Mmap functions).
Modify callers that were checking for ~(uptr)0 to now check for nullptr.

Other wrong use:
hwasan::MapDynamicShadow: checks for failure with ~(uptr)0 (while it's the syscall return value)
asan::FindDynamicShadowStart: same as above
__asan::PremapShadow: same as above

I guess after scavenging further, my initial patch is not correct, as other functions return nullptr on failure.
I am open to anything if we can get a consensus on what should be returned.

In D50910#1205003, @alekseyshl wrote:

Have you noticed any difference in performance? I tried the same back in March I have not noticed any perf gain on the couple of benchmarks. I am merely curious, I think this is a right change to make anyways.

Grammar/punctuation corrections in comments.

Account for 0 size, which is more common than one would expect.

In D50406#1194173, @vlad.tsyrklevich wrote:

malloc() would only be called during thread destruction, so it doesn't seem like there should be an issue unless malloc() causes a thread to destruct and even then this function shouldn't re-enter because of the order of when pthread_setspecific() is called. Perhaps I'm just failing to imagine a situation under which this could be an issue?

Random thought: isn't the introduction of malloc here (as opposed to an OS backed alternative like mmap) gonna mess compatibility with other Sanitizers that intercept it? (thinking of Scudo which is currently compatible with SafeStack but I haven't tested).