Author: rsmith Date: Mon Aug 28 18:52:13 2017 New Revision: 311970 URL: http://llvm.org/viewvc/llvm-project?rev=311970&view=rev Log: Improve constant expression evaluation of arrays of unknown bound.
The standard is not clear on how these are supposed to be handled, so we conservatively treat as non-constant any cases whose value is unknown or whose evaluation might result in undefined behavior. Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td cfe/trunk/include/clang/Basic/DiagnosticIDs.h cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=311970&r1=311969&r2=311970&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Mon Aug 28 18:52:13 2017 @@ -127,6 +127,10 @@ def note_constexpr_access_null : Note< def note_constexpr_access_past_end : Note< "%select{read of|assignment to|increment of|decrement of}0 " "dereferenced one-past-the-end pointer is not allowed in a constant expression">; +def note_constexpr_access_unsized_array : Note< + "%select{read of|assignment to|increment of|decrement of}0 " + "pointer to element of array without known bound " + "is not allowed in a constant expression">; def note_constexpr_access_inactive_union_member : Note< "%select{read of|assignment to|increment of|decrement of}0 " "member %1 of union with %select{active member %3|no active member}2 " @@ -154,6 +158,11 @@ def note_constexpr_baa_insufficient_alig def note_constexpr_baa_value_insufficient_alignment : Note< "value of the aligned pointer (%0) is not a multiple of the asserted %1 " "%plural{1:byte|:bytes}1">; +def note_constexpr_unsupported_unsized_array : Note< + "array-to-pointer decay of array member without known bound is not supported">; +def note_constexpr_unsized_array_indexed : Note< + "indexing of array without known bound is not allowed " + "in a constant expression">; def warn_integer_constant_overflow : Warning< "overflow in expression; result is %0 with type %1">, Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=311970&r1=311969&r2=311970&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original) +++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Mon Aug 28 18:52:13 2017 @@ -34,7 +34,7 @@ namespace clang { DIAG_SIZE_SERIALIZATION = 120, DIAG_SIZE_LEX = 400, DIAG_SIZE_PARSE = 500, - DIAG_SIZE_AST = 110, + DIAG_SIZE_AST = 150, DIAG_SIZE_COMMENT = 100, DIAG_SIZE_SEMA = 3500, DIAG_SIZE_ANALYSIS = 100 Modified: cfe/trunk/lib/AST/ExprConstant.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=311970&r1=311969&r2=311970&view=diff ============================================================================== --- cfe/trunk/lib/AST/ExprConstant.cpp (original) +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Aug 28 18:52:13 2017 @@ -141,6 +141,12 @@ namespace { return E && E->getType()->isPointerType() && tryUnwrapAllocSizeCall(E); } + /// The bound to claim that an array of unknown bound has. + /// The value in MostDerivedArraySize is undefined in this case. So, set it + /// to an arbitrary value that's likely to loudly break things if it's used. + static const uint64_t AssumedSizeForUnsizedArray = + std::numeric_limits<uint64_t>::max() / 2; + /// Determines if an LValue with the given LValueBase will have an unsized /// array in its designator. /// Find the path length and type of the most-derived subobject in the given @@ -148,7 +154,8 @@ namespace { static unsigned findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base, ArrayRef<APValue::LValuePathEntry> Path, - uint64_t &ArraySize, QualType &Type, bool &IsArray) { + uint64_t &ArraySize, QualType &Type, bool &IsArray, + bool &FirstEntryIsUnsizedArray) { // This only accepts LValueBases from APValues, and APValues don't support // arrays that lack size info. assert(!isBaseAnAllocSizeCall(Base) && @@ -158,12 +165,18 @@ namespace { for (unsigned I = 0, N = Path.size(); I != N; ++I) { if (Type->isArrayType()) { - const ConstantArrayType *CAT = - cast<ConstantArrayType>(Ctx.getAsArrayType(Type)); - Type = CAT->getElementType(); - ArraySize = CAT->getSize().getZExtValue(); + const ArrayType *AT = Ctx.getAsArrayType(Type); + Type = AT->getElementType(); MostDerivedLength = I + 1; IsArray = true; + + if (auto *CAT = dyn_cast<ConstantArrayType>(AT)) { + ArraySize = CAT->getSize().getZExtValue(); + } else { + assert(I == 0 && "unexpected unsized array designator"); + FirstEntryIsUnsizedArray = true; + ArraySize = AssumedSizeForUnsizedArray; + } } else if (Type->isAnyComplexType()) { const ComplexType *CT = Type->castAs<ComplexType>(); Type = CT->getElementType(); @@ -246,10 +259,12 @@ namespace { Entries.insert(Entries.end(), VEntries.begin(), VEntries.end()); if (V.getLValueBase()) { bool IsArray = false; + bool FirstIsUnsizedArray = false; MostDerivedPathLength = findMostDerivedSubobject( Ctx, V.getLValueBase(), V.getLValuePath(), MostDerivedArraySize, - MostDerivedType, IsArray); + MostDerivedType, IsArray, FirstIsUnsizedArray); MostDerivedIsArrayElement = IsArray; + FirstEntryIsAnUnsizedArray = FirstIsUnsizedArray; } } } @@ -318,7 +333,7 @@ namespace { // The value in MostDerivedArraySize is undefined in this case. So, set it // to an arbitrary value that's likely to loudly break things if it's // used. - MostDerivedArraySize = std::numeric_limits<uint64_t>::max() / 2; + MostDerivedArraySize = AssumedSizeForUnsizedArray; MostDerivedPathLength = Entries.size(); } /// Update this designator to refer to the given base or member of this @@ -350,6 +365,7 @@ namespace { MostDerivedArraySize = 2; MostDerivedPathLength = Entries.size(); } + void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E); void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E, const APSInt &N); /// Add N to the address of this subobject. @@ -357,6 +373,7 @@ namespace { if (Invalid || !N) return; uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue(); if (isMostDerivedAnUnsizedArray()) { + diagnoseUnsizedArrayPointerArithmetic(Info, E); // Can't verify -- trust that the user is doing the right thing (or if // not, trust that the caller will catch the bad behavior). // FIXME: Should we reject if this overflows, at least? @@ -1068,9 +1085,19 @@ bool SubobjectDesignator::checkSubobject setInvalid(); return false; } + // Note, we do not diagnose if isMostDerivedAnUnsizedArray(), because there + // must actually be at least one array element; even a VLA cannot have a + // bound of zero. And if our index is nonzero, we already had a CCEDiag. return true; } +void SubobjectDesignator::diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, + const Expr *E) { + Info.CCEDiag(E, diag::note_constexpr_unsized_array_indexed); + // Do not set the designator as invalid: we can represent this situation, + // and correct handling of __builtin_object_size requires us to do so. +} + void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info, const Expr *E, const APSInt &N) { @@ -1214,8 +1241,6 @@ namespace { IsNullPtr); else { assert(!InvalidBase && "APValues can't handle invalid LValue bases"); - assert(!Designator.FirstEntryIsAnUnsizedArray && - "Unsized array with a valid base?"); V = APValue(Base, Offset, Designator.Entries, Designator.IsOnePastTheEnd, CallIndex, IsNullPtr); } @@ -1288,10 +1313,14 @@ namespace { if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base)) Designator.addDeclUnchecked(D, Virtual); } - void addUnsizedArray(EvalInfo &Info, QualType ElemTy) { - assert(Designator.Entries.empty() && getType(Base)->isPointerType()); - assert(isBaseAnAllocSizeCall(Base) && - "Only alloc_size bases can have unsized arrays"); + void addUnsizedArray(EvalInfo &Info, const Expr *E, QualType ElemTy) { + if (!Designator.Entries.empty()) { + Info.CCEDiag(E, diag::note_constexpr_unsupported_unsized_array); + Designator.setInvalid(); + return; + } + + assert(getType(Base)->isPointerType() || getType(Base)->isArrayType()); Designator.FirstEntryIsAnUnsizedArray = true; Designator.addUnsizedArrayUnchecked(ElemTy); } @@ -2598,10 +2627,12 @@ findSubobject(EvalInfo &Info, const Expr if (Sub.Invalid) // A diagnostic will have already been produced. return handler.failed(); - if (Sub.isOnePastTheEnd()) { + if (Sub.isOnePastTheEnd() || Sub.isMostDerivedAnUnsizedArray()) { if (Info.getLangOpts().CPlusPlus11) - Info.FFDiag(E, diag::note_constexpr_access_past_end) - << handler.AccessKind; + Info.FFDiag(E, Sub.isOnePastTheEnd() + ? diag::note_constexpr_access_past_end + : diag::note_constexpr_access_unsized_array) + << handler.AccessKind; else Info.FFDiag(E); return handler.failed(); @@ -5460,7 +5491,7 @@ static bool evaluateLValueAsAllocSize(Ev Result.setInvalid(E); QualType Pointee = E->getType()->castAs<PointerType>()->getPointeeType(); - Result.addUnsizedArray(Info, Pointee); + Result.addUnsizedArray(Info, E, Pointee); return true; } @@ -5670,7 +5701,8 @@ bool PointerExprEvaluator::VisitCastExpr return true; } } - case CK_ArrayToPointerDecay: + + case CK_ArrayToPointerDecay: { if (SubExpr->isGLValue()) { if (!evaluateLValue(SubExpr, Result)) return false; @@ -5681,12 +5713,13 @@ bool PointerExprEvaluator::VisitCastExpr return false; } // The result is a pointer to the first element of the array. - if (const ConstantArrayType *CAT - = Info.Ctx.getAsConstantArrayType(SubExpr->getType())) + auto *AT = Info.Ctx.getAsArrayType(SubExpr->getType()); + if (auto *CAT = dyn_cast<ConstantArrayType>(AT)) Result.addArray(Info, E, CAT); else - Result.Designator.setInvalid(); + Result.addUnsizedArray(Info, E, AT->getElementType()); return true; + } case CK_FunctionToPointerDecay: return evaluateLValue(SubExpr, Result); @@ -5753,7 +5786,7 @@ bool PointerExprEvaluator::visitNonBuilt Result.setInvalid(E); QualType PointeeTy = E->getType()->castAs<PointerType>()->getPointeeType(); - Result.addUnsizedArray(Info, PointeeTy); + Result.addUnsizedArray(Info, E, PointeeTy); return true; } @@ -7314,7 +7347,8 @@ static const Expr *ignorePointerCastsAnd /// Please note: this function is specialized for how __builtin_object_size /// views "objects". /// -/// If this encounters an invalid RecordDecl, it will always return true. +/// If this encounters an invalid RecordDecl or otherwise cannot determine the +/// correct result, it will always return true. static bool isDesignatorAtObjectEnd(const ASTContext &Ctx, const LValue &LVal) { assert(!LVal.Designator.Invalid); @@ -7345,9 +7379,8 @@ static bool isDesignatorAtObjectEnd(cons unsigned I = 0; QualType BaseType = getType(Base); if (LVal.Designator.FirstEntryIsAnUnsizedArray) { - assert(isBaseAnAllocSizeCall(Base) && - "Unsized array in non-alloc_size call?"); - // If this is an alloc_size base, we should ignore the initial array index + // If we don't know the array bound, conservatively assume we're looking at + // the final array element. ++I; BaseType = BaseType->castAs<PointerType>()->getPointeeType(); } Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=311970&r1=311969&r2=311970&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original) +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Mon Aug 28 18:52:13 2017 @@ -604,6 +604,34 @@ static_assert(NATDCArray{}[1][1].n == 0, } +// FIXME: The rules in this case are unclear, but we conservatively choose to +// reject any cases where pointer arithmetic is not statically known to be +// valid. +namespace ArrayOfUnknownBound { + extern int arr[]; + constexpr int *a = arr; + constexpr int *b = &arr[0]; + static_assert(a == b, ""); + constexpr int *c = &arr[1]; // expected-error {{constant}} expected-note {{indexing of array without known bound}} + constexpr int *d = &a[1]; // expected-error {{constant}} expected-note {{indexing of array without known bound}} + constexpr int *e = a + 1; // expected-error {{constant}} expected-note {{indexing of array without known bound}} + + struct X { + int a; + int b[]; // expected-warning {{C99}} + }; + extern X x; + constexpr int *xb = x.b; // expected-error {{constant}} expected-note {{not supported}} + + struct Y { int a; }; + extern Y yarr[]; + constexpr Y *p = yarr; + constexpr int *q = &p->a; + + extern const int carr[]; // expected-note {{here}} + constexpr int n = carr[0]; // expected-error {{constant}} expected-note {{non-constexpr variable}} +} + namespace DependentValues { struct I { int n; typedef I V[10]; }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits