[llvm] r196626 - Fix the segfault reported in PR 11990.
Kaelyn Takata
rikka at google.com
Sun Dec 8 09:30:50 PST 2013
That's no problem. Just thought I'd ask, since it was a small fix for a
segfault in LLVM. ;) As an aside, I think at least part of why it doesn't
apply cleanly is due to the micro-optimization that was made to
isSizedDerivedType (r196604) shortly before I committed my fix (I had to
resolve a few conflicts when updating my local tree as I was about to
commit).
On Sat, Dec 7, 2013 at 1:19 PM, Bill Wendling <isanbard at gmail.com> wrote:
> 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/20131208/f3e1bf4e/attachment.html>
More information about the llvm-commits
mailing list