[llvm] r224998 - [COFF] Don't try to add quotes to already quoted linker directives

[Reid Kleckner]

I think I was just trying to be careful about quoting, rather than trying
to make some particular real-world linker directive work. Disabling the
quoting completely would be OK, given that MSVC probably supports multiple
arguments in one pragma via space separation.

On Wed, Feb 11, 2015 at 1:19 AM, Kuperstein, Michael M <
michael.m.kuperstein at intel.com> wrote:

> On second thought (and after running into this again), why do we even try
> to quote the comment, instead of assuming it's correctly quoted?
>
> E.g. given this:
> #pragma comment(linker,"/manifestdependency:type='foo' name='bar'")
> int main() {return 0;}
>
> MSVC 2013 does not try to add quotes, but, rather, simply fails:
> fatal error LNK1276: invalid directive 'name' found; does not start with
> '/'
>
> The problem is that the string can be quoted in different ways. One option
> (which we accept as of r224998) is:
> #pragma comment(linker,"\"/manifestdependency:type='foo' name='bar'\"")
>
> But this is by no means the only way to quote the string. Any
> correctly-quoted string works. E.g. VC will also accept:
> #pragma comment(linker,"/manifestdependency:\"type='foo' name='bar'\"")
>
> Reid, what do you think?
> Do we have cases where un-escaped linker directives containing spaces may
> come from somewhere that's not #pragma comment?
> If no, can we remove this code and fail like VC does? If yes, can they
> perhaps be quoted where they're created?
>
> The alternative would be to completely parse the possibly-quoted string
> and then re-quote it, which seems like a bit of overkill to me.
>
> Michael
>
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:
> llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Michael Kuperstein
> Sent: Tuesday, December 30, 2014 21:24
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm] r224998 - [COFF] Don't try to add quotes to already quoted
> linker directives
>
> Author: mkuper
> Date: Tue Dec 30 13:23:48 2014
> New Revision: 224998
>
> URL: http://llvm.org/viewvc/llvm-project?rev=224998&view=rev
> Log:
> [COFF] Don't try to add quotes to already quoted linker directives
>
> If a linker directive is already quoted, don't try to quote it again,
> otherwise it creates a mess.
> This pops up in places like:
> #pragma comment(linker,"\"/foo bar'\"")
>
> Differential Revision: http://reviews.llvm.org/D6792
>
> Modified:
>     llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
>     llvm/trunk/test/MC/COFF/linker-options.ll
>
> Modified: llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp?rev=224998&r1=224997&r2=224998&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp Tue Dec 30
> 13:23:48 2014
> @@ -932,7 +932,7 @@ emitModuleFlags(MCStreamer &Streamer,
>        StringRef Op = MDOption->getString();
>        // Lead with a space for consistency with our dllexport
> implementation.
>        std::string Escaped(" ");
> -      if (Op.find(" ") != StringRef::npos) {
> +      if (!Op.startswith("\"") && (Op.find(" ") != StringRef::npos)) {
>          // The PE-COFF spec says args with spaces must be quoted.  It
> doesn't say
>          // how to escape quotes, but it probably uses this algorithm:
>          // http://msdn.microsoft.com/en-us/library/17w5ykft(v=vs.85).aspx
>
> Modified: llvm/trunk/test/MC/COFF/linker-options.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/linker-options.ll?rev=224998&r1=224997&r2=224998&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/MC/COFF/linker-options.ll (original)
> +++ llvm/trunk/test/MC/COFF/linker-options.ll Tue Dec 30 13:23:48 2014
> @@ -1,6 +1,6 @@
>  ; RUN: llc -O0 -mtriple=i386-pc-win32 -filetype=asm -o - %s | FileCheck %s
>
> -!0 = !{i32 6, !"Linker Options", !{!{!"/DEFAULTLIB:msvcrt.lib"},
> !{!"/DEFAULTLIB:msvcrt.lib", !"/DEFAULTLIB:secur32.lib"},
> !{!"/DEFAULTLIB:C:\5Cpath to\5Casan_rt.lib"}, !{!"/with spaces"}}}
> +!0 = !{i32 6, !"Linker Options", !{!{!"/DEFAULTLIB:msvcrt.lib"},
> !{!"/DEFAULTLIB:msvcrt.lib", !"/DEFAULTLIB:secur32.lib"},
> !{!"/DEFAULTLIB:C:\5Cpath to\5Casan_rt.lib"}, !{!"/with spaces"},
> !{!"\22/quoted spaces\22"}}}
>
>  !llvm.module.flags = !{ !0 }
>
> @@ -14,4 +14,5 @@ define dllexport void @foo() {
>  ; CHECK: .ascii   " /DEFAULTLIB:secur32.lib"
>  ; CHECK: .ascii   " \"/DEFAULTLIB:C:\\path to\\asan_rt.lib\""
>  ; CHECK: .ascii   " \"/with spaces\""
> +; CHECK: .ascii   " \"/quoted spaces\""
>  ; CHECK: .ascii   " /EXPORT:_foo"
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150211/d63182ba/attachment.html>