[ThinLTO] Escape module paths when printing

[chrisjackson]

Summary

We have located a bug in AssemblyWriter::printModuleSummaryIndex(). This
function outputs path strings incorrectly. Backslashes in the string are not
correctly escaped. Hence, only windows environments are affected.

Consequently, if a path name contains a backslash followed by two hexadecimal
characters, the sequence is incorrectly interpreted when the output is read by
another component. This mangles the path and results in error.

This patch fixes this issue by calling printEscapedString() to output the paths.

[pcc]

Would it be possible to make the test case simpler? I'm thinking that you could start with an input file with an embedded path containing a backslash, pipe it through llvm-as | llvm-dis and check that the escaping is preserved. That would mean that it could run on non-Windows as well.

[dexonsmith]

test/Assembler/asm-path-writer.ll

1 ↗

(On Diff #154637)

It's unfortunate that this requires Windows. Is it possible to check in textual IR file where a path: has (properly escaped) backslashes in it, and round-trip it back to assembly?

[tejohnson]

test/Assembler/asm-path-writer.ll

1 ↗

(On Diff #154637)

Right, this is possible since the asm parsing support is also in now. All you should need is a module path and at least one corresponding GV entry. If you use the guid form instead of a name for the gv, it will not require any corresponding IR. I.e. it can just be something like:

^0 = module: (path: "someescapedpath", hash: (0, 0, 0, 0, 0))
^1 = gv: (guid: 17407585008595848568, summaries: (function: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1)))

(but using an escaped path reproducing the issue)

Then just round trip through llvm-as and llvm-dis and check the output as Peter and Duncan suggested.

[chrisjackson]

Followed suggestions to make the test simpler.

[tejohnson]

LGTM

[dexonsmith]

LGTM too.

llvm/trunk/lib/IR/AsmWriter.cpp

moduleVec[Machine.getModulePathSlot(ModPath.first())] = std::make_pair(
         // during the thin link.
         ModPath.second.first == -1u ? RegularLTOModuleName
                                     : (std::string)ModPath.first(),
         ModPath.second.second);
 
   unsigned i = 0;
   for (auto &ModPair : moduleVec) {
     Out << "^" << i++ << " = module: (";
-    Out << "path: \"" << ModPair.first << "\"";
-    Out << ", hash: (";
+    Out << "path: \"";
+    printEscapedString(ModPair.first, Out);
+    Out << "\", hash: (";
     FieldSeparator FS;
     for (auto Hash : ModPair.second)
       Out << FS << Hash;
     Out << "))\n";
   }
 
   // FIXME: Change AliasSummary to hold a ValueInfo instead of summary pointer
   // for aliasee (then update BitcodeWriter.cpp and remove get/setAliaseeGUID).

llvm/trunk/test/Assembler/asm-path-writer.ll

+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+; CHECK: ^0 = module: (path: ".\5Cf4folder\5Cabc.o", hash: (0, 0, 0, 0, 0))
+
+^0 = module: (path: ".\5Cf4folder\5Cabc.o", hash: (0, 0, 0, 0, 0))
+^1 = gv: (guid: 15822663052811949562, summaries: (function: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 2)))