Page MenuHomePhabricator
Differential D85559 Diff 284076 llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Changeset View

Standalone View

View Options

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Show First 20 Lines • Show All 3,499 Lines • ▼ Show 20 Lines void visitLibAtomicLoad(CallBase &CB) {
Value *DstPtr = CB.getArgOperand(2); Value *DstPtr = CB.getArgOperand(2);
Value *Ordering = CB.getArgOperand(3); Value *Ordering = CB.getArgOperand(3);
// Convert the call to have at least Acquire ordering to make sure // Convert the call to have at least Acquire ordering to make sure
// the shadow operations aren't reordered before it. // the shadow operations aren't reordered before it.
Value *NewOrdering = Value *NewOrdering =
IRB.CreateExtractElement(makeAddAcquireOrderingTable(IRB), Ordering); IRB.CreateExtractElement(makeAddAcquireOrderingTable(IRB), Ordering);
CB.setArgOperand(3, NewOrdering); CB.setArgOperand(3, NewOrdering);
IRBuilder<> NextIRB(CB.getNextNode()); Instruction *InsPoint = CB.getNextNode();
eugenisUnsubmitted
Done
ReplyInline Actions

Check if the function already exists.

eugenis: Check if the function already exists.
if (!InsPoint) {
eugenisUnsubmitted
Done
ReplyInline Actions

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.

eugenis: Better explicitly check that this an invoke and not call. Call always has a next instruction.
llvm::errs() << "MSAN -- cannot instrument libatomic call with no "
eugenisUnsubmitted
Done
ReplyInline Actions

addVisibility(GlobalValue::HiddenVisibility);

eugenis: addVisibility(GlobalValue::HiddenVisibility);
"successor. Ignoring!\n";
eugenisUnsubmitted
Done
ReplyInline Actions

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

eugenis: It has a successor. Two successors, actually. I'd rather say something like "invoke of…
return;
}
guiandAuthorUnsubmitted
Done
ReplyInline Actions

TODO: Remove this redudant call

guiand: TODO: Remove this redudant call
eugenisUnsubmitted
Done
ReplyInline Actions

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

eugenis: you've already checked it with assert(isa<CallInst>) - which could be changed to isTerminator()…
IRBuilder<> NextIRB(InsPoint);
NextIRB.SetCurrentDebugLocation(CB.getDebugLoc()); NextIRB.SetCurrentDebugLocation(CB.getDebugLoc());
Value *SrcShadowPtr, *SrcOriginPtr; Value *SrcShadowPtr, *SrcOriginPtr;
std::tie(SrcShadowPtr, SrcOriginPtr) = std::tie(SrcShadowPtr, SrcOriginPtr) =
guiandAuthorUnsubmitted
Done
ReplyInline Actions

TODO: Remove

guiand: TODO: Remove
getShadowOriginPtr(SrcPtr, NextIRB, NextIRB.getInt8Ty(), Align(1), getShadowOriginPtr(SrcPtr, NextIRB, NextIRB.getInt8Ty(), Align(1),
/*isStore*/ false); /*isStore*/ false);
Value *DstShadowPtr = Value *DstShadowPtr =
getShadowOriginPtr(DstPtr, NextIRB, NextIRB.getInt8Ty(), Align(1), getShadowOriginPtr(DstPtr, NextIRB, NextIRB.getInt8Ty(), Align(1),
/*isStore*/ true) /*isStore*/ true)
.first; .first;
NextIRB.CreateMemCpy(DstShadowPtr, Align(1), SrcShadowPtr, Align(1), Size); NextIRB.CreateMemCpy(DstShadowPtr, Align(1), SrcShadowPtr, Align(1), Size);
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines void visitCallBase(CallBase &CB) {
} }
LibFunc LF; LibFunc LF;
if (TLI->getLibFunc(CB, LF)) { if (TLI->getLibFunc(CB, LF)) {
// libatomic.a functions need to have special handling because there isn't // libatomic.a functions need to have special handling because there isn't
// a good way to intercept them or compile the library with // a good way to intercept them or compile the library with
// instrumentation. // instrumentation.
switch (LF) { switch (LF) {
case LibFunc_atomic_load: case LibFunc_atomic_load:
visitLibAtomicLoad(CB); visitLibAtomicLoad(CB);
eugenisUnsubmitted
Not Done
ReplyInline Actions

Probably need the same check for atomic_store.

eugenis: Probably need the same check for atomic_store.
guiandAuthorUnsubmitted
Done
ReplyInline Actions

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()

guiand: Although it would be good for consistency I don't think it's strictly needed since with…
eugenisUnsubmitted
Not Done
ReplyInline Actions

ah, good point. LGTM

eugenis: ah, good point. LGTM
return; return;
case LibFunc_atomic_store: case LibFunc_atomic_store:
visitLibAtomicStore(CB); visitLibAtomicStore(CB);
return; return;
default: default:
break; break;
} }
} }
▲ Show 20 Lines • Show All 1,706 Lines • Show Last 20 Lines