labath created this revision.
labath added reviewers: clayborg, lemo, JDevlieghere.
Herald added a subscriber: aprantl.

This adds support for auto-detection of path style to SymbolFileBreakpad
(similar to how r351328 did the same for DWARF). We guess each file
entry separately, as we have no idea which file came from which compile
units (and different compile units can have different path styles). The
breakpad generates should have already converted the paths to absolute
ones, so this guess should be reasonable accurate, but as always with
these kinds of things, it is hard to give guarantees about anything.

In an attempt to bring some unity to the path guessing logic, I move the
guessing logic from inside SymbolFileDWARF into the FileSpec class and
have both symbol files use it to implent their desired behavior.


https://reviews.llvm.org/D57895

Files:
  include/lldb/Utility/FileSpec.h
  lit/SymbolFile/Breakpad/Inputs/line-table-mixed-path-styles.syms
  lit/SymbolFile/Breakpad/line-table-discontinuous-file-ids.test
  lit/SymbolFile/Breakpad/line-table-edgecases.test
  lit/SymbolFile/Breakpad/line-table-missing-file.test
  lit/SymbolFile/Breakpad/line-table-mixed-path-styles.test
  lit/SymbolFile/Breakpad/line-table.test
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Utility/FileSpec.cpp
  unittests/Utility/FileSpecTest.cpp

Index: unittests/Utility/FileSpecTest.cpp
===================================================================
--- unittests/Utility/FileSpecTest.cpp
+++ unittests/Utility/FileSpecTest.cpp
@@ -189,6 +189,17 @@
   }
 }
 
+TEST(FileSpecTest, GuessPathStyle) {
+  EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("/foo/bar.txt"));
+  EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
+  EXPECT_EQ(FileSpec::Style::windows,
+            FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+            FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
+}
+
 TEST(FileSpecTest, GetNormalizedPath) {
   std::pair<const char *, const char *> posix_tests[] = {
       {"/foo/.././bar", "/bar"},
Index: source/Utility/FileSpec.cpp
===================================================================
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -365,6 +365,16 @@
   return a == b;
 }
 
+llvm::Optional<FileSpec::Style> FileSpec::GuessPathStyle(llvm::StringRef path) {
+  if (path.startswith("/"))
+    return Style::posix;
+  if (path.startswith(R"(\\)"))
+    return Style::windows;
+  if (path.size() > 3 && llvm::isAlpha(path[0]) && path.substr(1, 2) == R"(:\)")
+    return Style::windows;
+  return llvm::None;
+}
+
 //------------------------------------------------------------------
 // Dump the object to the supplied stream. If the object contains a valid
 // directory name, it will be displayed followed by a directory delimiter, and
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -820,22 +820,19 @@
   if (!die)
     return;
 
-  auto guess = [](llvm::StringRef str) {
-    if (str.startswith("/"))
-      return FileSpec::Style::posix;
-    if (str.size() > 3 && llvm::isAlpha(str[0]) && str.substr(1, 2) == ":\\")
-      return FileSpec::Style::windows;
-    return FileSpec::Style::native;
-  };
   llvm::StringRef comp_dir = removeHostnameFromPathname(
       die->GetAttributeValueAsString(m_dwarf, this, DW_AT_comp_dir, NULL));
   if (!comp_dir.empty()) {
-    m_comp_dir = resolveCompDir(FileSpec(comp_dir, guess(comp_dir)));
+    FileSpec::Style comp_dir_style =
+        FileSpec::GuessPathStyle(comp_dir).getValueOr(FileSpec::Style::native);
+    m_comp_dir = resolveCompDir(FileSpec(comp_dir, comp_dir_style));
   } else {
     // Try to detect the style based on the DW_AT_name attribute, but just store
     // the detected style in the m_comp_dir field.
-    m_comp_dir = FileSpec("", guess(die->GetAttributeValueAsString(
-                                  m_dwarf, this, DW_AT_name, NULL)));
+    const char *name =
+        die->GetAttributeValueAsString(m_dwarf, this, DW_AT_name, NULL);
+    m_comp_dir = FileSpec(
+        "", FileSpec::GuessPathStyle(name).getValueOr(FileSpec::Style::native));
   }
 }
 
Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===================================================================
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -398,7 +398,9 @@
 
     if (record->Number >= m_files->size())
       m_files->resize(record->Number + 1);
-    (*m_files)[record->Number] = FileSpec(record->Name);
+    FileSpec::Style style = FileSpec::GuessPathStyle(record->Name)
+                                .getValueOr(FileSpec::Style::native);
+    (*m_files)[record->Number] = FileSpec(record->Name, style);
   }
 }
 
Index: lit/SymbolFile/Breakpad/line-table.test
===================================================================
--- lit/SymbolFile/Breakpad/line-table.test
+++ lit/SymbolFile/Breakpad/line-table.test
@@ -1,5 +1,3 @@
-# XFAIL: system-windows
-
 # RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/line-table.out
 # RUN: %lldb %T/line-table.out -o "target symbols add -s line-table.out %S/Inputs/line-table.syms" \
 # RUN:   -s %s -o exit | FileCheck %s
Index: lit/SymbolFile/Breakpad/line-table-mixed-path-styles.test
===================================================================
--- /dev/null
+++ lit/SymbolFile/Breakpad/line-table-mixed-path-styles.test
@@ -0,0 +1,11 @@
+# RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/line-table-mixed-path-styles.out
+# RUN: %lldb %T/line-table-mixed-path-styles.out \
+# RUN:   -o "target symbols add -s line-table-mixed-path-styles.out %S/Inputs/line-table-mixed-path-styles.syms" \
+# RUN:   -s %s -o exit | FileCheck %s
+
+image dump line-table a.c
+# CHECK-LABEL: Line table for /tmp/a.c
+# CHECK-NEXT: 0x00000000004000b0: /tmp/a.c:1
+# CHECK-NEXT: 0x00000000004000b1: c:\tmp\b.c:1
+# CHECK-NEXT: 0x00000000004000b2:
+# CHECK-EMPTY:
Index: lit/SymbolFile/Breakpad/line-table-missing-file.test
===================================================================
--- lit/SymbolFile/Breakpad/line-table-missing-file.test
+++ lit/SymbolFile/Breakpad/line-table-missing-file.test
@@ -3,8 +3,6 @@
 # Right now, "something reasonable" means creating a line entry with an empty
 # file.
 
-# XFAIL: system-windows
-
 # RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/line-table-missing-file.out
 # RUN: %lldb %T/line-table-missing-file.out \
 # RUN:   -o "target symbols add -s line-table-missing-file.out %S/Inputs/line-table-missing-file.syms" \
Index: lit/SymbolFile/Breakpad/line-table-edgecases.test
===================================================================
--- lit/SymbolFile/Breakpad/line-table-edgecases.test
+++ lit/SymbolFile/Breakpad/line-table-edgecases.test
@@ -2,8 +2,6 @@
 # input contains a LINE record which does not belong to any function as well as
 # a FUNC record without any LINE records.
 
-# XFAIL: system-windows
-
 # RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/line-table-edgecases.out
 # RUN: %lldb %T/line-table-edgecases.out \
 # RUN:   -o "target symbols add -s line-table-edgecases.out %S/Inputs/line-table-edgecases.syms" \
Index: lit/SymbolFile/Breakpad/line-table-discontinuous-file-ids.test
===================================================================
--- lit/SymbolFile/Breakpad/line-table-discontinuous-file-ids.test
+++ lit/SymbolFile/Breakpad/line-table-discontinuous-file-ids.test
@@ -1,7 +1,5 @@
 # Test that we handle files which has gaps in the FILE record IDs.
 
-# XFAIL: system-windows
-
 # RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/line-table-discontinuous-file-ids.out
 # RUN: %lldb %T/line-table-discontinuous-file-ids.out \
 # RUN:   -o "target symbols add -s line-table-discontinuous-file-ids.out %S/Inputs/line-table-discontinuous-file-ids.syms" \
Index: lit/SymbolFile/Breakpad/Inputs/line-table-mixed-path-styles.syms
===================================================================
--- /dev/null
+++ lit/SymbolFile/Breakpad/Inputs/line-table-mixed-path-styles.syms
@@ -0,0 +1,7 @@
+MODULE Linux x86_64 761550E08086333960A9074A9CE2895C0 a.out
+INFO CODE_ID E05015768680393360A9074A9CE2895C
+FILE 0 /tmp/a.c
+FILE 1 c:\tmp\b.c
+FUNC b0 10 0 func
+b0 1 1 0
+b1 1 1 1
Index: include/lldb/Utility/FileSpec.h
===================================================================
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -242,6 +242,13 @@
 
   static bool Equal(const FileSpec &a, const FileSpec &b, bool full);
 
+  /// Attempt to guess path style for a given path string. It returns a style,
+  /// if it was able to make a reasonable guess, or None if it wasn't. The guess
+  /// will be correct if the input path was a valid absolute path on the system
+  /// which produced it. On other paths the result of this function is
+  /// unreliable (e.g. "c:\foo.txt" is a valid relative posix path).
+  static llvm::Optional<Style> GuessPathStyle(llvm::StringRef path);
+
   //------------------------------------------------------------------
   /// Case sensitivity of path.
   ///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to