r294800 - Don't let EvaluationModes dictate whether an invalid base is OK
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 14 11:15:17 PST 2017
Merged in r295087.
On Fri, Feb 10, 2017 at 5:00 PM, Hans Wennborg <hans at chromium.org> wrote:
> 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