[CLANG] [DebugInfo] Convert File name to native format

[kamleshbhalui]

Summary

Clang emits duplicate file entry in object file on windows platform(when windows style path appended with posix style path or vice versa).
which becomes problem for some debugger(not able to put break point on the file which has duplicate entry).

By making sure it's native path before creating DIFile above problem goes away.

Testcase Demonstration of problem on llvm-dev:
https://lists.llvm.org/pipermail/llvm-dev/2021-March/149501.html

[dblaikie]

@rnk @akhuang - I think we touched on this before maybe in some other thread/patch/discussion?

Ah, @probinson mentioned in the linked thread:

Is this https://bugs.llvm.org/show_bug.cgi?id=44170 which had a tentative patch at https://reviews.llvm.org/D71508 ?
The original complaint wasn't for Windows, but the lack of filepath canonicalization seems like a common symptom.

Might be best, @kamleshbhalui, if you could check if the other (D71508) patch relates to the same issue & if so, continue the discussion over there instead of forking it here.

[amccarth]

The previous discussions (that I participated in) were centered around the redirecting virtual filesystem, which creates paths in hybrid style and there is no "correct" way to make those native without taking breaking changes and making it less useful for writing platform-agnostic tests. But it's not clear that's relevant here.

I'm also told that clang _generally_ tries to avoid canonicalization and instead treats file paths as mostly opaque strings that can be concatenated. Personally, I prefer file paths in the style that the native system prefers, so I'd like to see this go through, but I'm not sure how compatible this would be with other aspects of clang's file path handling.

Another possible issue is that llvm::sys::path and other functions try to map Windows filesystem concepts to Posix ones. There are many cases where there isn't a perfect mapping. For example: Windows has a current working directory _per drive_, and so it can be hard to resolve paths like D:foo.ext, which are relative to something other than the "current working directory." Details like this can come into play in the immediate vicinity of the code change, so I have some trepidation.

It looks like the code change is for everyone, but the new test is specific to mingw. Is that the right place for the new test?

[kamleshbhalui]

In D99580#2659858, @amccarth wrote:

Another possible issue is that llvm::sys::path and other functions try to map Windows filesystem concepts to Posix ones. There are many cases where there isn't a perfect mapping. For example: Windows has a current working directory _per drive_, and so it can be hard to resolve paths like D:foo.ext, which are relative to something other than the "current working directory." Details like this can come into play in the immediate vicinity of the code change, so I have some trepidation.

Agree with you but this problem exist with and without this patch.

It looks like the code change is for everyone, but the new test is specific to mingw.

For Linux like platform it does not create problem because input file path is already in POSIX style, so having a test case will not test the change because even without the change it will pass.
Even on windows problem occur only when input file path specified in POSIX style(i.e. C:/foo/bar/test.c), if specify the path windows style(i.e. C:\foo\bar\test.c) then the fix not required.

Is that the right place for the new test?

In same directory their are many mingw related test, so added this test their, still if everyone strongly feels it should be in separate directory I will do that.

[kamleshbhalui]

In D99580#2659352, @dblaikie wrote:

@rnk @akhuang - I think we touched on this before maybe in some other thread/patch/discussion?

Ah, @probinson mentioned in the linked thread:

Is this https://bugs.llvm.org/show_bug.cgi?id=44170 which had a tentative patch at https://reviews.llvm.org/D71508 ?
The original complaint wasn't for Windows, but the lack of filepath canonicalization seems like a common symptom.

Might be best, @kamleshbhalui, if you could check if the other (D71508) patch relates to the same issue & if so, continue the discussion over there instead of forking it here.

@dblaikie It's not exactly same problem because PR44170 pops up only when compiling with -gdwarf-5 (on both linux and windows) although IR looks similar as when compiled with -gdwarf-4.
But this bug(for which current patch D99580 created) only can be reproduced on windows and with all dwarf version.

[kamleshbhalui]

Marked failed tests as unsupported on windows system
Because now clang emits native path for DIFile.

[kamleshbhalui]

clang formatting the patch.

[kamleshbhalui]

updated patch

[amccarth]

In D99580#2660040, @kamleshbhalui wrote:

In D99580#2659858, @amccarth wrote:

It looks like the code change is for everyone, but the new test is specific to mingw.

For Linux like platform it does not create problem because input file path is already in POSIX style, so having a test case will not test the change because even without the change it will pass.

The fix is on the code path for all platforms, so running the test on all platforms could help catch future regressions. There could also be unusual cases, such as cross compiling for Linux on Windows, and thus you could have Windows-style paths even though the target is Posix. Or someone might further refine the fix to make more cases work correctly for mingw but inadvertently cause problems for other platforms.

[kamleshbhalui]

Making changes effective only for windows

[kamleshbhalui]

ping?

[kamleshbhalui]

Making changes effective only for windows

[amccarth]

Sorry, I think I've lost track of some context while I was on vacation. I don't understand why several of the tests are now unsupported on Windows. Some of those seem like important tests.

If canonicalizing the file names to the platform's native style creates new test failures, then I think we need to wonder whether canonicalization is the right solution (and whether it's done at the right point. If it is, then I think we need to find a way to make these tests work cross-platform.

clang/lib/CodeGen/CGDebugInfo.cpp

 
 llvm::DIFile *
 CGDebugInfo::createFile(StringRef FileName,
                         Optional<llvm::DIFile::ChecksumInfo<StringRef>> CSInfo,
                         Optional<StringRef> Source) {
   StringRef Dir;
   StringRef File;
   std::string RemappedFile = remapDIPath(FileName);
+#ifdef _WIN32
+  // Convert RemappedFile to native style when Windows
+  SmallString<128> NativeFileName = StringRef(RemappedFile);
+  llvm::sys::path::native(NativeFileName);
+  RemappedFile = (std::string)NativeFileName.str();
+#endif
   std::string CurDir = remapDIPath(getCurrentDirname());
   SmallString<128> DirBuf;
   SmallString<128> FileBuf;
   if (llvm::sys::path::is_absolute(RemappedFile)) {
     // Strip the common prefix (if it is more than just "/") from current
     // directory and FileName for a more space-efficient encoding.
     auto FileIt = llvm::sys::path::begin(RemappedFile);
     auto FileE = llvm::sys::path::end(RemappedFile);
void CGDebugInfo::CreateCompileUnit() {
   uint64_t DwoId = 0;
   auto &CGOpts = CGM.getCodeGenOpts();
   // The DIFile used by the CU is distinct from the main source
   // file. Its directory part specifies what becomes the
   // DW_AT_comp_dir (the compilation directory), even if the source
   // file was specified with an absolute path.
   if (CSKind)
     CSInfo.emplace(*CSKind, Checksum);
-  llvm::DIFile *CUFile = DBuilder.createFile(
-      remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
-      getSource(SM, SM.getMainFileID()));
+  MainFileName = remapDIPath(MainFileName);
+#ifdef _WIN32
+  // Convert MainFileName to native style when windows
+  SmallString<128> NativeMainFileName = (StringRef)MainFileName;
+  llvm::sys::path::native(NativeMainFileName);
+  MainFileName = (std::string)NativeMainFileName.str();
+#endif
+  llvm::DIFile *CUFile =
+      DBuilder.createFile(MainFileName, remapDIPath(getCurrentDirname()),
+                          CSInfo, getSource(SM, SM.getMainFileID()));
 
   StringRef Sysroot, SDK;
   if (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB) {
     Sysroot = CGM.getHeaderSearchOpts().Sysroot;
     auto B = llvm::sys::path::rbegin(Sysroot);
     auto E = llvm::sys::path::rend(Sysroot);
     auto It = std::find_if(B, E, [](auto SDK) { return SDK.endswith(".sdk"); });
     if (It != E)

clang/test/CodeGen/debug-info-preprocessed-file.i

+// UNSUPPORTED: system-windows
 # 1 "/foo/bar/preprocessed-input.c"
 # 1 "<built-in>" 1
 # 1 "<built-in>" 3
 # 318 "<built-in>" 3
 # 1 "<command line>" 1
 # 1 "<built-in>" 2
 # 1 "preprocessed-input.c" 2
 
 // RUN: %clang -g -c -S -emit-llvm -o - %s | FileCheck %s
 // CHECK: !DICompileUnit(language: DW_LANG_C99, file: ![[FILE:[0-9]+]] 
 // CHECK: ![[FILE]] = !DIFile(filename: "/foo/bar/preprocessed-input.c"

clang/test/CodeGen/debug-prefix-map.c

+// UNSUPPORTED: system-windows
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - | FileCheck %s -check-prefix CHECK-NO-MAIN-FILE-NAME
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH=empty %s -emit-llvm -o - | FileCheck %s -check-prefix CHECK-EVIL
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -main-file-name debug-prefix-map.c | FileCheck %s
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -fdebug-compilation-dir %p | FileCheck %s -check-prefix CHECK-COMPILATION-DIR
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -isysroot %p -debugger-tuning=lldb | FileCheck %s -check-prefix CHECK-SYSROOT
 // RUN: %clang -g -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang -g -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s
 

clang/test/CodeGenCXX/debug-info-mingw.cpp

+// UNSUPPORTED: !system-windows
+// RUN: rm -rf %t/UNIQUE_DIR && mkdir -p %t/UNIQUE_DIR
+// RUN: cp %s %t/UNIQUE_DIR/debug-info-mingw.cpp
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -debug-info-kind=limited \
+// RUN: -main-file-name debug-info-mingw.cpp %t/UNIQUE_DIR/debug-info-mingw.cpp \
+// RUN: -emit-llvm -o - | FileCheck %s
+int main() {
+}
+// CHECK: !DIFile(filename: "{{.+}}\\UNIQUE_DIR\\debug-info-mingw.cpp",

clang/test/CodeGenCXX/linetable-fnbegin.cpp

+// UNSUPPORTED: system-windows
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
 // Test that the line table info for Foo<T>::bar() is pointing to the
 // right header file.
 // CHECK: define{{.*}}bar
 // CHECK-NOT: define
 // CHECK: ret {{.*}}, !dbg [[DBG:.*]]
 // CHECK: [[HPP:.*]] = !DIFile(filename: "./template.hpp",
 // CHECK: [[SP:.*]] = distinct !DISubprogram(name: "bar",
 // CHECK-SAME:                               file: [[HPP]], line: 22
 // CHECK-SAME:                               DISPFlagDefinition
 // We shouldn't need a lexical block for this function.
 // CHECK: [[DBG]] = !DILocation(line: 23, column: 3, scope: [[SP]])
 
-
 # 1 "./template.h" 1
 template <typename T>
 class Foo {
 public:
   int bar();
 };
 # 21 "./template.hpp"
 template <typename T>
 int Foo<T>::bar() {
   return 23;
 }
 int main (int argc, const char * argv[])
 {
   Foo<int> f;
   return f.bar();
 }

clang/test/Frontend/optimization-remark-line-directive.c

+// UNSUPPORTED: system-windows
 // This file tests -Rpass diagnostics together with #line
 // directives. We cannot map #line directives back to
 // a SourceLocation.
 
 // RUN: %clang_cc1 %s -Rpass=inline -debug-info-kind=line-tables-only -emit-llvm-only -verify -fno-experimental-new-pass-manager
 
 // The new PM inliner is not added to the default pipeline at O0, so we add
 // some optimizations to trigger it.
 // RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 -debug-info-kind=line-tables-only -emit-llvm-only -mllvm -mandatory-inlining-first=false -verify
 
 int foo(int x, int y) __attribute__((always_inline));
 int foo(int x, int y) { return x + y; }
 
 // expected-remark@+2 {{foo inlined into bar}} expected-note@+2 {{could not determine the original source location for /bad/path/to/original.c:1230:25}}
 #line 1230 "/bad/path/to/original.c"
 int bar(int j) { return foo(j, j - 2); }

clang/test/Frontend/stdin-input.c

+// UNSUPPORTED: system-windows
 // RUN: cat %s | %clang -emit-llvm -g -S \
 // RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
 // CHECK: ; ModuleID = 'test/foo.c'
 // CHECK: source_filename = "test/foo.c"
 // CHECK: !DIFile(filename: "test/foo.c"
 
 int main() {}

clang/test/Modules/module-debuginfo-prefix.m

+// UNSUPPORTED: system-windows
 // REQUIRES: asserts
 
 // Modules:
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \
 // RUN:   -fdebug-prefix-map=%S/Inputs=/OVERRIDE \
 // RUN:   -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \
 // RUN:   -I %S/Inputs -I %t -emit-llvm -o %t.ll \

clang/test/PCH/debug-info-pch-container-path.c

+// UNSUPPORTED: system-windows
 // REQUIRES: asserts
 
 // Modules:
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: cd %t
 
 // RUN: %clang_cc1 -fmodule-format=obj -emit-pch                \