1
0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 20:51:52 +01:00

[FunctionAttrs] Fix an iterator wraparound bug

Summary:
This change fixes an iterator wraparound bug in
`determinePointerReadAttrs`.

Ideally, ++'ing off the `end()` of an iplist should result in a failed
assert, but currently iplist seems to silently wrap to the head of the
list on `end()++`.  This is why the bad behavior is difficult to
demonstrate.

Reviewers: chandlerc, reames

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D14350

llvm-svn: 252386
This commit is contained in:
Sanjoy Das 2015-11-07 01:55:53 +00:00
parent 6a3a04f00f
commit 9dbcbc0397
2 changed files with 49 additions and 18 deletions

View File

@ -394,7 +394,6 @@ determinePointerReadAttrs(Argument *A,
while (!Worklist.empty()) {
Use *U = Worklist.pop_back_val();
Instruction *I = cast<Instruction>(U->getUser());
Value *V = U->get();
switch (I->getOpcode()) {
case Instruction::BitCast:
@ -438,24 +437,26 @@ determinePointerReadAttrs(Argument *A,
return Attribute::None;
}
Function::arg_iterator AI = F->arg_begin(), AE = F->arg_end();
CallSite::arg_iterator B = CS.arg_begin(), E = CS.arg_end();
for (CallSite::arg_iterator A = B; A != E; ++A, ++AI) {
if (A->get() == V) {
if (AI == AE) {
assert(F->isVarArg() &&
"More params than args in non-varargs call.");
return Attribute::None;
}
Captures &= !CS.doesNotCapture(A - B);
if (SCCNodes.count(&*AI))
continue;
if (!CS.onlyReadsMemory() && !CS.onlyReadsMemory(A - B))
return Attribute::None;
if (!CS.doesNotAccessMemory(A - B))
IsRead = true;
}
// Note: the callee and the two successor blocks *follow* the argument
// operands. This means there is no need to adjust UseIndex to account
// for these.
unsigned UseIndex = std::distance(CS.arg_begin(), U);
assert(UseIndex < CS.arg_size() && "Non-argument use?");
if (UseIndex >= F->arg_size()) {
assert(F->isVarArg() && "More params than args in non-varargs call");
return Attribute::None;
}
Captures &= !CS.doesNotCapture(UseIndex);
if (!SCCNodes.count(std::next(F->arg_begin(), UseIndex))) {
if (!CS.onlyReadsMemory() && !CS.onlyReadsMemory(UseIndex))
return Attribute::None;
if (!CS.doesNotAccessMemory(UseIndex))
IsRead = true;
}
AddUsersToWorklistIfCapturing();
break;
}

View File

@ -0,0 +1,30 @@
; RUN: opt -functionattrs -S < %s | FileCheck %s
; This checks for an iterator wraparound bug in FunctionAttrs. The previous
; "incorrect" behavior was inferring readonly for the %x argument in @caller.
; Inferring readonly for %x *is* actually correct, since @va_func is marked
; readonly, but FunctionAttrs was inferring readonly for the wrong reasons (and
; we _need_ the readonly on @va_func to trigger the problematic code path). It
; is possible that in the future FunctionAttrs becomes smart enough to infer
; readonly for %x for the right reasons, and at that point this test will have
; to be marked invalid.
declare void @llvm.va_start(i8*)
declare void @llvm.va_end(i8*)
define void @va_func(i32* readonly %b, ...) readonly nounwind {
; CHECK-LABEL: define void @va_func(i32* nocapture readonly %b, ...)
entry:
%valist = alloca i8
call void @llvm.va_start(i8* %valist)
call void @llvm.va_end(i8* %valist)
%x = call i32 @caller(i32* %b)
ret void
}
define i32 @caller(i32* %x) {
; CHECK-LABEL: define i32 @caller(i32* nocapture %x)
entry:
call void(i32*,...) @va_func(i32* null, i32 0, i32 0, i32 0, i32* %x)
ret i32 42
}