Add the -fdestroy-globals flag

[itessier]

Summary

The -fdestroy-globals flag can be used to disable global variable destructor
registration. It is intended to be used with embedded code that never exits.
Disabling registration allows the linker to garbage collect unused destructors
and vtables.

[vsk]

This is interesting. Do you have any results/metrics to share (e.g some any binary size reduction for projects you've looked at)?

lib/Frontend/CompilerInvocation.cpp

585

I think this will do the wrong thing when passed: -cc1 -fno-destroy-globals -fdestroy-globals. Using hasFlag here should fix the issue. I'm just noting this as a FYI, there's no need to update the diff right away, since I'd first like to find out if there are any correctness concerns / what the use cases for this functionality are.

[itessier]

In D35338#809146, @vsk wrote:

This is interesting. Do you have any results/metrics to share (e.g some any binary size reduction for projects you've looked at)?

I only tested this with Project Loon's avionics firmware which freed up ~1.2% of ROM space. A small amount, but for us every bit counts.

I did look at creating a templated wrapper class instead which uses placement-new to construct the wrapped type in a private char buffer. This results in the same effect as this patch (no dtor references are emitted). The only issue is the code is a big uglier to read, and we might forgot to use the wrapper (though a static checker could be added for this case). I figured it'd be best to add compiler support so other C++ based firmware projects can benefit as well.

[george.burgess.iv]

Thanks for working on this!

One small drive-by nit for you. Same "no need to update the diff this very second" as vsk; I can't LGTM this with confidence.

(Also, in the future, please try to include context with your patch. Doing so makes reviewing it on Phab much easier. :) )

include/clang/Frontend/CodeGenOptions.def

47

Nit: Please align the /// with the above/below ones.

[vsk]

That seems like a nice win and I like the convenience of this approach. That said I've just remembered that there's a thread on cfe-dev about this:
[RFC] Suppress C++ static destructor registration

I don't think a consensus was reached. From what I gather, some people think that the convenience of this flag makes it worth adding to clang, while others think that adding a non-standard compiler-specific flag is asking for trouble.

I suspect that we'll need more consensus to get this in tree. See Richard's comment:
"If someone's prepared to write up a paper for the C++ committee exploring what it would mean to standardize this behaviour and actually fix the problem for everyone, it would seem extremely reasonable for clang trunk to carry a flag to enable that behaviour (and it certainly doesn't have to wait for the committee to respond)."

[itessier]

That seems like a nice win and I like the convenience of this approach. That said I've just remembered that there's a thread on cfe-dev about this:
[RFC] Suppress C++ static destructor registration
I don't think a consensus was reached. From what I gather, some people think that the convenience of this flag makes it worth adding to clang, while others think that adding a non-standard compiler-specific flag is asking for trouble.

Given that firmware is a much different (or controlled) environment than a binary running on a full blown OS, would it be acceptable to name the flag -fbaremetal-destroy-globals, and only allow its use if the target triple's OS is set to none (e.g.: arm-none-eabi)?

include/clang/Driver/Options.td

   HelpText<"Don't use __cxa_atexit for calling destructors">;
 def fno_use_init_array : Flag<["-"], "fno-use-init-array">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Don't use .init_array instead of .ctors">;
+def fno_destroy_globals : Flag<["-"], "fno-destroy-globals">, Group<f_Group>, Flags<[CC1Option]>;
 def fno_unit_at_a_time : Flag<["-"], "fno-unit-at-a-time">, Group<f_Group>;
 def fno_unwind_tables : Flag<["-"], "fno-unwind-tables">, Group<f_Group>;
 def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group<f_Group>;

 def fuse_cxa_atexit : Flag<["-"], "fuse-cxa-atexit">, Group<f_Group>;
 def fuse_init_array : Flag<["-"], "fuse-init-array">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Use .init_array instead of .ctors">;
+def fdestroy_globals : Flag<["-"], "fdestroy-globals">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Register global variables for destruction">;
 def fno_var_tracking : Flag<["-"], "fno-var-tracking">, Group<clang_ignored_f_Group>;
 def fverbose_asm : Flag<["-"], "fverbose-asm">, Group<f_Group>;
 def fvisibility_EQ : Joined<["-"], "fvisibility=">, Group<f_Group>,

include/clang/Frontend/CodeGenOptions.def

 CODEGENOPT(CXAAtExit         , 1, 1) ///< Use __cxa_atexit for calling destructors.
 CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
                                      ///< aliases to base ctors when possible.
+CODEGENOPT(DestroyGlobals, 1, 1) ///< -fdestroy-globals
 CODEGENOPT(DataSections      , 1, 0) ///< Set when -fdata-sections is enabled.
 CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names.
 CODEGENOPT(DisableFPElim     , 1, 0) ///< Set when -fomit-frame-pointer is enabled.

lib/CodeGen/CGDeclCXX.cpp

       EmitDeclInit(*this, D, DeclAddr);
     if (CGM.isTypeConstant(D.getType(), true))
       EmitDeclInvariant(*this, D, DeclPtr);
-    else
+    else if (CGM.getCodeGenOpts().DestroyGlobals)
       EmitDeclDestroy(*this, D, DeclAddr);
     return;
   }

lib/Driver/ToolChains/Clang.cpp

       KernelOrKext)
     CmdArgs.push_back("-fno-use-cxa-atexit");
 
+  // -fdestroy-globals=1 is default.
+  if (!Args.hasFlag(options::OPT_fdestroy_globals,
+                    options::OPT_fno_destroy_globals, true))
+      CmdArgs.push_back("-fno-destroy-globals");
+
   // -fms-extensions=0 is default.
   if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
                    IsWindowsMSVC))

lib/Frontend/CompilerInvocation.cpp

   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
   Opts.CXAAtExit = !Args.hasArg(OPT_fno_use_cxa_atexit);
   Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases);
+  Opts.DestroyGlobals = !Args.hasArg(OPT_fno_destroy_globals);
   Opts.CodeModel = getCodeModel(Args, Diags);
   Opts.DebugPass = Args.getLastArgValue(OPT_mdebug_pass);
   Opts.DisableFPElim =

test/Driver/fdestroy-globals.cpp

+// RUN: %clang -fno-destroy-globals -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-FLAG-DISABLE
+// RUN: %clang -fdestroy-globals -### %s 2>&1    | FileCheck %s --check-prefix=CHECK-FLAG-ENABLE1
+// RUN: %clang -### %s 2>&1                      | FileCheck %s --check-prefix=CHECK-FLAG-ENABLE2
+
+// RUN: %clang_cc1 -fno-destroy-globals -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-CODE-DISABLE
+// RUN: %clang_cc1 -fdestroy-globals -emit-llvm -o - %s    | FileCheck %s --check-prefix=CHECK-CODE-ENABLE
+
+// CHECK-FLAG-DISABLE: "-cc1"
+// CHECK-FLAG-DISABLE: "-fno-destroy-globals"
+
+// CHECK-FLAG-ENABLE1: "-cc1"
+// CHECK-FLAG-ENABLE1-NOT: "-fno-destroy-globals"
+
+// CHECK-FLAG-ENABLE2: "-cc1"
+// CHECK-FLAG-ENABLE2-NOT: "-fno-destroy-globals"
+
+// CHECK-CODE-DISABLE-LABEL: define {{.*}} @__cxx_global_var_init
+// CHECK-CODE-DISABLE: call void @_ZN1AC1Ev{{.*}}
+// CHECK-CODE-DISABLE: ret void
+
+// CHECK-CODE-ENABLE-LABEL: define {{.*}} @__cxx_global_var_init
+// CHECK-CODE-ENABLE: call void @_ZN1AC1Ev{{.*}}
+// CHECK-CODE-ENABLE: %{{.*}} = call i32 @__cxa_atexit{{.*}}_ZN1AD1Ev
+// CHECK-CODE-ENABLE: ret void
+
+struct A {
+  virtual ~A() {}
+};
+
+A a;