Merged in r295087.
On Fri, Feb 10, 2017 at 5:00 PM, Hans Wennborg <h...@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...@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@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@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
