[llvm] r196626 - Fix the segfault reported in PR 11990.
Bill Wendling
isanbard at gmail.com
Sat Dec 7 13:19:08 PST 2013
I’m hesitant to do that, because it’s not a regression from 3.3 and we’re deep into the final stages of the release. Also the patch doesn’t apply cleanly...
-bw
On Dec 6, 2013, at 4:24 PM, Kaelyn Takata <rikka at google.com> wrote:
> Do we want to pull this into the release branch? The segfault has apparently existed since LLVM 3.0.
>
>
> On Fri, Dec 6, 2013 at 4:13 PM, Kaelyn Uhrain <rikka at google.com> wrote:
> Author: rikka
> Date: Fri Dec 6 18:13:34 2013
> New Revision: 196626
>
> URL: http://llvm.org/viewvc/llvm-project?rev=196626&view=rev
> Log:
> 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.
>
> Added:
> llvm/trunk/test/Verifier/recursive-type-1.ll
> llvm/trunk/test/Verifier/recursive-type-2.ll
> llvm/trunk/test/Verifier/recursive-type-3.ll
> Modified:
> llvm/trunk/include/llvm/IR/DerivedTypes.h
> llvm/trunk/include/llvm/IR/Type.h
> llvm/trunk/lib/IR/Type.cpp
> llvm/trunk/lib/IR/Verifier.cpp
>
> Modified: llvm/trunk/include/llvm/IR/DerivedTypes.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DerivedTypes.h?rev=196626&r1=196625&r2=196626&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/DerivedTypes.h (original)
> +++ llvm/trunk/include/llvm/IR/DerivedTypes.h Fri Dec 6 18:13:34 2013
> @@ -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; }
>
> Modified: llvm/trunk/include/llvm/IR/Type.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Type.h?rev=196626&r1=196625&r2=196626&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Type.h (original)
> +++ llvm/trunk/include/llvm/IR/Type.h Fri Dec 6 18:13:34 2013
> @@ -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.
>
> Modified: llvm/trunk/lib/IR/Type.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Type.cpp?rev=196626&r1=196625&r2=196626&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Type.cpp (original)
> +++ llvm/trunk/lib/IR/Type.cpp Fri Dec 6 18:13:34 2013
> @@ -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<const Type*, 4> *Visited) const {
> 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);
>
> - return cast<StructType>(this)->isSized();
> + return cast<StructType>(this)->isSized(Visited);
> }
>
> //===----------------------------------------------------------------------===//
> @@ -550,17 +550,20 @@ StructType *StructType::create(StringRef
> 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
>
> Modified: llvm/trunk/lib/IR/Verifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=196626&r1=196625&r2=196626&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Verifier.cpp (original)
> +++ llvm/trunk/lib/IR/Verifier.cpp Fri Dec 6 18:13:34 2013
> @@ -1861,11 +1861,12 @@ void Verifier::visitStoreInst(StoreInst
> }
>
> 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);
>
> Added: llvm/trunk/test/Verifier/recursive-type-1.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/recursive-type-1.ll?rev=196626&view=auto
> ==============================================================================
> --- llvm/trunk/test/Verifier/recursive-type-1.ll (added)
> +++ llvm/trunk/test/Verifier/recursive-type-1.ll Fri Dec 6 18:13:34 2013
> @@ -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
> +}
>
> Added: llvm/trunk/test/Verifier/recursive-type-2.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/recursive-type-2.ll?rev=196626&view=auto
> ==============================================================================
> --- llvm/trunk/test/Verifier/recursive-type-2.ll (added)
> +++ llvm/trunk/test/Verifier/recursive-type-2.ll Fri Dec 6 18:13:34 2013
> @@ -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
> +}
>
> Added: llvm/trunk/test/Verifier/recursive-type-3.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/recursive-type-3.ll?rev=196626&view=auto
> ==============================================================================
> --- llvm/trunk/test/Verifier/recursive-type-3.ll (added)
> +++ llvm/trunk/test/Verifier/recursive-type-3.ll Fri Dec 6 18:13:34 2013
> @@ -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
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131207/31084410/attachment.html>
More information about the llvm-commits
mailing list