[PATCH] D15599: [CodeGen] Fix a crash that occurs when attribute "naked" is attached to a c++ member function

[Reid Kleckner via cfe-commits]

rnk added a comment.

I guess I'm OK with the approach.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1967
@@ +1966,3 @@
+
+void CodeGenFunction::cleanupNakedFunction() {
+  llvm::SmallPtrSet<llvm::Instruction *, 8> InstrsToRemove;
----------------
This isn't the right approach. Look at Function::dropAllReferences and try modelling this code on that. This does a lot of unnecessary RAUW when we can just null out all operands and delete all instructions except for the inline asm CallInst.

================
Comment at: test/CodeGen/attr-naked.c:20
@@ -19,3 +19,3 @@
 __attribute((naked)) void t3(int x) {
-// CHECK: define void @t3(i32)
+// CHECK: define void @t3(i32 %x)
 // CHECK-NOT: alloca
----------------
unrelated change?

================
Comment at: test/CodeGenCXX/attr-naked.cpp:7
@@ +6,3 @@
+  int __attribute__((naked)) m2() { __asm__ volatile("retq"); }
+};
+
----------------
Can you add a test involving vtable thunks? I'm worried that we will clean up the thunk for a naked virtual method implementation. The code looks correct today, but I'd like to defend against that possibility in the future.

================
Comment at: test/CodeGenCXX/attr-naked.cpp:30
@@ +29,3 @@
+// CHECK: define internal i32 @"_ZZ4foo1iP6Class1ENK3$_0clEv"(%class.anon* %this) [[ATTR1:#[0-9]]]
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void asm sideeffect "retq
----------------
Won't this require asserts? We don't name BBs in NDEBUG builds.


http://reviews.llvm.org/D15599