[PATCH] D14350: [FunctionAttrs] Fix an iterator wraparound bug

[Sanjoy Das via llvm-commits]

sanjoy created this revision.
sanjoy added reviewers: chandlerc, reames.
sanjoy added a subscriber: llvm-commits.

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.

http://reviews.llvm.org/D14350

Files:
  lib/Transforms/IPO/FunctionAttrs.cpp
  test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll

Index: test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
===================================================================
--- /dev/null
+++ test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
@@ -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
+}
Index: lib/Transforms/IPO/FunctionAttrs.cpp
===================================================================
--- lib/Transforms/IPO/FunctionAttrs.cpp
+++ lib/Transforms/IPO/FunctionAttrs.cpp
@@ -442,9 +442,11 @@
 
       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) {
+      bool InVarArgSection = false;
+      for (CallSite::arg_iterator A = B; A != E; ++A) {
+        InVarArgSection |= (AI == AE);
         if (A->get() == V) {
-          if (AI == AE) {
+          if (InVarArgSection) {
             assert(F->isVarArg() &&
                    "More params than args in non-varargs call.");
             return Attribute::None;
@@ -457,6 +459,8 @@
           if (!CS.doesNotAccessMemory(A - B))
             IsRead = true;
         }
+        if (!InVarArgSection)
+          ++AI;
       }
       AddUsersToWorklistIfCapturing();
       break;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14350.39260.patch
Type: text/x-patch
Size: 2414 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151104/8b54e48e/attachment.bin>