Handle mixed-OS paths in DWARF reader

[eugene]

Summary

Make sure that DWARF line information generated by Windows can be properly read by Posix OS and vice versa.

[eugene]

clang-format

[aprantl]

This will also need a testcase.

lib/DebugInfo/DWARF/DWARFDebugLine.cpp

956

Shouldn't this function be provided in libSupport? For example, by adding a new enumerator sys::path::Style::any ?

[eugene]

Added a test

[zturner]

Is there some kind of field we can write inside the DWARF that can tell us what path syntax to assume? I'm a little weary about trying to guess. It seems like there could be lots of edge cases where we incorrectly determine what syntax a path is using.

lib/DebugInfo/DWARF/DWARFDebugLine.cpp

956

It seems pretty special purpose. If we were going to put something in support I would rather have it be something like sys::path::guess_path_syntax(const Twine &Path)

[eugene]

In D44290#1034041, @zturner wrote:

Is there some kind of field we can write inside the DWARF that can tell us what path syntax to assume?

As far as I know DWARF doesn't have a field like this.

I'm a little weary about trying to guess. It seems like there could be lots of edge cases where we incorrectly determine what syntax a path is using.

I share your worry for the general practice, but in this particular case I think it's ok.
If path is absolute on any OS (starts with / or X:) the code will treat it like it is absolute, and just won't try to concatenate more things together.

On Windows Android NDK produces binaries where different CUs have different path styles. Those binaries need to be symbolized on Linux and Mac OS, hence this change removing assumption that path style is always native.

[labath]

I think that adding a path-style field to dwarf (it would probably have to be a vendor extension) is an excellent idea. However, that won't help us with parsing the existing debug info corpus. This isn't going to be 100% correct (e.g. c:\foo.cpp is a valid relative path on unix), but I hope nobody actually uses file names where that matters. If someone like that turns up, then we'd probably have to add a some flag about how the user wants the paths to be treated, as there is no way we can figure out the correct behavior automatically.

[aprantl]

Oh well, I think you convinced me.

lib/DebugInfo/DWARF/DWARFDebugLine.cpp

Optional<StringRef> DWARFDebugLine::LineTable::getSourceByIndex(uint64_t FileIndex,
   if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
     return None;
   const FileNameEntry &Entry = Prologue.FileNames[FileIndex - 1];
   if (Optional<const char *> source = Entry.Source.getAsCString())
     return StringRef(*source);
   return None;
 }
 
+static bool isPathAbsoluteOnWindowsOrPosix(const Twine &Path) {
+  // Debug info can contain paths from any OS, not necessarily
+  // an OS we're currently running on. Moreover different compilation units can
+  // be compiled on different operating systems and linked together later.
+  return sys::path::is_absolute(Path, sys::path::Style::posix) ||
+         sys::path::is_absolute(Path, sys::path::Style::windows);
+}
+
 bool DWARFDebugLine::LineTable::getFileNameByIndex(uint64_t FileIndex,
                                                    const char *CompDir,
                                                    FileLineInfoKind Kind,
                                                    std::string &Result) const {
   if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
     return false;
   const FileNameEntry &Entry = Prologue.FileNames[FileIndex - 1];
   StringRef FileName = Entry.Name.getAsCString().getValue();
   if (Kind != FileLineInfoKind::AbsoluteFilePath ||
-      sys::path::is_absolute(FileName)) {
+      isPathAbsoluteOnWindowsOrPosix(FileName)) {
     Result = FileName;
     return true;
   }
 
   SmallString<16> FilePath;
   uint64_t IncludeDirIndex = Entry.DirIdx;
   StringRef IncludeDir;
   // Be defensive about the contents of Entry.
   if (IncludeDirIndex > 0 &&
       IncludeDirIndex <= Prologue.IncludeDirectories.size())
     IncludeDir = Prologue.IncludeDirectories[IncludeDirIndex - 1]
                      .getAsCString()
                      .getValue();
 
   // We may still need to append compilation directory of compile unit.
   // We know that FileName is not absolute, the only way to have an
   // absolute path at this point would be if IncludeDir is absolute.
   if (CompDir && Kind == FileLineInfoKind::AbsoluteFilePath &&
-      sys::path::is_relative(IncludeDir))
+      !isPathAbsoluteOnWindowsOrPosix(IncludeDir))
     sys::path::append(FilePath, CompDir);
 
   // sys::path::append skips empty strings.
   sys::path::append(FilePath, IncludeDir, FileName);
   Result = FilePath.str();
   return true;
 }