Page MenuHomePhabricator

add steps to preprocess file and reduce command line args

Authored by akhuang on Mar 15 2019, 4:17 PM.



-try to preprocess the file before reducing
-try to remove some command line arguments
-now requires a llvm bin directory since the generated crash script doesn't have an absolute path for clang

Diff Detail

rC Clang

Event Timeline

akhuang created this revision.Mar 15 2019, 4:17 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: cfe-commits. · View Herald Transcript
akhuang updated this revision to Diff 190929.Mar 15 2019, 5:14 PM
akhuang updated this revision to Diff 191128.Mar 18 2019, 11:02 AM

fix some typos

rnk added inline comments.Mar 18 2019, 11:57 AM
106–117 ↗(On Diff #191128)

Another way to do this without the complexity of swapping the files around is to construct the interestingness test to take the path to the file as a parameter ($1). CReduce always passes the path to the file being reduced as the first argument.

akhuang updated this revision to Diff 191192.Mar 18 2019, 3:34 PM

Modify interestingness test to take file as input

rnk added a comment.Mar 18 2019, 3:51 PM

Aside from the buglet, I'm happy with this, but I wanted to wait until @george.burgess.iv takes a look.

44 ↗(On Diff #191192)

This modifies cmd in place, so multiple calls to quote_cmd will get "more quoted" with more calls, right?

akhuang updated this revision to Diff 191199.Mar 18 2019, 4:00 PM
akhuang marked an inline comment as done.

fixed array copy mistake

akhuang updated this revision to Diff 191217.Mar 18 2019, 5:29 PM
akhuang marked 2 inline comments as done.

Fixed typo where it was writing the abspath of the file to the interestingness test.

akhuang marked an inline comment as done.Mar 18 2019, 5:39 PM
akhuang added inline comments.
106–117 ↗(On Diff #191128)

It seems like creduce isn't actually passing the filename as an argument (creduce had problems when I accidentally used the absolute path of the file as the default value when no arguments are found)

45 ↗(On Diff #191217)

Useless enumerate here. You could even use a list comprehension here

return ' '.join(arg if arg.startswith('$') else pipes.quote(arg) for arg in cmd)
161 ↗(On Diff #191217)

useless parenthesis around test

could be a list comprehension

result = [arg for arg in args if all(not arg.startswith(a) for a in opts_startswith)]
akhuang updated this revision to Diff 191409.Mar 19 2019, 3:34 PM

style things

Just a few style nits for you, and this LGTM. I assume rnk and serge-sans-paille are content, so I'm happy to check this in for you once these are addressed.


64 ↗(On Diff #191409)

nit: can be simplified to return all(msg not in crash_output for msg in expected_output)

116 ↗(On Diff #191409)

nit: looks like you can use returncode =, stdout=devnull) here

124 ↗(On Diff #191409)

same nit

243 ↗(On Diff #191409)

nit: please add a space: # FIXME

akhuang updated this revision to Diff 191598.Mar 20 2019, 4:02 PM
akhuang marked 4 inline comments as done.

style nits, fixed thing in getting path to clang

LGTM; thanks again!

Will land shortly

This revision is now accepted and ready to land.Mar 20 2019, 4:51 PM
This revision was automatically updated to reflect the committed changes.

@akhuang Thanks for getting this committed. Since it seems like a lot of this is taken from my script, could you please add me as a reviewer for the next patch so that I know which bits still need to be upstreamed?

@arichardson Will add you next time, sorry I didn't do so on this one!