<div dir="ltr">Hi Hans!<div><br></div><div>This fixes PR<span style="font-size:12.8px">31843, which is a release blocker. Once the bots seem happy with it, can we merge this into the 4.0 branch, please?</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">(Richard okayed this when he LGTM'ed the patch)</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Thanks,</span></div><div><span style="font-size:12.8px">George</span></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 10, 2017 at 2:52 PM, George Burgess IV via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: gbiv<br>
Date: Fri Feb 10 16:52:29 2017<br>
New Revision: 294800<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=294800&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=294800&view=rev</a><br>
Log:<br>
Don't let EvaluationModes dictate whether an invalid base is OK<br>
<br>
What we want to actually control this behavior is something more local<br>
than an EvalutationMode. Please see the linked revision for more<br>
discussion on why/etc.<br>
<br>
This fixes PR31843.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D29469" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2946<wbr>9</a><br>
<br>
Modified:<br>
cfe/trunk/lib/AST/ExprConstant<wbr>.cpp<br>
cfe/trunk/test/CodeGen/object-<wbr>size.c<br>
cfe/trunk/test/Sema/builtin-ob<wbr>ject-size.c<br>
<br>
Modified: cfe/trunk/lib/AST/ExprConstant<wbr>.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=294800&r1=294799&r2=294800&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/AST/ExprCo<wbr>nstant.cpp?rev=294800&r1=29479<wbr>9&r2=294800&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/AST/ExprConstant<wbr>.cpp (original)<br>
+++ cfe/trunk/lib/AST/ExprConstant<wbr>.cpp Fri Feb 10 16:52:29 2017<br>
@@ -616,10 +616,12 @@ namespace {<br>
/// gets a chance to look at it.<br>
EM_PotentialConstantExpressio<wbr>nUnevaluated,<br>
<br>
- /// Evaluate as a constant expression. Continue evaluating if either:<br>
- /// - We find a MemberExpr with a base that can't be evaluated.<br>
- /// - We find a variable initialized with a call to a function that has<br>
- /// the alloc_size attribute on it.<br>
+ /// Evaluate as a constant expression. In certain scenarios, if:<br>
+ /// - we find a MemberExpr with a base that can't be evaluated, or<br>
+ /// - we find a variable initialized with a call to a function that has<br>
+ /// the alloc_size attribute on it<br>
+ /// then we may consider evaluation to have succeeded.<br>
+ ///<br>
/// In either case, the LValue returned shall have an invalid base; in the<br>
/// former, the base will be the invalid MemberExpr, in the latter, the<br>
/// base will be either the alloc_size CallExpr or a CastExpr wrapping<br>
@@ -902,10 +904,6 @@ namespace {<br>
return KeepGoing;<br>
}<br>
<br>
- bool allowInvalidBaseExpr() const {<br>
- return EvalMode == EM_OffsetFold;<br>
- }<br>
-<br>
class ArrayInitLoopIndex {<br>
EvalInfo &Info;<br>
uint64_t OuterIndex;<br>
@@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev<br>
static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,<br>
const LValue &This, const Expr *E,<br>
bool AllowNonLiteralTypes = false);<br>
-static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info);<br>
-static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo &Info);<br>
+static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,<br>
+ bool InvalidBaseOK = false);<br>
+static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo &Info,<br>
+ bool InvalidBaseOK = false);<br>
static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,<br>
EvalInfo &Info);<br>
static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info);<br>
@@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase<br>
: public ExprEvaluatorBase<Derived> {<br>
protected:<br>
LValue &Result;<br>
+ bool InvalidBaseOK;<br>
typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;<br>
typedef ExprEvaluatorBase<Derived> ExprEvaluatorBaseTy;<br>
<br>
@@ -4843,9 +4844,14 @@ protected:<br>
return true;<br>
}<br>
<br>
+ bool evaluatePointer(const Expr *E, LValue &Result) {<br>
+ return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);<br>
+ }<br>
+<br>
public:<br>
- LValueExprEvaluatorBase(EvalIn<wbr>fo &Info, LValue &Result) :<br>
- ExprEvaluatorBaseTy(Info), Result(Result) {}<br>
+ LValueExprEvaluatorBase(EvalIn<wbr>fo &Info, LValue &Result, bool InvalidBaseOK)<br>
+ : ExprEvaluatorBaseTy(Info), Result(Result),<br>
+ InvalidBaseOK(InvalidBaseOK) {}<br>
<br>
bool Success(const APValue &V, const Expr *E) {<br>
Result.setFrom(this->Info.Ctx<wbr>, V);<br>
@@ -4857,7 +4863,7 @@ public:<br>
QualType BaseTy;<br>
bool EvalOK;<br>
if (E->isArrow()) {<br>
- EvalOK = EvaluatePointer(E->getBase(), Result, this->Info);<br>
+ EvalOK = evaluatePointer(E->getBase(), Result);<br>
BaseTy = E->getBase()->getType()->castA<wbr>s<PointerType>()->getPointeeTy<wbr>pe();<br>
} else if (E->getBase()->isRValue()) {<br>
assert(E->getBase()->getType(<wbr>)->isRecordType());<br>
@@ -4868,7 +4874,7 @@ public:<br>
BaseTy = E->getBase()->getType();<br>
}<br>
if (!EvalOK) {<br>
- if (!this->Info.allowInvalidBaseE<wbr>xpr())<br>
+ if (!InvalidBaseOK)<br>
return false;<br>
Result.setInvalid(E);<br>
return true;<br>
@@ -4962,8 +4968,8 @@ namespace {<br>
class LValueExprEvaluator<br>
: public LValueExprEvaluatorBase<LValue<wbr>ExprEvaluator> {<br>
public:<br>
- LValueExprEvaluator(EvalInfo &Info, LValue &Result) :<br>
- LValueExprEvaluatorBaseTy(Info<wbr>, Result) {}<br>
+ LValueExprEvaluator(EvalInfo &Info, LValue &Result, bool InvalidBaseOK) :<br>
+ LValueExprEvaluatorBaseTy(Info<wbr>, Result, InvalidBaseOK) {}<br>
<br>
bool VisitVarDecl(const Expr *E, const VarDecl *VD);<br>
bool VisitUnaryPreIncDec(const UnaryOperator *UO);<br>
@@ -5016,10 +5022,11 @@ public:<br>
/// * function designators in C, and<br>
/// * "extern void" objects<br>
/// * @selector() expressions in Objective-C<br>
-static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info) {<br>
+static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,<br>
+ bool InvalidBaseOK) {<br>
assert(E->isGLValue() || E->getType()->isFunctionType() ||<br>
E->getType()->isVoidType() || isa<ObjCSelectorExpr>(E));<br>
- return LValueExprEvaluator(Info, Result).Visit(E);<br>
+ return LValueExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);<br>
}<br>
<br>
bool LValueExprEvaluator::VisitDecl<wbr>RefExpr(const DeclRefExpr *E) {<br>
@@ -5180,7 +5187,7 @@ bool LValueExprEvaluator::VisitArra<wbr>ySubs<br>
if (E->getBase()->getType()->isVe<wbr>ctorType())<br>
return Error(E);<br>
<br>
- if (!EvaluatePointer(E->getBase()<wbr>, Result, Info))<br>
+ if (!evaluatePointer(E->getBase()<wbr>, Result))<br>
return false;<br>
<br>
APSInt Index;<br>
@@ -5191,7 +5198,7 @@ bool LValueExprEvaluator::VisitArra<wbr>ySubs<br>
}<br>
<br>
bool LValueExprEvaluator::VisitUnar<wbr>yDeref(const UnaryOperator *E) {<br>
- return EvaluatePointer(E->getSubExpr(<wbr>), Result, Info);<br>
+ return evaluatePointer(E->getSubExpr(<wbr>), Result);<br>
}<br>
<br>
bool LValueExprEvaluator::VisitUnar<wbr>yReal(const UnaryOperator *E) {<br>
@@ -5339,7 +5346,7 @@ static bool getBytesReturnedByAllocSizeC<br>
/// and mark Result's Base as invalid.<br>
static bool evaluateLValueAsAllocSize(Eval<wbr>Info &Info, APValue::LValueBase Base,<br>
LValue &Result) {<br>
- if (!Info.allowInvalidBaseExpr() || Base.isNull())<br>
+ if (Base.isNull())<br>
return false;<br>
<br>
// Because we do no form of static analysis, we only support const variables.<br>
@@ -5373,17 +5380,27 @@ namespace {<br>
class PointerExprEvaluator<br>
: public ExprEvaluatorBase<PointerExprE<wbr>valuator> {<br>
LValue &Result;<br>
+ bool InvalidBaseOK;<br>
<br>
bool Success(const Expr *E) {<br>
Result.set(E);<br>
return true;<br>
}<br>
<br>
+ bool evaluateLValue(const Expr *E, LValue &Result) {<br>
+ return EvaluateLValue(E, Result, Info, InvalidBaseOK);<br>
+ }<br>
+<br>
+ bool evaluatePointer(const Expr *E, LValue &Result) {<br>
+ return EvaluatePointer(E, Result, Info, InvalidBaseOK);<br>
+ }<br>
+<br>
bool visitNonBuiltinCallExpr(const CallExpr *E);<br>
public:<br>
<br>
- PointerExprEvaluator(EvalInfo &info, LValue &Result)<br>
- : ExprEvaluatorBaseTy(info), Result(Result) {}<br>
+ PointerExprEvaluator(EvalInfo &info, LValue &Result, bool InvalidBaseOK)<br>
+ : ExprEvaluatorBaseTy(info), Result(Result),<br>
+ InvalidBaseOK(InvalidBaseOK) {}<br>
<br>
bool Success(const APValue &V, const Expr *E) {<br>
Result.setFrom(Info.Ctx, V);<br>
@@ -5430,9 +5447,10 @@ public:<br>
};<br>
} // end anonymous namespace<br>
<br>
-static bool EvaluatePointer(const Expr* E, LValue& Result, EvalInfo &Info) {<br>
+static bool EvaluatePointer(const Expr* E, LValue& Result, EvalInfo &Info,<br>
+ bool InvalidBaseOK) {<br>
assert(E->isRValue() && E->getType()->hasPointerRepres<wbr>entation());<br>
- return PointerExprEvaluator(Info, Result).Visit(E);<br>
+ return PointerExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);<br>
}<br>
<br>
bool PointerExprEvaluator::VisitBin<wbr>aryOperator(const BinaryOperator *E) {<br>
@@ -5445,7 +5463,7 @@ bool PointerExprEvaluator::VisitBin<wbr>aryOp<br>
if (IExp->getType()->isPointerTyp<wbr>e())<br>
std::swap(PExp, IExp);<br>
<br>
- bool EvalPtrOK = EvaluatePointer(PExp, Result, Info);<br>
+ bool EvalPtrOK = evaluatePointer(PExp, Result);<br>
if (!EvalPtrOK && !Info.noteFailure())<br>
return false;<br>
<br>
@@ -5461,7 +5479,7 @@ bool PointerExprEvaluator::VisitBin<wbr>aryOp<br>
}<br>
<br>
bool PointerExprEvaluator::VisitUna<wbr>ryAddrOf(const UnaryOperator *E) {<br>
- return EvaluateLValue(E->getSubExpr()<wbr>, Result, Info);<br>
+ return evaluateLValue(E->getSubExpr()<wbr>, Result);<br>
}<br>
<br>
bool PointerExprEvaluator::VisitCas<wbr>tExpr(const CastExpr* E) {<br>
@@ -5495,7 +5513,7 @@ bool PointerExprEvaluator::VisitCas<wbr>tExpr<br>
<br>
case CK_DerivedToBase:<br>
case CK_UncheckedDerivedToBase:<br>
- if (!EvaluatePointer(E->getSubExp<wbr>r(), Result, Info))<br>
+ if (!evaluatePointer(E->getSubExp<wbr>r(), Result))<br>
return false;<br>
if (!Result.Base && Result.Offset.isZero())<br>
return true;<br>
@@ -5542,7 +5560,7 @@ bool PointerExprEvaluator::VisitCas<wbr>tExpr<br>
}<br>
case CK_ArrayToPointerDecay:<br>
if (SubExpr->isGLValue()) {<br>
- if (!EvaluateLValue(SubExpr, Result, Info))<br>
+ if (!evaluateLValue(SubExpr, Result))<br>
return false;<br>
} else {<br>
Result.set(SubExpr, Info.CurrentCall->Index);<br>
@@ -5559,18 +5577,19 @@ bool PointerExprEvaluator::VisitCas<wbr>tExpr<br>
return true;<br>
<br>
case CK_FunctionToPointerDecay:<br>
- return EvaluateLValue(SubExpr, Result, Info);<br>
+ return evaluateLValue(SubExpr, Result);<br>
<br>
case CK_LValueToRValue: {<br>
LValue LVal;<br>
- if (!EvaluateLValue(E->getSubExpr<wbr>(), LVal, Info))<br>
+ if (!evaluateLValue(E->getSubExpr<wbr>(), LVal))<br>
return false;<br>
<br>
APValue RVal;<br>
// Note, we use the subexpression's type in order to retain cv-qualifiers.<br>
if (!handleLValueToRValueConversi<wbr>on(Info, E, E->getSubExpr()->getType(),<br>
LVal, RVal))<br>
- return evaluateLValueAsAllocSize(Info<wbr>, LVal.Base, Result);<br>
+ return InvalidBaseOK &&<br>
+ evaluateLValueAsAllocSize(Inf<wbr>o, LVal.Base, Result);<br>
return Success(RVal, E);<br>
}<br>
}<br>
@@ -5615,7 +5634,7 @@ bool PointerExprEvaluator::visitNon<wbr>Built<br>
if (ExprEvaluatorBaseTy::VisitCal<wbr>lExpr(E))<br>
return true;<br>
<br>
- if (!(Info.allowInvalidBaseExpr() && getAllocSizeAttr(E)))<br>
+ if (!(InvalidBaseOK && getAllocSizeAttr(E)))<br>
return false;<br>
<br>
Result.setInvalid(E);<br>
@@ -5638,12 +5657,12 @@ bool PointerExprEvaluator::VisitBui<wbr>ltinC<br>
unsigned BuiltinOp) {<br>
switch (BuiltinOp) {<br>
case Builtin::BI__builtin_addressof<wbr>:<br>
- return EvaluateLValue(E->getArg(0), Result, Info);<br>
+ return evaluateLValue(E->getArg(0), Result);<br>
case Builtin::BI__builtin_assume_al<wbr>igned: {<br>
// We need to be very careful here because: if the pointer does not have the<br>
// asserted alignment, then the behavior is undefined, and undefined<br>
// behavior is non-constant.<br>
- if (!EvaluatePointer(E->getArg(0)<wbr>, Result, Info))<br>
+ if (!evaluatePointer(E->getArg(0)<wbr>, Result))<br>
return false;<br>
<br>
LValue OffsetResult(Result);<br>
@@ -6279,7 +6298,7 @@ class TemporaryExprEvaluator<br>
: public LValueExprEvaluatorBase<Tempor<wbr>aryExprEvaluator> {<br>
public:<br>
TemporaryExprEvaluator(EvalIn<wbr>fo &Info, LValue &Result) :<br>
- LValueExprEvaluatorBaseTy(Info<wbr>, Result) {}<br>
+ LValueExprEvaluatorBaseTy(Info<wbr>, Result, false) {}<br>
<br>
/// Visit an expression which constructs the value of this temporary.<br>
bool VisitConstructExpr(const Expr *E) {<br>
@@ -7383,7 +7402,8 @@ static bool tryEvaluateBuiltinObjectSize<br>
if (!EvaluateAsRValue(Info, E, RVal))<br>
return false;<br>
LVal.setFrom(Info.Ctx, RVal);<br>
- } else if (!EvaluatePointer(ignorePointe<wbr>rCastsAndParens(E), LVal, Info))<br>
+ } else if (!EvaluatePointer(ignorePointe<wbr>rCastsAndParens(E), LVal, Info,<br>
+ /*InvalidBaseOK=*/true))<br>
return false;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/CodeGen/object-<wbr>size.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/object-size.c?rev=294800&r1=294799&r2=294800&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/CodeGen/o<wbr>bject-size.c?rev=294800&r1=294<wbr>799&r2=294800&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CodeGen/object-<wbr>size.c (original)<br>
+++ cfe/trunk/test/CodeGen/object-<wbr>size.c Fri Feb 10 16:52:29 2017<br>
@@ -549,3 +549,22 @@ int incomplete_and_function_types(<wbr>) {<br>
// CHECK: store i32 0<br>
gi = __builtin_object_size(incomple<wbr>te_char_array, 3);<br>
}<br>
+<br>
+// Flips between the pointer and lvalue evaluator a lot.<br>
+void deeply_nested() {<br>
+ struct {<br>
+ struct {<br>
+ struct {<br>
+ struct {<br>
+ int e[2];<br>
+ char f; // Inhibit our writing-off-the-end check<br>
+ } d[2];<br>
+ } c[2];<br>
+ } b[2];<br>
+ } *a;<br>
+<br>
+ // CHECK: store i32 4<br>
+ gi = __builtin_object_size(&a->b[1]<wbr>.c[1].d[1].e[1], 1);<br>
+ // CHECK: store i32 4<br>
+ gi = __builtin_object_size(&a->b[1]<wbr>.c[1].d[1].e[1], 3);<br>
+}<br>
<br>
Modified: cfe/trunk/test/Sema/builtin-ob<wbr>ject-size.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-object-size.c?rev=294800&r1=294799&r2=294800&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Sema/buil<wbr>tin-object-size.c?rev=294800&r<wbr>1=294799&r2=294800&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Sema/builtin-ob<wbr>ject-size.c (original)<br>
+++ cfe/trunk/test/Sema/builtin-ob<wbr>ject-size.c Fri Feb 10 16:52:29 2017<br>
@@ -76,3 +76,18 @@ int pr28314(void) {<br>
a += __builtin_object_size(p3->b, 0);<br>
return a;<br>
}<br>
+<br>
+int pr31843() {<br>
+ int n = 0;<br>
+<br>
+ struct { int f; } a;<br>
+ int b;<br>
+ n += __builtin_object_size(({&(b ? &a : &a)->f; pr31843;}), 0); // expected-warning{{expression result unused}}<br>
+<br>
+ struct statfs { char f_mntonname[1024];};<br>
+ struct statfs *outStatFSBuf;<br>
+ n += __builtin_object_size(outStatF<wbr>SBuf->f_mntonname ? "" : "", 1); // expected-warning{{address of array}}<br>
+ n += __builtin_object_size(outStatF<wbr>SBuf->f_mntonname ?: "", 1);<br>
+<br>
+ return n;<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>