LGTM

LGTM

What's the binary size overhead? I assume most of it comes from adding personality functions to noexcept but !nounwind functions?

address comments

addressed review comments

We are still interested in gold compatibility, I think.

.

I've committed a better fix in r369138.

This build is on r369069 and still has the problem:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/23577

I'll submit this as is and then refactor in a follow up.
Thanks!

It's a bit hard to use, you need to build libcxx/libcxxabi with msan as well.
See here:
https://github.com/google/sanitizers/wiki/MemorySanitizerBootstrappingClang

Hi, MSan is complaining about this change:

28574==WARNING: MemorySanitizer: use-of-uninitialized-value

#0 0x130075d in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::lowerPrintfForGpu(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:576:39
#1 0x12f8287 in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:613:10
#2 0x544a96d in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1750:27
#3 0x544a96d in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1863
#4 0x9e58b7 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:892:12
#5 0x7f84fa91f2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#6 0x91ce99 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/opt+0x91ce99)

LGTM

LGTM

LGTM

LGTM

LGTM

added a test

LGTM

This fixes http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18689.
I've added a pass that preservesCFG; following passes use LI but not DT. Without the transitive dependency, the pass manager kills DT before my pass as there are no future users, but then calls LoopInfoWrapperPass::verify that assumes that DT is still alive.

OK, sure.
As I understand functon passes also have better data locality by running a sequence of passes over a single function. Not sure how important that is.
LGTM

I don't know what is the best practice here. Other sanitizers have both module and function passes, is there a reason to do things differently here?
@chandlerc

In D64299#1588931, @dvyukov wrote:

In D64299#1588251, @eugenis wrote:

This feels like the right trade-off.

TSan should be fine with a mutex that is never destroyed, but it will not be able to catch lock-after-destroy bugs on such mutex obviously.
@dvyukov

Effectively pthread_mutex_destroy is not trivial under tsan so, yes, there will be a number of negative consequences:

• no lock-after-destroy races
• no races between lock/unlock and a subsequent destory
• no racy use-after-free detection for destroy vs free
• incorrect stack trace for a mutex that happened to reuse the address later
• resource leaks for mutexes
• no reporting of unlock of locked mutex
• no auto-unlock for destroyed mutexes, which will look like the thread is taking infinite number of locks recursively, which will quickly check-fail in deadlock detector

LGTM

This feels like the right trade-off.

LGTM

.

address review comments

LGTM, but please update the summary:

An alternative would be to disallow -fsanitize=function ...

Tested by adding a bunch of fake attributes at the beginning to push the interesting ones out of 0..64 range, and also with ASan.

oops, found another place to update

In D64663#1583672, @pcc wrote:

I would prefer to keep this the way it is with an array so that it is obvious what the size/alignment are.

It would be good to have a static assertion of some sort, though. It looks like Attribute::EndAttrKinds could be used for this purpose.

std::bitset is word-aligned at least in libc++ implementation, so it would waste 4 bytes here.
SmallBitVector has only 64 bits of inline storage.

addressed review comments

It's sanitize_memtag in the child revision.
Actually, we were fine without this until your change on Jule 10 pushed our pending attribute over the limit :)

I've moved the bitset into the padding of NumAttrSets so hopefully this will not waste any memory.

PTAL. I've added tests for various frame layouts. Stg offset overflow is covered by existing tests (settag.ll).

Made aarch64.irg.sp IntrInaccessibleMemOnly cause it has side effects.
Simplified SDAG a little by matching intrinsic directly w/o going through an SDag node for IRGstack.
Added a bunch of tests.

I'm a bit worried about performance implications.
isBytewiseValue is now doing some clearly unnecessary work.
Could you sanity check that it does not affect compilation times?

This is not a lot of code, so LGTM, but I agree that constant folding would be even better.

This seems correct. Any idea why it has not been done this way from the start?

LGTM

LGTM