Page MenuHomePhabricator

[analyzer] Support spaces in compiler path and arguments.
ClosedPublic

Authored by ayartsev on Apr 29 2015, 4:15 PM.

Details

Reviewers
jordan_rose
Summary

The patch fixes errors that occur if a path to default compiler has spaces or if an argument with spaces is given to compiler (e.g. via -I).

The precedent: running scan-build that uses clang compiled by VS ends up with
error: error reading 'Files\Microsoft'
error: error reading 'Visual'
error: error reading 'Studio'
error: error reading '11.0\VC\include'

Ok to commit?

Diff Detail

Event Timeline

ayartsev updated this revision to Diff 24664.Apr 29 2015, 4:15 PM
ayartsev retitled this revision from to [analyzer] Support spaces in compiler path and arguments..
ayartsev updated this object.
ayartsev edited the test plan for this revision. (Show Details)
ayartsev added a reviewer: jordan_rose.
ayartsev added subscribers: zaks.anna, krememek, Unknown Object (MLST).
jordan_rose edited edge metadata.Apr 30 2015, 9:29 AM

Thanks, good catch.

tools/scan-build/ccc-analyzer
182–188

I think we can just pass true to quotewords' 'keep' parameter.

Ok to commit the solution with passing true to quotewords?

tools/scan-build/ccc-analyzer
182–188

Yes, it works.

jordan_rose accepted this revision.Apr 30 2015, 6:24 PM
jordan_rose edited edge metadata.

Yes, thanks!

This revision is now accepted and ready to land.Apr 30 2015, 6:24 PM
ayartsev updated this revision to Diff 25105.May 6 2015, 4:54 PM
ayartsev edited edge metadata.

The patch, committed as r236423 caused at least two types of regressions:

  1. (not reproduced on Windows) :

clang: error: no such file or directory: '"-cc1"'
clang: error: no such file or directory: '"-triple"'
...

  1. Macro definitions with quotes were corrupted ( e.g. -DVAL_CONFIGURE="\"'CFLAGS=-fno-vectorize' 'CC=ccc-analyzer'\"" )

In the updated patch I restricted wrapping an argument with quotes to cases when an argument has spaces and arguments first symbol is not a quotation mark.
Ok to commit?

This just seems wrong. What if I have a relative path that starts with a directory name that begins with a quote character? Maybe I deserve what I get, but we should figure out why this is happening and fix it rather than just ignore arguments beginning with quotes.

ayartsev updated this revision to Diff 26356.May 22 2015, 3:35 PM

Another try: we preserve all quotes and backslashes as is by passing 'true' to quotewords() and only remove quotes from macro definitions and arguments without spaces.

What about macro definitions that appear as one argument? "-DFOO=abc"?

I think we need to take a step back here. Why are some arguments being quoted and others not? Is that something we can control in scan-build itself? Is it something the build system does wrong?

The $line that hold arguments is the raw output from Clang with -### option passed. An output is consistent: each item in the output is double-quoted, each macro definition is split into "-D" and "definition".
In scan-build we call 'quotewords' to extract items. If we call 'quotewords' with the false 'keep' parameter we "remove all quotes and backslashes that are not themselves backslash-escaped or inside of single quotes" (perldoc). Here the problem with paths that have spaces arises (at least under Windows).
If we pass true to 'quotewords' 'keep' parameter we preserve all quotes and backslashes and this worked for me under Windows, but the regressions were reported in comments to r236423 like the following:
clang: error: no such file or directory: '"-cc1"'
clang: error: no such file or directory: '"-analyze"'
...

and regressions caused by extra quotes around macro definitions.
That's how matters stand.
So I came to the following solution: pass true to 'quotewords' to preserve arguments unmodified and then strip quotes from every macro definition and from all arguments that have no spaces (I've looked through the gcc options manual, it looks like macro definitions and paths are the only arguments that can have spaces if I am not missing something).

It's really not a good idea to hardcode some list of commands that behave a certain way.

What you're telling me is that @items is correctly separated, but that when we actually spawn a subprocess we fail to preserve that. Alternately, perhaps @items is different on Windows vs. Linux, and then we should address that. I really think we shouldn't try to come up with something ad hoc.

Finally addressed issues with arguments. I had a patch for D8774 (Prevent ccc/c++-analyzer from hanging on Windows) applied locally (because I was unable to use scan-build without that patch) and that was the reason for the arguments being processed differently .
I replaced all calls to 'system' and 'exec' (that pass arguments to the system's command shell for processing) with calls to pipe form of 'open' and this solution worked consistently under Windows 7, OS X, Ubuntu and different Windows perl ports. Merged this patch with D8774. Please review!

ayartsev closed this revision.Aug 25 2015, 2:25 PM

Merged with D8774.