Fix for segfault reported in PR 11990

[Kaelyn Takata]

Here's a patch that fixes the segfault in the verifier when trying to
determine the size of a type of the form "%rt = type { %rt }" while
checking an alloca of the type.

I suspect this is also a candidate for the release branch, as it is a
crasher that has apparently existed since the 3.0 release.

Cheers,
Kaelyn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131204/16a17ebb/attachment.html>
-------------- next part --------------
diff --git a/include/llvm/IR/DerivedTypes.h b/include/llvm/IR/DerivedTypes.h
index e279e60..758ef71 100644
--- a/include/llvm/IR/DerivedTypes.h
+++ b/include/llvm/IR/DerivedTypes.h
@@ -249,7 +249,7 @@ public:
   bool isOpaque() const { return (getSubclassData() & SCDB_HasBody) == 0; }
 
   /// isSized - Return true if this is a sized type.
-  bool isSized() const;
+  bool isSized(SmallPtrSet<const Type*, 4> *Visited = 0) const;
   
   /// hasName - Return true if this is a named struct that has a non-empty name.
   bool hasName() const { return SymbolTableEntry != 0; }
diff --git a/include/llvm/IR/Type.h b/include/llvm/IR/Type.h
index 3cfb84e..9e833db 100644
--- a/include/llvm/IR/Type.h
+++ b/include/llvm/IR/Type.h
@@ -16,6 +16,7 @@
 #define LLVM_IR_TYPE_H
 
 #include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm/Support/DataTypes.h"
@@ -275,7 +276,7 @@ public:
   /// get the actual size for a particular target, it is reasonable to use the
   /// DataLayout subsystem to do this.
   ///
-  bool isSized() const {
+  bool isSized(SmallPtrSet<const Type*, 4> *Visited = 0) const {
     // If it's a primitive, it is always sized.
     if (getTypeID() == IntegerTyID || isFloatingPointTy() ||
         getTypeID() == PointerTyID ||
@@ -287,7 +288,7 @@ public:
         getTypeID() != VectorTyID)
       return false;
     // Otherwise we have to try harder to decide.
-    return isSizedDerivedType();
+    return isSizedDerivedType(Visited);
   }
 
   /// getPrimitiveSizeInBits - Return the basic size of this type if it is a
@@ -429,7 +430,7 @@ private:
   /// isSizedDerivedType - Derived types like structures and arrays are sized
   /// iff all of the members of the type are sized as well.  Since asking for
   /// their size is relatively uncommon, move this operation out of line.
-  bool isSizedDerivedType() const;
+  bool isSizedDerivedType(SmallPtrSet<const Type*, 4> *Visited = 0) const;
 };
 
 // Printing of types.
diff --git a/lib/IR/Type.cpp b/lib/IR/Type.cpp
index 432cbc9..802463c 100644
--- a/lib/IR/Type.cpp
+++ b/lib/IR/Type.cpp
@@ -155,20 +155,20 @@ int Type::getFPMantissaWidth() const {
 /// isSizedDerivedType - Derived types like structures and arrays are sized
 /// iff all of the members of the type are sized as well.  Since asking for
 /// their size is relatively uncommon, move this operation out of line.
-bool Type::isSizedDerivedType() const {
+bool Type::isSizedDerivedType(SmallPtrSet<const Type*, 4> *Visited) const {
   if (this->isIntegerTy())
     return true;
 
   if (const ArrayType *ATy = dyn_cast<ArrayType>(this))
-    return ATy->getElementType()->isSized();
+    return ATy->getElementType()->isSized(Visited);
 
   if (const VectorType *VTy = dyn_cast<VectorType>(this))
-    return VTy->getElementType()->isSized();
+    return VTy->getElementType()->isSized(Visited);
 
   if (!this->isStructTy()) 
     return false;
 
-  return cast<StructType>(this)->isSized();
+  return cast<StructType>(this)->isSized(Visited);
 }
 
 //===----------------------------------------------------------------------===//
@@ -556,17 +556,20 @@ StructType *StructType::create(StringRef Name, Type *type, ...) {
   return llvm::StructType::create(Ctx, StructFields, Name);
 }
 
-bool StructType::isSized() const {
+bool StructType::isSized(SmallPtrSet<const Type*, 4> *Visited) const {
   if ((getSubclassData() & SCDB_IsSized) != 0)
     return true;
   if (isOpaque())
     return false;
 
+  if (Visited && !Visited->insert(this))
+    return false;
+
   // Okay, our struct is sized if all of the elements are, but if one of the
   // elements is opaque, the struct isn't sized *yet*, but may become sized in
   // the future, so just bail out without caching.
   for (element_iterator I = element_begin(), E = element_end(); I != E; ++I)
-    if (!(*I)->isSized())
+    if (!(*I)->isSized(Visited))
       return false;
 
   // Here we cheat a bit and cast away const-ness. The goal is to memoize when
diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index da6b573..febd29f 100644
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -1861,11 +1861,12 @@ void Verifier::visitStoreInst(StoreInst &SI) {
 }
 
 void Verifier::visitAllocaInst(AllocaInst &AI) {
+  SmallPtrSet<const Type*, 4> Visited;
   PointerType *PTy = AI.getType();
   Assert1(PTy->getAddressSpace() == 0,
           "Allocation instruction pointer not in the generic address space!",
           &AI);
-  Assert1(PTy->getElementType()->isSized(), "Cannot allocate unsized type",
+  Assert1(PTy->getElementType()->isSized(&Visited), "Cannot allocate unsized type",
           &AI);
   Assert1(AI.getArraySize()->getType()->isIntegerTy(),
           "Alloca array size must have integer type", &AI);
diff --git a/test/Verifier/recursive-type-1.ll b/test/Verifier/recursive-type-1.ll
new file mode 100644
index 0000000..4a39957
--- /dev/null
+++ b/test/Verifier/recursive-type-1.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+%rt2 = type { i32, { i8, %rt2, i8 }, i32 }
+
+define i32 @main() nounwind {
+entry:
+  ; Check that recursive types trigger an error instead of segfaulting, when
+  ; the recursion isn't through a pointer to the type.
+  ; CHECK: Cannot allocate unsized type
+  %0 = alloca %rt2
+  ret i32 0
+}
diff --git a/test/Verifier/recursive-type-2.ll b/test/Verifier/recursive-type-2.ll
new file mode 100644
index 0000000..5f2f66f
--- /dev/null
+++ b/test/Verifier/recursive-type-2.ll
@@ -0,0 +1,14 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+%rt1 = type { i32, { i8, %rt2, i8 }, i32 }
+%rt2 = type { i64, { i6, %rt3 } }
+%rt3 = type { %rt1 }
+
+define i32 @main() nounwind {
+entry:
+  ; Check that mutually recursive types trigger an error instead of segfaulting,
+  ; when the recursion isn't through a pointer to the type.
+  ; CHECK: Cannot allocate unsized type
+  %0 = alloca %rt2
+  ret i32 0
+}
diff --git a/test/Verifier/recursive-type-3.ll b/test/Verifier/recursive-type-3.ll
new file mode 100644
index 0000000..8968fb5
--- /dev/null
+++ b/test/Verifier/recursive-type-3.ll
@@ -0,0 +1,11 @@
+; RUN: llvm-as %s -o /dev/null 2>&1
+
+%rt2 = type { i32, { i8, %rt2*, i8 }, i32 }
+
+define i32 @main() nounwind {
+entry:
+  ; Check that linked-list-style recursive types where the recursion is through
+  ; a pointer of the type is valid for an alloca.
+  %0 = alloca %rt2
+  ret i32 0
+}