From: George Burgess IV Date: Fri, 4 Sep 2015 21:28:13 +0000 (+0000) Subject: Increase accuracy of __builtin_object_size. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1270c8463202754b4093cc7d75481afbe31808ba;p=clang Increase accuracy of __builtin_object_size. Improvements: - For all types, we would give up in a case such as: __builtin_object_size((char*)&foo, N); even if we could provide an answer to __builtin_object_size(&foo, N); We now provide the same answer for both of the above examples in all cases. - For type=1|3, we now support subobjects with unknown bases, as long as the designator is valid. Thanks to Richard Smith for the review + design planning. Review: http://reviews.llvm.org/D12169 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@246877 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index 8aea10d516..dea85d84a2 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -492,7 +492,11 @@ namespace { /// optimizer if we don't constant fold them here, but in an unevaluated /// context we try to fold them immediately since the optimizer never /// gets a chance to look at it. - EM_PotentialConstantExpressionUnevaluated + EM_PotentialConstantExpressionUnevaluated, + + /// Evaluate as a constant expression. Continue evaluating if we find a + /// MemberExpr with a base that can't be evaluated. + EM_DesignatorFold, } EvalMode; /// Are we checking whether the expression is a potential constant @@ -595,6 +599,7 @@ namespace { case EM_PotentialConstantExpression: case EM_ConstantExpressionUnevaluated: case EM_PotentialConstantExpressionUnevaluated: + case EM_DesignatorFold: HasActiveDiagnostic = false; return OptionalDiagnostic(); } @@ -674,6 +679,7 @@ namespace { case EM_ConstantExpression: case EM_ConstantExpressionUnevaluated: case EM_ConstantFold: + case EM_DesignatorFold: return false; } llvm_unreachable("Missed EvalMode case"); @@ -702,10 +708,15 @@ namespace { case EM_ConstantExpressionUnevaluated: case EM_ConstantFold: case EM_IgnoreSideEffects: + case EM_DesignatorFold: return false; } llvm_unreachable("Missed EvalMode case"); } + + bool allowInvalidBaseExpr() const { + return EvalMode == EM_DesignatorFold; + } }; /// Object used to treat all foldable expressions as constant expressions. @@ -736,6 +747,21 @@ namespace { } }; + /// RAII object used to treat the current evaluation as the correct pointer + /// offset fold for the current EvalMode + struct FoldOffsetRAII { + EvalInfo &Info; + EvalInfo::EvaluationMode OldMode; + explicit FoldOffsetRAII(EvalInfo &Info, bool Subobject) + : Info(Info), OldMode(Info.EvalMode) { + if (!Info.checkingPotentialConstantExpression()) + Info.EvalMode = Subobject ? EvalInfo::EM_DesignatorFold + : EvalInfo::EM_ConstantFold; + } + + ~FoldOffsetRAII() { Info.EvalMode = OldMode; } + }; + /// RAII object used to suppress diagnostics and side-effects from a /// speculative evaluation. class SpeculativeEvaluationRAII { @@ -917,7 +943,8 @@ namespace { struct LValue { APValue::LValueBase Base; CharUnits Offset; - unsigned CallIndex; + bool InvalidBase : 1; + unsigned CallIndex : 31; SubobjectDesignator Designator; const APValue::LValueBase getLValueBase() const { return Base; } @@ -938,17 +965,23 @@ namespace { assert(V.isLValue()); Base = V.getLValueBase(); Offset = V.getLValueOffset(); + InvalidBase = false; CallIndex = V.getLValueCallIndex(); Designator = SubobjectDesignator(Ctx, V); } - void set(APValue::LValueBase B, unsigned I = 0) { + void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) { Base = B; Offset = CharUnits::Zero(); + InvalidBase = BInvalid; CallIndex = I; Designator = SubobjectDesignator(getType(B)); } + void setInvalid(APValue::LValueBase B, unsigned I = 0) { + set(B, I, true); + } + // Check that this LValue is not based on a null pointer. If it is, produce // a diagnostic and mark the designator as invalid. bool checkNullPointer(EvalInfo &Info, const Expr *E, @@ -4368,20 +4401,23 @@ public: bool VisitMemberExpr(const MemberExpr *E) { // Handle non-static data members. QualType BaseTy; + bool EvalOK; if (E->isArrow()) { - if (!EvaluatePointer(E->getBase(), Result, this->Info)) - return false; + EvalOK = EvaluatePointer(E->getBase(), Result, this->Info); BaseTy = E->getBase()->getType()->castAs()->getPointeeType(); } else if (E->getBase()->isRValue()) { assert(E->getBase()->getType()->isRecordType()); - if (!EvaluateTemporary(E->getBase(), Result, this->Info)) - return false; + EvalOK = EvaluateTemporary(E->getBase(), Result, this->Info); BaseTy = E->getBase()->getType(); } else { - if (!this->Visit(E->getBase())) - return false; + EvalOK = this->Visit(E->getBase()); BaseTy = E->getBase()->getType(); } + if (!EvalOK) { + if (!this->Info.allowInvalidBaseExpr()) + return false; + Result.setInvalid(E->getBase()); + } const ValueDecl *MD = E->getMemberDecl(); if (const FieldDecl *FD = dyn_cast(E->getMemberDecl())) { @@ -4793,7 +4829,7 @@ public: bool VisitObjCStringLiteral(const ObjCStringLiteral *E) { return Success(E); } bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E) - { return Success(E); } + { return Success(E); } bool VisitAddrLabelExpr(const AddrLabelExpr *E) { return Success(E); } bool VisitCallExpr(const CallExpr *E); @@ -4919,6 +4955,7 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr* E) { unsigned Size = Info.Ctx.getTypeSize(E->getType()); uint64_t N = Value.getInt().extOrTrunc(Size).getZExtValue(); Result.Base = (Expr*)nullptr; + Result.InvalidBase = false; Result.Offset = CharUnits::fromQuantity(N); Result.CallIndex = 0; Result.Designator.setInvalid(); @@ -6211,6 +6248,26 @@ static QualType getObjectType(APValue::LValueBase B) { return QualType(); } +/// A more selective version of E->IgnoreParenCasts for +/// TryEvaluateBuiltinObjectSize. This ignores casts/parens that serve only to +/// change the type of E. +/// Ex. For E = `(short*)((char*)(&foo))`, returns `&foo` +/// +/// Always returns an RValue with a pointer representation. +static const Expr *ignorePointerCastsAndParens(const Expr *E) { + assert(E->isRValue() && E->getType()->hasPointerRepresentation()); + + auto *NoParens = E->IgnoreParens(); + auto *Cast = dyn_cast(NoParens); + if (Cast == nullptr || Cast->getCastKind() == CK_DerivedToBase) + return NoParens; + + auto *SubExpr = Cast->getSubExpr(); + if (!SubExpr->getType()->hasPointerRepresentation() || !SubExpr->isRValue()) + return NoParens; + return ignorePointerCastsAndParens(SubExpr); +} + bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E, unsigned Type) { // Determine the denoted object. @@ -6220,23 +6277,35 @@ bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E, // If there are any, but we can determine the pointed-to object anyway, then // ignore the side-effects. SpeculativeEvaluationRAII SpeculativeEval(Info); - FoldConstant Fold(Info, true); - if (!EvaluatePointer(E->getArg(0), Base, Info)) + FoldOffsetRAII Fold(Info, Type & 1); + const Expr *Ptr = ignorePointerCastsAndParens(E->getArg(0)); + if (!EvaluatePointer(Ptr, Base, Info)) return false; } CharUnits BaseOffset = Base.getLValueOffset(); - - // If we point to before the start of the object, there are no - // accessible bytes. - if (BaseOffset < CharUnits::Zero()) + // If we point to before the start of the object, there are no accessible + // bytes. + if (BaseOffset.isNegative()) return Success(0, E); - // MostDerivedType is null if we're dealing with a literal such as nullptr or - // (char*)0x100000. Lower it to LLVM in either case so it can figure out what - // to do with it. - // FIXME(gbiv): Try to do a better job with this in clang. - if (Base.Designator.MostDerivedType.isNull()) + // In the case where we're not dealing with a subobject, we discard the + // subobject bit. + if (!Base.Designator.Invalid && Base.Designator.Entries.empty()) + Type = Type & ~1U; + + // If Type & 1 is 0, we need to be able to statically guarantee that the bytes + // exist. If we can't verify the base, then we can't do that. + // + // As a special case, we produce a valid object size for an unknown object + // with a known designator if Type & 1 is 1. For instance: + // + // extern struct X { char buff[32]; int a, b, c; } *p; + // int a = __builtin_object_size(p->buff + 4, 3); // returns 28 + // int b = __builtin_object_size(p->buff + 4, 2); // returns 0, not 40 + // + // This matches GCC's behavior. + if ((Type & 1) == 0 && Base.InvalidBase) return Error(E); // If Type & 1 is 0, the object in question is the complete object; reset to @@ -6256,16 +6325,6 @@ bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E, } } - // FIXME: We should produce a valid object size for an unknown object with a - // known designator, if Type & 1 is 1. For instance: - // - // extern struct X { char buff[32]; int a, b, c; } *p; - // int a = __builtin_object_size(p->buff + 4, 3); // returns 28 - // int b = __builtin_object_size(p->buff + 4, 2); // returns 0, not 40 - // - // This is GCC's behavior. We currently don't do this, but (hopefully) will in - // the near future. - // If it is not possible to determine which objects ptr points to at compile // time, __builtin_object_size should return (size_t) -1 for type 0 or 1 // and (size_t) 0 for type 2 or 3. @@ -6280,14 +6339,15 @@ bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E, End.Designator.Entries.size() == End.Designator.MostDerivedPathLength) { // We got a pointer to an array. Step to its end. AmountToAdd = End.Designator.MostDerivedArraySize - - End.Designator.Entries.back().ArrayIndex; - } else if (End.Designator.IsOnePastTheEnd) { + End.Designator.Entries.back().ArrayIndex; + } else if (End.Designator.isOnePastTheEnd()) { // We're already pointing at the end of the object. AmountToAdd = 0; } - if (End.Designator.MostDerivedType->isIncompleteType() || - End.Designator.MostDerivedType->isFunctionType()) + QualType PointeeType = End.Designator.MostDerivedType; + assert(!PointeeType.isNull()); + if (PointeeType->isIncompleteType() || PointeeType->isFunctionType()) return Error(E); if (!HandleLValueArrayAdjustment(Info, E, End, End.Designator.MostDerivedType, @@ -6331,6 +6391,7 @@ bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) { case EvalInfo::EM_ConstantFold: case EvalInfo::EM_EvaluateForOverflow: case EvalInfo::EM_IgnoreSideEffects: + case EvalInfo::EM_DesignatorFold: // Leave it to IR generation. return Error(E); case EvalInfo::EM_ConstantExpressionUnevaluated: diff --git a/test/CodeGen/object-size.c b/test/CodeGen/object-size.c index ffd50cf9cf..367318d6cc 100644 --- a/test/CodeGen/object-size.c +++ b/test/CodeGen/object-size.c @@ -161,6 +161,15 @@ void test19() { gi = __builtin_object_size(&foo.a, 2); // CHECK: store i32 4 gi = __builtin_object_size(&foo.a, 3); + + // CHECK: store i32 4 + gi = __builtin_object_size(&foo.b, 0); + // CHECK: store i32 4 + gi = __builtin_object_size(&foo.b, 1); + // CHECK: store i32 4 + gi = __builtin_object_size(&foo.b, 2); + // CHECK: store i32 4 + gi = __builtin_object_size(&foo.b, 3); } // CHECK: @test20 @@ -221,25 +230,59 @@ void test22() { gi = __builtin_object_size(&t[9].t[10], 2); // CHECK: store i32 0 gi = __builtin_object_size(&t[9].t[10], 3); + + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[0] + sizeof(t), 0); + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[0] + sizeof(t), 1); + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[0] + sizeof(t), 2); + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[0] + sizeof(t), 3); + + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 0); + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 1); + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 2); + // CHECK: store i32 0 + gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 3); } -struct Test23Ty { int t[10]; }; +struct Test23Ty { int a; int t[10]; }; // CHECK: @test23 -void test23(struct Test22Ty *p) { +void test23(struct Test23Ty *p) { // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) gi = __builtin_object_size(p, 0); // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) gi = __builtin_object_size(p, 1); // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) gi = __builtin_object_size(p, 2); - // Note: this is currently fixed at 0 because LLVM doesn't have sufficient // data to correctly handle type=3 // CHECK: store i32 0 gi = __builtin_object_size(p, 3); -} + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&p->a, 0); + // CHECK: store i32 4 + gi = __builtin_object_size(&p->a, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(&p->a, 2); + // CHECK: store i32 4 + gi = __builtin_object_size(&p->a, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&p->t[5], 0); + // CHECK: store i32 20 + gi = __builtin_object_size(&p->t[5], 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(&p->t[5], 2); + // CHECK: store i32 20 + gi = __builtin_object_size(&p->t[5], 3); +} // PR24493 -- ICE if __builtin_object_size called with NULL and (Type & 1) != 0 // CHECK: @test24 @@ -280,3 +323,71 @@ void test25() { // CHECK: store i32 0 gi = __builtin_object_size((void*)0 + 0x1000, 3); } + +// CHECK: @test26 +void test26() { + struct { int v[10]; } t[10]; + + // CHECK: store i32 316 + gi = __builtin_object_size(&t[1].v[11], 0); + // CHECK: store i32 312 + gi = __builtin_object_size(&t[1].v[12], 1); + // CHECK: store i32 308 + gi = __builtin_object_size(&t[1].v[13], 2); + // CHECK: store i32 0 + gi = __builtin_object_size(&t[1].v[14], 3); +} + +struct Test27IncompleteTy; + +// CHECK: @test27 +void test27(struct Test27IncompleteTy *t) { + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(t, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(t, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(t, 2); + // Note: this is currently fixed at 0 because LLVM doesn't have sufficient + // data to correctly handle type=3 + // CHECK: store i32 0 + gi = __builtin_object_size(t, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false) + gi = __builtin_object_size(&test27, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false) + gi = __builtin_object_size(&test27, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 true) + gi = __builtin_object_size(&test27, 2); + // Note: this is currently fixed at 0 because LLVM doesn't have sufficient + // data to correctly handle type=3 + // CHECK: store i32 0 + gi = __builtin_object_size(&test27, 3); +} + +// The intent of this test is to ensure that __builtin_object_size treats `&foo` +// and `(T*)&foo` identically, when used as the pointer argument. +// CHECK: @test28 +void test28() { + struct { int v[10]; } t[10]; + +#define addCasts(s) ((char*)((short*)(s))) + // CHECK: store i32 360 + gi = __builtin_object_size(addCasts(&t[1]), 0); + // CHECK: store i32 360 + gi = __builtin_object_size(addCasts(&t[1]), 1); + // CHECK: store i32 360 + gi = __builtin_object_size(addCasts(&t[1]), 2); + // CHECK: store i32 360 + gi = __builtin_object_size(addCasts(&t[1]), 3); + + // CHECK: store i32 356 + gi = __builtin_object_size(addCasts(&t[1].v[1]), 0); + // CHECK: store i32 36 + gi = __builtin_object_size(addCasts(&t[1].v[1]), 1); + // CHECK: store i32 356 + gi = __builtin_object_size(addCasts(&t[1].v[1]), 2); + // CHECK: store i32 36 + gi = __builtin_object_size(addCasts(&t[1].v[1]), 3); +#undef addCasts +}