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

[spatel]

Summary

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

[efriedma]

lib/CodeGen/CGClass.cpp

410

Not sure this comment really adds anything, unless you want to cite the standard.

test/CodeGenCXX/catch-undef-behavior.cpp

391

We probably want a test which checks the output when ubsan isn't enabled.

[spatel]

Patch updated:

1. Removed comment that didn't add value.
2. Added test with no sanitizing, reduced from PR35909.

[efriedma]

LGTM

lib/CodeGen/CGClass.cpp

if (NullCheckValue) {
 
     llvm::Value *IsNull = Builder.CreateIsNull(BaseAddr.getPointer());
     Builder.CreateCondBr(IsNull, CastNull, CastNotNull);
     EmitBlock(CastNotNull);
   }
 
   // Apply the offset.
   llvm::Value *Value = Builder.CreateBitCast(BaseAddr.getPointer(), Int8PtrTy);
-  Value = Builder.CreateGEP(Value, Builder.CreateNeg(NonVirtualOffset),
+  Value = Builder.CreateInBoundsGEP(Value, Builder.CreateNeg(NonVirtualOffset),
                                     "sub.ptr");
 
   // Just cast.
   Value = Builder.CreateBitCast(Value, DerivedPtrTy);
 
   // Produce a PHI if we had a null-check.
   if (NullCheckValue) {
     Builder.CreateBr(CastEnd);
     EmitBlock(CastNull);

test/CodeGenCXX/catch-undef-behavior.cpp

 // Make sure we check the alignment of the pointer after subtracting any
 // offset. The pointer before subtraction doesn't need to be aligned for
 // the destination type.
 
 // CHECK-LABEL: define void @_Z16downcast_pointerP1B(%class.B* %b)
 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]], {{.*}} ], {{.*}}
   // Objectsize check goes here
   // CHECK: [[C_INT:%.+]] = ptrtoint %class.C* [[FROM_PHI]] to i64
   // CHECK-NEXT: [[MASKED:%.+]] = and i64 [[C_INT]], 15
   // CHECK-NEXT: [[TEST:%.+]] = icmp eq i64 [[MASKED]], 0
   // AND the alignment test with the objectsize test.
   // CHECK-NEXT: [[AND:%.+]] = and i1 {{.*}}, [[TEST]]
   // CHECK-NEXT: br i1 [[AND]]
 }
 
 // CHECK-LABEL: define void @_Z18downcast_referenceR1B(%class.B* dereferenceable({{[0-9]+}}) %b)
 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
   // CHECK-NEXT: [[MASKED:%.+]] = and i64 [[C_INT]], 15
   // CHECK-NEXT: [[TEST:%.+]] = icmp eq i64 [[MASKED]], 0
   // AND the alignment test with the objectsize test.
   // CHECK:      [[AND:%.+]] = and i1 {{.*}}, [[TEST]]
   // CHECK-NEXT: br i1 [[AND]]

test/CodeGenCXX/derived-cast.cpp

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+class A {
+    int a;
+};
+
+class B {
+    int b;
+public:
+    A *getAsA();
+};
+
+class X : public A, public B {
+    int x;
+};
+
+// PR35909 - https://bugs.llvm.org/show_bug.cgi?id=35909
+
+A *B::getAsA() {
+  return static_cast<X*>(this);
+
+  // CHECK-LABEL: define %class.A* @_ZN1B6getAsAEv
+  // CHECK: %[[THIS:.*]] = load %class.B*, %class.B**
+  // CHECK-NEXT: %[[BC:.*]] = bitcast %class.B* %[[THIS]] to i8*
+  // CHECK-NEXT: getelementptr inbounds i8, i8* %[[BC]], i64 -4
+}
+