CFLAA Bugfix -- Tag untrackable pointers (inttoptr/ptrtoint)

[george.burgess.iv]

Summary

// Hal already LGTM'ed this for style, I'm putting it here so any discussions about design are published, as requested. :)

This patch notes that pointers passed to inttoptr/ptrtoint can escape.

• Used StratifiedAttrs instead of one-way edges because StratifiedAttrs don't union everything (the advantage of one-way edges), and we get propagation of the Unknown attribute to sets "below" the Unknown one for free.

• Explicitly omitted from this patch: special handling of Unknown StratifiedAttrs. An Unknown attribute on some set only forces a MayAlias if the set it's being compared to has any StratifiedAttrs set (interacts with an argument not marked as noalias, a global, or inttoptr/ptrtoint). I can't see how this can be broken, but feedback is appreciated.

[majnemer]

lib/Analysis/CFLAliasAnalysis.cpp

269–274

Please bind pointer tokens to the variable names.

[george.burgess.iv]

Addressed David's feedback. Thanks for catching that :)

[• dberlin]

LGTM algorithmically

[george.burgess.iv]

r231741

lib/Analysis/CFLAliasAnalysis.cpp

const StratifiedIndex StratifiedLink::SetSentinel =
   std::numeric_limits<StratifiedIndex>::max();
 
 namespace {
 // StratifiedInfo Attribute things.
 typedef unsigned StratifiedAttr;
 LLVM_CONSTEXPR unsigned MaxStratifiedAttrIndex = NumStratifiedAttrs;
 LLVM_CONSTEXPR unsigned AttrAllIndex = 0;
 LLVM_CONSTEXPR unsigned AttrGlobalIndex = 1;
-LLVM_CONSTEXPR unsigned AttrFirstArgIndex = 2;
+LLVM_CONSTEXPR unsigned AttrUnknownIndex = 2;
+LLVM_CONSTEXPR unsigned AttrFirstArgIndex = 3;
 LLVM_CONSTEXPR unsigned AttrLastArgIndex = MaxStratifiedAttrIndex;
 LLVM_CONSTEXPR unsigned AttrMaxNumArgs = AttrLastArgIndex - AttrFirstArgIndex;
 
 LLVM_CONSTEXPR StratifiedAttr AttrNone = 0;
+LLVM_CONSTEXPR StratifiedAttr AttrUnknown = 1 << AttrUnknownIndex;
 LLVM_CONSTEXPR StratifiedAttr AttrAll = ~AttrNone;
 
 // \brief StratifiedSets call for knowledge of "direction", so this is how we
 // represent that locally.
 enum class Level { Same, Above, Below };
 
 // \brief Edges can be one of four "weights" -- each weight must have an inverse
 // weight (Assign has Assign; Reference has Dereference).

 public:
   GetEdgesVisitor(CFLAliasAnalysis &AA, SmallVectorImpl<Edge> &Output)
       : AA(AA), Output(Output) {}
 
   void visitInstruction(Instruction &) {
     llvm_unreachable("Unsupported instruction encountered");
   }
 
+  void visitPtrToIntInst(PtrToIntInst &Inst) {
+    auto *Ptr = Inst.getOperand(0);
+    Output.push_back(Edge(Ptr, Ptr, EdgeType::Assign, AttrUnknown));
+  }
+
+  void visitIntToPtrInst(IntToPtrInst &Inst) {
+    auto *Ptr = &Inst;
+    Output.push_back(Edge(Ptr, Ptr, EdgeType::Assign, AttrUnknown));
+  }
+
   void visitCastInst(CastInst &Inst) {
     Output.push_back(Edge(&Inst, Inst.getOperand(0), EdgeType::Assign,
                           AttrNone));
   }
 
   void visitBinaryOperator(BinaryOperator &Inst) {
     auto *Op1 = Inst.getOperand(0);
     auto *Op2 = Inst.getOperand(1);
while (!Worklist.empty()) {
         case Level::Below:
           Added = Builder.addBelow(CurValue, OtherValue);
           break;
         case Level::Same:
           Added = Builder.addWith(CurValue, OtherValue);
           break;
         }
 
-        if (Added) {
-          auto Aliasing = Weight.second;
-          if (auto MaybeCurIndex = valueToAttrIndex(CurValue))
-            Aliasing.set(*MaybeCurIndex);
-          if (auto MaybeOtherIndex = valueToAttrIndex(OtherValue))
-            Aliasing.set(*MaybeOtherIndex);
-          Builder.noteAttributes(CurValue, Aliasing);
-          Builder.noteAttributes(OtherValue, Aliasing);
+
+        if (Added)
           Worklist.push_back(OtherNode);
-        }
-      }
-    }
-  }
 
   // There are times when we end up with parameters not in our graph (i.e. if
   // it's only used as the condition of a branch). Other bits of code depend on
   // things that were present during construction being present in the graph.
   // So, we add all present arguments here.
   for (auto &Arg : Fn->args()) {
     Builder.add(&Arg);
   }

test/Analysis/CFLAliasAnalysis/branch-alias.ll

+; Makes sure that we give up on some pathological cases with inttoptr/ptrtoint
+;
+; @ptr_test was generated from the following C code:
+;  void ptr_test() {
+;    int* A;
+;    unsigned long RefCopy = 0;
+;    for (int i = 0; i < 8*sizeof(&A); ++i) {
+;      if ((unsigned long)&A & (1UL << i))
+;        RefCopy |= 1UL << i;
+;    }
+;
+;    int** AliasA1 = (int**)RefCopy;
+;    int* ShouldAliasA = *AliasA1;
+;  }
+
+; RUN: opt < %s -cfl-aa -aa-eval -print-may-aliases -disable-output 2>&1 | FileCheck %s
+
+; CHECK: Function: ptr_test
+define void @ptr_test() #0 {
+  ; CHECK: MayAlias: i32** %A, i32** %ShouldAliasA
+  ; CHECK-NOT: %AliasA1
+entry:
+  %A = alloca i32*, align 8
+  %RefCopy = alloca i64, align 8
+  %i = alloca i32, align 4
+  %AliasA1 = alloca i32**, align 8
+  %ShouldAliasA = alloca i32*, align 8
+  store i64 0, i64* %RefCopy, align 8
+  store i32 0, i32* %i, align 4
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.inc, %entry
+  %0 = load i32, i32* %i, align 4
+  %conv = sext i32 %0 to i64
+  %cmp = icmp ult i64 %conv, 64
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %1 = ptrtoint i32** %A to i64
+  %2 = load i32, i32* %i, align 4
+  %sh_prom = zext i32 %2 to i64
+  %shl = shl i64 1, %sh_prom
+  %and = and i64 %1, %shl
+  %tobool = icmp ne i64 %and, 0
+  br i1 %tobool, label %if.then, label %if.end
+
+if.then:                                          ; preds = %for.body
+  %3 = load i32, i32* %i, align 4
+  %sh_prom2 = zext i32 %3 to i64
+  %shl3 = shl i64 1, %sh_prom2
+  %4 = load i64, i64* %RefCopy, align 8
+  %or = or i64 %4, %shl3
+  store i64 %or, i64* %RefCopy, align 8
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %for.body
+  br label %for.inc
+
+for.inc:                                          ; preds = %if.end
+  %5 = load i32, i32* %i, align 4
+  %inc = add nsw i32 %5, 1
+  store i32 %inc, i32* %i, align 4
+  br label %for.cond
+
+for.end:                                          ; preds = %for.cond
+  %6 = load i64, i64* %RefCopy, align 8
+  %7 = inttoptr i64 %6 to i32**
+  store i32** %7, i32*** %AliasA1, align 8
+  %8 = load i32**, i32*** %AliasA1, align 8
+  %9 = load i32*, i32** %8, align 8
+  store i32* %9, i32** %ShouldAliasA, align 8
+  ret void
+}