From 04fddf0d1f3af5e79e9b00bdaa66ea19fe566a56 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sat, 15 Aug 2009 02:50:32 +0000 Subject: [PATCH] Fix for PR4721: adjust CodeGen and ASTContext so that we have a consistent model for handling size expressions for VLAs. The model is essentially as follows: VLA types own their associated expression. In some cases, we need to create multiple VLA types to represent a given VLA (for canonical types, or qualifiers on array types, or type merging). If we need to create multiple types based off of the same VLA declaration, we use the new refcounting functionality so they can all own the expression. The VLASizeMap in CodeGenFunction then uses the size expression to identify the group of VLA types based off of the same original declaration. I'm not particularly attached to the VLA types owning the expression, but we're stuck with at least until someone comes up with a way to walk the VLA expressions for a declaration. I did the parallel fix in ASTContext for DependentSizedArrayType, but I haven't really looked closely at it, so there might still be issues there. I'll clean up the code duplication in ASTContext in a followup commit. llvm-svn: 79071 --- clang/lib/AST/ASTContext.cpp | 12 ++++++++---- clang/lib/CodeGen/CodeGenFunction.cpp | 4 ++-- clang/lib/CodeGen/CodeGenFunction.h | 5 ++++- clang/test/CodeGen/2009-08-14-vararray-crash.c | 11 +++++++++++ 4 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 clang/test/CodeGen/2009-08-14-vararray-crash.c diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index c33a74ee95ae..dad09babccc2 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2105,14 +2105,16 @@ CanQualType ASTContext::getCanonicalType(QualType T) { if (DependentSizedArrayType *DSAT = dyn_cast(AT)) return CanQualType::CreateUnsafe( getDependentSizedArrayType(NewEltTy, - DSAT->getSizeExpr(), + DSAT->getSizeExpr() ? + DSAT->getSizeExpr()->Retain() : 0, DSAT->getSizeModifier(), DSAT->getIndexTypeQualifier(), DSAT->getBracketsRange())); VariableArrayType *VAT = cast(AT); return CanQualType::CreateUnsafe(getVariableArrayType(NewEltTy, - VAT->getSizeExpr(), + VAT->getSizeExpr() ? + VAT->getSizeExpr()->Retain() : 0, VAT->getSizeModifier(), VAT->getIndexTypeQualifier(), VAT->getBracketsRange())); @@ -2304,14 +2306,16 @@ const ArrayType *ASTContext::getAsArrayType(QualType T) { = dyn_cast(ATy)) return cast( getDependentSizedArrayType(NewEltTy, - DSAT->getSizeExpr(), + DSAT->getSizeExpr() ? + DSAT->getSizeExpr()->Retain() : 0, DSAT->getSizeModifier(), DSAT->getIndexTypeQualifier(), DSAT->getBracketsRange())); const VariableArrayType *VAT = cast(ATy); return cast(getVariableArrayType(NewEltTy, - VAT->getSizeExpr(), + VAT->getSizeExpr() ? + VAT->getSizeExpr()->Retain() : 0, VAT->getSizeModifier(), VAT->getIndexTypeQualifier(), VAT->getBracketsRange())); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index e4a83bce0296..1e8d05261db2 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -477,7 +477,7 @@ void CodeGenFunction::EmitIndirectSwitches() { } llvm::Value *CodeGenFunction::GetVLASize(const VariableArrayType *VAT) { - llvm::Value *&SizeEntry = VLASizeMap[VAT]; + llvm::Value *&SizeEntry = VLASizeMap[VAT->getSizeExpr()]; assert(SizeEntry && "Did not emit size for type"); return SizeEntry; @@ -490,7 +490,7 @@ llvm::Value *CodeGenFunction::EmitVLASize(QualType Ty) { EnsureInsertPoint(); if (const VariableArrayType *VAT = getContext().getAsVariableArrayType(Ty)) { - llvm::Value *&SizeEntry = VLASizeMap[VAT]; + llvm::Value *&SizeEntry = VLASizeMap[VAT->getSizeExpr()]; if (!SizeEntry) { const llvm::Type *SizeTy = ConvertType(getContext().getSizeType()); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index aa4bcceae267..b4469c503b3f 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -224,9 +224,12 @@ private: llvm::BasicBlock *InvokeDest; // VLASizeMap - This keeps track of the associated size for each VLA type. + // We track this by the size expression rather than the type itself because + // in certain situations, like a const qualifier applied to an VLA typedef, + // multiple VLA types can share the same size expression. // FIXME: Maybe this could be a stack of maps that is pushed/popped as we // enter/leave scopes. - llvm::DenseMap VLASizeMap; + llvm::DenseMap VLASizeMap; /// DidCallStackSave - Whether llvm.stacksave has been called. Used to avoid /// calling llvm.stacksave for multiple VLAs in the same scope. diff --git a/clang/test/CodeGen/2009-08-14-vararray-crash.c b/clang/test/CodeGen/2009-08-14-vararray-crash.c new file mode 100644 index 000000000000..40e071bd19a0 --- /dev/null +++ b/clang/test/CodeGen/2009-08-14-vararray-crash.c @@ -0,0 +1,11 @@ +// RUN: clang-cc -emit-llvm < %s + +void sum1(int rb) { + typedef unsigned char imgrow[rb]; + typedef imgrow img[rb]; + + const img *br; + int y; + + (*br)[y]; +} -- GitLab