[PATCH] D59725: Additions to creduce script

Tue Mar 26 13:57:17 PDT 2019

akhuang marked 8 inline comments as done.
akhuang added inline comments.

================
Comment at: clang/utils/creduce-clang-crash.py:145
+      matches = re.findall(stacktrace_re, crash_output)
+      result = filter(lambda x: x and x.strip() not in filters, matches)[:5]
+      for msg in result:
----------------
arichardson wrote:
> george.burgess.iv wrote:
> > nit: please prefer `[x for x in matches if x and x.strip() not in filters][:5]`. py3's filter returns a generator, whereas py2's returns a list.
> Stack traces also look different on macOS and it would be nice to handle that too.
> 
> Here's a sample I got from adding a llvm_unreachable at a random location:
> ```
> My unreachable message...
> UNREACHABLE executed at /Users/alex/cheri/llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4468!
> Stack dump:
> 0.	Program arguments: /Users/alex/cheri/llvm-project/cmake-build-debug/bin/opt -mtriple=cheri-unknown-freebsd -mcpu=cheri128 -mattr=+cheri128 -target-abi purecap -relocation-model pic -S -instcombine -simplifycfg /Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll -o - 
> 1.	Running pass 'Function Pass Manager' on module '/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll'.
> 2.	Running pass 'Combine redundant instructions' on function '@cannot_fold_tag_unknown'
> 0  libLLVMSupport.dylib     0x0000000114515a9d llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 45
> 1  libLLVMSupport.dylib     0x00000001145153f1 llvm::sys::RunSignalHandlers() + 65
> 2  libLLVMSupport.dylib     0x0000000114515fbf SignalHandler(int) + 111
> 3  libsystem_platform.dylib 0x00007fff5b637b3d _sigtramp + 29
> 4  libsystem_platform.dylib 0x00007ffee20d0cf0 _sigtramp + 2259259856
> 5  libsystem_c.dylib        0x00007fff5b4f51c9 abort + 127
> 6  libLLVMSupport.dylib     0x000000011446bb12 llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) + 162
> 7  libLLVMInstCombine.dylib 0x0000000112c345c8 llvm::InstCombiner::foldICmpUsingKnownBits(llvm::ICmpInst&) + 4136
> 8  libLLVMInstCombine.dylib 0x0000000112c34d19 llvm::InstCombiner::visitICmpInst(llvm::ICmpInst&) + 569
> 9  libLLVMInstCombine.dylib 0x0000000112bb9cf2 llvm::InstCombiner::run() + 1522
> 10 libLLVMInstCombine.dylib 0x0000000112bbb310 combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, llvm::LoopInfo*) + 624
> 11 libLLVMInstCombine.dylib 0x0000000112bbb6d6 llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) + 214
> 12 libLLVMCore.dylib        0x0000000111c0bb4d llvm::FPPassManager::runOnFunction(llvm::Function&) + 317
> 13 libLLVMCore.dylib        0x0000000111c0be83 llvm::FPPassManager::runOnModule(llvm::Module&) + 99
> 14 libLLVMCore.dylib        0x0000000111c0c2c4 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) + 420
> 15 libLLVMCore.dylib        0x0000000111c0c036 llvm::legacy::PassManagerImpl::run(llvm::Module&) + 182
> 16 opt                      0x000000010db6657b main + 7163
> 17 libdyld.dylib            0x00007fff5b44ced9 start + 1
> ```
I changed the regex to ignore the # at the beginning of the line - I think that should cover the mac os stack trace 

================
Comment at: clang/utils/creduce-clang-crash.py:303
+                                    opts_startswith=["-O"])
+    self.clang_args = new_args
+    verbose_print("Simplified command:", quote_cmd(self.get_crash_cmd()))
----------------
arichardson wrote:
> george.burgess.iv wrote:
> > FWIW, opportunistically trying to add `-fsyntax-only` may help here: if the crash is in the frontend, it means that non-repros will stop before codegen, rather than trying to generate object files, or whatever they were trying to generate in the first place.
> Yes that sounds like a good idea! I just do -emit-llvm to avoid assembly output but for parser/sema crashes -fsyntax-only would save some time.
> 
> Another one I found useful was `-Werror=implicit-int` to get more readable test cases from creduce: https://github.com/CTSRD-CHERI/llvm-project/blob/master/clang/utils/creduce_crash_testcase.py#L851
> 
> Without that flag lots of test cases look really weird due to the implicit int and various inferred semicolons.
> 
Sounds good-- I added `-fsyntax-only`, `-emit-llvm` and `-Werror=implicit-int`

================
Comment at: clang/utils/creduce-clang-crash.py:64
+
+class Reduce:
+  def __init__(self, crash_script, file_to_reduce):
----------------
arichardson wrote:
> Does this need to be `Reduce(object):` for python2? 
I think it still works, but adding `object` makes sense. 

================
Comment at: clang/utils/creduce-clang-crash.py:123
+    # Look for specific error messages
+    regexes = [r"Assertion `(.+)' failed",
+               r"fatal error: backend error: (.+)",
----------------
arichardson wrote:
> Some operating systems use a different assertion format (see my reduce script: https://github.com/CTSRD-CHERI/llvm-project/blob/master/clang/utils/creduce_crash_testcase.py#L662)
> 
> For MacOS/FreeBSD we need to also handle `r"Assertion failed: \(.+\),"`. Over the past two years I have also had cases where the other message formats have been useful so I would quite like to see them added here as well.
> 
> 
added the error messages from your script

================
Comment at: clang/utils/creduce-clang-crash.py:205
+    p = subprocess.Popen(cmd,
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.STDOUT)
----------------
arichardson wrote:
> If we are writing the preprocessed output to that tempfile anyway, we could use `stdout=tmpfile`?
> 
> For python3 this would be simpler with `subprocess.check_call()` but I'm not sure python 2.7 has it.
I think python2.7 has `check_call` - switched to using that

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59725/new/

https://reviews.llvm.org/D59725