From 1978426a94e3d3b53f488ffc56516e6e87906a0b Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Thu, 7 May 2009 18:08:34 +0000 Subject: [PATCH] Revert r70876 and add a testcase (@c7) showing the problem: bits captured, but the pointer marked nocapture. In fact I now recall that this problem is why only readnone functions returning void were considered before! However keep a small fix that was also in r70876: a readnone function returning void can result in bits being captured if it unwinds, so test for this. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@71168 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/CaptureTracking.cpp | 77 ++++++++----------- .../FunctionAttrs/2008-12-31-NoCapture.ll | 27 ++++--- 2 files changed, 48 insertions(+), 56 deletions(-) diff --git a/lib/Analysis/CaptureTracking.cpp b/lib/Analysis/CaptureTracking.cpp index 773f53d2cd2..a19b8e4f94d 100644 --- a/lib/Analysis/CaptureTracking.cpp +++ b/lib/Analysis/CaptureTracking.cpp @@ -49,7 +49,13 @@ bool llvm::PointerMayBeCaptured(const Value *V, bool ReturnCaptures) { switch (I->getOpcode()) { case Instruction::Call: case Instruction::Invoke: { - CallSite CS(I); + CallSite CS = CallSite::get(I); + // Not captured if the callee is readonly, doesn't return a copy through + // its return value and doesn't unwind (a readonly function can leak bits + // by throwing an exception or not depending on the input value). + if (CS.onlyReadsMemory() && CS.doesNotThrow() && + I->getType() == Type::VoidTy) + break; // Not captured if only passed via 'nocapture' arguments. Note that // calling a function pointer does not in itself cause the pointer to @@ -58,69 +64,46 @@ bool llvm::PointerMayBeCaptured(const Value *V, bool ReturnCaptures) { // that loading a value from a pointer does not cause the pointer to be // captured, even though the loaded value might be the pointer itself // (think of self-referential objects). - bool MayBeCaptured = false; CallSite::arg_iterator B = CS.arg_begin(), E = CS.arg_end(); for (CallSite::arg_iterator A = B; A != E; ++A) - if (A->get() == V && !CS.paramHasAttr(A-B+1, Attribute::NoCapture)) { - // The parameter is not marked 'nocapture' - handled by generic code - // below. - MayBeCaptured = true; - break; - } - if (!MayBeCaptured) - // Only passed via 'nocapture' arguments, or is the called function - - // not captured. - continue; - if (!CS.doesNotThrow()) - // Even a readonly function can leak bits by throwing an exception or - // not depending on the input value. - return true; - // Fall through to the generic code. + if (A->get() == V && !CS.paramHasAttr(A - B + 1, Attribute::NoCapture)) + // The parameter is not marked 'nocapture' - captured. + return true; + // Only passed via 'nocapture' arguments, or is the called function - not + // captured. break; } case Instruction::Free: // Freeing a pointer does not cause it to be captured. - continue; + break; case Instruction::Load: // Loading from a pointer does not cause it to be captured. - continue; + break; case Instruction::Ret: if (ReturnCaptures) return true; - continue; + break; case Instruction::Store: if (V == I->getOperand(0)) // Stored the pointer - it may be captured. return true; // Storing to the pointee does not cause the pointer to be captured. - continue; - } - - // If it may write to memory and isn't one of the special cases above, - // be conservative and assume the pointer is captured. - if (I->mayWriteToMemory()) + break; + case Instruction::BitCast: + case Instruction::GetElementPtr: + case Instruction::PHI: + case Instruction::Select: + // The original value is not captured via this if the new value isn't. + for (Instruction::use_iterator UI = I->use_begin(), UE = I->use_end(); + UI != UE; ++UI) { + Use *U = &UI.getUse(); + if (Visited.insert(U)) + Worklist.push_back(U); + } + break; + default: + // Something else - be conservative and say it is captured. return true; - - // If the instruction doesn't write memory, it can only capture by - // having its own value depend on the input value. - const Type* Ty = I->getType(); - if (Ty == Type::VoidTy) - // The value of an instruction can't be a copy if it can't contain any - // information. - continue; - if (!isa(Ty)) - // At the moment, we don't track non-pointer values, so be conservative - // and assume the pointer is captured. - // FIXME: Track these too. This would need to be done very carefully as - // it is easy to leak bits via control flow if integer values are allowed. - return true; - - // The original value is not captured via this if the new value isn't. - for (Instruction::use_iterator UI = I->use_begin(), UE = I->use_end(); - UI != UE; ++UI) { - Use *U = &UI.getUse(); - if (Visited.insert(U)) - Worklist.push_back(U); } } diff --git a/test/Transforms/FunctionAttrs/2008-12-31-NoCapture.ll b/test/Transforms/FunctionAttrs/2008-12-31-NoCapture.ll index aec134a8c7f..39a64e6d36c 100644 --- a/test/Transforms/FunctionAttrs/2008-12-31-NoCapture.ll +++ b/test/Transforms/FunctionAttrs/2008-12-31-NoCapture.ll @@ -49,6 +49,21 @@ ret1: ret i1 1 } +define i1* @lookup_bit(i32* %q, i32 %bitno) readnone nounwind { + %tmp = ptrtoint i32* %q to i32 + %tmp2 = lshr i32 %tmp, %bitno + %bit = and i32 %tmp2, 1 + %lookup = getelementptr [2 x i1]* @lookup_table, i32 0, i32 %bit + ret i1* %lookup +} + +define i1 @c7(i32* %q, i32 %bitno) { + %ptr = call i1* @lookup_bit(i32* %q, i32 %bitno) + %val = load i1* %ptr + ret i1 %val +} + + define i32 @nc1(i32* %q, i32* %p, i1 %b) { e: br label %l @@ -79,14 +94,8 @@ define void @nc4(i8* %p) { ret void } -define void @nc5(void (i8*)* %p, i8* %r) { - call void %p(i8* %r) - call void %p(i8* nocapture %r) - ret void -} - -declare i8* @external_identity(i8*) readonly nounwind -define void @nc6(i8* %p) { - call i8* @external_identity(i8* %p) +define void @nc5(void (i8*)* %f, i8* %p) { + call void %f(i8* %p) readonly nounwind + call void %f(i8* nocapture %p) ret void }