RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-26 12:43:36 +01:00
Code Issues Projects Releases Wiki Activity

[SimplifyCFG] Stop inserting calls to llvm.trap for UB

Browse Source 
SimplifyCFG had logic to insert calls to llvm.trap for two very
particular IR patterns: stores and invokes of undef/null.

While InstCombine canonicalizes certain undefined behavior IR patterns
to stores of undef, phase ordering means that this cannot be relied upon
in general.

There are much better tools than llvm.trap: UBSan and ASan.

N.B. I could be argued into reverting this change if a clear argument as
to why it is important that we synthesize llvm.trap for stores, I'd be
hard pressed to see why it'd be useful for invokes...

llvm-svn: 273778
This commit is contained in:
David Majnemer 2016-06-25 08:04:19 +00:00
parent 3c5c29ea74
commit 9f643fc17a
10 changed files with 30 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/llvm/Transforms/Utils/Local.h
View File

@ -296,7 +296,7 @@ unsigned removeAllNonTerminatorAndEHPadInstructions(BasicBlock *BB);
/// Insert an unreachable instruction before the specified
/// instruction, making it and the rest of the code in the block dead.
unsigned changeToUnreachable(Instruction *I, bool UseLLVMTrap);
unsigned changeToUnreachable(Instruction *I);
/// Replace 'BB's terminator with one that does not have an unwind successor
/// block. Rewrites `invoke` to `call`, etc. Updates any PHIs in unwind

6
lib/CodeGen/WinEHPrepare.cpp
View File

@ -963,9 +963,9 @@ void WinEHPrepare::removeImplausibleInstructions(Function &F) {
BasicBlock::iterator CallI =
std::prev(BB->getTerminator()->getIterator());
auto *CI = cast<CallInst>(&*CallI);
changeToUnreachable(CI, /*UseLLVMTrap=*/false);
changeToUnreachable(CI);
} else {
changeToUnreachable(&I, /*UseLLVMTrap=*/false);
changeToUnreachable(&I);
}
// There are no more instructions in the block (except for unreachable),
@ -986,7 +986,7 @@ void WinEHPrepare::removeImplausibleInstructions(Function &F) {
IsUnreachableCleanupret = CRI->getCleanupPad() != CleanupPad;
if (IsUnreachableRet || IsUnreachableCatchret ||
IsUnreachableCleanupret) {
changeToUnreachable(TI, /*UseLLVMTrap=*/false);
changeToUnreachable(TI);
} else if (isa<InvokeInst>(TI)) {
if (Personality == EHPersonality::MSVC_CXX && CleanupPad) {
// Invokes within a cleanuppad for the MSVC++ personality never

2
lib/Transforms/IPO/PruneEH.cpp
View File

@ -258,7 +258,7 @@ void PruneEH::DeleteBasicBlock(BasicBlock *BB) {
if (TokenInst) {
if (!isa<TerminatorInst>(TokenInst))
changeToUnreachable(TokenInst->getNextNode(), /*UseLLVMTrap=*/false);
changeToUnreachable(TokenInst->getNextNode());
} else {
// Get the list of successors of this block.
std::vector<BasicBlock *> Succs(succ_begin(BB), succ_end(BB));

3
lib/Transforms/Scalar/SCCP.cpp
View File

@ -1802,8 +1802,7 @@ static bool runIPSCCP(Module &M, const DataLayout &DL,
DEBUG(dbgs() << " BasicBlock Dead:" << *BB);
++NumDeadBlocks;
NumInstRemoved +=
changeToUnreachable(BB->getFirstNonPHI(), /*UseLLVMTrap=*/false);
NumInstRemoved += changeToUnreachable(BB->getFirstNonPHI());
MadeChanges = true;

2
lib/Transforms/Utils/InlineFunction.cpp
View File

@ -1833,7 +1833,7 @@ bool llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
// As such, we replace the cleanupret with unreachable.
if (auto *CleanupRet = dyn_cast<CleanupReturnInst>(BB->getTerminator()))
if (CleanupRet->unwindsToCaller() && EHPadForCallUnwindsLocally)
changeToUnreachable(CleanupRet, /*UseLLVMTrap=*/false);
changeToUnreachable(CleanupRet);
Instruction *I = BB->getFirstNonPHI();
if (!I->isEHPad())

60
lib/Transforms/Utils/Local.cpp
View File

@ -42,11 +42,13 @@
#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/PatternMatch.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
using namespace llvm;
using namespace llvm::PatternMatch;
#define DEBUG_TYPE "local"
@ -1310,21 +1312,13 @@ unsigned llvm::removeAllNonTerminatorAndEHPadInstructions(BasicBlock *BB) {
return NumDeadInst;
}
unsigned llvm::changeToUnreachable(Instruction *I, bool UseLLVMTrap) {
unsigned llvm::changeToUnreachable(Instruction *I) {
BasicBlock *BB = I->getParent();
// Loop over all of the successors, removing BB's entry from any PHI
// nodes.
for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
(*SI)->removePredecessor(BB);
for (BasicBlock *Succ : successors(BB))
Succ->removePredecessor(BB);
// Insert a call to llvm.trap right before this. This turns the undefined
// behavior into a hard fail instead of falling through into random code.
if (UseLLVMTrap) {
Function *TrapFn =
Intrinsic::getDeclaration(BB->getParent()->getParent(), Intrinsic::trap);
CallInst *CallTrap = CallInst::Create(TrapFn, "", I);
CallTrap->setDebugLoc(I->getDebugLoc());
}
new UnreachableInst(I->getContext(), I);
// All instructions after this are dead.
@ -1374,22 +1368,14 @@ static bool markAliveBlocks(Function &F,
// Do a quick scan of the basic block, turning any obviously unreachable
// instructions into LLVM unreachable insts. The instruction combining pass
// canonicalizes unreachable insts into stores to null or undef.
for (BasicBlock::iterator BBI = BB->begin(), E = BB->end(); BBI != E;++BBI){
for (Instruction &I : *BB) {
// Assumptions that are known to be false are equivalent to unreachable.
// Also, if the condition is undefined, then we make the choice most
// beneficial to the optimizer, and choose that to also be unreachable.
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(BBI)) {
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(&I)) {
if (II->getIntrinsicID() == Intrinsic::assume) {
bool MakeUnreachable = false;
if (isa<UndefValue>(II->getArgOperand(0)))
MakeUnreachable = true;
else if (ConstantInt *Cond =
dyn_cast<ConstantInt>(II->getArgOperand(0)))
MakeUnreachable = Cond->isZero();
if (MakeUnreachable) {
// Don't insert a call to llvm.trap right before the unreachable.
changeToUnreachable(&*BBI, false);
if (match(II->getArgOperand(0), m_CombineOr(m_Zero(), m_Undef()))) {
changeToUnreachable(II);
Changed = true;
break;
}
@ -1404,19 +1390,19 @@ static bool markAliveBlocks(Function &F,
// Note: unlike in llvm.assume, it is not "obviously profitable" for
// guards to treat `undef` as `false` since a guard on `undef` can
// still be useful for widening.
if (auto *CI = dyn_cast<ConstantInt>(II->getArgOperand(0)))
if (CI->isZero() && !isa<UnreachableInst>(II->getNextNode())) {
changeToUnreachable(II->getNextNode(), /*UseLLVMTrap=*/ false);
if (match(II->getArgOperand(0), m_Zero()))
if (!isa<UnreachableInst>(II->getNextNode())) {
changeToUnreachable(II->getNextNode());
Changed = true;
break;
}
}
}
if (CallInst *CI = dyn_cast<CallInst>(BBI)) {
if (auto *CI = dyn_cast<CallInst>(&I)) {
Value *Callee = CI->getCalledValue();
if (isa<ConstantPointerNull>(Callee) || isa<UndefValue>(Callee)) {
changeToUnreachable(CI, /*UseLLVMTrap=*/false);
changeToUnreachable(CI);
Changed = true;
break;
}
@ -1424,10 +1410,8 @@ static bool markAliveBlocks(Function &F,
// If we found a call to a no-return function, insert an unreachable
// instruction after it. Make sure there isn't *already* one there
// though.
++BBI;
if (!isa<UnreachableInst>(BBI)) {
// Don't insert a call to llvm.trap right before the unreachable.
changeToUnreachable(&*BBI, false);
if (!isa<UnreachableInst>(CI->getNextNode())) {
changeToUnreachable(CI->getNextNode());
Changed = true;
}
break;
@ -1437,7 +1421,7 @@ static bool markAliveBlocks(Function &F,
// Store to undef and store to null are undefined and used to signal that
// they should be changed to unreachable by passes that can't modify the
// CFG.
if (StoreInst *SI = dyn_cast<StoreInst>(BBI)) {
if (auto *SI = dyn_cast<StoreInst>(&I)) {
// Don't touch volatile stores.
if (SI->isVolatile()) continue;
@ -1446,7 +1430,7 @@ static bool markAliveBlocks(Function &F,
if (isa<UndefValue>(Ptr) ||
(isa<ConstantPointerNull>(Ptr) &&
SI->getPointerAddressSpace() == 0)) {
changeToUnreachable(SI, true);
changeToUnreachable(SI);
Changed = true;
break;
}
@ -1458,7 +1442,7 @@ static bool markAliveBlocks(Function &F,
// Turn invokes that call 'nounwind' functions into ordinary calls.
Value *Callee = II->getCalledValue();
if (isa<ConstantPointerNull>(Callee) || isa<UndefValue>(Callee)) {
changeToUnreachable(II, true);
changeToUnreachable(II);
Changed = true;
} else if (II->doesNotThrow() && canSimplifyInvokeNoUnwind(&F)) {
if (II->use_empty() && II->onlyReadsMemory()) {
@ -1511,9 +1495,9 @@ static bool markAliveBlocks(Function &F,
}
Changed |= ConstantFoldTerminator(BB, true);
for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
if (Reachable.insert(*SI).second)
Worklist.push_back(*SI);
for (BasicBlock *Succ : successors(BB))
if (Reachable.insert(Succ).second)
Worklist.push_back(Succ);
} while (!Worklist.empty());
return Changed;
}

2
lib/Transforms/Utils/LoopSimplify.cpp
View File

@ -491,7 +491,7 @@ ReprocessLoop:
// Zap the dead pred's terminator and replace it with unreachable.
TerminatorInst *TI = P->getTerminator();
changeToUnreachable(TI, /*UseLLVMTrap=*/false);
changeToUnreachable(TI);
Changed = true;
}
}

2
test/Transforms/SimplifyCFG/invoke.ll
View File

@ -12,7 +12,6 @@ declare i32 @fn()
; CHECK-LABEL: @f1(
define i8* @f1() nounwind uwtable ssp personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
entry:
; CHECK: call void @llvm.trap()
; CHECK: unreachable
%call = invoke noalias i8* undef()
to label %invoke.cont unwind label %lpad
@ -31,7 +30,6 @@ lpad:
; CHECK-LABEL: @f2(
define i8* @f2() nounwind uwtable ssp personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
entry:
; CHECK: call void @llvm.trap()
; CHECK: unreachable
%call = invoke noalias i8* null()
to label %invoke.cont unwind label %lpad

22
test/Transforms/SimplifyCFG/trap-debugloc.ll
View File

@ -1,22 +0,0 @@
; RUN: opt -S -simplifycfg < %s | FileCheck %s
; Radar 9342286
; Assign DebugLoc to trap instruction.
define void @foo() nounwind ssp !dbg !0 {
; CHECK: call void @llvm.trap(), !dbg
store i32 42, i32* null, !dbg !5
ret void, !dbg !7
}
!llvm.dbg.cu = !{!2}
!llvm.module.flags = !{!10}
!0 = distinct !DISubprogram(name: "foo", line: 3, isLocal: false, isDefinition: true, virtualIndex: 6, isOptimized: false, unit: !2, file: !8, scope: !1, type: !3)
!1 = !DIFile(filename: "foo.c", directory: "/private/tmp")
!2 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "Apple clang version 3.0 (tags/Apple/clang-206.1) (based on LLVM 3.0svn)", isOptimized: true, emissionKind: FullDebug, file: !8, enums: !{}, retainedTypes: !{})
!3 = !DISubroutineType(types: !4)
!4 = !{null}
!5 = !DILocation(line: 4, column: 2, scope: !6)
!6 = distinct !DILexicalBlock(line: 3, column: 12, file: !8, scope: !0)
!7 = !DILocation(line: 5, column: 1, scope: !6)
!8 = !DIFile(filename: "foo.c", directory: "/private/tmp")
!10 = !{i32 1, !"Debug Info Version", i32 3}

1
test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
View File

@ -28,7 +28,6 @@ entry:
ret void
; CHECK-LABEL: @test2(
; CHECK: call void @llvm.trap
; CHECK: unreachable
}