<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Sun, Jun 11, 2017 at 6:15 PM Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 06/09/2017 12:29 AM, David Blaikie via llvm-commits wrote:<br>
> Author: dblaikie<br>
> Date: Fri Jun 9 02:29:03 2017<br>
> New Revision: 305056<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305056&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=305056&view=rev</a><br>
> Log:<br>
> bugpoint: disabling symbolication of bugpoint-executed programs<br>
><br>
> Initial implementation - needs similar work/testing for other tools<br>
> bugpoint invokes (llc, lli I think, maybe more).<br>
><br>
> Alternatively (as suggested by chandlerc@) an environment variable could<br>
> be used. This would allow the option to pass transparently through user<br>
> scripts, pass to compilers if they happened to be LLVM-ish, etc.<br>
><br>
> I worry a bit about using cl::opt in the crash handling code - LLVM<br>
> might crash early, perhaps before the cl::opt is properly initialized?<br>
> Or at least before arguments have been parsed?<br>
><br>
> - should be OK since it defaults to "pretty", so if the crash is very<br>
> early in opt parsing, etc, then crash reports will still be symbolized.<br>
><br>
> I shyed away from doing this with an environment variable when I<br>
> realized that would require copying the existing environment and<br>
> appending the env variable of interest. But it seems there's no existing<br>
> LLVM API for accessing the environment (even the Support tests for<br>
> process launching have their own ifdefs for getting the environment). It<br>
> could be added, but seemed like a higher bar/untested codepath to<br>
> actually add environment variables.<br>
><br>
> Most importantly, this reduces the runtime of test/BugPoint/metadata.ll<br>
> in a split-dwarf Debug build from 1m34s to 6.5s by avoiding a lot of<br>
> symbolication. (this wasn't a problem for non-split-dwarf builds only<br>
> because the executable was too large to map into memory (due to bugpoint<br>
> setting a 400MB memory (including address space - not sure why? Going to<br>
> remove that) limit on the child process) so symbolication would fail<br>
> fast & wouldn't spend all that time parsing DWARF, etc)<br>
The 400MB limit is probably to catch out of memory conditions in the<br>
sub-process. (i.e. some bugs show up as huge bogus allocation attempts<br>
and we want to be able to reduce those too.) It can probably be fine<br>
tuned, but having some limit is definitely useful.<br></blockquote><div><br>Oh, sure - wasn't planning to remove the limit entirely, but to remove the /address space/ limit, while keeping the resident memory limit. As you say, it's probably quite reasonable for bugpoint to stop the child processes from runaway memory allocation - but I'm not sure of any particular risk of a child process just memory mapping a lot of files, as such.<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
><br>
> Reviewers: chandlerc, dannyb<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D33804" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33804</a><br>
><br>
> Added:<br>
> llvm/trunk/test/BugPoint/unsymbolized.ll<br>
> Modified:<br>
> llvm/trunk/lib/Support/Signals.cpp<br>
> llvm/trunk/tools/bugpoint/OptimizerDriver.cpp<br>
><br>
> Modified: llvm/trunk/lib/Support/Signals.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Signals.cpp?rev=305056&r1=305055&r2=305056&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Signals.cpp?rev=305056&r1=305055&r2=305056&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Support/Signals.cpp (original)<br>
> +++ llvm/trunk/lib/Support/Signals.cpp Fri Jun 9 02:29:03 2017<br>
> @@ -26,15 +26,21 @@<br>
> #include "llvm/Support/Program.h"<br>
> #include "llvm/Support/StringSaver.h"<br>
> #include "llvm/Support/raw_ostream.h"<br>
> +#include "llvm/Support/Options.h"<br>
> #include <vector><br>
><br>
> -namespace llvm {<br>
> -<br>
> //===----------------------------------------------------------------------===//<br>
> //=== WARNING: Implementation here must contain only TRULY operating system<br>
> //=== independent code.<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> +using namespace llvm;<br>
> +<br>
> +static cl::opt<bool><br>
> + DisableSymbolication("disable-symbolication",<br>
> + cl::desc("Disable symbolizing crash backtraces."),<br>
> + cl::init(false), cl::Hidden);<br>
> +<br>
> static ManagedStatic<std::vector<std::pair<void (*)(void *), void *>>><br>
> CallBacksToRun;<br>
> void sys::RunSignalHandlers() {<br>
> @@ -44,9 +50,6 @@ void sys::RunSignalHandlers() {<br>
> I.first(I.second);<br>
> CallBacksToRun->clear();<br>
> }<br>
> -}<br>
> -<br>
> -using namespace llvm;<br>
><br>
> static bool findModulesAndOffsets(void **StackTrace, int Depth,<br>
> const char **Modules, intptr_t *Offsets,<br>
> @@ -70,6 +73,9 @@ static bool printSymbolizedStackTrace(St<br>
> static bool printSymbolizedStackTrace(StringRef Argv0,<br>
> void **StackTrace, int Depth,<br>
> llvm::raw_ostream &OS) {<br>
> + if (DisableSymbolication)<br>
> + return false;<br>
> +<br>
> // Don't recursively invoke the llvm-symbolizer binary.<br>
> if (Argv0.find("llvm-symbolizer") != std::string::npos)<br>
> return false;<br>
><br>
> Added: llvm/trunk/test/BugPoint/unsymbolized.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/BugPoint/unsymbolized.ll?rev=305056&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/BugPoint/unsymbolized.ll?rev=305056&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/BugPoint/unsymbolized.ll (added)<br>
> +++ llvm/trunk/test/BugPoint/unsymbolized.ll Fri Jun 9 02:29:03 2017<br>
> @@ -0,0 +1,21 @@<br>
> +; REQUIRES: loadable_module<br>
> +; RUN: echo "import sys" > %t.py<br>
> +; RUN: echo "print('args = ' + str(sys.argv))" >> %t.py<br>
> +; RUN: echo "exit(1)" >> %t.py<br>
> +; RUN: not bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t -bugpoint-crashcalls -opt-command="%python" -opt-args %t.py | FileCheck %s<br>
> +; RUN: not --crash opt -load %llvmshlibdir/BugpointPasses%shlibext %s -bugpoint-crashcalls -disable-symbolication 2>&1 | FileCheck --check-prefix=CRASH %s<br>
> +<br>
> +; Test that bugpoint disables symbolication on the opt tool to reduce runtime overhead when opt crashes<br>
> +; CHECK: args = {{.*}}'-disable-symbolication'<br>
> +<br>
> +; Test that opt, when it crashes & is passed -disable-symbolication, doesn't symbolicate.<br>
> +; In theory this test should maybe be in test/tools/opt or<br>
> +; test/Transforms, but since there doesn't seem to be another convenient way to<br>
> +; crash opt, apart from the BugpointPasses dynamic plugin, this is the spot for<br>
> +; now.<br>
> +; CRASH-NOT: Signals.inc<br>
> +<br>
> +define void @f() {<br>
> + call void @f()<br>
> + ret void<br>
> +}<br>
><br>
> Modified: llvm/trunk/tools/bugpoint/OptimizerDriver.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/OptimizerDriver.cpp?rev=305056&r1=305055&r2=305056&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/OptimizerDriver.cpp?rev=305056&r1=305055&r2=305056&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/tools/bugpoint/OptimizerDriver.cpp (original)<br>
> +++ llvm/trunk/tools/bugpoint/OptimizerDriver.cpp Fri Jun 9 02:29:03 2017<br>
> @@ -202,10 +202,11 @@ bool BugDriver::runPasses(Module *Progra<br>
> } else<br>
> Args.push_back(tool.c_str());<br>
><br>
> - Args.push_back("-o");<br>
> - Args.push_back(OutputFilename.c_str());<br>
> for (unsigned i = 0, e = OptArgs.size(); i != e; ++i)<br>
> Args.push_back(OptArgs[i].c_str());<br>
> + Args.push_back("-disable-symbolication");<br>
> + Args.push_back("-o");<br>
> + Args.push_back(OutputFilename.c_str());<br>
> std::vector<std::string> pass_args;<br>
> for (unsigned i = 0, e = PluginLoader::getNumPlugins(); i != e; ++i) {<br>
> pass_args.push_back(std::string("-load"));<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
</blockquote></div></div>