[LLD][ELF] Add FORCE_LLD_DIAGNOSTICS_CRASH to force LLD to crash

[bd1976llvm]

Summary

Add FORCE_LLD_DIAGNOSTICS_CRASH inspired by the existing FORCE_CLANG_DIAGNOSTICS_CRASH.

This is particularly useful for people customizing LLD as they may want to modify the crash reporting behavior.

[emaste]

LGTM

[peter.smith]

IIUC this is primarily so that a test can be written for a crash message? Possibly worth adding an additional message "Crashing due to enviroment variable FORCE_LLD_DIAGNOSTICS_CRASH" as I can imagine someone setting the environment variable then forgetting and wondering why LLD keeps crashing. Alternatively would a --crash option be easier to manage? I don't think we can replicate the reproducer that the clang driver uses with this option.

[bd1976llvm]

Added useful reminder message as suggested by Peter.

[bd1976llvm]

In D128195#3596592, @peter.smith wrote:

Alternatively would a --crash option be easier to manage? I don't think we can replicate the reproducer that the clang driver uses with this option.

I'm not opposed. I have a slight preference for an env variable as it allows you to simulate crashes on systems where it is difficult to control command line switches.

[tschuett]

Entry in ReleaseNotes or man page?

[bd1976llvm]

In D128195#3596905, @tschuett wrote:

Entry in ReleaseNotes or man page?

I'll add a ReleaseNote but I don't think we want to document this otherwise as it is really a developer tool not a feature we want anyone to rely on. Thanks.

[bd1976llvm]

Add ReleaseNote.

[jhenderson]

lld/ELF/Driver.cpp

515 ↗

(On Diff #438465)

Two nits:

1. not clang-formatted
2. This probably should come under the same formatting rules as errors and warning messages (see https://llvm.org/docs/CodingStandards.html#error-and-warning-messages) so should start with a lower-case letter.

lld/test/ELF/crash-report.test

2

Personal preference: I don't know about anybody else, but I'm much more used to # and ## being used in lit tests where they can be.

4

[bd1976llvm]

Addressed style comments.

[bd1976llvm]

Fix typo.

[bd1976llvm]

Remove accidentally committed comments.

[jhenderson]

LGTM, but you probably want to wait for @peter.smith and/or @MaskRay.

[peter.smith]

LGTM from me. Thanks for the update. Will be worth giving MaskRay some time to comment as well.

[MaskRay]

If this is intended for all ports, you may use lld/tools/lld/lld.cpp

lld/test/ELF/crash-report.test

9

Is this reliable on Windows?

Also, LLVM_ENABLE_BACKTRACES=off builds do not have this diagnostic.

[bd1976llvm]

Move to lld.cpp

[bd1976llvm]

lld/test/ELF/crash-report.test

9

It seems to be reliable on windows- we have been running this test for a while now downstream with no failures.

Also, LLVM_ENABLE_BACKTRACES=off builds do not have this diagnostic.

Great review comment. However, I'm not sure how to make this test pass with LLVM_ENABLE_BACKTRACES=off without adding too much complexity. Do you/anyone else have an idea? Thanks.

[MaskRay]

lld/docs/ReleaseNotes.rst

32

Since you have the differential link, add it.

lld/test/ELF/crash-report.test

9

We may need to change test/lit.cfg.py to UNSUPPORTED: the test if LLVM_ENABLE_BACKTRACES=off.

If it is too troublesome, don't check the detailed diagnostic.

[bd1976llvm]

Ensure added lit test passes for LLVM_ENABLE_BACKTRACES=off builds.
Added differential URL to release note.
Used regex so that test doesn't care about contents of the bug report URL.

[bd1976llvm]

lld/test/ELF/crash-report.test

9

It wasn't troublesome at all. Thanks for the suggestion.

[bd1976llvm]

• Removed the LLD_IN_TEST un-setting in the lit test as we have moved the code earlier now.
• Removed checking for the period/full-stop in the lit test to allow customizers to extend the sentence without adjusting this test.

lld/docs/ReleaseNotes.rst

 =================================================
 
 ELF Improvements
 ----------------
 
 * ``-z pack-relative-relocs`` is now available to support ``DT_RELR`` for glibc 2.36+.
   (`D120701 <https://reviews.llvm.org/D120701>`_)
 * ``--no-fortran-common`` (pre 12.0.0 behavior) is now the default.
+* ``FORCE_LLD_DIAGNOSTICS_CRASH`` environment variable is now available to force LLD to crash.
+  (`D128195 <https://reviews.llvm.org/D128195>`_)
 
 Breaking changes
 ----------------
 
 * The GNU ld incompatible ``--no-define-common`` has been removed.
 * The obscure ``-dc``/``-dp`` options have been removed.
 * ``-d`` is now ignored.
 * If a prevailing COMDAT group defines STB_WEAK symbol, having a STB_GLOBAL symbol in a non-prevailing group is now rejected with a diagnostic.

lld/test/ELF/crash-report.test

+# REQUIRES: backtrace
+
+## Test the diagnostics produced when LLD crashes.
+
+# RUN: env FORCE_LLD_DIAGNOSTICS_CRASH=1 not --crash ld.lld -o /dev/null 2>&1 | FileCheck %s
+
+## Check the crash text has the correct structure.
+# CHECK: crashing due to environment variable FORCE_LLD_DIAGNOSTICS_CRASH
+# CHECK: PLEASE submit a bug report to {{.*}} and include the crash backtrace
+# CHECK: Stack dump:

lld/test/lit.cfg.py

 
 # Indirectly check if the mt.exe Microsoft utility exists by searching for
 # cvtres, which always accompanies it.  Alternatively, check if we can use
 # libxml2 to merge manifests.
 if (lit.util.which('cvtres', config.environment['PATH']) or
         config.have_libxml2):
     config.available_features.add('manifest_tool')
 
+if config.enable_backtrace:
+    config.available_features.add('backtrace')
+
 if config.have_libxar:
     config.available_features.add('xar')
 
 if config.have_libxml2:
     config.available_features.add('libxml2')
 
 if config.have_dia_sdk:
     config.available_features.add("diasdk")

lld/test/lit.site.cfg.py.in

 @LIT_SITE_CFG_IN_HEADER@
 
 import lit.util
 
 config.have_dia_sdk = lit.util.pythonize_bool("@LLVM_ENABLE_DIA_SDK@")
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
 config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
 config.llvm_shlib_dir = "@SHLIBDIR@"
 config.llvm_shlib_ext = "@SHLIBEXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
+config.enable_backtrace = @ENABLE_BACKTRACES@
 config.errc_messages = "@LLVM_LIT_ERRC_MESSAGES@"
 config.lld_obj_root = "@LLD_BINARY_DIR@"
 config.lld_libs_dir = lit_config.substitute("@CURRENT_LIBS_DIR@")
 config.lld_tools_dir = lit_config.substitute("@CURRENT_TOOLS_DIR@")
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @LLVM_HAVE_LIBXAR@

lld/tools/lld/lld.cpp

static unsigned inTestVerbosity() {
   StringRef(getenv("LLD_IN_TEST")).getAsInteger(10, v);
   return v;
 }
 
 int main(int argc, const char **argv) {
   InitLLVM x(argc, argv);
   sys::Process::UseANSIEscapeCodes(true);
 
+  if (::getenv("FORCE_LLD_DIAGNOSTICS_CRASH")) {
+    llvm::errs()
+        << "crashing due to environment variable FORCE_LLD_DIAGNOSTICS_CRASH\n";
+    LLVM_BUILTIN_TRAP;
+  }
+
   // Not running in lit tests, just take the shortest codepath with global
   // exception handling and no memory cleanup on exit.
   if (!inTestVerbosity())
     return lldMain(argc, argv, llvm::outs(), llvm::errs());
 
   Optional<int> mainRet;
   CrashRecoveryContext::Enable();