Additions to creduce script

[akhuang]

Summary

Some more additions to the script - mainly reducing the clang args after the creduce run by removing them one by one and seeing if the crash reproduces. Other things:

• remove the --crash flag when "fatal error" occurs
• fixed to read stack trace functions from the top
• run creduce on a copy of the original file

[akhuang]

Fix some typos, pass --tidy flag to creduce

[akhuang]

reuploaded diff with full context

[george.burgess.iv]

Thanks for this!

clang/utils/creduce-clang-crash.py

137

Please expand a bit on why 5 was chosen (is there some deep reason behind it, or does it just seem like a sensible number?)

145

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.

193

I think _ is an open file descriptor; please os.close it if we don't want to use it

199

Same nit

202

was this intended to use cmd?

205

Do we want to check the exit code of this? Or do we assume that if clang crashes during preprocessing, we'll just see a different error during check_expected_output? (In the latter case, please add a comment)

222

Is it intentional to group multiple consecutive non-dashed args? e.g. it seems that clang -ffoo bar baz will turn into ['clang', '-ffoo bar baz']

223

Should we be shlex.quote'ing y here (and probably in the return x + [y] below)?

251

IMO, if even extra_arg is in new_args, we should still move it near the end. Arg ordering matters in clang, generally with later args taking precedence over earlier ones. e.g. the -g$N args in https://godbolt.org/z/Mua8cs

279

If we're replacing other args with their effective negation, does it also make sense to replace all debug-ish options with -g0?

296

Might not want to have a space here; -DFOO=1 is valid (same for -I below)

306

Probably want to do a similar thing for -Xclang (which, as far as this code is concerned, acts a lot like -mllvm)

362

I'm unclear on why we do a partial simplification of clang args here, then a full reduction after creduce. Is there a disadvantage to instead doing:

r.write_interestingness_test()
r.clang_preprocess()
r.reduce_clang_args()
r.run_creduce()
r.reduce_clang_args()

?

The final reduce_clang_args being there to remove extra -D/-I/etc. args if preprocessing failed somehow, to remove -std=foo args if those aren't relevant after reduction, etc.

The advantage to this being that we no longer need to maintain a simplify function, and creduce runs with a relatively minimal set of args to start with.

In any case, can we please add comments explaining why we chose whatever order we end up going with, especially where subtleties make it counter to what someone might naively expect?

[akhuang]

clang/utils/creduce-clang-crash.py

137

There is no deep reason - it was an arbitrary smallish number to hopefully not only pick up common stack trace functions

202

yep

205

I think checking the exit code is a good idea

222

I guess that was originally the intention, although now that I think of it it makes more sense to group at most one argument.

223

It quotes everything right before writing to file - are there reasons to quote here instead?

279

I guess -g0 is not a cc1 option, nor is -gdwarf? So this is essentially just removing -gcodeview. I'm actually not sure what the other flags do.

362

Basically the disadvantage is that clang takes longer to run before the reduction, so it takes unnecessary time to iterate through all the arguments beforehand.
And yeah, the final reduce_clang_args is there to minimize the clang arguments needed to reproduce the crash on the reduced source file.

If that makes sense, I can add comments for this-

[rnk]

I recently discovered NamedTemporaryFile, maybe that would help simplify up the various mkstemp usages.

[akhuang]

Style nits, added comments

[george.burgess.iv]

clang/utils/creduce-clang-crash.py

223

We're shlex.spliting groups below, and I assume the intent is Reduce.ungroup_args(Reduce.group_args_by_dash(args)) == args.

If we don't want to quote here, we can also have ungroup_args and group_args_by_dash deal in lists of nonempty lists.

279

Ah, I didn't realize this was dealing with cc1 args. My mistake.

I'm not immediately sure either, but grepping through the code, it looks like -debug-info-kind= may be the main interesting one to us here.

303

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.

306

(You can ignore this comment if we're dealing in cc1; -Xclang is just "pass this directly as a cc1 arg")

362

Eh, I don't have a strong opinion here. My inclination is to prefer a simpler reduction script if the difference is len(args) clang invocations, since I assume creduce is going to invoke clang way more than len(args) times. I see a docstring detailing that simplify is only meant to speed things up now, so I'm content with the way things are.

[akhuang]

clang/utils/creduce-clang-crash.py

223

good point- I guess the whole grouping thing is unnecessarily complicated, so I got rid of it and it now removes the next arg in try_remove_arg_by_index

306

ah, ok.

[akhuang]

fix issue with grouping two command line args together

[arichardson]

clang/utils/creduce-clang-crash.py

64

Does this need to be Reduce(object): for python2?

123

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.

145

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

205

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.

303

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.

310

I would move this before the remove_arg_by_index call since all llvm args start with a - and try_remove_arg_by_index will cause lots of invalid command lines to be created otherwise.

362

I think it makes sense to remove some clang args before running creduce since removal of some flags can massively speed up reduction later (e.g. not emitting debug info or compiling at -O0, only doing -emit-llvm, etc) if they avoid expensive optimizations that don't cause the crash.

[akhuang]

clang/utils/creduce-clang-crash.py

64

I think it still works, but adding object makes sense.

123

added the error messages from your script

145

I changed the regex to ignore the # at the beginning of the line - I think that should cover the mac os stack trace

205

I think python2.7 has check_call - switched to using that

303

Sounds good-- I added -fsyntax-only, -emit-llvm and -Werror=implicit-int

[akhuang]

added to error message regexes and command line flags

[george.burgess.iv]

Only a few more nits on my side, and this LGTM. WDYT, arichardson?

clang/utils/creduce-clang-crash.py

137

Sorry -- should've been clearer. I meant "in the comment in the code, please expand a bit [...]" :)

198

Did we want to use NamedTemporaryFile here as rnk suggested?

(If not, you can lift the os.closes to immediately after this line.)

206

Similar question about NamedTemporaryFile.

Please note that you'll probably have to pass delete=False, since apparently delete=True sets O_TEMPORARY on Windows, which... might follow the file across renames? I'm unsure.

362

Agreed. My question was more "why do we have special reduction code on both sides of this instead of just reduce_clang_args'ing on both sides of the run_creduce." It wasn't clear to me that simplify_clang_args was only intended to speed things up, but now it is. :)

[akhuang]

change mkstemp to NamedTemporaryFile and add decode(utf-8) so it works on python3.5

clang/utils/creduce-clang-crash.py

198

switched to using NamedTemporaryFile here -

206

moved to NamedTemporaryFile with comment about delete=False

[arichardson]

In D59725#1443990, @george.burgess.iv wrote:

Only a few more nits on my side, and this LGTM. WDYT, arichardson?

LGTM with the minor tempfile changes.

clang/utils/creduce-clang-crash.py

201

with tempfile.NamedTemporaryFile() as empty_file:? Definitely works with python 3 and 2.7 docs say This file-like object can be used in a with statement, just like a normal file..

210

Use a with statement?

212

Some crash messages might include the line numbers, do you think it makes sense to fall back to running with -E but without -P and also checking that? I do it in my script but I'm not sure preprocessing saves that much time since creduce will try to remove those statements early.

[akhuang]

Add preprocessing with clang -E only;
use with for opening files

clang/utils/creduce-clang-crash.py

212

Makes sense-- in my experience preprocessing is still quite a bit faster than letting creduce remove all the statements.

[arichardson]

LGTM once the tempfile is deleted.

clang/utils/creduce-clang-crash.py

208

I believe we are currently not deleting this temporary file.
Can delete=False be removed since we are using shutil.copy() instead of a move?

[akhuang]

clang/utils/creduce-clang-crash.py

208

Yeah, I think that should be fine.

[akhuang]

Tmpfile was not being removed

[lebedev.ri]

I've stumbled into an issue with the script:
It got a line: clang: /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' failed.`
And produced the following grep: grep 'L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' t.log || exit 1
But that doesn't work, escapes should be applied. it should be
grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run indvars!\"' t.log || exit 1

[arichardson]

In D59725#1477042, @lebedev.ri wrote:

I've stumbled into an issue with the script:
It got a line: clang: /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' failed.`
And produced the following grep: grep 'L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' t.log || exit 1
But that doesn't work, escapes should be applied. it should be
grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run indvars!\"' t.log || exit 1

In my script I use grep -F " + shlex.quote(self.args.crash_message) which should escape both the message and ensure that the grep argument is not interpreted as a regex.

[lebedev.ri]

cfe/trunk/utils/creduce-clang-crash.py

185 ↗

(On Diff #192867)

In D59725#1477362, @arichardson wrote:

In D59725#1477042, @lebedev.ri wrote:

I've stumbled into an issue with the script:
It got a line: clang: /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' failed.`
And produced the following grep: grep 'L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' t.log || exit 1
But that doesn't work, escapes should be applied. it should be
grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run indvars!\"' t.log || exit 1

In my script I use grep -F " + shlex.quote(self.args.crash_message) which should escape both the message and ensure that the grep argument is not interpreted as a regex.

Great to hear!
It appears that the script is currently more primitive than that currently though.

[akhuang]

cfe/trunk/utils/creduce-clang-crash.py

185 ↗

(On Diff #192867)

Thanks; I added a -F flag (r359216), and it seems like that fixes the issue.

[lebedev.ri]

cfe/trunk/utils/creduce-clang-crash.py

185 ↗

(On Diff #192867)

Thanks!
What about shlex? How do you know bash won't expand that as regex already?

[akhuang]

cfe/trunk/utils/creduce-clang-crash.py

185 ↗

(On Diff #192867)

Sorry, not sure what you mean by bash expanding shlex as regex? It should be printing a quoted string to the bash script.

[lebedev.ri]

cfe/trunk/utils/creduce-clang-crash.py

185 ↗

(On Diff #192867)

$ ls konsole*
'konsole*'   konsole-ACNHfh.history   konsole-CVnmsn.history   konsole-HhEJRA.history
$ echo "konsole\*" | grep -F "konsole*"
$ echo "konsole\*" | grep -F "konsole\*"
konsole\*

The regex still needs to be escaped, regardless of -F.
Otherwise bash will expand it.

clang/utils/creduce-clang-crash.py

 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
+
+Output files:
+  *.reduced.sh -- crash reproducer with minimal arguments
+  *.reduced.cpp -- the reduced file
+  *.test.sh -- interestingness test for C-Reduce
 """
 
-from argparse import ArgumentParser
+from __future__ import print_function
+from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
 import stat
 import sys
 import subprocess
 import pipes
 import shlex
 import tempfile
 import shutil
 from distutils.spawn import find_executable
 
 verbose = False
-llvm_bin = None
 creduce_cmd = None
+clang_cmd = None
 not_cmd = None
 
+def verbose_print(*args, **kwargs):
+  if verbose:
+    print(*args, **kwargs)
+
 def check_file(fname):
   if not os.path.isfile(fname):
     sys.exit("ERROR: %s does not exist" % (fname))
   return fname
 
 def check_cmd(cmd_name, cmd_dir, cmd_path=None):
   """
   Returns absolute path to cmd_path if it is given,
   or absolute path to cmd_dir/cmd_name.
   """
   if cmd_path:
     cmd = find_executable(cmd_path)
     if cmd:
       return cmd
-    sys.exit("ERROR: executable %s not found" % (cmd_path))
+    sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
   cmd = find_executable(cmd_name, path=cmd_dir)
   if cmd:
     return cmd
-  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
+
+  if not cmd_dir:
+    cmd_dir = "$PATH"
+  sys.exit("ERROR: `%s` not found in %s" % (cmd_name, cmd_dir))
 
 def quote_cmd(cmd):
-  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
-                  for arg in cmd)
+  return ' '.join(pipes.quote(arg) for arg in cmd)
+
+def write_to_script(text, filename):
+  with open(filename, 'w') as f:
+    f.write(text)
+  os.chmod(filename, os.stat(filename).st_mode | stat.S_IEXEC)
+
+class Reduce:
+  def __init__(self, crash_script, file_to_reduce):
+    crash_script_name, crash_script_ext = os.path.splitext(crash_script)
+    file_reduce_name, file_reduce_ext = os.path.splitext(file_to_reduce)
+
+    self.testfile = file_reduce_name + '.test.sh'
+    self.crash_script = crash_script_name + '.reduced' + crash_script_ext
+    self.file_to_reduce = file_reduce_name + '.reduced' + file_reduce_ext
+    shutil.copy(file_to_reduce, self.file_to_reduce)
+
+    self.clang = clang_cmd
+    self.clang_args = []
+    self.expected_output = []
+    self.is_crash = True
+    self.creduce_flags = ["--tidy"]
+
+    self.read_clang_args(crash_script, file_to_reduce)
+    self.read_expected_output()
+
+  def get_crash_cmd(self, cmd=None, args=None, filename=None):
+    if not cmd:
+      cmd = self.clang
+    if not args:
+      args = self.clang_args
+    if not filename:
+      filename = self.file_to_reduce
+
+    return [cmd] + args + [filename]
 
-def get_crash_cmd(crash_script):
+  def read_clang_args(self, crash_script, filename):
+    print("\nReading arguments from crash script...")
     with open(crash_script) as f:
       # Assume clang call is on the last line of the script
       line = f.readlines()[-1]
       cmd = shlex.split(line)
 
-    # Overwrite the script's clang with the user's clang path
-    new_clang = check_cmd('clang', llvm_bin)
-    cmd[0] = pipes.quote(new_clang)
-    return cmd
-
-def has_expected_output(crash_cmd, expected_output):
-  p = subprocess.Popen(crash_cmd,
+    # Remove clang and filename from the command
+    # Assume the last occurrence of the filename is the clang input file
+    del cmd[0]
+    for i in range(len(cmd)-1, -1, -1):
+      if cmd[i] == filename:
+        del cmd[i]
+        break
+    self.clang_args = cmd
+    verbose_print("Clang arguments:", quote_cmd(self.clang_args))
+
+  def read_expected_output(self):
+    print("\nGetting expected crash output...")
+    p = subprocess.Popen(self.get_crash_cmd(),
                          stdout=subprocess.PIPE,
                          stderr=subprocess.STDOUT)
     crash_output, _ = p.communicate()
-  return all(msg in crash_output for msg in expected_output)
+    result = []
+
+    # Remove color codes
+    ansi_escape = r'\x1b\[[0-?]*m'
+    crash_output = re.sub(ansi_escape, '', crash_output)
+
+    # Look for specific error messages
+    regexes = [r"Assertion `(.+)' failed",
+               r"fatal error: backend error: (.+)",
+               r"LLVM ERROR: (.+)"]
+    for msg_re in regexes:
+      match = re.search(msg_re, crash_output)
+      if match:
+        msg = match.group(1)
+        result = [msg]
+        print("Found message:", msg)
+
+        if "fatal error:" in msg_re:
+          self.is_crash = False
+        break
+
+    # If no message was found, use the top five stack trace functions,
+    # ignoring some common functions
+    if not result:
+      stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
+      filters = ["PrintStackTraceSignalHandler",
+                 "llvm::sys::RunSignalHandlers",
+                 "SignalHandler", "__restore_rt", "gsignal", "abort"]
+      matches = re.findall(stacktrace_re, crash_output)
+      result = filter(lambda x: x and x.strip() not in filters, matches)[:5]
+      for msg in result:
+        print("Found stack trace function:", msg)
+
+    if not result:
+      print("ERROR: no crash was found")
+      print("The crash output was:\n========\n%s========" % crash_output)
+      sys.exit(1)
+
+    self.expected_output = result
+
+  def check_expected_output(self, args=None, filename=None):
+    if not args:
+      args = self.clang_args
+    if not filename:
+      filename = self.file_to_reduce
 
-def get_expected_output(crash_cmd):
-  p = subprocess.Popen(crash_cmd,
+    p = subprocess.Popen(self.get_crash_cmd(args=args, filename=filename),
                          stdout=subprocess.PIPE,
                          stderr=subprocess.STDOUT)
     crash_output, _ = p.communicate()
+    return all(msg in crash_output for msg in self.expected_output)
 
-  # If there is an assertion failure, use that;
-  # otherwise use the last five stack trace functions
-  assertion_re = r'Assertion `([^\']+)\' failed'
-  assertion_match = re.search(assertion_re, crash_output)
-  if assertion_match:
-    return [assertion_match.group(1)]
-  else:
-    stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
-    matches = re.findall(stacktrace_re, crash_output)
-    return matches[-5:]
+  def write_interestingness_test(self):
+    print("\nCreating the interestingness test...")
+
+    crash_flag = "--crash" if self.is_crash else ""
 
-def write_interestingness_test(testfile, crash_cmd, expected_output,
-                               file_to_reduce):
-  filename = os.path.basename(file_to_reduce)
-  if filename not in crash_cmd:
-    sys.exit("ERROR: expected %s to be in the crash command" % filename)
-
-  # Replace all instances of file_to_reduce with a command line variable
-  output = ['#!/bin/bash',
-            'if [ -z "$1" ] ; then',
-            '  f=%s' % (pipes.quote(filename)),
-            'else',
-            '  f="$1"',
-            'fi']
-  cmd = ['$f' if s == filename else s for s in crash_cmd]
-
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
-                                                      quote_cmd(cmd)))
-
-  for msg in expected_output:
-    output.append('grep %s t.log || exit 1' % pipes.quote(msg))
-
-  with open(testfile, 'w') as f:
-    f.write('\n'.join(output))
-  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+    output = "#!/bin/bash\n%s %s %s >& t.log || exit 1\n" % \
+        (pipes.quote(not_cmd), crash_flag, quote_cmd(self.get_crash_cmd()))
 
-def check_interestingness(testfile, file_to_reduce):
-  testfile = os.path.abspath(testfile)
+    for msg in self.expected_output:
+      output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+
+    write_to_script(output, self.testfile)
+    self.check_interestingness()
+
+  def check_interestingness(self):
+    testfile = os.path.abspath(self.testfile)
 
     # Check that the test considers the original file interesting
     with open(os.devnull, 'w') as devnull:
       returncode = subprocess.call(testfile, stdout=devnull)
     if returncode:
       sys.exit("The interestingness test does not pass for the original file.")
 
     # Check that an empty file is not interesting
+    # Instead of modifying the filename in the test file, just run the command
     _, empty_file = tempfile.mkstemp()
-  with open(os.devnull, 'w') as devnull:
-    returncode = subprocess.call([testfile, empty_file], stdout=devnull)
-  os.remove(empty_file)
-  if not returncode:
+    if self.check_expected_output(filename=empty_file):
       sys.exit("The interestingness test passes for an empty file.")
 
-def clang_preprocess(file_to_reduce, crash_cmd, expected_output):
+  def clang_preprocess(self):
+    print("\nTrying to preprocess the source file...")
     _, tmpfile = tempfile.mkstemp()
-  shutil.copy(file_to_reduce, tmpfile)
 
-  cmd = crash_cmd + ['-E', '-P']
-  p = subprocess.Popen(cmd,
+    cmd = self.get_crash_cmd() + ['-E', '-P']
+    p = subprocess.Popen(self.get_crash_cmd() + ['-E', '-P'],
                          stdout=subprocess.PIPE,
                          stderr=subprocess.STDOUT)
     preprocessed, _ = p.communicate()
 
-  with open(file_to_reduce, 'w') as f:
+    with open(tmpfile, 'w') as f:
       f.write(preprocessed)
 
-  if has_expected_output(crash_cmd, expected_output):
-    if verbose:
-      print("Successfuly preprocessed with %s" % (quote_cmd(cmd)))
-    os.remove(tmpfile)
+    if self.check_expected_output(filename=tmpfile):
+      print("Successfully preprocessed")
+      shutil.move(tmpfile, self.file_to_reduce)
     else:
-    if verbose:
-      print("Failed to preprocess with %s" % (quote_cmd(cmd)))
-    shutil.move(tmpfile, file_to_reduce)
-
+      print("Failed -- file no longer crashes after preprocessing")
+      os.remove(tmpfile)
 
-def filter_args(args, opts_startswith=[]):
-  result = [arg for arg in args if all(not arg.startswith(a) for a in
-                                       opts_startswith)]
+  @staticmethod
+  def group_args_by_dash(args):
+    # Heuristic to group arguments for removal, based on whether they
+    # begin with "-"
+    def append_flags(x, y):
+      if len(x) > 0 and x[-1].startswith('-') and not y.startswith('-'):
+        x[-1] += ' ' + y
+        return x
+      return x + [y]
+
+    return reduce(append_flags, args, [])
+
+  @staticmethod
+  def ungroup_args(args):
+    return [arg for group in args for arg in shlex.split(group)]
+
+  @staticmethod
+  def filter_args(args, opts_startswith=[], opts_one_arg_startswith=[]):
+    result = []
+    skip_next = False
+    for arg in args:
+      if skip_next:
+        skip_next = False
+        continue
+      if any(arg.startswith(a) for a in opts_startswith):
+        continue
+      if any(arg.startswith(a) for a in opts_one_arg_startswith):
+        skip_next = True
+        continue
+      result.append(arg)
     return result
 
-def try_remove_args(cmd, expected_output, msg=None, extra_arg=None, **kwargs):
-  new_cmd = filter_args(cmd, **kwargs)
-  if extra_arg and extra_arg not in new_cmd:
-    new_cmd.append(extra_arg)
-  if new_cmd != cmd and has_expected_output(new_cmd, expected_output):
-    if msg and verbose:
-      print(msg)
-    return new_cmd
-  return cmd
-
-def simplify_crash_cmd(crash_cmd, expected_output):
-  new_cmd = try_remove_args(crash_cmd, expected_output,
+  def try_remove_args(self, args, msg=None, extra_arg=None, **kwargs):
+    new_args = self.filter_args(args, **kwargs)
+    if extra_arg and extra_arg not in new_args:
+      new_args.append(extra_arg)
+
+    if (new_args != args and
+        self.check_expected_output(args=self.ungroup_args(new_args))):
+      if msg:
+        verbose_print(msg)
+      return new_args
+    return args
+
+  def try_remove_arg_by_index(self, args, index):
+    new_args = args[:index] + args[index+1:]
+    if self.check_expected_output(args=self.ungroup_args(new_args)):
+      verbose_print("Removed %s" % args[index])
+      return new_args, index
+    return args, index+1
+
+  def simplify_clang_args(self):
+    print("\nSimplifying the clang command...")
+
+    # Remove some clang arguments to speed up the interestingness test
+    new_args = self.clang_args
+    new_args = self.try_remove_args(new_args,
                                     msg="Removed debug info options",
                                     opts_startswith=["-gcodeview",
                                                      "-dwarf-column-info",
                                                      "-debug-info-kind=",
                                                      "-debugger-tuning=",
                                                      "-gdwarf"])
-  new_cmd = try_remove_args(new_cmd, expected_output,
+    new_args = self.try_remove_args(new_args,
                                     msg="Replaced -W options with -w",
                                     extra_arg='-w',
                                     opts_startswith=["-W"])
-  new_cmd = try_remove_args(new_cmd, expected_output,
+    new_args = self.try_remove_args(new_args,
                                     msg="Replaced optimization level with -O0",
                                     extra_arg="-O0",
                                     opts_startswith=["-O"])
-  return new_cmd
+    self.clang_args = new_args
+    verbose_print("Simplified command:", quote_cmd(self.get_crash_cmd()))
+
+  def reduce_clang_args(self):
+    print("\nReducing the clang crash command...")
+    new_args = self.group_args_by_dash(self.clang_args)
+
+    new_args = self.try_remove_args(new_args, msg="Removed -D options",
+                                    opts_startswith=["-D "])
+    new_args = self.try_remove_args(new_args, msg="Removed -I options",
+                                    opts_startswith=["-I "])
+
+    i = 0
+    while i < len(new_args):
+      new_args, i = self.try_remove_arg_by_index(new_args, i)
+    new_args = self.ungroup_args(new_args)
+
+    # Remove other cases that aren't covered by the heuristic
+    new_args = self.try_remove_args(new_args, msg="Removed -mllvm",
+                                    opts_one_arg_startswith=["-mllvm"])
+
+    self.clang_args = new_args
+
+    reduced_cmd = quote_cmd(self.get_crash_cmd())
+    write_to_script(reduced_cmd, self.crash_script)
+    print("Reduced command:", reduced_cmd)
+
+  def run_creduce(self):
+    print("\nRunning C-Reduce...")
+    try:
+      p = subprocess.Popen([creduce_cmd] + self.creduce_flags +
+                           [self.testfile, self.file_to_reduce])
+      p.communicate()
+    except KeyboardInterrupt:
+      # Hack to kill C-Reduce because it jumps into its own pgid
+      print('\n\nctrl-c detected, killed creduce')
+      p.kill()
 
 def main():
   global verbose
-  global llvm_bin
   global creduce_cmd
+  global clang_cmd
   global not_cmd
 
-  parser = ArgumentParser(description=__doc__)
+  parser = ArgumentParser(description=__doc__,
+                          formatter_class=RawTextHelpFormatter)
   parser.add_argument('crash_script', type=str, nargs=1,
                       help="Name of the script that generates the crash.")
   parser.add_argument('file_to_reduce', type=str, nargs=1,
                       help="Name of the file to be reduced.")
   parser.add_argument('--llvm-bin', dest='llvm_bin', type=str,
-                      required=True, help="Path to the LLVM bin directory.")
+                      help="Path to the LLVM bin directory.")
   parser.add_argument('--llvm-not', dest='llvm_not', type=str,
                       help="The path to the `not` executable. "
                       "By default uses the llvm-bin directory.")
+  parser.add_argument('--clang', dest='clang', type=str,
+                      help="The path to the `clang` executable. "
+                      "By default uses the llvm-bin directory.")
   parser.add_argument('--creduce', dest='creduce', type=str,
                       help="The path to the `creduce` executable. "
                       "Required if `creduce` is not in PATH environment.")
   parser.add_argument('-v', '--verbose', action='store_true')
   args = parser.parse_args()
 
   verbose = args.verbose
-  llvm_bin = os.path.abspath(args.llvm_bin)
+  llvm_bin = os.path.abspath(args.llvm_bin) if args.llvm_bin else None
   creduce_cmd = check_cmd('creduce', None, args.creduce)
+  clang_cmd = check_cmd('clang', llvm_bin, args.clang)
   not_cmd = check_cmd('not', llvm_bin, args.llvm_not)
+
   crash_script = check_file(args.crash_script[0])
   file_to_reduce = check_file(args.file_to_reduce[0])
 
-  print("\nParsing the crash script and getting expected output...")
-  crash_cmd = get_crash_cmd(crash_script)
-
-  expected_output = get_expected_output(crash_cmd)
-  if len(expected_output) < 1:
-    sys.exit("ERROR: no crash was found")
-
-  print("\nSimplifying the crash command...")
-  crash_cmd = simplify_crash_cmd(crash_cmd, expected_output)
-
-  print("\nWriting interestingness test to file...")
-  testfile = os.path.splitext(file_to_reduce)[0] + '.test.sh'
-  write_interestingness_test(testfile, crash_cmd, expected_output,
-                             file_to_reduce)
-  check_interestingness(testfile, file_to_reduce)
-
-  print("\nPreprocessing the file to reduce...")
-  clang_preprocess(file_to_reduce, crash_cmd, expected_output)
-
-  print("\nRunning C-Reduce...")
-  try:
-    p = subprocess.Popen([creduce_cmd, testfile, file_to_reduce])
-    p.communicate()
-  except KeyboardInterrupt:
-    # Hack to kill C-Reduce because it jumps into its own pgid
-    print('\n\nctrl-c detected, killed creduce')
-    p.kill()
-
-  # FIXME: reduce the clang crash command
+  r = Reduce(crash_script, file_to_reduce)
+  r.simplify_clang_args()
+  r.write_interestingness_test()
+  r.clang_preprocess()
+  r.run_creduce()
+  r.reduce_clang_args()
 
 if __name__ == '__main__':
   main()