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
