bugpoint: Prototype disabling symbolication of bugpoint-executed programs

[dblaikie]

Summary

This shows the basic idea, using command line arguments. Further work
would be done to plumb through the argument to other tools (llc, lli).

This wouldn't handle things like external scripts (bugpoint's
-compile-command feature) which would need to explicitly opt-in by
passing the argument to any LLVM tools they invoke to avoid the
performance penalty of symbolication.

Alternatively (as suggested by chandlerc@) an environment variable would
be used. This would allow the option to pass transparently through user
scripts, pass to compilers if they happened to be LLVM-ish, etc.

I worry a bit about using cl::opt in the crash handling code - LLVM
might crash early, perhaps before the cl::opt is properly initialized?
Or at least before arguments have been parsed?

I shyed away from doing this with an environment variable when I
realized that would require copying the existing environment and
appending the env variable of interest. But it seems there's no existing
LLVM API for accessing the environment (even the Support tests for
process launching have their own ifdefs for getting the environment). It
could be added, but seemed like a higher bar/untested codepath to
actually add environment variables.

Anyway - so looking for folks thoughts/preferences/etc. This change as
is does fix the original issue I came across, with this change
test/BugPoint/metadata.ll goes from 1m34s to 6.5s (& I think it hits all
the other bugpoint tests too - so I guess they don't have loads of
coverage on the different programs that can be executed, etc)

[• dberlin]

I think this is the right thing to do given what we've seen. Nobody has come up with a use case where they current crawl backtraces in their scripts, etc.
If it becomes common, we can always revisit.

[chandlerc]

I'm pretty happy to start simple with a flag and go from there.

lib/Support/Signals.cpp

39–42 ↗

(On Diff #101121)

Especially because this is initialized to false initially, I think it'll be fine even if we crash in weird places.

tools/bugpoint/ToolRunner.cpp

66–67 ↗

(On Diff #101121)

I assume this was just an experiment?

85–88 ↗

(On Diff #101121)

same...

llvm/trunk/lib/Support/Signals.cpp

 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Options.h"
 #include <vector>
 
-namespace llvm {
-
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only TRULY operating system
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
+using namespace llvm;
+
+static cl::opt<bool>
+    DisableSymbolication("disable-symbolication",
+                         cl::desc("Disable symbolizing crash backtraces."),
+                         cl::init(false), cl::Hidden);
+
 static ManagedStatic<std::vector<std::pair<void (*)(void *), void *>>>
     CallBacksToRun;
 void sys::RunSignalHandlers() {
   if (!CallBacksToRun.isConstructed())
     return;
   for (auto &I : *CallBacksToRun)
     I.first(I.second);
   CallBacksToRun->clear();
 }
-}
-
-using namespace llvm;
 
 static bool findModulesAndOffsets(void **StackTrace, int Depth,
                                   const char **Modules, intptr_t *Offsets,
                                   const char *MainExecutableName,
                                   StringSaver &StrPool);
 
 /// Format a pointer value as hexadecimal. Zero pad it out so its always the
 /// same width.
 static FormattedNumber format_ptr(void *PC) {
   // Each byte is two hex digits plus 2 for the 0x prefix.
   unsigned PtrWidth = 2 + 2 * sizeof(void *);
   return format_hex((uint64_t)PC, PtrWidth);
 }
 
 static bool printSymbolizedStackTrace(StringRef Argv0,
                                       void **StackTrace, int Depth,
                                       llvm::raw_ostream &OS)
   LLVM_ATTRIBUTE_USED;
 
 /// Helper that launches llvm-symbolizer and symbolizes a backtrace.
 static bool printSymbolizedStackTrace(StringRef Argv0,
                                       void **StackTrace, int Depth,
                                       llvm::raw_ostream &OS) {
+  if (DisableSymbolication)
+    return false;
+
   // Don't recursively invoke the llvm-symbolizer binary.
   if (Argv0.find("llvm-symbolizer") != std::string::npos)
     return false;
 
   // FIXME: Subtract necessary number from StackTrace entries to turn return addresses
   // into actual instruction addresses.
   // Use llvm-symbolizer tool to symbolize the stack traces. First look for it
   // alongside our binary, then in $PATH.

llvm/trunk/test/BugPoint/unsymbolized.ll

+; REQUIRES: loadable_module
+; RUN: echo "import sys" > %t.py
+; RUN: echo "print('args = ' + str(sys.argv))" >> %t.py
+; RUN: echo "exit(1)" >> %t.py
+; RUN: not bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t -bugpoint-crashcalls -opt-command="%python" -opt-args %t.py | FileCheck %s
+; RUN: not --crash opt -load %llvmshlibdir/BugpointPasses%shlibext %s -bugpoint-crashcalls -disable-symbolication 2>&1 | FileCheck --check-prefix=CRASH %s
+
+; Test that bugpoint disables symbolication on the opt tool to reduce runtime overhead when opt crashes
+; CHECK: args = {{.*}}'-disable-symbolication'
+
+; Test that opt, when it crashes & is passed -disable-symbolication, doesn't symbolicate.
+; In theory this test should maybe be in test/tools/opt or
+; test/Transforms, but since there doesn't seem to be another convenient way to
+; crash opt, apart from the BugpointPasses dynamic plugin, this is the spot for
+; now.
+; CRASH-NOT: Signals.inc
+
+define void @f() {
+  call void @f()
+  ret void
+}

llvm/trunk/tools/bugpoint/OptimizerDriver.cpp

bool BugDriver::runPasses(Module *Program,
   if (UseValgrind) {
     Args.push_back("valgrind");
     Args.push_back("--error-exitcode=1");
     Args.push_back("-q");
     Args.push_back(tool.c_str());
   } else
     Args.push_back(tool.c_str());
 
-  Args.push_back("-o");
-  Args.push_back(OutputFilename.c_str());
   for (unsigned i = 0, e = OptArgs.size(); i != e; ++i)
     Args.push_back(OptArgs[i].c_str());
+  Args.push_back("-disable-symbolication");
+  Args.push_back("-o");
+  Args.push_back(OutputFilename.c_str());
   std::vector<std::string> pass_args;
   for (unsigned i = 0, e = PluginLoader::getNumPlugins(); i != e; ++i) {
     pass_args.push_back(std::string("-load"));
     pass_args.push_back(PluginLoader::getPlugin(i));
   }
   for (std::vector<std::string>::const_iterator I = Passes.begin(),
                                                 E = Passes.end();
        I != E; ++I)