[llvm] r196626 - Fix the segfault reported in PR 11990.

Kaelyn Takata rikka at google.com
Fri Dec 6 16:24:12 PST 2013


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/20131206/583f6c3d/attachment.html>


More information about the llvm-commits mailing list