Jump Theading/GVN bug

[Romanova, Katya]

Hello,

I encountered a problem triggered by Jump-Threading optimization.  This pass is creating an unreachable block with an instruction that is not well formed, which then causes the subsequent GVN pass to enter an infinite loop.

Here are the details:
Consider the following small testcase. I reduced it from a huge project that was failing during LTO. Unfortunately,  I was not given any source files, only the bitcode.

-----------------------------------------------------------------------------
small.ll
----------------------------------------------------------------
%"class.ls_file::Path" = type { [1024 x i8], i8*, i8*, i8*}

; Function Attrs: nounwind sspstrong uwtable
define void @foo(%"class.ls_file::Path"* %this, i8* nocapture readonly %pPath) #0 {
  %1 = load i8* %pPath, align 1
  switch i8 %1, label %.loopexit.i.i [
    i8 92, label %2
    i8 47, label %2
  ]

; <label>:2                                       ; preds = %0, %0
  %3 = load i8* %pPath, align 1
  %4 = icmp eq i8 %3, 46
  br i1 %4, label %.critedge.i.i, label %.outer.i.i

.critedge.i.i:                                    ; preds = %.critedge.i.i, %.critedge.i.i, %2
  %.0.i.i = phi i8* [ %pPath, %2 ], [ %5, %.critedge.i.i ], [ %5, %.critedge.i.i ]
  %5 = getelementptr inbounds i8* %.0.i.i, i64 1
  %6 = load i8* %5, align 1
  switch i8 %6, label %.outer.i.i [
    i8 92, label %.critedge.i.i
    i8 47, label %.critedge.i.i
  ]

.outer.i.i:                                       ; preds = %.critedge.i.i, %2
  %.2.ph2.i.i = phi i8* [ %5, %.critedge.i.i ], [ %pPath, %2 ]
  %7 = load i8* %.2.ph2.i.i, align 1
  %8 = icmp eq i8 %7, 0
  br label %.loopexit.i.i

.loopexit.i.i:                                    ; preds = %7
  ret void
}


If you run GVN after jump threading, opt will go into infinite recursion.

opt  -jump-threading small.ll -o test2.o
opt -gvn test2.o -o test3.o
Segmentation fault (core dumped)


Here's why it's happening:

Jump threading transforms this block:

.critedge.i.i:                                    ; preds = %.critedge.i.i, %.critedge.i.i, %2
  %.0.i.i = phi i8* [ %pPath, %2 ], [ %5, %.critedge.i.i ], [ %5, %.critedge.i.i ]
  %5 = getelementptr inbounds i8* %.0.i.i, i64 1
  %6 = load i8* %5, align 1
  switch i8 %6, label %.outer.i.i [
    i8 92, label %.critedge.i.i
    i8 47, label %.critedge.i.i
  ]

into the following block:

.critedge.i.i:                                    ; preds = %.critedge.i.i, %.critedge.i.i
  %2 = getelementptr inbounds i8* %2, i64 1
  %3 = load i8* %2, align 1
  switch i8 %3, label %.outer.i.i [
    i8 92, label %.critedge.i.i
    i8 47, label %.critedge.i.i
  ]

Note that the block .critedge.i.i is now unreachable and it contains the following instruction:
%2 = getelementptr inbounds i8* %2, i64 1

This instruction doesn't appear to be well formed. However, one could argue that it is - formally %2 def does dominate all %2 uses because both are located in an unreachable block, so every path to the use is passing through the def (because there are 0 paths like that).
That is likely the reason why -verify doesn't complain about it.


And this is where the problem actually happens:
When -gvn pass is invoked afterwards, it goes into an infinite recursion in function 'PHITranslateSubExpr' (located in PHITransAddr.cpp). When processing a GEP instruction (%2), this function calls itself recursively with its first operand (which is also %2).

// Handle getelementptr with at least one PHI translatable operand.
  if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Inst)) {
    SmallVector<Value*, 8> GEPOps;
    bool AnyChanged = false;
    for (unsigned i = 0, e = GEP->getNumOperands(); i != e; ++i) {
      Value *GEPOp = PHITranslateSubExpr(GEP->getOperand(i), CurBB, PredBB, DT);                     // <- it goes into infinite recursion here.
      if (GEPOp == 0) return 0;

There are several different ways to fix this problem.


(1)                        We have to remove the unreachable code immediately after or during Jump-Threading pass;

(2)                        GVN should skip unreachable code;

(3)                        GVN shouldn't recursively invoke the same function with the same arguments.


Here is an extra check that implements the 3rd approach. Please review.

===================================================================
--- PHITransAddr.cpp    (revision 229821)
+++ PHITransAddr.cpp    (working copy)
@@ -217,10 +217,13 @@
     SmallVector<Value*, 8> GEPOps;
     bool AnyChanged = false;
     for (unsigned i = 0, e = GEP->getNumOperands(); i != e; ++i) {
-      Value *GEPOp = PHITranslateSubExpr(GEP->getOperand(i), CurBB, PredBB, DT);
+      Value *Operand= GEP->getOperand(i);
+      if (GEP==Operand)
+        continue;
+      Value *GEPOp = PHITranslateSubExpr(Operand, CurBB, PredBB, DT);
       if (!GEPOp) return nullptr;

-      AnyChanged |= GEPOp != GEP->getOperand(i);
+      AnyChanged |= GEPOp != Operand;
       GEPOps.push_back(GEPOp);
     }

===================================================================
One thing to note here is - unreachable code, if not removed, could push invalid code through subsequent passes. This invalid code will not be caught by the verifier. If  these passes are not paranoid enough, the bad code may cause all sorts of issues. The case above is one example.


Katya.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150222/905514e2/attachment.html>