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