r293568 - PR28739: Check that integer values fit into 64 bits before extracting them as 64 bit values for pointer arithmetic.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 30 15:30:26 PST 2017
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}}
+}
More information about the cfe-commits
mailing list