[docs][Bugpoint]Add notes about multiple crashes

[jsji]

Summary

When reducing case for a CodeGenCrash, bugpoint may generate a new
reduced
testcase that exposes/causes another crash or break something due to
limitation.

Bugpoint does not distiguish different crashes currently,
so when this happens, bugpoint will go on reducing for the new crash,
or just abort, we can't get the case reduced for the origial crash.

An advice is added into usage doc to connect to recommend checking error
message with scripts and `-compile-command`.

[jsji]

Fix typos in testcase.

[jsji]

Ping ...

[jsji]

Ping..
and add the most recent code modifier & patch reviewers . Thanks!

[vsk]

I think you've identified a real, pernicious problem, but am not sure that this is the best approach for addressing it. One potential problem here is that slight variations in the error message can confuse the reducer. If e.g. some location information changes in between messages, the reduction will stop too early.

Maybe we need better documentation/examples about how to write check scripts that validate the error message?

[jsji]

Thanks @vsk for the valuable comments.

In D66832#1664105, @vsk wrote:

I think you've identified a real, pernicious problem, but am not sure that this is the best approach for addressing it. One potential problem here is that slight variations in the error message can confuse the reducer. If e.g. some location information changes in between messages, the reduction will stop too early.

Maybe we need better documentation/examples about how to write check scripts that validate the error message?

Yes, you are absolutely right!
In some complex cases, the change might looks trivial, eg: location information, or call stack change.
And, yes, we should be able to use -custom-command and write a simple script to validate.

eg, for the testcase in llvm/test/BugPoint/crash-sameerror.ll, we should be able to use a custom-command line like:

$ cat t1.sh
#!/bin/bash
!(llc < $1 2>&1|grep "LLVM ERROR: Cannot select: intrinsic %llvm.ppc.altivec.dss" )

$ bugpoint -compile-custom -compile-command ./t1.sh t.ll

I think the major point of this patch is to make it easier for beginners, for common scenarios:
for most simple code gen crash, we can simply use follow command to reduce the testcases.

$ bugpoint --llc-safe t.ll -same-error 

[jsji]

I think it would be great to make the powerful bugpoint tool easier to use, especially for beginners, for common simple scenarios.

But if we don't think this is really necessary or in the wrong direction of design, I am OK to abandon this patch.

[vsk]

In D66832#1664974, @jsji wrote:

I think it would be great to make the powerful bugpoint tool easier to use, especially for beginners, for common simple scenarios.

But if we don't think this is really necessary or in the wrong direction of design, I am OK to abandon this patch.

I see the need to make error validation with bugpoint more beginner-friendly, but am worried about situations in which the mode proposed here could confuse beginners (what if the first error isn't the interesting one? or reductions stop too early?). I don't have a better alternative to suggest. Perhaps we could repost @mehdi_amini's writeup in the "Search for a string in the output" section within http://blog.llvm.org/2015/11/reduce-your-testcases-with-bugpoint-and.html, or link to it from the bugpoint docs.

[jsji]

Abandon the --same-error change, update docs instead.

[jsji]

> I see the need to make error validation with bugpoint more beginner-friendly, but am worried about situations in which the mode proposed here could confuse beginners (what if the first error isn't the interesting one? or reductions stop too early?). I don't have a better alternative to suggest. Perhaps we could repost @mehdi_amini's writeup in the "Search for a string in the output" section within http://blog.llvm.org/2015/11/reduce-your-testcases-with-bugpoint-and.html, or link to it from the bugpoint docs.

Make sense, the more flag, the more confusion. So I abandon the original change.

I am not sure whether it is appropriate to point to a writeup in blog, so I just simple refer it to the command in the official documents.
@vsk Any comments?

[jsji]

Ping? @vsk Do you think it worth updating Bugpoint.rst? If so, any comments? Thanks.

[vsk]

Thanks, this is quite helpful.

llvm/test/BugPoint/crash-sameerror.ll

+; Test that bugpoint can stick to same error (ignore non-relevant errors)
+;
+; RUN: bugpoint -llc-safe %s -output-prefix %t -silence-passes -same-error > /dev/null
+; REQUIRES: powerpc-registered-target
+target triple = "powerpc64le-unknown-linux-gnu"
+
+define dso_local void @test1(i32 signext %a) local_unnamed_addr #0 {
+entry:
+; This should cause codegen error, the argument should be a constant.
+  tail call void @llvm.ppc.altivec.dss(i32 %a) #1
+  ret void
+}
+declare void @llvm.ppc.altivec.dss(i32) #1
+
+; This should trigger bugpoint limitation when reducing attributes
+; verify failed: Attribute 'optnone' requires 'noinline'! 
+attributes #0 = { optnone noinline nounwind}
+attributes #1 = { nounwind }

llvm/tools/bugpoint/CrashDebugger.cpp

cl::opt<bool> NoStripDebugInfo("disable-strip-debuginfo",
                                cl::desc("Do not strip debug info metadata"),
                                cl::init(false));
 cl::opt<bool> NoStripDebugTypeInfo("disable-strip-debug-types",
                                cl::desc("Do not strip debug type info metadata"),
                                cl::init(false));
 cl::opt<bool> VerboseErrors("verbose-errors",
                             cl::desc("Print the output of crashing program"),
                             cl::init(false));
+cl::opt<bool> SameError("same-error",
+                            cl::desc("Focus on 1st crash error, treat other new crash as success"),
+                            cl::init(false));
+
 }
 
 namespace llvm {
 class ReducePassList : public ListReducer<std::string> {
   BugDriver &BD;
 
 public:
   ReducePassList(BugDriver &bd) : BD(bd) {}
Error BugDriver::debugOptimizerCrash(const std::string &ID) {
 
   EmitProgressBitcode(*Program, ID);
 
   return DebugACrash(*this, TestForOptimizerCrash);
 }
 
 static bool TestForCodeGenCrash(const BugDriver &BD, Module *M) {
   if (Error E = BD.compileProgram(*M)) {
+    bool SameCrash = true;
+    handleAllErrors(std::move(E), [&](const ErrorInfoBase &EIB) {
+      static std::string CrashMsgs = "";
+      if (SameError) {
+        StringRef str(EIB.message());
+        auto Msgs = str.split("Error Message:\n");
+        if (CrashMsgs.empty())
+          CrashMsgs = Msgs.second;
+        else {
+          SameCrash = CrashMsgs.compare(Msgs.second.str()) == 0;
+        }
+      }
       if (VerboseErrors)
-      errs() << toString(std::move(E)) << "\n";
+        errs() << EIB.message() << "\n";
       else {
-      consumeError(std::move(E));
         errs() << "<crash>\n";
       }
+    });
+    if (SameError) {
+      // Only retrun true when we are getting same crash
+      return SameCrash;
+    }
     return true; // Tool is still crashing.
   }
   errs() << '\n';
   return false;
 }
 
 /// debugCodeGeneratorCrash - This method is called when the code generator
 /// crashes on an input.  It attempts to reduce the input as much as possible
 /// while still causing the code generator to crash.
 Error BugDriver::debugCodeGeneratorCrash() {
   errs() << "*** Debugging code generator crash!\n";
 
   return DebugACrash(*this, TestForCodeGenCrash);
 }

llvm/tools/bugpoint/ToolRunner.cpp

 }
 
 static Error ProcessFailure(StringRef ProgPath, ArrayRef<StringRef> Args,
                             unsigned Timeout = 0, unsigned MemoryLimit = 0) {
   std::ostringstream OS;
   OS << "\nError running tool:\n ";
   for (StringRef Arg : Args)
     OS << " " << Arg.str();
-  OS << "\n";
+  OS << "\nError Message:\n";
 
   // Rerun the compiler, capturing any error messages to print them.
   SmallString<128> ErrorFilename;
   std::error_code EC = sys::fs::createTemporaryFile(
       "bugpoint.program_error_messages", "", ErrorFilename);
   if (EC) {
     errs() << "Error making unique filename: " << EC.message() << "\n";
     exit(1);