george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.

This patch fixes PR26741, and makes us handle inheritance more sanely.

Broken code:

```
struct Foo { char a[1]; };
struct Bar : Foo {};

int break() {
  Bar *b;
  int size = __builtin_object_size(b->a, 1);
  assert(size == -1); // Fires; size is 1.
}
```

Because we're now handling inheritance, this patch has a few fun things in it 
(see: the new loop at ExprConstant.cpp:6489) so that we aren't overly 
conservative when inheritance is involved. I'm not entirely thrilled with how 
we determine if a base class is considered to be at the end of a derived class; 
better approaches are appreciated.

http://reviews.llvm.org/D17746

Files:
  lib/AST/ExprConstant.cpp
  test/CodeGen/object-size.cpp

Index: test/CodeGen/object-size.cpp
===================================================================
--- test/CodeGen/object-size.cpp
+++ test/CodeGen/object-size.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s
 
 // C++-specific tests for __builtin_object_size
 
@@ -62,3 +62,110 @@
   // CHECK: store i32 16
   gi = __builtin_object_size(&c->bs[0].buf[0], 3);
 }
+
+// CHECK-LABEL: define void @_Z5test3v()
+void test3() {
+  struct A { int i; char buf[1]; };
+  struct B : A {};
+  struct C : B {};
+  struct D { int i; B bs[1]; } *d;
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&d->bs[0].buf[0], 0);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&d->bs[0].buf[0], 1);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
+  gi = __builtin_object_size(&d->bs[0].buf[0], 2);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->bs[0].buf[0], 3);
+}
+
+// CHECK-LABEL: define void @_Z5test4v()
+void test4() {
+  struct A { int i; char buf[1]; };
+  struct B : A { char c; };
+  struct C { int i; B bs[1]; } *c;
+
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&c->bs[0].buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&c->bs[0].buf[0], 3);
+}
+
+// CHECK-LABEL: define void @_Z5test5v()
+void test5() {
+  struct A { int i; char buf[1]; };
+  struct B : A { char c; };
+  struct C : B {};
+  struct D { int i; B bs[1]; } *d;
+
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->bs[0].buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->bs[0].buf[0], 3);
+}
+
+// CHECK-LABEL: define void @_Z5test6v()
+void test6() {
+  // A   A
+  // |   |
+  // B   C
+  //  \ /
+  //   D
+  struct A { int i; char buf[1]; };
+  struct B : A {};
+  struct C : A {};
+  struct D : B, C {} *d;
+
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->B::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->B::buf[0], 3);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&d->C::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->C::buf[0], 3);
+
+  D &dr = *d;
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&dr.B::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&dr.B::buf[0], 3);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&dr.C::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&dr.C::buf[0], 3);
+
+  D &&drval = static_cast<D&&>(*d);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&drval.B::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&drval.B::buf[0], 3);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&drval.C::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&drval.C::buf[0], 3);
+
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&static_cast<D&&>(drval).B::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&static_cast<D&&>(drval).B::buf[0], 3);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&static_cast<D&&>(drval).C::buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&static_cast<D&&>(drval).C::buf[0], 3);
+}
+
+// CHECK-LABEL: define void @_Z5test7v()
+void test7() {
+  // Always give up when virtual inheritance is involved.
+  struct A { int i; char buf[1]; };
+  struct B : virtual A {};
+  struct C : virtual A {};
+  struct D : B, C {} *d;
+
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->buf[0], 1);
+  // CHECK: store i32 1
+  gi = __builtin_object_size(&d->buf[0], 3);
+}
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -6447,6 +6447,27 @@
 static bool isDesignatorAtObjectEnd(const ASTContext &Ctx, const LValue &LVal) {
   assert(!LVal.Designator.Invalid);
 
+  auto IsBaseAtEndOfDerived = [&Ctx](const CXXRecordDecl *Base,
+                                     const CXXRecordDecl *Derived) {
+    assert(Derived->isDerivedFrom(Base) && "Derived must be derived from Base");
+
+    // Cheap check: if we have members in the derived class, we know the
+    // designator can't be at the end.
+    if (!Derived->field_empty())
+      return false;
+
+    // Just give up when virtual inheritance is involved.
+    if (Derived->isVirtuallyDerivedFrom(Base))
+      return false;
+
+    // Otherwise, check the actual layout of the objects to see if Base lies at
+    // the end of Derived's layout.
+    auto &DerivedLayout = Ctx.getASTRecordLayout(Derived);
+    CharUnits BaseOffset = DerivedLayout.getBaseClassOffset(Base);
+    CharUnits BaseSize = Ctx.getASTRecordLayout(Base).getSize();
+    return BaseOffset + BaseSize == DerivedLayout.getSize();
+  };
+
   auto IsLastFieldDecl = [&Ctx](const FieldDecl *FD) {
     if (FD->getParent()->isUnion())
       return true;
@@ -6456,6 +6477,47 @@
 
   auto &Base = LVal.getLValueBase();
   if (auto *ME = dyn_cast_or_null<MemberExpr>(Base.dyn_cast<const Expr *>())) {
+    // The given MemberExpr's base may have implicit derived to base
+    // conversions. Make a best-effort attempt to find the most derived
+    // object, and check to see if we're at the end of that.
+    //
+    // Because we're meant to interpret this "as written", we won't traverse
+    // through explicit casts. In the worst case, we answer a bit more
+    // conservatively than we need to.
+    QualType RecentBaseTy = ME->getBase()->getType();
+    const Expr *Derived = ME->getBase();
+    while (auto *CE = dyn_cast<ImplicitCastExpr>(Derived)) {
+      if (CE->getCastKind() != CK_DerivedToBase &&
+          CE->getCastKind() != CK_UncheckedDerivedToBase)
+        break;
+
+      Derived = CE->getSubExpr();
+      QualType BaseTy = RecentBaseTy;
+      QualType DerivedTy = Derived->getType();
+      if (ME->isArrow()) {
+        assert(BaseTy->isPointerType() && DerivedTy->isPointerType() &&
+               "Derived->Base removed a pointer.");
+        BaseTy = BaseTy->castAs<PointerType>()->getPointeeType();
+        DerivedTy = DerivedTy->castAs<PointerType>()->getPointeeType();
+      }
+
+      const CXXRecordDecl *BaseDecl = BaseTy->getAsCXXRecordDecl();
+      const CXXRecordDecl *DerivedDecl = DerivedTy->getAsCXXRecordDecl();
+      assert(BaseDecl && DerivedDecl &&
+             "Derived->Base casts on non-CXXRecordDecl types");
+      assert(BaseDecl != DerivedDecl &&
+             "Classes can't be derived from themselves");
+
+      // This check can't just be done once at the end of the loop because we
+      // may be in an inheritance heierarchy where the most derived type
+      // inherits from multiple `Base`s. We can, however, skip doing it on each
+      // individual base in this cast chain.
+      if (!IsBaseAtEndOfDerived(BaseDecl, DerivedDecl))
+        return false;
+
+      RecentBaseTy = Derived->getType();
+    }
+
     if (auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
       if (!IsLastFieldDecl(FD))
         return false;
@@ -6489,9 +6551,14 @@
         return false;
       BaseType = FD->getType();
     } else {
-      assert(getAsBaseClass(LVal.Designator.Entries[I]) != nullptr &&
-             "Expecting cast to a base class");
-      return false;
+      const CXXRecordDecl *Base = getAsBaseClass(LVal.Designator.Entries[I]);
+      const CXXRecordDecl *Derived = BaseType->getAsCXXRecordDecl();
+      assert(Base && Derived &&
+             "Expected two CXXRecordDecls for base->derived conversion");
+
+      if (!IsBaseAtEndOfDerived(Base, Derived))
+        return false;
+      BaseType = QualType(Base->getTypeForDecl(), 0);
     }
   }
   return true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to