[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)

[Sanjay Patel via Phabricator via cfe-commits]

spatel created this revision.
spatel added reviewers: efriedma, hfinkel, rjmccall, rsmith.
Herald added a subscriber: mcrosier.

I'm not sure if the code comment is adequate or even correct, but hopefully the change itself is valid.

Eli cited this section of the standard in PR35909 ( https://bugs.llvm.org/show_bug.cgi?id=35909 ):
[expr.static.cast] p11: "If the prvalue of type “pointer to cv1 B” points to a B that is actually a subobject of an object of type D, the resulting pointer points to the enclosing object of type D. Otherwise, the behavior is undefined."

In the motivating case in the bug report, LLVM can't eliminate a nullptr check because a GEP is not marked with 'inbounds':

  class A {
      int a;
  };
  class B {
      int b;
  public:
      A *getAsA();
  };
  class X : public A, public B {
      int x;
  };
  
  A *B::getAsA() {
      if (b == 42) {
          auto temp = static_cast<X*>(this);
          //__builtin_assume(temp != nullptr);
          return temp;
      }
      return nullptr;
  }
  
  void helper(A *);
  
  void test(B *b) {
      auto temp = b->getAsA();
      if (temp)
          return helper(temp);
  }


https://reviews.llvm.org/D42249

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/catch-undef-behavior.cpp


Index: test/CodeGenCXX/catch-undef-behavior.cpp
===================================================================
--- test/CodeGenCXX/catch-undef-behavior.cpp
+++ test/CodeGenCXX/catch-undef-behavior.cpp
@@ -371,7 +371,7 @@
 void downcast_pointer(B *b) {
   (void) static_cast<C*>(b);
   // Alignment check from EmitTypeCheck(TCK_DowncastPointer, ...)
-  // CHECK: [[SUB:%[.a-z0-9]*]] = getelementptr i8, i8* {{.*}}, i64 -16
+  // CHECK: [[SUB:%[.a-z0-9]*]] = getelementptr inbounds i8, i8* {{.*}}, i64 -16
   // CHECK-NEXT: [[C:%.+]] = bitcast i8* [[SUB]] to %class.C*
   // null check goes here
   // CHECK: [[FROM_PHI:%.+]] = phi %class.C* [ [[C]], {{.*}} ], {{.*}}
@@ -388,7 +388,7 @@
 void downcast_reference(B &b) {
   (void) static_cast<C&>(b);
   // Alignment check from EmitTypeCheck(TCK_DowncastReference, ...)
-  // CHECK:      [[SUB:%[.a-z0-9]*]] = getelementptr i8, i8* {{.*}}, i64 -16
+  // CHECK:      [[SUB:%[.a-z0-9]*]] = getelementptr inbounds i8, i8* {{.*}}, i64 -16
   // CHECK-NEXT: [[C:%.+]] = bitcast i8* [[SUB]] to %class.C*
   // Objectsize check goes here
   // CHECK:      [[C_INT:%.+]] = ptrtoint %class.C* [[C]] to i64
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -406,8 +406,10 @@
 
   // Apply the offset.
   llvm::Value *Value = Builder.CreateBitCast(BaseAddr.getPointer(), Int8PtrTy);
-  Value = Builder.CreateGEP(Value, Builder.CreateNeg(NonVirtualOffset),
-                            "sub.ptr");
+
+  // The GEP is to a derived object, so this GEP must be 'inbounds'.
+  Value = Builder.CreateInBoundsGEP(Value, Builder.CreateNeg(NonVirtualOffset),
+                                    "sub.ptr");
 
   // Just cast.
   Value = Builder.CreateBitCast(Value, DerivedPtrTy);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42249.130433.patch
Type: text/x-patch
Size: 1823 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180118/69b13f73/attachment.bin>