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

[Kuperstein, Michael M]

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.