r245323 - Make __builtin_object_size always answer correctly
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 18 11:18:27 PDT 2015
Author: gbiv
Date: Tue Aug 18 13:18:27 2015
New Revision: 245323
URL: http://llvm.org/viewvc/llvm-project?rev=245323&view=rev
Log:
Make __builtin_object_size always answer correctly
__builtin_object_size would return incorrect answers for many uses where
type=3. This fixes the inaccuracy by making us emit 0 instead of LLVM's
objectsize intrinsic.
Additionally, there are many cases where we would emit suboptimal (but
correct) answers, such as when arrays are involved. This patch fixes
some of these cases (please see new tests in test/CodeGen/object-size.c
for specifics on which cases are improved)
Patch mostly by Richard Smith.
Differential Revision: http://reviews.llvm.org/D12000
This fixes PR15212.
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/object-size.c
cfe/trunk/test/Sema/const-eval.c
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=245323&r1=245322&r2=245323&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Aug 18 13:18:27 2015
@@ -967,10 +967,6 @@ namespace {
// Check this LValue refers to an object. If not, set the designator to be
// invalid and emit a diagnostic.
bool checkSubobject(EvalInfo &Info, const Expr *E, CheckSubobjectKind CSK) {
- // Outside C++11, do not build a designator referring to a subobject of
- // any object: we won't use such a designator for anything.
- if (!Info.getLangOpts().CPlusPlus11)
- Designator.setInvalid();
return (CSK == CSK_ArrayToPointer || checkNullPointer(Info, E, CSK)) &&
Designator.checkSubobject(Info, E, CSK);
}
@@ -2713,8 +2709,7 @@ static bool handleLValueToRValueConversi
// Check for special cases where there is no existing APValue to look at.
const Expr *Base = LVal.Base.dyn_cast<const Expr*>();
- if (!LVal.Designator.Invalid && Base && !LVal.CallIndex &&
- !Type.isVolatileQualified()) {
+ if (Base && !LVal.CallIndex && !Type.isVolatileQualified()) {
if (const CompoundLiteralExpr *CLE = dyn_cast<CompoundLiteralExpr>(Base)) {
// In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating the
// initializer until now for such expressions. Such an expression can't be
@@ -5998,8 +5993,7 @@ public:
bool VisitSizeOfPackExpr(const SizeOfPackExpr *E);
private:
- static QualType GetObjectType(APValue::LValueBase B);
- bool TryEvaluateBuiltinObjectSize(const CallExpr *E);
+ bool TryEvaluateBuiltinObjectSize(const CallExpr *E, unsigned Type);
// FIXME: Missing: array subscript of vector, member of vector
};
} // end anonymous namespace
@@ -6171,7 +6165,7 @@ static bool EvaluateBuiltinConstantP(AST
/// Retrieves the "underlying object type" of the given expression,
/// as used by __builtin_object_size.
-QualType IntExprEvaluator::GetObjectType(APValue::LValueBase B) {
+static QualType getObjectType(APValue::LValueBase B) {
if (const ValueDecl *D = B.dyn_cast<const ValueDecl*>()) {
if (const VarDecl *VD = dyn_cast<VarDecl>(D))
return VD->getType();
@@ -6183,49 +6177,87 @@ QualType IntExprEvaluator::GetObjectType
return QualType();
}
-bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E) {
+bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E,
+ unsigned Type) {
+ // Determine the denoted object.
LValue Base;
-
{
// The operand of __builtin_object_size is never evaluated for side-effects.
// 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))
return false;
}
- if (!Base.getLValueBase()) {
- // 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.
- llvm::APSInt TypeIntVaue;
- const Expr *ExprType = E->getArg(1);
- if (!ExprType->EvaluateAsInt(TypeIntVaue, Info.Ctx))
- return false;
- if (TypeIntVaue == 0 || TypeIntVaue == 1)
- return Success(-1, E);
- if (TypeIntVaue == 2 || TypeIntVaue == 3)
- return Success(0, E);
- return Error(E);
+ CharUnits BaseOffset = Base.getLValueOffset();
+
+ // If we point to before the start of the object, there are no
+ // accessible bytes.
+ if (BaseOffset < CharUnits::Zero())
+ return Success(0, E);
+
+ // If Type & 1 is 0, the object in question is the complete object; reset to
+ // a complete object designator in that case.
+ //
+ // If Type is 1 and we've lost track of the subobject, just find the complete
+ // object instead. (If Type is 3, that's not correct behavior and we should
+ // return 0 instead.)
+ LValue End = Base;
+ if (((Type & 1) == 0) || (End.Designator.Invalid && Type == 1)) {
+ QualType T = getObjectType(End.getLValueBase());
+ if (T.isNull())
+ End.Designator.setInvalid();
+ else {
+ End.Designator = SubobjectDesignator(T);
+ End.Offset = CharUnits::Zero();
+ }
}
- QualType T = GetObjectType(Base.getLValueBase());
- if (T.isNull() ||
- T->isIncompleteType() ||
- T->isFunctionType() ||
- T->isVariablyModifiedType() ||
- T->isDependentType())
+ // 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.
+ if (End.Designator.Invalid)
+ return false;
+
+ // According to the GCC documentation, we want the size of the subobject
+ // denoted by the pointer. But that's not quite right -- what we actually
+ // want is the size of the immediately-enclosing array, if there is one.
+ int64_t AmountToAdd = 1;
+ if (End.Designator.MostDerivedArraySize &&
+ 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) {
+ // We're already pointing at the end of the object.
+ AmountToAdd = 0;
+ }
+
+ if (End.Designator.MostDerivedType->isIncompleteType() ||
+ End.Designator.MostDerivedType->isFunctionType())
return Error(E);
- CharUnits Size = Info.Ctx.getTypeSizeInChars(T);
- CharUnits Offset = Base.getLValueOffset();
+ if (!HandleLValueArrayAdjustment(Info, E, End, End.Designator.MostDerivedType,
+ AmountToAdd))
+ return false;
+
+ auto EndOffset = End.getLValueOffset();
+ if (BaseOffset > EndOffset)
+ return Success(0, E);
- if (!Offset.isNegative() && Offset <= Size)
- Size -= Offset;
- else
- Size = CharUnits::Zero();
- return Success(Size, E);
+ return Success(EndOffset - BaseOffset, E);
}
bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) {
@@ -6234,17 +6266,21 @@ bool IntExprEvaluator::VisitCallExpr(con
return ExprEvaluatorBaseTy::VisitCallExpr(E);
case Builtin::BI__builtin_object_size: {
- if (TryEvaluateBuiltinObjectSize(E))
+ // The type was checked when we built the expression.
+ unsigned Type =
+ E->getArg(1)->EvaluateKnownConstInt(Info.Ctx).getZExtValue();
+ assert(Type <= 3 && "unexpected type");
+
+ if (TryEvaluateBuiltinObjectSize(E, Type))
return true;
// If evaluating the argument has side-effects, we can't determine the size
// of the object, and so we lower it to unknown now. CodeGen relies on us to
// handle all cases where the expression has side-effects.
- if (E->getArg(0)->HasSideEffects(Info.Ctx)) {
- if (E->getArg(1)->EvaluateKnownConstInt(Info.Ctx).getZExtValue() <= 1)
- return Success(-1ULL, E);
- return Success(0, E);
- }
+ // Likewise, if Type is 3, we must handle this because CodeGen cannot give a
+ // conservatively correct answer in that case.
+ if (E->getArg(0)->HasSideEffects(Info.Ctx) || Type == 3)
+ return Success((Type & 2) ? 0 : -1, E);
// Expression had no side effects, but we couldn't statically determine the
// size of the referenced object.
@@ -6254,10 +6290,12 @@ bool IntExprEvaluator::VisitCallExpr(con
case EvalInfo::EM_ConstantFold:
case EvalInfo::EM_EvaluateForOverflow:
case EvalInfo::EM_IgnoreSideEffects:
+ // Leave it to IR generation.
return Error(E);
case EvalInfo::EM_ConstantExpressionUnevaluated:
case EvalInfo::EM_PotentialConstantExpressionUnevaluated:
- return Success(-1ULL, E);
+ // Reduce it to a constant now.
+ return Success((Type & 2) ? 0 : -1, E);
}
}
Modified: cfe/trunk/test/CodeGen/object-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/object-size.c?rev=245323&r1=245322&r2=245323&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/object-size.c (original)
+++ cfe/trunk/test/CodeGen/object-size.c Tue Aug 18 13:18:27 2015
@@ -146,3 +146,96 @@ unsigned test18(int cond) {
// CHECK: call i64 @llvm.objectsize.i64
return __builtin_object_size(cond ? a : b, 0);
}
+
+// CHECK: @test19
+void test19() {
+ struct {
+ int a, b;
+ } foo;
+
+ // CHECK: store i32 8
+ gi = __builtin_object_size(&foo.a, 0);
+ // CHECK: store i32 4
+ gi = __builtin_object_size(&foo.a, 1);
+ // CHECK: store i32 8
+ gi = __builtin_object_size(&foo.a, 2);
+ // CHECK: store i32 4
+ gi = __builtin_object_size(&foo.a, 3);
+}
+
+// CHECK: @test20
+void test20() {
+ struct { int t[10]; } t[10];
+
+ // CHECK: store i32 380
+ gi = __builtin_object_size(&t[0].t[5], 0);
+ // CHECK: store i32 20
+ gi = __builtin_object_size(&t[0].t[5], 1);
+ // CHECK: store i32 380
+ gi = __builtin_object_size(&t[0].t[5], 2);
+ // CHECK: store i32 20
+ gi = __builtin_object_size(&t[0].t[5], 3);
+}
+
+// CHECK: @test21
+void test21() {
+ struct { int t; } t;
+
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t + 1, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t + 1, 1);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t + 1, 2);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t + 1, 3);
+
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t.t + 1, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t.t + 1, 1);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t.t + 1, 2);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t.t + 1, 3);
+}
+
+// CHECK: @test22
+void test22() {
+ struct { int t[10]; } t[10];
+
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[10], 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[10], 1);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[10], 2);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[10], 3);
+
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[9].t[10], 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[9].t[10], 1);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[9].t[10], 2);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(&t[9].t[10], 3);
+}
+
+struct Test23Ty { int t[10]; };
+
+// CHECK: @test23
+void test23(struct Test22Ty *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);
+}
Modified: cfe/trunk/test/Sema/const-eval.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/const-eval.c?rev=245323&r1=245322&r2=245323&view=diff
==============================================================================
--- cfe/trunk/test/Sema/const-eval.c (original)
+++ cfe/trunk/test/Sema/const-eval.c Tue Aug 18 13:18:27 2015
@@ -118,10 +118,6 @@ float varfloat;
const float constfloat = 0;
EVAL_EXPR(43, varfloat && constfloat) // expected-error {{must have a constant size}}
-// <rdar://problem/11205586>
-// (Make sure we continue to reject this.)
-EVAL_EXPR(44, "x"[0]); // expected-error {{variable length array}}
-
// <rdar://problem/10962435>
EVAL_EXPR(45, ((char*)-1) + 1 == 0 ? 1 : -1)
EVAL_EXPR(46, ((char*)-1) + 1 < (char*) -1 ? 1 : -1)
More information about the cfe-commits
mailing list