r246877 - Increase accuracy of __builtin_object_size.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 4 14:28:13 PDT 2015
Author: gbiv
Date: Fri Sep 4 16:28:13 2015
New Revision: 246877
URL: http://llvm.org/viewvc/llvm-project?rev=246877&view=rev
Log:
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
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/object-size.c
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=246877&r1=246876&r2=246877&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Sep 4 16:28:13 2015
@@ -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<PointerType>()->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<FieldDecl>(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
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::L
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<CastExpr>(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::TryEvaluateBuilti
// 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::TryEvaluateBuilti
}
}
- // 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::TryEvaluateBuilti
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(con
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:
Modified: cfe/trunk/test/CodeGen/object-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/object-size.c?rev=246877&r1=246876&r2=246877&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/object-size.c (original)
+++ cfe/trunk/test/CodeGen/object-size.c Fri Sep 4 16:28:13 2015
@@ -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
+}
More information about the cfe-commits
mailing list