[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)
This revision was automatically updated to reflect the committed changes. Closed by commit rL322950: [CodeGenCXX] annotate a GEP to a derived class with inbounds (PR35909) (authored by spatel, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42249?vs=130488=130616#toc Repository: rL LLVM https://reviews.llvm.org/D42249 Files: cfe/trunk/lib/CodeGen/CGClass.cpp cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp cfe/trunk/test/CodeGenCXX/derived-cast.cpp Index: cfe/trunk/test/CodeGenCXX/derived-cast.cpp === --- cfe/trunk/test/CodeGenCXX/derived-cast.cpp +++ cfe/trunk/test/CodeGenCXX/derived-cast.cpp @@ -0,0 +1,27 @@ +// 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(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 +} + Index: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp === --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp @@ -371,7 +371,7 @@ void downcast_pointer(B *b) { (void) static_cast (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 ) { (void) static_cast (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: cfe/trunk/lib/CodeGen/CGClass.cpp === --- cfe/trunk/lib/CodeGen/CGClass.cpp +++ cfe/trunk/lib/CodeGen/CGClass.cpp @@ -406,8 +406,8 @@ // Apply the offset. llvm::Value *Value = Builder.CreateBitCast(BaseAddr.getPointer(), Int8PtrTy); - Value = Builder.CreateGEP(Value, Builder.CreateNeg(NonVirtualOffset), -"sub.ptr"); + Value = Builder.CreateInBoundsGEP(Value, Builder.CreateNeg(NonVirtualOffset), +"sub.ptr"); // Just cast. Value = Builder.CreateBitCast(Value, DerivedPtrTy); Index: cfe/trunk/test/CodeGenCXX/derived-cast.cpp === --- cfe/trunk/test/CodeGenCXX/derived-cast.cpp +++ cfe/trunk/test/CodeGenCXX/derived-cast.cpp @@ -0,0 +1,27 @@ +// 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 (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 +} + Index: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp === --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp @@ -371,7 +371,7 @@ void downcast_pointer(B *b) { (void) static_cast (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 ) { (void) static_cast (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:
[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D42249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)
spatel updated this revision to Diff 130488. spatel added a comment. Patch updated: 1. Removed comment that didn't add value. 2. Added test with no sanitizing, reduced from PR35909. https://reviews.llvm.org/D42249 Files: lib/CodeGen/CGClass.cpp test/CodeGenCXX/catch-undef-behavior.cpp test/CodeGenCXX/derived-cast.cpp Index: test/CodeGenCXX/derived-cast.cpp === --- test/CodeGenCXX/derived-cast.cpp +++ test/CodeGenCXX/derived-cast.cpp @@ -0,0 +1,27 @@ +// 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(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 +} + 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 (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 ) { (void) static_cast (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,8 @@ // Apply the offset. llvm::Value *Value = Builder.CreateBitCast(BaseAddr.getPointer(), Int8PtrTy); - Value = Builder.CreateGEP(Value, Builder.CreateNeg(NonVirtualOffset), -"sub.ptr"); + Value = Builder.CreateInBoundsGEP(Value, Builder.CreateNeg(NonVirtualOffset), +"sub.ptr"); // Just cast. Value = Builder.CreateBitCast(Value, DerivedPtrTy); Index: test/CodeGenCXX/derived-cast.cpp === --- test/CodeGenCXX/derived-cast.cpp +++ test/CodeGenCXX/derived-cast.cpp @@ -0,0 +1,27 @@ +// 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 (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 +} + 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 (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 ) { (void) static_cast (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,8 @@ // Apply the offset.
[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)
efriedma added inline comments. Comment at: lib/CodeGen/CGClass.cpp:410 + + // The GEP is to a derived object, so this GEP must be 'inbounds'. + Value = Builder.CreateInBoundsGEP(Value, Builder.CreateNeg(NonVirtualOffset), Not sure this comment really adds anything, unless you want to cite the standard. Comment at: test/CodeGenCXX/catch-undef-behavior.cpp:391 // 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* We probably want a test which checks the output when ubsan isn't enabled. https://reviews.llvm.org/D42249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)
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(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 (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 ) { (void) static_cast (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 (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 ) { (void) static_cast (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