george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.
This patch aims to fix a bug encountered by compiling code with C++14.
Specifically, when evaluating `p` in `__builtin_object_size(p, n)`, we start
speculatively evaluating. Currently, clang pretends that side-effects have
always occurred when we start speculatively evaluating. This prevents us from
reading any locals in C++14. So, the following code compiles fine in C++11
mode, but fails to compile with C++14:
```
void foo(char *p) __attribute__((enable_if(__builtin_object_size(p, 0) != -1,
"")));
void runFoo() {
char buf[5];
foo(buf);
}
```
This patch makes ExprConstant note when we've actually failed (read: may have
an unmodeled side-effect), and makes us act as conservatively before if a
failure happened. If a failure hasn't happened, we can be more aggressive.
http://reviews.llvm.org/D18540
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
test/SemaCXX/builtin-object-size-cxx14.cpp
test/SemaCXX/constant-expression-cxx1y.cpp
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===================================================================
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -179,12 +179,10 @@
static_assert(!test1(100), "");
static_assert(!test1(101), ""); // expected-error {{constant expression}} expected-note {{in call to 'test1(101)'}}
- // FIXME: We should be able to reject this before it's called
- constexpr void f() {
+ constexpr void f() { // expected-error{{constexpr function never produces a constant expression}} expected-note@+2{{assignment to dereferenced one-past-the-end pointer is not allowed in a constant expression}}
char foo[10] = { "z" }; // expected-note {{here}}
- foo[10] = 'x'; // expected-warning {{past the end}} expected-note {{assignment to dereferenced one-past-the-end pointer}}
+ foo[10] = 'x'; // expected-warning {{past the end}}
}
- constexpr int k = (f(), 0); // expected-error {{constant expression}} expected-note {{in call}}
}
namespace array_resize {
Index: test/SemaCXX/builtin-object-size-cxx14.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/builtin-object-size-cxx14.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
+
+namespace basic {
+// Ensuring that __bos can be used in constexpr functions without anything
+// sketchy going on...
+constexpr int bos0() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 0);
+}
+
+constexpr int bos1() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 1);
+}
+
+constexpr int bos2() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 2);
+}
+
+constexpr int bos3() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 3);
+}
+
+static_assert(bos0() == sizeof(char) * 5, "");
+static_assert(bos1() == sizeof(char) * 5, "");
+static_assert(bos2() == sizeof(char) * 5, "");
+static_assert(bos3() == sizeof(char) * 5, "");
+}
+
+namespace in_enable_if {
+// The code that prompted these changes was __bos in enable_if
+
+void copy5CharsInto(char *buf) // expected-note{{candidate}}
+ __attribute__((enable_if(__builtin_object_size(buf, 0) != -1 &&
+ __builtin_object_size(buf, 0) > 5,
+ "")));
+
+// We use different EvalModes for __bos with type 0 versus 1. Ensure 1 works,
+// too...
+void copy5CharsIntoStrict(char *buf) // expected-note{{candidate}}
+ __attribute__((enable_if(__builtin_object_size(buf, 1) != -1 &&
+ __builtin_object_size(buf, 1) > 5,
+ "")));
+
+struct LargeStruct {
+ int pad;
+ char buf[6];
+ int pad2;
+};
+
+struct SmallStruct {
+ int pad;
+ char buf[5];
+ int pad2;
+};
+
+void noWriteToBuf() {
+ char buf[6];
+ copy5CharsInto(buf);
+
+ LargeStruct large;
+ copy5CharsIntoStrict(large.buf);
+}
+
+void initTheBuf() {
+ char buf[6] = {};
+ copy5CharsInto(buf);
+
+ LargeStruct large = {0, {}, 0};
+ copy5CharsIntoStrict(large.buf);
+}
+
+int getI();
+void initTheBufWithALoop() {
+ char buf[6] = {};
+ for (unsigned I = getI(); I != sizeof(buf); ++I)
+ buf[I] = I;
+ copy5CharsInto(buf);
+
+ LargeStruct large;
+ for (unsigned I = getI(); I != sizeof(buf); ++I)
+ large.buf[I] = I;
+ copy5CharsIntoStrict(large.buf);
+}
+
+void tooSmallBuf() {
+ char buf[5];
+ copy5CharsInto(buf); // expected-error{{no matching function for call}}
+
+ SmallStruct small;
+ copy5CharsIntoStrict(small.buf); // expected-error{{no matching function for call}}
+}
+}
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -478,6 +478,9 @@
/// fold (not just why it's not strictly a constant expression)?
bool HasFoldFailureDiagnostic;
+ /// \brief Whether or not we're currently speculatively evaluating.
+ bool IsSpeculativelyEvaluating;
+
enum EvaluationMode {
/// Evaluate as a constant expression. Stop if we find that the expression
/// is not a constant expression.
@@ -542,7 +545,8 @@
BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
EvaluatingDecl((const ValueDecl *)nullptr),
EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
- HasFoldFailureDiagnostic(false), EvalMode(Mode) {}
+ HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+ EvalMode(Mode) {}
void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
EvaluatingDecl = Base;
@@ -764,6 +768,29 @@
llvm_unreachable("Missed EvalMode case");
}
+ /// Notes that we failed to evaluate an expression, and determine if we
+ /// should keep evaluating.
+ ///
+ /// Currently, this is used as a fancy way of noting that there *may* have
+ /// been a side-effect that we didn't see (and can't model).
+ ///
+ /// Note that this is sometimes *not* called when a failure happens. The
+ /// only time we really care about this case is when we're going to try to
+ /// evaluate things after the failure. It's therefore the responsibility of
+ /// the caller that wants to continue evaluating after a failure to call
+ /// this.
+ bool noteFailure() {
+ EvalStatus.HasFailure = true;
+ return keepEvaluatingAfterFailure();
+ }
+
+ /// Returns whether a side-effect that we can't model might have occurred.
+ /// Moreover, whether we've seen a side-effect, or if we failed to evaluate
+ /// something that might produce a side-effect.
+ bool sideEffectsMightHaveOccurred() const {
+ return EvalStatus.HasSideEffects || EvalStatus.HasFailure;
+ }
+
bool allowInvalidBaseExpr() const {
return EvalMode == EM_DesignatorFold;
}
@@ -817,18 +844,20 @@
class SpeculativeEvaluationRAII {
EvalInfo &Info;
Expr::EvalStatus Old;
+ bool OldSpecEval;
public:
- SpeculativeEvaluationRAII(EvalInfo &Info,
- SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)
- : Info(Info), Old(Info.EvalStatus) {
+ SpeculativeEvaluationRAII(
+ EvalInfo &Info, SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)
+ : Info(Info), Old(Info.EvalStatus),
+ OldSpecEval(Info.IsSpeculativelyEvaluating) {
Info.EvalStatus.Diag = NewDiag;
// If we're speculatively evaluating, we may have skipped over some
// evaluations and missed out a side effect.
- Info.EvalStatus.HasSideEffects = true;
}
~SpeculativeEvaluationRAII() {
Info.EvalStatus = Old;
+ Info.IsSpeculativelyEvaluating = OldSpecEval;
}
};
@@ -2612,6 +2641,10 @@
return CompleteObject();
}
+ // Don't allow the speculative evaluation of writes.
+ if (AK != AK_Read && Info.IsSpeculativelyEvaluating)
+ return CompleteObject();
+
CallStackFrame *Frame = nullptr;
if (LVal.CallIndex) {
Frame = Info.getCallFrame(LVal.CallIndex);
@@ -2785,12 +2818,12 @@
}
// In C++1y, we can't safely access any mutable state when we might be
- // evaluating after an unmodeled side effect or an evaluation failure.
+ // evaluating after an unmodeled side effect.
//
// FIXME: Not all local state is mutable. Allow local constant subobjects
// to be read here (but take care with 'mutable' fields).
if (Frame && Info.getLangOpts().CPlusPlus14 &&
- (Info.EvalStatus.HasSideEffects || Info.keepEvaluatingAfterFailure()))
+ Info.sideEffectsMightHaveOccurred())
return CompleteObject();
return CompleteObject(BaseVal, BaseType);
@@ -3247,7 +3280,7 @@
assert(BO->getOpcode() == BO_PtrMemD || BO->getOpcode() == BO_PtrMemI);
if (!EvaluateObjectArgument(Info, BO->getLHS(), LV)) {
- if (Info.keepEvaluatingAfterFailure()) {
+ if (Info.noteFailure()) {
MemberPtr MemPtr;
EvaluateMemberPointer(BO->getRHS(), MemPtr, Info);
}
@@ -3541,7 +3574,7 @@
// FIXME: This isn't quite right; if we're performing aggregate
// initialization, each braced subexpression is its own full-expression.
FullExpressionRAII Scope(Info);
- if (!EvaluateDecl(Info, DclIt) && !Info.keepEvaluatingAfterFailure())
+ if (!EvaluateDecl(Info, DclIt) && !Info.noteFailure())
return ESR_Failed;
}
return ESR_Succeeded;
@@ -3816,7 +3849,7 @@
if (!Evaluate(ArgValues[I - Args.begin()], Info, *I)) {
// If we're checking for a potential constant expression, evaluate all
// initializers even if some of them fail.
- if (!Info.keepEvaluatingAfterFailure())
+ if (!Info.noteFailure())
return false;
Success = false;
}
@@ -4008,7 +4041,7 @@
*Value, FD))) {
// If we're checking for a potential constant expression, evaluate all
// initializers even if some of them fail.
- if (!Info.keepEvaluatingAfterFailure())
+ if (!Info.noteFailure())
return false;
Success = false;
}
@@ -4041,6 +4074,9 @@
template<typename ConditionalOperator>
void CheckPotentialConstantConditional(const ConditionalOperator *E) {
assert(Info.checkingPotentialConstantExpression());
+ assert(
+ Info.EvalStatus.HasFailure &&
+ "Need multiple speculative eval RAIIs below if we haven't failed yet.");
// Speculatively evaluate both arms.
{
@@ -4065,6 +4101,7 @@
bool HandleConditionalOperator(const ConditionalOperator *E) {
bool BoolResult;
if (!EvaluateAsBooleanCondition(E->getCond(), BoolResult, Info)) {
+ Info.noteFailure();
if (Info.checkingPotentialConstantExpression())
CheckPotentialConstantConditional(E);
return false;
@@ -4853,7 +4890,7 @@
// The overall lvalue result is the result of evaluating the LHS.
if (!this->Visit(CAO->getLHS())) {
- if (Info.keepEvaluatingAfterFailure())
+ if (Info.noteFailure())
Evaluate(RHS, this->Info, CAO->getRHS());
return false;
}
@@ -4874,7 +4911,7 @@
APValue NewVal;
if (!this->Visit(E->getLHS())) {
- if (Info.keepEvaluatingAfterFailure())
+ if (Info.noteFailure())
Evaluate(NewVal, this->Info, E->getRHS());
return false;
}
@@ -4962,7 +4999,7 @@
std::swap(PExp, IExp);
bool EvalPtrOK = EvaluatePointer(PExp, Result, Info);
- if (!EvalPtrOK && !Info.keepEvaluatingAfterFailure())
+ if (!EvalPtrOK && !Info.noteFailure())
return false;
llvm::APSInt Offset;
@@ -5465,7 +5502,7 @@
APValue &FieldVal = Result.getStructBase(ElementNo);
if (!EvaluateInPlace(FieldVal, Info, Subobject, Init)) {
- if (!Info.keepEvaluatingAfterFailure())
+ if (!Info.noteFailure())
return false;
Success = false;
}
@@ -5503,7 +5540,7 @@
if (!EvaluateInPlace(FieldVal, Info, Subobject, Init) ||
(Field->isBitField() && !truncateBitfieldValue(Info, Init,
FieldVal, Field))) {
- if (!Info.keepEvaluatingAfterFailure())
+ if (!Info.noteFailure())
return false;
Success = false;
}
@@ -5953,7 +5990,7 @@
Info, Subobject, Init) ||
!HandleLValueArrayAdjustment(Info, Init, Subobject,
CAT->getElementType(), 1)) {
- if (!Info.keepEvaluatingAfterFailure())
+ if (!Info.noteFailure())
return false;
Success = false;
}
@@ -7180,7 +7217,7 @@
assert(E->getLHS()->getType()->isIntegralOrEnumerationType() &&
E->getRHS()->getType()->isIntegralOrEnumerationType());
- if (LHSResult.Failed && !Info.keepEvaluatingAfterFailure())
+ if (LHSResult.Failed && !Info.noteFailure())
return false; // Ignore RHS;
return true;
@@ -7333,7 +7370,7 @@
}
bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
- if (!Info.keepEvaluatingAfterFailure() && E->isAssignmentOp())
+ if (E->isAssignmentOp() && !Info.noteFailure())
return Error(E);
if (DataRecursiveIntBinOpEvaluator::shouldEnqueue(E))
@@ -7358,7 +7395,7 @@
} else {
LHSOK = EvaluateComplex(E->getLHS(), LHS, Info);
}
- if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.noteFailure())
return false;
if (E->getRHS()->getType()->isRealFloatingType()) {
@@ -7406,7 +7443,7 @@
APFloat RHS(0.0), LHS(0.0);
bool LHSOK = EvaluateFloat(E->getRHS(), RHS, Info);
- if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.noteFailure())
return false;
if (!EvaluateFloat(E->getLHS(), LHS, Info) || !LHSOK)
@@ -7440,7 +7477,7 @@
LValue LHSValue, RHSValue;
bool LHSOK = EvaluatePointer(E->getLHS(), LHSValue, Info);
- if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.noteFailure())
return false;
if (!EvaluatePointer(E->getRHS(), RHSValue, Info) || !LHSOK)
@@ -7657,7 +7694,7 @@
MemberPtr LHSValue, RHSValue;
bool LHSOK = EvaluateMemberPointer(E->getLHS(), LHSValue, Info);
- if (!LHSOK && Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.noteFailure())
return false;
if (!EvaluateMemberPointer(E->getRHS(), RHSValue, Info) || !LHSOK)
@@ -8229,7 +8266,7 @@
APFloat RHS(0.0);
bool LHSOK = EvaluateFloat(E->getLHS(), Result, Info);
- if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.noteFailure())
return false;
return EvaluateFloat(E->getRHS(), RHS, Info) && LHSOK &&
handleFloatFloatBinOp(Info, E, Result, E->getOpcode(), RHS);
@@ -8506,7 +8543,7 @@
} else {
LHSOK = Visit(E->getLHS());
}
- if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.noteFailure())
return false;
ComplexValue RHS;
@@ -9643,9 +9680,12 @@
for (ArrayRef<const Expr*>::iterator I = Args.begin(), E = Args.end();
I != E; ++I) {
if ((*I)->isValueDependent() ||
- !Evaluate(ArgValues[I - Args.begin()], Info, *I))
- // If evaluation fails, throw away the argument entirely.
+ !Evaluate(ArgValues[I - Args.begin()], Info, *I)) {
+ // If evaluation fails, throw away the argument entirely, and note that
+ // something might have failed.
ArgValues[I - Args.begin()] = APValue();
+ Info.noteFailure();
+ }
if (Info.EvalStatus.HasSideEffects)
return false;
}
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -538,6 +538,13 @@
/// Likewise, INT_MAX + 1 can be folded to INT_MIN, but has UB.
bool HasUndefinedBehavior;
+ /// \brief Whether the evaluation was unable to reduce a construct to a
+ /// value.
+ ///
+ /// For example, in foo(bar, baz, qux), if evaluating bar fails, we may
+ /// try to evaluate baz and qux anyway.
+ bool HasFailure;
+
/// Diag - If this is non-null, it will be filled in with a stack of notes
/// indicating why evaluation failed (or why it failed to produce a constant
/// expression).
@@ -548,7 +555,8 @@
SmallVectorImpl<PartialDiagnosticAt> *Diag;
EvalStatus()
- : HasSideEffects(false), HasUndefinedBehavior(false), Diag(nullptr) {}
+ : HasSideEffects(false), HasUndefinedBehavior(false), HasFailure(false),
+ Diag(nullptr) {}
// hasSideEffects - Return true if the evaluated expression has
// side effects.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
