Author: rsmith Date: Mon Jan 30 17:30:26 2017 New Revision: 293568 URL: http://llvm.org/viewvc/llvm-project?rev=293568&view=rev Log: PR28739: Check that integer values fit into 64 bits before extracting them as 64 bit values for pointer arithmetic.
This fixes various ways to tickle an assertion in constant expression evaluation when using __int128. Longer term, we need to figure out what should happen here: either any kind of overflow in offset calculation should result in a non-constant value or we should truncate to 64 bits. In C++11 onwards, we're effectively already checking for overflow because we strictly enforce array bounds checks, but even there some forms of overflow can slip past undetected. Modified: cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/test/Sema/const-eval.c cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Modified: cfe/trunk/lib/AST/ExprConstant.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=293568&r1=293567&r2=293568&view=diff ============================================================================== --- cfe/trunk/lib/AST/ExprConstant.cpp (original) +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Jan 30 17:30:26 2017 @@ -1460,9 +1460,17 @@ static bool EvaluateIgnoredValue(EvalInf /// Sign- or zero-extend a value to 64 bits. If it's already 64 bits, just /// return its existing value. -static int64_t getExtValue(const APSInt &Value) { - return Value.isSigned() ? Value.getSExtValue() - : static_cast<int64_t>(Value.getZExtValue()); +static bool getExtValue(EvalInfo &Info, const Expr *E, const APSInt &Value, + int64_t &Result) { + if (Value.isSigned() ? Value.getMinSignedBits() > 64 + : Value.getActiveBits() > 64) { + Info.FFDiag(E); + return false; + } + + Result = Value.isSigned() ? Value.getSExtValue() + : static_cast<int64_t>(Value.getZExtValue()); + return true; } /// Should this call expression be treated as a string literal? @@ -3184,7 +3192,9 @@ struct CompoundAssignSubobjectHandler { return false; } - int64_t Offset = getExtValue(RHS.getInt()); + int64_t Offset; + if (!getExtValue(Info, E, RHS.getInt(), Offset)) + return false; if (Opcode == BO_Sub) Offset = -Offset; @@ -5148,8 +5158,10 @@ bool LValueExprEvaluator::VisitArraySubs if (!EvaluateInteger(E->getIdx(), Index, Info)) return false; - return HandleLValueArrayAdjustment(Info, E, Result, E->getType(), - getExtValue(Index)); + int64_t Offset; + if (!getExtValue(Info, E, Index, Offset)) + return false; + return HandleLValueArrayAdjustment(Info, E, Result, E->getType(), Offset); } bool LValueExprEvaluator::VisitUnaryDeref(const UnaryOperator *E) { @@ -5415,7 +5427,9 @@ bool PointerExprEvaluator::VisitBinaryOp if (!EvaluateInteger(IExp, Offset, Info) || !EvalPtrOK) return false; - int64_t AdditionalOffset = getExtValue(Offset); + int64_t AdditionalOffset; + if (!getExtValue(Info, E, Offset, AdditionalOffset)) + return false; if (E->getOpcode() == BO_Sub) AdditionalOffset = -AdditionalOffset; @@ -5614,14 +5628,14 @@ bool PointerExprEvaluator::VisitBuiltinC APSInt Alignment; if (!EvaluateInteger(E->getArg(1), Alignment, Info)) return false; - CharUnits Align = CharUnits::fromQuantity(getExtValue(Alignment)); + CharUnits Align = CharUnits::fromQuantity(Alignment.getZExtValue()); if (E->getNumArgs() > 2) { APSInt Offset; if (!EvaluateInteger(E->getArg(2), Offset, Info)) return false; - int64_t AdditionalOffset = -getExtValue(Offset); + int64_t AdditionalOffset = -Offset.getZExtValue(); OffsetResult.Offset += CharUnits::fromQuantity(AdditionalOffset); } @@ -5638,12 +5652,11 @@ bool PointerExprEvaluator::VisitBuiltinC if (BaseAlignment < Align) { Result.Designator.setInvalid(); - // FIXME: Quantities here cast to integers because the plural modifier - // does not work on APSInts yet. + // FIXME: Add support to Diagnostic for long / long long. CCEDiag(E->getArg(0), diag::note_constexpr_baa_insufficient_alignment) << 0 - << (int) BaseAlignment.getQuantity() - << (unsigned) getExtValue(Alignment); + << (unsigned)BaseAlignment.getQuantity() + << (unsigned)Align.getQuantity(); return false; } } @@ -5651,18 +5664,14 @@ bool PointerExprEvaluator::VisitBuiltinC // The offset must also have the correct alignment. if (OffsetResult.Offset.alignTo(Align) != OffsetResult.Offset) { Result.Designator.setInvalid(); - APSInt Offset(64, false); - Offset = OffsetResult.Offset.getQuantity(); - - if (OffsetResult.Base) - CCEDiag(E->getArg(0), - diag::note_constexpr_baa_insufficient_alignment) << 1 - << (int) getExtValue(Offset) << (unsigned) getExtValue(Alignment); - else - CCEDiag(E->getArg(0), - diag::note_constexpr_baa_value_insufficient_alignment) - << Offset << (unsigned) getExtValue(Alignment); + (OffsetResult.Base + ? CCEDiag(E->getArg(0), + diag::note_constexpr_baa_insufficient_alignment) << 1 + : CCEDiag(E->getArg(0), + diag::note_constexpr_baa_value_insufficient_alignment)) + << (int)OffsetResult.Offset.getQuantity() + << (unsigned)Align.getQuantity(); return false; } @@ -7968,12 +7977,13 @@ bool DataRecursiveIntBinOpEvaluator:: // Handle cases like (unsigned long)&a + 4. if (E->isAdditiveOp() && LHSVal.isLValue() && RHSVal.isInt()) { Result = LHSVal; - CharUnits AdditionalOffset = - CharUnits::fromQuantity(RHSVal.getInt().getZExtValue()); + int64_t Offset; + if (!getExtValue(Info, E, RHSVal.getInt(), Offset)) + return false; if (E->getOpcode() == BO_Add) - Result.getLValueOffset() += AdditionalOffset; + Result.getLValueOffset() += CharUnits::fromQuantity(Offset); else - Result.getLValueOffset() -= AdditionalOffset; + Result.getLValueOffset() -= CharUnits::fromQuantity(Offset); return true; } @@ -7981,8 +7991,10 @@ bool DataRecursiveIntBinOpEvaluator:: if (E->getOpcode() == BO_Add && RHSVal.isLValue() && LHSVal.isInt()) { Result = RHSVal; - Result.getLValueOffset() += - CharUnits::fromQuantity(LHSVal.getInt().getZExtValue()); + int64_t Offset; + if (!getExtValue(Info, E, LHSVal.getInt(), Offset)) + return false; + Result.getLValueOffset() += CharUnits::fromQuantity(Offset); return true; } Modified: cfe/trunk/test/Sema/const-eval.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/const-eval.c?rev=293568&r1=293567&r2=293568&view=diff ============================================================================== --- cfe/trunk/test/Sema/const-eval.c (original) +++ cfe/trunk/test/Sema/const-eval.c Mon Jan 30 17:30:26 2017 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux %s -Wno-tautological-pointer-compare +// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s -Wno-tautological-pointer-compare #define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char qq[expr];}); int x; @@ -126,7 +126,7 @@ EVAL_EXPR(48, &x != &x - 1 ? 1 : -1) EVAL_EXPR(49, &x < &x - 100 ? 1 : -1) // expected-error {{must have a constant size}} extern struct Test50S Test50; -EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned)&Test50 + 10)) // expected-error {{must have a constant size}} +EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned long)&Test50 + 10)) // expected-error {{must have a constant size}} // <rdar://problem/11874571> EVAL_EXPR(51, 0 != (float)1e99) @@ -137,3 +137,15 @@ void PR21945() { int i = (({}), 0l); } void PR24622(); struct PR24622 {} pr24622; EVAL_EXPR(52, &pr24622 == (void *)&PR24622); // expected-error {{must have a constant size}} + +// Reject cases that would require more than 64 bits of pointer offset to +// represent. +// FIXME: We don't check for all offset overflow, just immediate adjustments +// that don't fit in 64 bits. We should consistently either check all offset +// adjustments or truncate to 64 bits everywhere. +void *PR28739a = (__int128)(unsigned long)-1 + &PR28739a; // expected-error {{not a compile-time constant}} +void *PR28739b = &PR28739b + (__int128)(unsigned long)-1; // expected-error {{not a compile-time constant}} +__int128 PR28739c = (&PR28739c + (__int128)(unsigned long)-1) - &PR28739c; // expected-error {{not a compile-time constant}} +void *PR28739d = &(&PR28739d)[(__int128)(unsigned long)-1]; // expected-error {{not a compile-time constant}} +__int128 PR28739e = (__int128)(unsigned long)-1 + (long)&PR28739e; // expected-error {{not a compile-time constant}} +__int128 PR28739f = (long)&PR28739f + (__int128)(unsigned long)-1; // expected-error {{not a compile-time constant}} Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp?rev=293568&r1=293567&r2=293568&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp (original) +++ cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Mon Jan 30 17:30:26 2017 @@ -977,3 +977,8 @@ void run() { foo(); } static_assert(sum(Cs) == 'a' + 'b', ""); // expected-error{{not an integral constant expression}} expected-note{{in call to 'sum(Cs)'}} constexpr int S = sum(Cs); // expected-error{{must be initialized by a constant expression}} expected-note{{in call}} } + +constexpr void PR28739(int n) { // expected-error {{never produces a constant}} + int *p = &n; + p += (__int128)(unsigned long)-1; // expected-note {{subexpression}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits