CodeGen: Don't try to canonicalize Unix-style paths in CodeView debug info.

[pcc]

Summary

Most importantly, we should not replace slashes with backslashes
because that would invalidate the path.

[zturner]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

117–125 ↗

(On Diff #141784)

Shouldn't we instead just use something like LLVM_ON_WIN32 etc?

[pcc]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

117–125 ↗

(On Diff #141784)

I don't think so. Imagine that you have something like a ThinLTO distributed build where bitcode files are copied to a remote machine where code generation happens. In that case the code generator won't necessarily be running the same operating system as the bitcode producer was, and the consumer of the object file will be expecting a path style consistent with the bitcode producer.

[rnk]

Can you elaborate on the use case? What tool do you expect will consume these Unix style paths? What aspect of the path is invalidated by changing the slash style?

I think the right way to address these distributed cross-compilation use cases is probably going to be to have a flag like "-fdebug-compilation-dir" that the build system supplies with a Windows-style drive letter path.

[pcc]

In D45473#1063321, @rnk wrote:

Can you elaborate on the use case? What tool do you expect will consume these Unix style paths?

We can use the paths to provide better linker error messages. See D45467.

What aspect of the path is invalidated by changing the slash style?

It means for example that users can't copy the path out of the linker error message and paste it to their editor.

I think the right way to address these distributed cross-compilation use cases is probably going to be to have a flag like "-fdebug-compilation-dir" that the build system supplies with a Windows-style drive letter path.

Yes this is one example of a case where a "foreign" path ends up being embedded in the directory component of a debug info path and the code generator will need to respect that and not the path convention of the host machine. A similar situation is where the frontend is run on the local machine and the backend runs on the remote machine -- this is the distributed thinlto situation. In that case the frontend will "naturally" embed host-style paths in the directory component of debug info paths and the backend will again need to respect them.

[rnk]

In D45473#1063531, @pcc wrote:

In D45473#1063321, @rnk wrote:

Can you elaborate on the use case? What tool do you expect will consume these Unix style paths?

We can use the paths to provide better linker error messages. See D45467.

For this use case, is it actually important to preserve symlinks? Would it be enough to make the linker canonicalize the slashes to native format when printing the diagnostic?

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

117–125 ↗

(On Diff #141784)

Yes, this definitely shouldn't be conditional on macros, it's deep in LLVM codegen. llc should do the same thing with IR everywhere.

[pcc]

In D45473#1063737, @rnk wrote:

In D45473#1063531, @pcc wrote:

In D45473#1063321, @rnk wrote:

Can you elaborate on the use case? What tool do you expect will consume these Unix style paths?

We can use the paths to provide better linker error messages. See D45467.

For this use case, is it actually important to preserve symlinks? Would it be enough to make the linker canonicalize the slashes to native format when printing the diagnostic?

Assuming that we don't care about symlinks, that would work I suppose, but it wouldn't necessarily do the right thing in distributed links, and it would mean that any other tools that consume CodeView on non-Windows (e.g. LLDB remote debugging a Windows binary) would need to do the same thing.

[rnk]

OK, here are the two competing use cases that I am imagining:

1. (yours) Full cross-compilation from Unix, with linker diagnostics referring to files that exist locally on Unix.
2. Distributed cross-compilation on Unix, local linking on Windows with the VC linker.

I think the correct solution to 2 is going to be to use -fdebug-compilation-dir and -fdebug-prefix-map=/unix/path=C:/windows/path so that these DIFile directory components start with a drive letter that makes sense on the system doing the link.

Basically, if we have a DIFile directory that starts with /, let's trust that the producer wants Unix paths in their CodeView.

[pcc]

In D45473#1064430, @rnk wrote:

OK, here are the two competing use cases that I am imagining:

1. (yours) Full cross-compilation from Unix, with linker diagnostics referring to files that exist locally on Unix.
2. Distributed cross-compilation on Unix, local linking on Windows with the VC linker.

I think the correct solution to 2 is going to be to use -fdebug-compilation-dir and -fdebug-prefix-map=/unix/path=C:/windows/path so that these DIFile directory components start with a drive letter that makes sense on the system doing the link.

Basically, if we have a DIFile directory that starts with /, let's trust that the producer wants Unix paths in their CodeView.

My thoughts exactly.

llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 
 StringRef CodeViewDebug::getFullFilepath(const DIFile *File) {
   std::string &Filepath = FileToFilepathMap[File];
   if (!Filepath.empty())
     return Filepath;
 
   StringRef Dir = File->getDirectory(), Filename = File->getFilename();
 
+  // If this is a Unix-style path, just use it as is. Don't try to canonicalize
+  // it textually because one of the path components could be a symlink.
+  if (!Dir.empty() && Dir[0] == '/') {
+    Filepath = Dir;
+    if (Dir.back() != '/')
+      Filepath += '/';
+    Filepath += Filename;
+    return Filepath;
+  }
+
   // Clang emits directory and relative filename info into the IR, but CodeView
   // operates on full paths.  We could change Clang to emit full paths too, but
   // that would increase the IR size and probably not needed for other users.
   // For now, just concatenate and canonicalize the path here.
   if (Filename.find(':') == 1)
     Filepath = Filename;
   else
     Filepath = (Dir + "\\" + Filename).str();

llvm/trunk/test/DebugInfo/COFF/enum.ll

 ; CHECK-NEXT:    ]
 ; CHECK-NEXT:    UnderlyingType: int (0x74)
 ; CHECK-NEXT:    FieldListType: <field list> (0x1000)
 ; CHECK-NEXT:    Name: E
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  StringId (0x1002) {
 ; CHECK-NEXT:    TypeLeafKind: LF_STRING_ID (0x1605)
 ; CHECK-NEXT:    Id: 0x0
-; CHECK-NEXT:    StringData: \<stdin>
+; CHECK-NEXT:    StringData: /foo/bar.cpp
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  UdtSourceLine (0x1003) {
 ; CHECK-NEXT:    TypeLeafKind: LF_UDT_SRC_LINE (0x1606)
 ; CHECK-NEXT:    UDT: E (0x1001)
-; CHECK-NEXT:    SourceFile: \<stdin> (0x1002)
+; CHECK-NEXT:    SourceFile: /foo/bar.cpp (0x1002)
 ; CHECK-NEXT:    LineNumber: 1
 ; CHECK_NEXT  }
 
 source_filename = "test/DebugInfo/COFF/enum.ll"
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
 target triple = "i686-pc-windows-msvc18.0.0"
 
 @"\01?e@@3W4E@@A" = global i32 0, align 4, !dbg !0
 
 !llvm.dbg.cu = !{!2}
 !llvm.module.flags = !{!11, !12}
 !llvm.ident = !{!13}
 
 !0 = distinct !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
 !1 = !DIGlobalVariable(name: "e", linkageName: "\01?e@@3W4E@@A", scope: !2, file: !6, line: 2, type: !5, isLocal: false, isDefinition: true)
 !2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 3.9.0 (trunk 272790) (llvm/trunk 272813)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !10)
 !3 = !DIFile(filename: "-", directory: "/")
 !4 = !{!5}
 !5 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "E", file: !6, line: 1, baseType: !7, size: 32, align: 32, elements: !8)
-!6 = !DIFile(filename: "<stdin>", directory: "/")
+!6 = !DIFile(filename: "bar.cpp", directory: "/foo")
 !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
 !8 = !{!9}
 !9 = !DIEnumerator(name: "BLAH", value: 0)
 !10 = !{!0}
 !11 = !{i32 2, !"CodeView", i32 1}
 !12 = !{i32 2, !"Debug Info Version", i32 3}
 !13 = !{!"clang version 3.9.0 (trunk 272790) (llvm/trunk 272813)"}
 

llvm/trunk/test/DebugInfo/COFF/types-array-advanced.ll

 ; CHECK:     VShape: 0x0
 ; CHECK:     SizeOf: 4
 ; CHECK:     Name: incomplete_struct
 ; CHECK:     LinkageName: .?AUincomplete_struct@@
 ; CHECK:   }
 ; CHECK:   StringId (0x100D) {
 ; CHECK:     TypeLeafKind: LF_STRING_ID (0x1605)
 ; CHECK:     Id: 0x0
-; CHECK:     StringData: \t.cpp
+; CHECK:     StringData: /t.cpp
 ; CHECK:   }
 ; CHECK:   UdtSourceLine (0x100E) {
 ; CHECK:     TypeLeafKind: LF_UDT_SRC_LINE (0x1606)
 ; CHECK:     UDT: incomplete_struct (0x100C)
-; CHECK:     SourceFile: \t.cpp (0x100D)
+; CHECK:     SourceFile: /t.cpp (0x100D)
 ; CHECK:     LineNumber: 4
 ; CHECK:   }
 ; CHECK:   Modifier (0x100F) {
 ; CHECK:     TypeLeafKind: LF_MODIFIER (0x1001)
 ; CHECK:     ModifiedType: int (0x74)
 ; CHECK:     Modifiers [ (0x3)
 ; CHECK:       Const (0x1)
 ; CHECK:       Volatile (0x2)

llvm/trunk/test/DebugInfo/COFF/types-non-virtual-methods.ll

 ; CHECK:     DerivedFrom: 0x0
 ; CHECK:     VShape: 0x0
 ; CHECK:     SizeOf: 1
 ; CHECK:     Name: A
 ; CHECK:   }
 ; CHECK:   StringId (0x1008) {
 ; CHECK:     TypeLeafKind: LF_STRING_ID (0x1605)
 ; CHECK:     Id: 0x0
-; CHECK:     StringData: \t.cpp
+; CHECK:     StringData: /t.cpp
 ; CHECK:   }
 ; CHECK:   UdtSourceLine (0x1009) {
 ; CHECK:     TypeLeafKind: LF_UDT_SRC_LINE (0x1606)
 ; CHECK:     UDT: A (0x1007)
-; CHECK:     SourceFile: \t.cpp (0x1008)
+; CHECK:     SourceFile: /t.cpp (0x1008)
 ; CHECK:     LineNumber: 1
 ; CHECK:   }
 ; CHECK:   Class (0x100A) {
 ; CHECK:     TypeLeafKind: LF_CLASS (0x1504)
 ; CHECK:     MemberCount: 0
 ; CHECK:     Properties [ (0x80)
 ; CHECK:       ForwardReference (0x80)
 ; CHECK:     ]

 ; CHECK:     DerivedFrom: 0x0
 ; CHECK:     VShape: 0x0
 ; CHECK:     SizeOf: 1
 ; CHECK:     Name: B
 ; CHECK:   }
 ; CHECK:   UdtSourceLine (0x1014) {
 ; CHECK:     TypeLeafKind: LF_UDT_SRC_LINE (0x1606)
 ; CHECK:     UDT: B (0x1013)
-; CHECK:     SourceFile: \t.cpp (0x1008)
+; CHECK:     SourceFile: /t.cpp (0x1008)
 ; CHECK:     LineNumber: 11
 ; CHECK:   }
 ; CHECK: ]
 
 
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
 target triple = "i686-pc-windows-msvc"