Fix quoting of #pragma comment for PS4

[ygao]

Summary

Hi,
This is the PS4 counterpart to r229376, which quotes the library name if the name contains space.
Can someone take a look whether this looks good to go in?

• Gao

[rsmith]

lib/CodeGen/TargetInfo.cpp

1653–1655 ↗

(On Diff #29941)

What happens if the library name contains an embedded \ character?

[ygao]

lib/CodeGen/TargetInfo.cpp

1653–1655 ↗

(On Diff #29941)

Are you thinking of the evil case where the library name ends with a backslash and then when we expand it, it might escape the separating whitespace? When I test it, the embedded backslash character always gets converted into an escape sequence "\5C" and hence looks harmless.

[rsmith]

lib/CodeGen/TargetInfo.cpp

1653–1655 ↗

(On Diff #29941)

Curious. What is doing that escaping, and why is it not also escaping spaces?

[ygao]

Would this be more clear as:
return (Lib.find(" ") != StringRef::npos) ? "\"" + Lib.str() + "\"" : Lib;
~Aaron

Seems like a good idea. I have a slight preference of if statement over the ternary operator if that is okay with you.

[ygao]

I don't have a strong feeling about it, but it seems like this wouldn't require a static helper function given how short it is (either way).
~Aaron

Sure. I'll inline that.

lib/CodeGen/TargetInfo.cpp

1653–1655 ↗

(On Diff #29953)

I did some research in this, and found that on Windows, the following characters are not acceptable as part of a file name or a folder name:

List of character:
\ / : * ? " < > |
End of list

Since the PS4 development is expected to be done in a Windows host environment, the backslash problem does not really impact us.

On a cross-development environment, e.g., a Linux host environment, special characters may appear in file names, and that will pose problems for quoting. For example, if a library contains both a space and a double-quote, and one cross-compiles for a Windows target, and clang tries to quote the library name. I am not sure what would be a good way to fix that. Just don't do that?

In a little experiment, I created on Linux a file with the name being a single double-quote and then I try to access the file on Windows, the file name on Windows did not display as a single double-quote but a tilda followed by 1.

Back to your question of where the escaping takes place in clang. I did not find anything in clang that does escaping, and I am coming to believe that no escaping was done, contrary to my prior statement. In the earlier testing I was doing a "clang -cc1 -triple x86_64-pc-win32 -emit-llvm", and the PrintEscapedString() in AsmWriter.cpp is printing the metadata in escaped format, but that is different than saying that the string has been converted to an escaped format. Very sorry for making the misleading statement.

[mkuper]

LGTM

cfe/trunk/lib/CodeGen/TargetInfo.cpp

 class PS4TargetCodeGenInfo : public X86_64TargetCodeGenInfo {
 public:
   PS4TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, X86AVXABILevel AVXLevel)
     : X86_64TargetCodeGenInfo(CGT, AVXLevel) {}
 
   void getDependentLibraryOption(llvm::StringRef Lib,
                                  llvm::SmallString<24> &Opt) const override {
     Opt = "\01";
+    // If the argument contains a space, enclose it in quotes.
+    if (Lib.find(" ") != StringRef::npos)
+      Opt += "\"" + Lib.str() + "\"";
+    else
       Opt += Lib;
   }
 };
 
 static std::string qualifyWindowsLibrary(llvm::StringRef Lib) {
   // If the argument does not end in .lib, automatically add the suffix.
   // If the argument contains a space, enclose it in quotes.
   // This matches the behavior of MSVC.
   bool Quote = (Lib.find(" ") != StringRef::npos);

cfe/trunk/test/CodeGen/pragma-comment.c

 // LINUX: !{!"-lmsvcrt.lib"}
 // LINUX: !{!"-lkernel32"}
 // LINUX: !{!"-lUSER32.LIB"}
 // LINUX: !{!" /bar=2"}
 
 // PS4: !{!"\01msvcrt.lib"}
 // PS4: !{!"\01kernel32"}
 // PS4: !{!"\01USER32.LIB"}
+// PS4: !{!"\01\22with space\22"}