diff --git a/include/clang/Basic/DiagnosticASTKinds.td b/include/clang/Basic/DiagnosticASTKinds.td index b3cba2066edd8248d84d304ac757657b5c8a5d9d..215580b2e9b6669779ee89b0c0e63bbc7edab45f 100644 --- a/include/clang/Basic/DiagnosticASTKinds.td +++ b/include/clang/Basic/DiagnosticASTKinds.td @@ -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_alignment : Note< 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">, diff --git a/include/clang/Basic/DiagnosticIDs.h b/include/clang/Basic/DiagnosticIDs.h index ca04ccacf603e93dd480c20c4088ea728532682f..a62d6af744c91a2db580eda31170e9b028b9117f 100644 --- a/include/clang/Basic/DiagnosticIDs.h +++ b/include/clang/Basic/DiagnosticIDs.h @@ -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_CROSSTU = 100, DIAG_SIZE_SEMA = 3500, diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index ecce50ed23cc27766d5af7434b340a6f437110c8..c25b3b6322fe5c1e7f0f562fad51f63b77cd1d90 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -62,7 +62,13 @@ namespace { static QualType getType(APValue::LValueBase B) { if (!B) return QualType(); if (const ValueDecl *D = B.dyn_cast<const ValueDecl*>()) - return D->getType(); + // FIXME: It's unclear where we're supposed to take the type from, and + // this actually matters for arrays of unknown bound. Using the type of + // the most recent declaration isn't clearly correct in general. Eg: + // + // extern int arr[]; void f() { extern int arr[3]; }; + // constexpr int *p = &arr[1]; // valid? + return cast<ValueDecl>(D->getMostRecentDecl())->getType(); const Expr *Base = B.get<const Expr*>(); @@ -141,6 +147,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 +160,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 +171,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 +265,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 +339,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 +371,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 +379,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? @@ -1094,9 +1117,19 @@ bool SubobjectDesignator::checkSubobject(EvalInfo &Info, const Expr *E, 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) { @@ -1240,8 +1273,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); } @@ -1314,10 +1345,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); } @@ -2624,10 +2659,12 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj, 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(); @@ -5487,7 +5524,7 @@ static bool evaluateLValueAsAllocSize(EvalInfo &Info, APValue::LValueBase Base, Result.setInvalid(E); QualType Pointee = E->getType()->castAs<PointerType>()->getPointeeType(); - Result.addUnsizedArray(Info, Pointee); + Result.addUnsizedArray(Info, E, Pointee); return true; } @@ -5697,7 +5734,8 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr* E) { return true; } } - case CK_ArrayToPointerDecay: + + case CK_ArrayToPointerDecay: { if (SubExpr->isGLValue()) { if (!evaluateLValue(SubExpr, Result)) return false; @@ -5708,12 +5746,13 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr* E) { 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); @@ -5780,7 +5819,7 @@ bool PointerExprEvaluator::visitNonBuiltinCallExpr(const CallExpr *E) { Result.setInvalid(E); QualType PointeeTy = E->getType()->castAs<PointerType>()->getPointeeType(); - Result.addUnsizedArray(Info, PointeeTy); + Result.addUnsizedArray(Info, E, PointeeTy); return true; } @@ -7341,7 +7380,8 @@ static const Expr *ignorePointerCastsAndParens(const Expr *E) { /// 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); @@ -7372,9 +7412,8 @@ static bool isDesignatorAtObjectEnd(const ASTContext &Ctx, const LValue &LVal) { 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(); } diff --git a/test/SemaCXX/constant-expression-cxx11.cpp b/test/SemaCXX/constant-expression-cxx11.cpp index e64f2d5a335dc229796a63ce1165b9944ee2546c..68b82c7d96fdd956822af6f6a9e436657b421252 100644 --- a/test/SemaCXX/constant-expression-cxx11.cpp +++ b/test/SemaCXX/constant-expression-cxx11.cpp @@ -604,20 +604,31 @@ static_assert(NATDCArray{}[1][1].n == 0, ""); } -// Tests for indexes into arrays of unknown bounds. +// Per current CWG direction, we reject any cases where pointer arithmetic is +// not statically known to be valid. namespace ArrayOfUnknownBound { - // This is a corner case of the language where it's not clear whether this - // should be an error: When we see the initializer for Z::a, the bounds of - // Z::b aren't known yet, but they will be known by the end of the translation - // unit, so the compiler can in theory check the indexing into Z::b. - // For the time being, as long as this is unclear, we want to make sure that - // this compiles. - struct Z { - static const void *const a[]; - static const void *const b[]; + 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}} }; - constexpr const void *Z::a[] = {&b[0], &b[1]}; - constexpr const void *Z::b[] = {&a[0], &a[1]}; + 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 { diff --git a/test/SemaCXX/constexpr-array-unknown-bound.cpp b/test/SemaCXX/constexpr-array-unknown-bound.cpp new file mode 100644 index 0000000000000000000000000000000000000000..395c4cbd127b24832ef1ae2fb22e70ff00b5dd13 --- /dev/null +++ b/test/SemaCXX/constexpr-array-unknown-bound.cpp @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify + +const extern int arr[]; +constexpr auto p = arr; // ok +constexpr int f(int i) {return p[i];} // expected-note {{read of dereferenced one-past-the-end pointer}} + +constexpr int arr[] {1, 2, 3}; +constexpr auto p2 = arr + 2; // ok +constexpr int x = f(2); // ok +constexpr int y = f(3); // expected-error {{constant expression}} +// expected-note-re@-1 {{in call to 'f({{.*}})'}} + +// FIXME: consider permitting this case +struct A {int m[];} a; +constexpr auto p3 = a.m; // expected-error {{constant expression}} expected-note {{without known bound}} +constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{without known bound}} + +void g(int i) { + int arr[i]; + constexpr auto *p = arr + 2; // expected-error {{constant expression}} expected-note {{without known bound}} + + // FIXME: Give a better diagnostic here. The issue is that computing + // sizeof(*arr2) within the array indexing fails due to the VLA. + int arr2[2][i]; + constexpr int m = ((void)arr2[2], 0); // expected-error {{constant expression}} +} diff --git a/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp b/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp index c1fcedd58d1344c8c0648c2ae219b1dd48215af8..d0fb25c047a9610fe4f3995a53047877031b775b 100644 --- a/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp +++ b/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp @@ -23,6 +23,9 @@ namespace Array { A<const char*, &(&x)[1]> h; // expected-error {{refers to subobject '&x + 1'}} A<const char*, 0> i; // expected-error {{not allowed in a converted constant}} A<const char*, nullptr> j; + + extern char aub[]; + A<char[], aub> k; } namespace Function {