<div dir="ltr"><div dir="ltr"><div dir="ltr">On Thu, Jan 10, 2019 at 3:01 AM Martin Storsjö <<a href="mailto:martin@martin.st" target="_blank">martin@martin.st</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, 9 Jan 2019, David Greene wrote:<br>
<br>
> I've come across a curious and pernicious problem in llvm-rc.<br>
> absolute.test checks that llvm-rc can accept a filename that is an<br>
> absolute path.  And it works just fine.  Until you run it with a file<br>
> that starts with "/c."<br>
<br>
Hmm, that's rather unfortunate indeed.<br>
<br>
FWIW, this test doesn't test specifically whether llvm-rc can accept an <br>
absolute filename as command line argument - all the llvm-rc tests run <br>
llvm-rc with absolute filenames as arguments. This test checks whether <br>
llvm-rc can handle an absolute filename reference within a rc file.<br>
<br>
I presume you run into the same issue on all other tests in <br>
test/tools/llvm-rc as well?<br>
<br>
> These will fail:<br>
><br>
> llvm-rc /crawl/through/some/path/to/my.rc<br>
> llvm-rc /c/some/path/to/my.rc<br>
><br>
> The option parser ends up interpreting "/" as an option prefix and then<br>
> the parser matches it to this in tools/llvm-rc/Opts.td:<br>
><br>
> def CODEPAGE : JoinedOrSeparate<[ "/", "-" ], "C">,<br>
>               HelpText<"Set the codepage used for input strings.">;<br>
><br>
> The test then fails with:<br>
>  Exactly one input file should be provided.<br>
><br>
> The same problem happens with files that begin with "/r"<br>
> (/read/the/path/to/my.rc) "/sl" (/slink/along/the/path/to/my.rc) or any<br>
> other path that happens to begin with the same text as an option in<br>
> Opts.td.<br>
><br>
> This triggered on one of our builders that just happens to build to a<br>
> path that begins with "/c."  Presumably none of the existing Buildbots<br>
> build to paths that cause problems.<br>
><br>
> It's easy enough to construct a test for this, but I'm not sure how/if<br>
> llvm-rc should be fixed.  I don't know why it accepts both "/" and "-"<br>
> as option prefixes.  As this mostly seems related to Windows (resource<br>
> files), should tests be UNSUPPORTED on every other platform?  Or is<br>
> llvm-rc intended to be a cross-platform way to create resource files?<br>
<br>
It's definitely intended as a cross-platform tool for generating windows <br>
resource files, to allow for cross compilation etc.<br>
<br>
> If the latter, then it seems like options ought to use the "/" prefix on<br>
> Windows and "-" everywhere else so as not to conflict with path<br>
> specifiers.<br>
<br>
Well, build scripts that call llvm-rc might be using either (more or less <br>
agnostic of what platform it runs on). I personally prefer always using <br>
"-" everywhere though (which also is supported on windows, and also <br>
supported by the original microsoft tools, even if their help listings <br>
only display the form with a "/").<br>
<br>
FWIW, lld-link also implements the same form of options using both <br>
prefixes, but there's less risk of unintended matches as most option names <br>
are full words, not single-char abbreviations.<br>
<br>
One way of disambiguating between option and pathname for the sake of the <br>
tests, would be to add '--' before the path arguments, which seems to be <br>
handled by the LLVM options parser at least. Does that sound sensible to <br>
you (and others CC:d)?<br></blockquote><div><br></div><div>That sounds like the thing to do. It's also how we handle the same issue in clang-cl, where /Users/foo/file.c is interpreted as the /U (undefine macro) flag with"sers/foo/file.c" as argument instead of the intended macOS-style path.</div><div><br></div><div>clang-cl also has a dedicated warning for this (<a href="http://reviews.llvm.org/rL293305">http://reviews.llvm.org/rL293305</a>), might be useful for some of llvm-rc's flags that are prone to this problem as well (/c, /r, maybe /sl)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
// Martin<br>
<br>
</blockquote></div></div></div>