From b95d0907fc6859b5f502a108e8793fa5335bf580 Mon Sep 17 00:00:00 2001 From: Kaelyn Uhrain Date: Sat, 7 Dec 2013 00:13:34 +0000 Subject: [PATCH] Fix the segfault reported in PR 11990. The sefault occurs due to an infinite loop when the verifier tries to determine the size of a type of the form "%rt = type { %rt }" while checking an alloca of the type. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@196626 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/DerivedTypes.h | 2 +- include/llvm/IR/Type.h | 7 ++++--- lib/IR/Type.cpp | 15 +++++++++------ lib/IR/Verifier.cpp | 3 ++- test/Verifier/recursive-type-1.ll | 12 ++++++++++++ test/Verifier/recursive-type-2.ll | 14 ++++++++++++++ test/Verifier/recursive-type-3.ll | 11 +++++++++++ 7 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 test/Verifier/recursive-type-1.ll create mode 100644 test/Verifier/recursive-type-2.ll create mode 100644 test/Verifier/recursive-type-3.ll diff --git a/include/llvm/IR/DerivedTypes.h b/include/llvm/IR/DerivedTypes.h index e279e60e476..758ef71a1f0 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 *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 3cfb84edd82..9e833dbe6ff 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 *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 *Visited = 0) const; }; // Printing of types. diff --git a/lib/IR/Type.cpp b/lib/IR/Type.cpp index 03b1122e9e4..86f2d89dd33 100644 --- a/lib/IR/Type.cpp +++ b/lib/IR/Type.cpp @@ -155,14 +155,14 @@ 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 *Visited) const { if (const ArrayType *ATy = dyn_cast(this)) - return ATy->getElementType()->isSized(); + return ATy->getElementType()->isSized(Visited); if (const VectorType *VTy = dyn_cast(this)) - return VTy->getElementType()->isSized(); + return VTy->getElementType()->isSized(Visited); - return cast(this)->isSized(); + return cast(this)->isSized(Visited); } //===----------------------------------------------------------------------===// @@ -550,17 +550,20 @@ StructType *StructType::create(StringRef Name, Type *type, ...) { return llvm::StructType::create(Ctx, StructFields, Name); } -bool StructType::isSized() const { +bool StructType::isSized(SmallPtrSet *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 da6b573a0c3..febd29f0725 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 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 00000000000..4a399575956 --- /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 00000000000..5f2f66fa1b1 --- /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 00000000000..8968fb5eb61 --- /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 +}