Skip to content

Commit 91beab6

Browse files
committed
Revert "Recommit DwarfEHPrepare: insert extra unwind paths for stack protector to instrument"
This broke Objective-C autorelease / retainAutoreleasedReturnValue, see comments on the code review. > This is a mitigation patch for > https://bugs.chromium.org/p/llvm/issues/detail?id=30, where existing stack > protection is skipped if a function is returned through by an unwinder rather > than the normal call/return path. The recent patch D139254 added the ability to > instrument a visible unwind path, at least in the IR case (I'm working on the > SelectionDAG instrumentation too) but there are still invisible unwinds it > can't reach. > > So this patch adds logic to DwarfEHPrepare that goes through a function, > converting any call that might throw into an invoke to a simple resume cleanup, > and adding cleanup clauses to existing landingpads that lack them. Obviously we > don't really want to do this if it's wasted effort, so I also exposed > requiresStackProtector from the actual StackProtector code to skip the extra > paths if they won't be used. > > Changes: > * Move test to AArch64 directory as it relies on target presence. > * Re-add Dominator-tree maintenance. Accidentally cherry-picked wrong patch. > * Skip adding paths on Windows EH functions. > > https://reviews.llvm.org/D143637 This reverts commit 2d69068.
1 parent a7bb8e2 commit 91beab6

File tree

2 files changed

+0
-272
lines changed

2 files changed

+0
-272
lines changed

llvm/lib/CodeGen/DwarfEHPrepare.cpp

Lines changed: 0 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
#include "llvm/Analysis/DomTreeUpdater.h"
1919
#include "llvm/Analysis/TargetTransformInfo.h"
2020
#include "llvm/CodeGen/RuntimeLibcalls.h"
21-
#include "llvm/CodeGen/StackProtector.h"
2221
#include "llvm/CodeGen/TargetLowering.h"
2322
#include "llvm/CodeGen/TargetPassConfig.h"
2423
#include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -29,7 +28,6 @@
2928
#include "llvm/IR/Dominators.h"
3029
#include "llvm/IR/EHPersonalities.h"
3130
#include "llvm/IR/Function.h"
32-
#include "llvm/IR/IRBuilder.h"
3331
#include "llvm/IR/Instructions.h"
3432
#include "llvm/IR/Module.h"
3533
#include "llvm/IR/Type.h"
@@ -39,7 +37,6 @@
3937
#include "llvm/Target/TargetMachine.h"
4038
#include "llvm/TargetParser/Triple.h"
4139
#include "llvm/Transforms/Utils/Local.h"
42-
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
4340
#include <cstddef>
4441

4542
using namespace llvm;
@@ -169,136 +166,7 @@ size_t DwarfEHPrepare::pruneUnreachableResumes(
169166
return ResumesLeft;
170167
}
171168

172-
/// If a landingpad block doesn't already have a cleanup case, add one
173-
/// that feeds directly into a resume instruction.
174-
static void addCleanupResumeToLandingPad(BasicBlock &BB, DomTreeUpdater *DTU) {
175-
LandingPadInst *LP = BB.getLandingPadInst();
176-
if (LP->isCleanup())
177-
return;
178-
179-
// There will usually be code testing for the other kinds of exception
180-
// immediately after the landingpad. Working out the far end of that chain is
181-
// tricky, so put our test for the new cleanup case (i.e. selector == 0) at
182-
// the beginning.
183-
BasicBlock *ContBB = SplitBlock(&BB, LP->getNextNode(), DTU);
184-
BB.getTerminator()->eraseFromParent();
185-
186-
LP->setCleanup(true);
187-
IRBuilder<> B(&BB);
188-
Value *Selector = B.CreateExtractValue(LP, 1);
189-
Value *Cmp = B.CreateICmpEQ(Selector, ConstantInt::get(Selector->getType(), 0));
190-
191-
Function *F = BB.getParent();
192-
LLVMContext &Ctx = F->getContext();
193-
BasicBlock *ResumeBB = BasicBlock::Create(Ctx, "resume", F);
194-
ResumeInst::Create(LP, ResumeBB);
195-
196-
B.CreateCondBr(Cmp, ResumeBB, ContBB);
197-
if (DTU) {
198-
SmallVector<DominatorTree::UpdateType> Updates;
199-
Updates.push_back({DominatorTree::Insert, &BB, ResumeBB});
200-
DTU->applyUpdates(Updates);
201-
}
202-
}
203-
204-
/// Create a basic block that has a `landingpad` instruction feeding
205-
/// directly into a `resume`. Will be set to the unwind destination of a new
206-
/// invoke.
207-
static BasicBlock *createCleanupResumeBB(Function &F, Type *LandingPadTy) {
208-
LLVMContext &Ctx = F.getContext();
209-
BasicBlock *BB = BasicBlock::Create(Ctx, "cleanup_resume", &F);
210-
IRBuilder<> B(BB);
211-
212-
// If this is going to be the only landingpad in the function, synthesize the
213-
// standard type all ABIs use, which is essentially `{ ptr, i32 }`.
214-
if (!LandingPadTy)
215-
LandingPadTy =
216-
StructType::get(Type::getInt8PtrTy(Ctx), IntegerType::get(Ctx, 32));
217-
218-
LandingPadInst *Except = B.CreateLandingPad(LandingPadTy, 0);
219-
Except->setCleanup(true);
220-
B.CreateResume(Except);
221-
return BB;
222-
}
223-
224-
/// Convert a call that might throw into an invoke that unwinds to the specified
225-
/// simple landingpad/resume block.
226-
static void changeCallToInvokeResume(CallInst &CI, BasicBlock *CleanupResumeBB,
227-
DomTreeUpdater *DTU) {
228-
BasicBlock *BB = CI.getParent();
229-
BasicBlock *ContBB = SplitBlock(BB, &CI, DTU);
230-
BB->getTerminator()->eraseFromParent();
231-
232-
IRBuilder<> B(BB);
233-
SmallVector<Value *> Args(CI.args());
234-
SmallVector<OperandBundleDef> Bundles;
235-
CI.getOperandBundlesAsDefs(Bundles);
236-
InvokeInst *NewCall =
237-
B.CreateInvoke(CI.getFunctionType(), CI.getCalledOperand(), ContBB,
238-
CleanupResumeBB, Args, Bundles, CI.getName());
239-
NewCall->setAttributes(CI.getAttributes());
240-
NewCall->setCallingConv(CI.getCallingConv());
241-
NewCall->copyMetadata(CI);
242-
243-
if (DTU) {
244-
SmallVector<DominatorTree::UpdateType> Updates;
245-
Updates.push_back({DominatorTree::Insert, BB, CleanupResumeBB});
246-
DTU->applyUpdates(Updates);
247-
}
248-
CI.replaceAllUsesWith(NewCall);
249-
CI.eraseFromParent();
250-
}
251-
252-
/// Ensure that any call in this function that might throw has an associated
253-
/// cleanup/resume that the stack protector can instrument later. Existing
254-
/// invokes will get an added `cleanup` clause if needed, calls will be
255-
/// converted to an invoke with trivial unwind followup.
256-
static void addCleanupPathsForStackProtector(Function &F, DomTreeUpdater *DTU) {
257-
// First add cleanup -> resume paths to all existing landingpads, noting what
258-
// type landingpads in this function actually have along the way.
259-
Type *LandingPadTy = nullptr;
260-
for (Function::iterator FI = F.begin(); FI != F.end(); ++FI) {
261-
BasicBlock &BB = *FI;
262-
if (LandingPadInst *LP = BB.getLandingPadInst()) {
263-
// We can assume the type is broadly compatible with { ptr, i32 } since
264-
// other parts of this pass already try to extract values from it.
265-
LandingPadTy = LP->getType();
266-
addCleanupResumeToLandingPad(BB, DTU);
267-
}
268-
}
269-
270-
// Next convert any call that might throw into an invoke to a resume
271-
// instruction for later instrumentation.
272-
BasicBlock *CleanupResumeBB = nullptr;
273-
for (Function::iterator FI = F.begin(); FI != F.end(); ++FI) {
274-
BasicBlock &BB = *FI;
275-
for (Instruction &I : BB) {
276-
CallInst *CI = dyn_cast<CallInst>(&I);
277-
if (!CI || CI->doesNotThrow())
278-
continue;
279-
280-
// Tail calls cannot use our stack so no need to check whether it was
281-
// corrupted.
282-
if (CI->isTailCall())
283-
continue;
284-
285-
if (!CleanupResumeBB)
286-
CleanupResumeBB = createCleanupResumeBB(F, LandingPadTy);
287-
288-
changeCallToInvokeResume(*CI, CleanupResumeBB, DTU);
289-
290-
// This block has been split, start again on its continuation.
291-
break;
292-
}
293-
}
294-
}
295-
296169
bool DwarfEHPrepare::InsertUnwindResumeCalls() {
297-
if (F.hasPersonalityFn() &&
298-
!isScopedEHPersonality(classifyEHPersonality(F.getPersonalityFn())) &&
299-
StackProtector::requiresStackProtector(&F, nullptr))
300-
addCleanupPathsForStackProtector(F, DTU);
301-
302170
SmallVector<ResumeInst *, 16> Resumes;
303171
SmallVector<LandingPadInst *, 16> CleanupLPads;
304172
if (F.doesNotThrow())

llvm/test/CodeGen/AArch64/safestack-unwind.ll

Lines changed: 0 additions & 140 deletions
This file was deleted.

0 commit comments

Comments
 (0)