[MSAN] Reintroduce libatomic load/store instrumentation

[guiand]

Summary

Have the front-end use the `nounwind` attribute on atomic libcalls.
This prevents us from seeing `invoke __atomic_load` in MSAN, which
is problematic as it has no successor for instrumentation to be added.

[guiand]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3518

TODO: Remove

[eugenis]

__libatomic_load might come at the end of the function, with no succeeding BB

Not exactly. It may come at the end of a BB.

We should probably fix it in clang by adding nounwind to libatomic calls, and ignore invokes here.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3508

Check if the function already exists.

3510

addVisibility(GlobalValue::HiddenVisibility);

[guiand]

Simplified by returning to the old implementation, but having libatomic calls made nounwind (so we never see them as invokes).

[guiand]

Rebased on master

[guiand]

Rebased on master (again)

[eugenis]

clang/lib/CodeGen/CGAtomic.cpp

315

This needs a clang test, and better move it to a separate change.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3509

Better explicitly check that this an invoke and not call. Call always has a next instruction.

And move this check to the parent function to apply regular call instrumentation to the unrecognized call.

3511

It has a successor. Two successors, actually.
I'd rather say something like "invoke of __atomic_load".

[guiand]

Separated out the frontend change. Addressed other comments.

[guiand]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3513

TODO: Remove this redudant call

[eugenis]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3513

you've already checked it with assert(isa<CallInst>) - which could be changed to isTerminator(), and this could be simplified to NextIRB(CB.getNextNode()).

[guiand]

Addressed comments

[eugenis]

LGTM with 1 comment

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3576

Probably need the same check for atomic_store.

[guiand]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3576

Although it would be good for consistency I don't think it's strictly needed since with atomic_store we don't need to use getNextNode()

[eugenis]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3576

ah, good point. LGTM

[guiand]

Committed as 97de0188dd5d845ff90c8ac779a2ea09688b17df