r294800 - Don't let EvaluationModes dictate whether an invalid base is OK
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 10 17:00:53 PST 2017
Sgtm. Go ahead and merge when the bots have chewed on it for a bit,
otherwise I'll do it next week.
Thanks,
Hans
On Fri, Feb 10, 2017 at 3:06 PM, George Burgess IV
<george.burgess.iv at gmail.com> wrote:
> Hi Hans!
>
> This fixes PR31843, which is a release blocker. Once the bots seem happy
> with it, can we merge this into the 4.0 branch, please?
>
> (Richard okayed this when he LGTM'ed the patch)
>
> Thanks,
> George
>
> On Fri, Feb 10, 2017 at 2:52 PM, George Burgess IV via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: gbiv
>> Date: Fri Feb 10 16:52:29 2017
>> New Revision: 294800
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=294800&view=rev
>> Log:
>> Don't let EvaluationModes dictate whether an invalid base is OK
>>
>> What we want to actually control this behavior is something more local
>> than an EvalutationMode. Please see the linked revision for more
>> discussion on why/etc.
>>
>> This fixes PR31843.
>>
>> Differential Revision: https://reviews.llvm.org/D29469
>>
>> Modified:
>> cfe/trunk/lib/AST/ExprConstant.cpp
>> cfe/trunk/test/CodeGen/object-size.c
>> cfe/trunk/test/Sema/builtin-object-size.c
>>
>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=294800&r1=294799&r2=294800&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb 10 16:52:29 2017
>> @@ -616,10 +616,12 @@ namespace {
>> /// gets a chance to look at it.
>> EM_PotentialConstantExpressionUnevaluated,
>>
>> - /// Evaluate as a constant expression. Continue evaluating if
>> either:
>> - /// - We find a MemberExpr with a base that can't be evaluated.
>> - /// - We find a variable initialized with a call to a function that
>> has
>> - /// the alloc_size attribute on it.
>> + /// Evaluate as a constant expression. In certain scenarios, if:
>> + /// - we find a MemberExpr with a base that can't be evaluated, or
>> + /// - we find a variable initialized with a call to a function that
>> has
>> + /// the alloc_size attribute on it
>> + /// then we may consider evaluation to have succeeded.
>> + ///
>> /// In either case, the LValue returned shall have an invalid base;
>> in the
>> /// former, the base will be the invalid MemberExpr, in the latter,
>> the
>> /// base will be either the alloc_size CallExpr or a CastExpr
>> wrapping
>> @@ -902,10 +904,6 @@ namespace {
>> return KeepGoing;
>> }
>>
>> - bool allowInvalidBaseExpr() const {
>> - return EvalMode == EM_OffsetFold;
>> - }
>> -
>> class ArrayInitLoopIndex {
>> EvalInfo &Info;
>> uint64_t OuterIndex;
>> @@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev
>> static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
>> const LValue &This, const Expr *E,
>> bool AllowNonLiteralTypes = false);
>> -static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo
>> &Info);
>> -static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>> &Info);
>> +static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
>> + bool InvalidBaseOK = false);
>> +static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>> &Info,
>> + bool InvalidBaseOK = false);
>> static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
>> EvalInfo &Info);
>> static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo
>> &Info);
>> @@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase
>> : public ExprEvaluatorBase<Derived> {
>> protected:
>> LValue &Result;
>> + bool InvalidBaseOK;
>> typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;
>> typedef ExprEvaluatorBase<Derived> ExprEvaluatorBaseTy;
>>
>> @@ -4843,9 +4844,14 @@ protected:
>> return true;
>> }
>>
>> + bool evaluatePointer(const Expr *E, LValue &Result) {
>> + return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);
>> + }
>> +
>> public:
>> - LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result) :
>> - ExprEvaluatorBaseTy(Info), Result(Result) {}
>> + LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool
>> InvalidBaseOK)
>> + : ExprEvaluatorBaseTy(Info), Result(Result),
>> + InvalidBaseOK(InvalidBaseOK) {}
>>
>> bool Success(const APValue &V, const Expr *E) {
>> Result.setFrom(this->Info.Ctx, V);
>> @@ -4857,7 +4863,7 @@ public:
>> QualType BaseTy;
>> bool EvalOK;
>> if (E->isArrow()) {
>> - EvalOK = EvaluatePointer(E->getBase(), Result, this->Info);
>> + EvalOK = evaluatePointer(E->getBase(), Result);
>> BaseTy =
>> E->getBase()->getType()->castAs<PointerType>()->getPointeeType();
>> } else if (E->getBase()->isRValue()) {
>> assert(E->getBase()->getType()->isRecordType());
>> @@ -4868,7 +4874,7 @@ public:
>> BaseTy = E->getBase()->getType();
>> }
>> if (!EvalOK) {
>> - if (!this->Info.allowInvalidBaseExpr())
>> + if (!InvalidBaseOK)
>> return false;
>> Result.setInvalid(E);
>> return true;
>> @@ -4962,8 +4968,8 @@ namespace {
>> class LValueExprEvaluator
>> : public LValueExprEvaluatorBase<LValueExprEvaluator> {
>> public:
>> - LValueExprEvaluator(EvalInfo &Info, LValue &Result) :
>> - LValueExprEvaluatorBaseTy(Info, Result) {}
>> + LValueExprEvaluator(EvalInfo &Info, LValue &Result, bool InvalidBaseOK)
>> :
>> + LValueExprEvaluatorBaseTy(Info, Result, InvalidBaseOK) {}
>>
>> bool VisitVarDecl(const Expr *E, const VarDecl *VD);
>> bool VisitUnaryPreIncDec(const UnaryOperator *UO);
>> @@ -5016,10 +5022,11 @@ public:
>> /// * function designators in C, and
>> /// * "extern void" objects
>> /// * @selector() expressions in Objective-C
>> -static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info)
>> {
>> +static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
>> + bool InvalidBaseOK) {
>> assert(E->isGLValue() || E->getType()->isFunctionType() ||
>> E->getType()->isVoidType() || isa<ObjCSelectorExpr>(E));
>> - return LValueExprEvaluator(Info, Result).Visit(E);
>> + return LValueExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);
>> }
>>
>> bool LValueExprEvaluator::VisitDeclRefExpr(const DeclRefExpr *E) {
>> @@ -5180,7 +5187,7 @@ bool LValueExprEvaluator::VisitArraySubs
>> if (E->getBase()->getType()->isVectorType())
>> return Error(E);
>>
>> - if (!EvaluatePointer(E->getBase(), Result, Info))
>> + if (!evaluatePointer(E->getBase(), Result))
>> return false;
>>
>> APSInt Index;
>> @@ -5191,7 +5198,7 @@ bool LValueExprEvaluator::VisitArraySubs
>> }
>>
>> bool LValueExprEvaluator::VisitUnaryDeref(const UnaryOperator *E) {
>> - return EvaluatePointer(E->getSubExpr(), Result, Info);
>> + return evaluatePointer(E->getSubExpr(), Result);
>> }
>>
>> bool LValueExprEvaluator::VisitUnaryReal(const UnaryOperator *E) {
>> @@ -5339,7 +5346,7 @@ static bool getBytesReturnedByAllocSizeC
>> /// and mark Result's Base as invalid.
>> static bool evaluateLValueAsAllocSize(EvalInfo &Info, APValue::LValueBase
>> Base,
>> LValue &Result) {
>> - if (!Info.allowInvalidBaseExpr() || Base.isNull())
>> + if (Base.isNull())
>> return false;
>>
>> // Because we do no form of static analysis, we only support const
>> variables.
>> @@ -5373,17 +5380,27 @@ namespace {
>> class PointerExprEvaluator
>> : public ExprEvaluatorBase<PointerExprEvaluator> {
>> LValue &Result;
>> + bool InvalidBaseOK;
>>
>> bool Success(const Expr *E) {
>> Result.set(E);
>> return true;
>> }
>>
>> + bool evaluateLValue(const Expr *E, LValue &Result) {
>> + return EvaluateLValue(E, Result, Info, InvalidBaseOK);
>> + }
>> +
>> + bool evaluatePointer(const Expr *E, LValue &Result) {
>> + return EvaluatePointer(E, Result, Info, InvalidBaseOK);
>> + }
>> +
>> bool visitNonBuiltinCallExpr(const CallExpr *E);
>> public:
>>
>> - PointerExprEvaluator(EvalInfo &info, LValue &Result)
>> - : ExprEvaluatorBaseTy(info), Result(Result) {}
>> + PointerExprEvaluator(EvalInfo &info, LValue &Result, bool
>> InvalidBaseOK)
>> + : ExprEvaluatorBaseTy(info), Result(Result),
>> + InvalidBaseOK(InvalidBaseOK) {}
>>
>> bool Success(const APValue &V, const Expr *E) {
>> Result.setFrom(Info.Ctx, V);
>> @@ -5430,9 +5447,10 @@ public:
>> };
>> } // end anonymous namespace
>>
>> -static bool EvaluatePointer(const Expr* E, LValue& Result, EvalInfo
>> &Info) {
>> +static bool EvaluatePointer(const Expr* E, LValue& Result, EvalInfo
>> &Info,
>> + bool InvalidBaseOK) {
>> assert(E->isRValue() && E->getType()->hasPointerRepresentation());
>> - return PointerExprEvaluator(Info, Result).Visit(E);
>> + return PointerExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);
>> }
>>
>> bool PointerExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
>> @@ -5445,7 +5463,7 @@ bool PointerExprEvaluator::VisitBinaryOp
>> if (IExp->getType()->isPointerType())
>> std::swap(PExp, IExp);
>>
>> - bool EvalPtrOK = EvaluatePointer(PExp, Result, Info);
>> + bool EvalPtrOK = evaluatePointer(PExp, Result);
>> if (!EvalPtrOK && !Info.noteFailure())
>> return false;
>>
>> @@ -5461,7 +5479,7 @@ bool PointerExprEvaluator::VisitBinaryOp
>> }
>>
>> bool PointerExprEvaluator::VisitUnaryAddrOf(const UnaryOperator *E) {
>> - return EvaluateLValue(E->getSubExpr(), Result, Info);
>> + return evaluateLValue(E->getSubExpr(), Result);
>> }
>>
>> bool PointerExprEvaluator::VisitCastExpr(const CastExpr* E) {
>> @@ -5495,7 +5513,7 @@ bool PointerExprEvaluator::VisitCastExpr
>>
>> case CK_DerivedToBase:
>> case CK_UncheckedDerivedToBase:
>> - if (!EvaluatePointer(E->getSubExpr(), Result, Info))
>> + if (!evaluatePointer(E->getSubExpr(), Result))
>> return false;
>> if (!Result.Base && Result.Offset.isZero())
>> return true;
>> @@ -5542,7 +5560,7 @@ bool PointerExprEvaluator::VisitCastExpr
>> }
>> case CK_ArrayToPointerDecay:
>> if (SubExpr->isGLValue()) {
>> - if (!EvaluateLValue(SubExpr, Result, Info))
>> + if (!evaluateLValue(SubExpr, Result))
>> return false;
>> } else {
>> Result.set(SubExpr, Info.CurrentCall->Index);
>> @@ -5559,18 +5577,19 @@ bool PointerExprEvaluator::VisitCastExpr
>> return true;
>>
>> case CK_FunctionToPointerDecay:
>> - return EvaluateLValue(SubExpr, Result, Info);
>> + return evaluateLValue(SubExpr, Result);
>>
>> case CK_LValueToRValue: {
>> LValue LVal;
>> - if (!EvaluateLValue(E->getSubExpr(), LVal, Info))
>> + if (!evaluateLValue(E->getSubExpr(), LVal))
>> return false;
>>
>> APValue RVal;
>> // Note, we use the subexpression's type in order to retain
>> cv-qualifiers.
>> if (!handleLValueToRValueConversion(Info, E,
>> E->getSubExpr()->getType(),
>> LVal, RVal))
>> - return evaluateLValueAsAllocSize(Info, LVal.Base, Result);
>> + return InvalidBaseOK &&
>> + evaluateLValueAsAllocSize(Info, LVal.Base, Result);
>> return Success(RVal, E);
>> }
>> }
>> @@ -5615,7 +5634,7 @@ bool PointerExprEvaluator::visitNonBuilt
>> if (ExprEvaluatorBaseTy::VisitCallExpr(E))
>> return true;
>>
>> - if (!(Info.allowInvalidBaseExpr() && getAllocSizeAttr(E)))
>> + if (!(InvalidBaseOK && getAllocSizeAttr(E)))
>> return false;
>>
>> Result.setInvalid(E);
>> @@ -5638,12 +5657,12 @@ bool PointerExprEvaluator::VisitBuiltinC
>> unsigned BuiltinOp) {
>> switch (BuiltinOp) {
>> case Builtin::BI__builtin_addressof:
>> - return EvaluateLValue(E->getArg(0), Result, Info);
>> + return evaluateLValue(E->getArg(0), Result);
>> case Builtin::BI__builtin_assume_aligned: {
>> // We need to be very careful here because: if the pointer does not
>> have the
>> // asserted alignment, then the behavior is undefined, and undefined
>> // behavior is non-constant.
>> - if (!EvaluatePointer(E->getArg(0), Result, Info))
>> + if (!evaluatePointer(E->getArg(0), Result))
>> return false;
>>
>> LValue OffsetResult(Result);
>> @@ -6279,7 +6298,7 @@ class TemporaryExprEvaluator
>> : public LValueExprEvaluatorBase<TemporaryExprEvaluator> {
>> public:
>> TemporaryExprEvaluator(EvalInfo &Info, LValue &Result) :
>> - LValueExprEvaluatorBaseTy(Info, Result) {}
>> + LValueExprEvaluatorBaseTy(Info, Result, false) {}
>>
>> /// Visit an expression which constructs the value of this temporary.
>> bool VisitConstructExpr(const Expr *E) {
>> @@ -7383,7 +7402,8 @@ static bool tryEvaluateBuiltinObjectSize
>> if (!EvaluateAsRValue(Info, E, RVal))
>> return false;
>> LVal.setFrom(Info.Ctx, RVal);
>> - } else if (!EvaluatePointer(ignorePointerCastsAndParens(E), LVal,
>> Info))
>> + } else if (!EvaluatePointer(ignorePointerCastsAndParens(E), LVal,
>> Info,
>> + /*InvalidBaseOK=*/true))
>> return false;
>> }
>>
>>
>> Modified: cfe/trunk/test/CodeGen/object-size.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/object-size.c?rev=294800&r1=294799&r2=294800&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/object-size.c (original)
>> +++ cfe/trunk/test/CodeGen/object-size.c Fri Feb 10 16:52:29 2017
>> @@ -549,3 +549,22 @@ int incomplete_and_function_types() {
>> // CHECK: store i32 0
>> gi = __builtin_object_size(incomplete_char_array, 3);
>> }
>> +
>> +// Flips between the pointer and lvalue evaluator a lot.
>> +void deeply_nested() {
>> + struct {
>> + struct {
>> + struct {
>> + struct {
>> + int e[2];
>> + char f; // Inhibit our writing-off-the-end check
>> + } d[2];
>> + } c[2];
>> + } b[2];
>> + } *a;
>> +
>> + // CHECK: store i32 4
>> + gi = __builtin_object_size(&a->b[1].c[1].d[1].e[1], 1);
>> + // CHECK: store i32 4
>> + gi = __builtin_object_size(&a->b[1].c[1].d[1].e[1], 3);
>> +}
>>
>> Modified: cfe/trunk/test/Sema/builtin-object-size.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-object-size.c?rev=294800&r1=294799&r2=294800&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/builtin-object-size.c (original)
>> +++ cfe/trunk/test/Sema/builtin-object-size.c Fri Feb 10 16:52:29 2017
>> @@ -76,3 +76,18 @@ int pr28314(void) {
>> a += __builtin_object_size(p3->b, 0);
>> return a;
>> }
>> +
>> +int pr31843() {
>> + int n = 0;
>> +
>> + struct { int f; } a;
>> + int b;
>> + n += __builtin_object_size(({&(b ? &a : &a)->f; pr31843;}), 0); //
>> expected-warning{{expression result unused}}
>> +
>> + struct statfs { char f_mntonname[1024];};
>> + struct statfs *outStatFSBuf;
>> + n += __builtin_object_size(outStatFSBuf->f_mntonname ? "" : "", 1); //
>> expected-warning{{address of array}}
>> + n += __builtin_object_size(outStatFSBuf->f_mntonname ?: "", 1);
>> +
>> + return n;
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list