[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);



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);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits