]> granicus.if.org Git - clang/commitdiff
Increase accuracy of __builtin_object_size.
authorGeorge Burgess IV <george.burgess.iv@gmail.com>
Fri, 4 Sep 2015 21:28:13 +0000 (21:28 +0000)
committerGeorge Burgess IV <george.burgess.iv@gmail.com>
Fri, 4 Sep 2015 21:28:13 +0000 (21:28 +0000)
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

lib/AST/ExprConstant.cpp
test/CodeGen/object-size.c

index 8aea10d516e9ae790eb40b8d1138692b502be39f..dea85d84a2d7a72a33c1bb6a2f91b41e1a932d42 100644 (file)
@@ -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(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<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::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:
index ffd50cf9cf58db66f3a4f0522f7f4a0ecde918da..367318d6cc202acff0df31ca1f9b18709b57db3e 100644 (file)
@@ -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
+}