[PATCH] D46685: [CodeGen] Disable structor optimizations at -O0

[Pavel Labath via Phabricator via cfe-commits]

labath created this revision.
labath added reviewers: rjmccall, aprantl.

Merging of complete and base structors can degrade debug quality as it
will leave the debugger unable to locate the full object structor. This
is apparent when evaluating an expression in the debugger which requires
constructing an object of class which has had this optimization applied
to it.  When compiling the expression, we pretend that the class and
its methods have been defined in another compilation unit, so the
expression compiler assumes the structor definition must be available,
whereas in reality the complete structor may have been omitted and all
it's uses replaced by the base structor.

This improves debug quality on non-darwin platforms (darwin does not
have -mconstructor-aliases on by default, so it is spared these
problems) and enable us to remove some workarounds from LLDB which attempt to
mitigate this issue.

This is tested by strenghtening the check in the existing
ctor-dtor-alias test. In the other aliasing tests I add -O1 to compiler
options to make sure the aliasing kicks in.

PS: Of the three ways we can do structor optimizations, only the RAUW
actually causes problems as it makes the symbol completely disappear, so
technically it should be sufficient to weaken RAUW to one of the other two
for -O0, but disabling optimizations completely looked like a more
principled solution.


Repository:
  rC Clang

https://reviews.llvm.org/D46685

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/constructor-alias.cpp
  test/CodeGenCXX/ctor-dtor-alias.cpp
  test/CodeGenCXX/dllexport-alias.cpp
  test/CodeGenCXX/virtual-bases.cpp


Index: test/CodeGenCXX/virtual-bases.cpp
===================================================================
--- test/CodeGenCXX/virtual-bases.cpp
+++ test/CodeGenCXX/virtual-bases.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin10 -mconstructor-aliases | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin10 -mconstructor-aliases -O1 -disable-llvm-passes | FileCheck %s
 
 struct A { 
   A();
Index: test/CodeGenCXX/dllexport-alias.cpp
===================================================================
--- test/CodeGenCXX/dllexport-alias.cpp
+++ test/CodeGenCXX/dllexport-alias.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-windows-gnu -mconstructor-aliases %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -mconstructor-aliases %s -S -emit-llvm -O1 -disable-llvm-passes -o - | FileCheck %s
 
 // This test assumes that the C1 constructor will be aliased to the C2
 // constructor, and the D1 destructor to the D2. It then checks that the aliases
Index: test/CodeGenCXX/ctor-dtor-alias.cpp
===================================================================
--- test/CodeGenCXX/ctor-dtor-alias.cpp
+++ test/CodeGenCXX/ctor-dtor-alias.cpp
@@ -77,11 +77,12 @@
   // CHECK1: call i32 @__cxa_atexit{{.*}}_ZN5test41AD2Ev
   // CHECK1: define linkonce_odr void @_ZN5test41AD2Ev({{.*}} comdat align
 
-  // test that we don't do this optimization at -O0 so that the debugger can
-  // see both destructors.
+  // test that we don't do this optimization at -O0 and call the complete
+  // destructor for B instead. This enables the debugger to see both
+  // destructors.
   // NOOPT: define internal void @__cxx_global_var_init.2()
-  // NOOPT: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD2Ev
-  // NOOPT: define linkonce_odr void @_ZN5test41BD2Ev({{.*}} comdat align
+  // NOOPT: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD1Ev
+  // NOOPT: define linkonce_odr void @_ZN5test41BD1Ev({{.*}} comdat align
   struct A {
     virtual ~A() {}
   };
Index: test/CodeGenCXX/constructor-alias.cpp
===================================================================
--- test/CodeGenCXX/constructor-alias.cpp
+++ test/CodeGenCXX/constructor-alias.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -triple mipsel--linux-gnu -mconstructor-aliases -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple mipsel--linux-gnu -mconstructor-aliases -O1 -disable-llvm-passes -o - %s | FileCheck %s
 
 // The target attribute code used to get confused with aliases. Make sure
 // we don't crash when an alias is used.
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -3616,6 +3616,11 @@
   if (MD->getParent()->getNumVBases())
     return StructorCodegen::Emit;
 
+  // Omitting the complete structor can degrade debug quality as the debugger
+  // cannot locate the complete structor symbol anymore.
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+    return StructorCodegen::Emit;
+
   GlobalDecl AliasDecl;
   if (const auto *DD = dyn_cast<CXXDestructorDecl>(MD)) {
     AliasDecl = GlobalDecl(DD, Dtor_Complete);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46685.146104.patch
Type: text/x-patch
Size: 3238 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180510/18705eed/attachment.bin>