[Preprocessor] Honor absolute paths in diagnostics

[aganea]

Summary

Previously, -fdiagnostics-absolute-paths did not have an effect in some cases, for example: when using relative include paths, or relative CPP paths, or --show-includes, or -E or displaying notes. We have a peculiar use-case on our end with Fastbuild, where all this was exposed: CPP files are being are preprocessed on one PC, then compiled on another PC (which doesn't have the source-code); then the compiler's stdout is displayed on the first PC.

[aganea]

Ping!

[hans]

I will try to take a look, but can you please expand the patch description a little to make it more clear exactly what you're proposing to change? Sorry for all the questions below, I'm just trying to understand exactly what the issue is.

Previously, -fdiagnostics-absolute-paths did not have an effect in some cases, for example: when using relative include paths, or relative CPP paths

Are you saying the diagnostics were not using absolute paths in those cases and this patch fixes that?

, or --show-includes, or -E

Those aren't diagnostics, so that's not surprising.

or displaying notes.

What notes?

We have a peculiar use-case on our end with Fastbuild, where all this was exposed: CPP files are being are preprocessed on one PC, then compiled on another PC (which doesn't have the source-code); then the compiler's stdout is displayed on the first PC.

And what is the final problem? That diagnostics from the compiler's stdout are not absolute because they came from the preprocessed code that doesn't include the absolute paths?

[aganea]

Thanks for getting back Hans!

In D63648#1589119, @hans wrote:

Are you saying the diagnostics were not using absolute paths in those cases and this patch fixes that?

Yes.

, or --show-includes, or -E

Those aren't diagnostics, so that's not surprising.

What would suggest in that case? Add a new -fpreprocessor-absolute-paths option? Or change the name of -fdiagnostics-absolute-paths for another name that applies to both diagnostics and the preprocessor output?

or displaying notes.

What notes?

That is, the additional output starting with note: issued along error messages: t.cc:4:5: note: candidate function not viable: no known conversion from 'vector<map<[...], float>>' to 'vector<map<[...], double>>' for 1st argument;
Sometimes notes are displaying an extra path, sometimes it's just the note alone.

We have a peculiar use-case on our end with Fastbuild, where all this was exposed: CPP files are being are preprocessed on one PC, then compiled on another PC (which doesn't have the source-code); then the compiler's stdout is displayed on the first PC.

And what is the final problem? That diagnostics from the compiler's stdout are not absolute because they came from the preprocessed code that doesn't include the absolute paths?

Exactly. When clicking on a warning/error, those relative paths prevent Visual Studio from jumping to the right location.

Please let me know if further clarification is needed.

[thakis]

Those aren't diagnostics, so that's not surprising.

What would suggest in that case? Add a new -fpreprocessor-absolute-paths option? Or change the name of -fdiagnostics-absolute-paths for another name that applies to both diagnostics and the preprocessor output?

clang uses the absolute-ness you use on the input. If you pass absolute paths to clang and to -I, then --show-includes etc will print absolute paths:

$ cat test.cc
#include "foo/test.h"
$ out/gn/bin/clang-cl /showIncludes /c test.cc
Note: including file: ./foo/test.h
$ out/gn/bin/clang-cl /showIncludes /c test.cc -I$PWD
Note: including file: /Users/thakis/src/llvm-project/foo/test.h

[aganea]

The problem is that it is not cl.exe's behavior - it always makes paths absolute:

F:\svn\test>cl /c test.cc /showIncludes
Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cc
Note: including file: f:\svn\test\foo/test.h

F:\svn\test>c:

C:\Windows\System32>cl /c f:test.cc /showIncludes
Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cc
Note: including file: f:\svn\test\foo/test.h

So maybe Driver::IsCLMode() should take precedence over -fdiagnostics-absolute-paths when using /showIncludes?

The behavior between clang-cl and cl is also very different when using -E:

C:\Windows\System32>f:\svn\build\Debug\bin\clang-cl /c f:test.cc /E
clang-cl: error: no such file or directory: 'f:test.cc'
clang-cl: error: no input files

C:\Windows\System32>f:

F:\svn\test>f:\svn\build\Debug\bin\clang-cl /c test.cc /E
# 1 "test.cc"
# 1 "<built-in>" 1
# 1 "<built-in>" 3
# 361 "<built-in>" 3
# 1 "<command line>" 1
# 1 "<built-in>" 2
# 1 "test.cc" 2
# 1 "./foo/test.h" 1
void f();
# 1 "test.cc" 2


F:\svn\test>cl /c test.cc /E
Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cc
#line 1 "test.cc"
#line 1 "f:\\svn\\test\\foo/test.h"
void f();
#line 2 "test.cc"

F:\svn\test>cl /c test.cc /E /FC
Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cc
#line 1 "f:\\svn\\test\\test.cc"
#line 1 "f:\\svn\\test\\foo\\test.h"
void f();
#line 2 "f:\\svn\\test\\test.cc"

So maybe I should also implement [[ https://docs.microsoft.com/en-us/cpp/build/reference/fc-full-path-of-source-code-file-in-diagnostics?view=vs-2019 | /FC ]] along the way, and make the defaults match between clang-cl and cl? WDYT?

[thakis]

So maybe Driver::IsCLMode() should take precedence over -fdiagnostics-absolute-paths when using /showIncludes?

Definitely not. We want builds to be deterministic; this includes them being independent of the build directory. (This matters for distributed compilation caching.)

So maybe I should also implement [[ https://docs.microsoft.com/en-us/cpp/build/reference/fc-full-path-of-source-code-file-in-diagnostics?view=vs-2019 | /FC ]] along the way, and make the defaults match between clang-cl and cl? WDYT?

We intentionally don't implement /FC for the same reasons, see the discussion on D23816.

If you want absolute paths in the output, you can pass absolute paths to clang-cl and you'll get absolute paths in the output. Does that not work for you?

[aganea]

It totally makes sense, thanks for the explanation Nico! Let's forget about cl compatibility, that wasn't my initial intent.

We always pass relative paths on the cmd-line, for all the reasons you've mentioned. We also pass -fdiagnostics-absolute-paths hoping to fix the Visual Studio jump-to-location issue I've mentioned above. However that doesn't work, because we do:

(locally, on my PC)
  $ clang-cl file.cpp -Isome/relative/path/ -fdiagnostics-absolute-paths /showIncludes -E >file_pre.cpp

(and then, on a remote PC)
  $ clang-cl file_pre.cpp -fdiagnostics-absolute-paths
(the remote stdout is then displayed on my PC)

-fdiagnostics-absolute-paths has no effect in the latter case, because the paths are extracted from the preprocessed file, and paths can't be absolutized anymore on the remote PC because it doesn't have the source code (it is only a worker server).
So we end with relative paths in diagnostics, and Visual Studio can't jump to the location of the diagnostic.

What would you suggest? Use absolute paths along with -fdebug-prefix-map?

[thakis]

In D63648#1591469, @aganea wrote:

It totally makes sense, thanks for the explanation Nico! Let's forget about cl compatibility, that wasn't my initial intent.

We always pass relative paths on the cmd-line, for all the reasons you've mentioned. We also pass -fdiagnostics-absolute-paths hoping to fix the Visual Studio jump-to-location issue I've mentioned above. However that doesn't work, because we do:

(locally, on my PC)
  $ clang-cl file.cpp -Isome/relative/path/ -fdiagnostics-absolute-paths /showIncludes -E >file_pre.cpp

(and then, on a remote PC)
  $ clang-cl file_pre.cpp -fdiagnostics-absolute-paths
(the remote stdout is then displayed on my PC)

-fdiagnostics-absolute-paths has no effect in the latter case, because the paths are extracted from the preprocessed file, and paths can't be absolutized anymore on the remote PC because it doesn't have the source code (it is only a worker server).
So we end with relative paths in diagnostics, and Visual Studio can't jump to the location of the diagnostic.

What would you suggest? Use absolute paths along with -fdebug-prefix-map?

I see, thanks for the details. A few more questions (sorry!):

• Does fastbuild have something like distcc-pump? (In pump mode, distcc sends the raw cc files and all included headers to the server, and preprocessing happens remotely – http://manpages.ubuntu.com/manpages/bionic/man1/distcc.1.html#how%20distcc-pump%20mode%20works) I think this would address this (?)

• IIRC we used to use fdiagnostics-absolute-paths in Chromium but eventually stopped because someone figured out how to make MSVC's jump-to-diag work even with relative paths (?)

I'm not sure what the Right Fix is here – I can see use cases for -E both with relative and with absolute paths, even if -fdiagnostics-absolute-paths :-/

I think -fdebug-prefix-map doesn't affect -E output as far as I know.

[aganea]

In D63648#1593619, @thakis wrote:

• Does fastbuild have something like distcc-pump?

Fastbuild works like plain distcc and unfortunately it does not have pump mode.

• IIRC we used to use fdiagnostics-absolute-paths in Chromium but eventually stopped because someone figured out how to make MSVC's jump-to-diag work even with relative paths (?)

Let me know if you find out what was your solution ;) Otherwise we could absolutize paths in Fastbuild.

While I think we will go the relative paths route - do you think the changes in this patch related to --show-includes and note: make sense? Could -fdiagnostics-absolute-paths affect --show-includes as well? I am just wondering. Otherwise I'll drop the patch.

[thakis]

Maybe we should just implement full /FC mode that affects paths everywhere (diagnostics, __FILE__, etc). Then people who need this more than deterministic, build-directory-independent builds can use that. (But it does break distributed build caching, so it's not that great.)

Not sure.

But -fdiagnostics-absolute-paths should imho not affect /showIncludes output, since that's not a diagnostic.

include/clang/Frontend/TextDiagnostic.h

public:
   ///                      for any prefix.
   /// \param Columns The number of columns to use in line-wrapping, 0 disables
   ///                all line-wrapping.
   /// \param ShowColors Enable colorizing of the message.
   static void printDiagnosticMessage(raw_ostream &OS, bool IsSupplemental,
                                      StringRef Message, unsigned CurrentColumn,
                                      unsigned Columns, bool ShowColors);
 
+  /// Canonicalize a file path iff we want it absolute
+  static StringRef canonicalizePath(StringRef Filename, const SourceManager &SM,
+                                    SmallVectorImpl<char> &AbsolutePath);
+
 protected:
   void emitDiagnosticMessage(FullSourceLoc Loc, PresumedLoc PLoc,
                              DiagnosticsEngine::Level Level, StringRef Message,
                              ArrayRef<CharSourceRange> Ranges,
                              DiagOrStoredDiag D) override;
 
   void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
                          DiagnosticsEngine::Level Level,
protected:
 
   void emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc,
                           StringRef ModuleName) override;
 
   void emitBuildingModuleLocation(FullSourceLoc Loc, PresumedLoc PLoc,
                                   StringRef ModuleName) override;
 
 private:
-  void emitFilename(StringRef Filename, const SourceManager &SM);
+  static std::string emitFilename(StringRef Filename, const SourceManager &SM);
+  static std::string emitFilename(FullSourceLoc Loc, PresumedLoc PLoc);
 
   void emitSnippetAndCaret(FullSourceLoc Loc, DiagnosticsEngine::Level Level,
                            SmallVectorImpl<CharSourceRange> &Ranges,
                            ArrayRef<FixItHint> Hints);
 
   void emitSnippet(StringRef SourceLine);
 
   void emitParseableFixits(ArrayRef<FixItHint> Hints, const SourceManager &SM);
 };
 
 } // end namespace clang
 
 #endif

lib/Frontend/HeaderIncludeGen.cpp

 //===-- HeaderIncludeGen.cpp - Generate Header Includes -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Frontend/DependencyOutputOptions.h"
-#include "clang/Frontend/Utils.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Frontend/TextDiagnostic.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
+
 using namespace clang;
 
 namespace {
 class HeaderIncludesCallback : public PPCallbacks {
   SourceManager &SM;
   raw_ostream *OutputFile;
   const DependencyOutputOptions &DepOpts;
   unsigned CurrentIncludeDepth;
void HeaderIncludesCallback::FileChanged(SourceLocation Loc,
 
   // Dump the header include information we are past the predefines buffer or
   // are showing all headers and this isn't the magic implicit <command line>
   // header.
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "<command line>" and "<built-in>" in a bunch of places.
   if (ShowHeader && Reason == PPCallbacks::EnterFile &&
       UserLoc.getFilename() != StringRef("<command line>")) {
-    PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth,
-                    MSStyle);
+    SmallString<128> AbsolutePath;
+    StringRef Filename = TextDiagnostic::canonicalizePath(UserLoc.getFilename(),
+                                                          SM, AbsolutePath);
+    PrintHeaderInfo(OutputFile, Filename, ShowDepth, IncludeDepth, MSStyle);
   }
 }

lib/Frontend/PrintPreprocessedOutput.cpp

 //===--- PrintPreprocessedOutput.cpp - Implement the -E mode --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This code simply runs the preprocessor on the input file and prints out the
 // result.  This is the traditional behavior of the -E option.
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Frontend/Utils.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
+#include "clang/Frontend/TextDiagnostic.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Pragma.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstdio>
+
 using namespace clang;
 
 /// PrintMacroDefinition - Print a macro definition in a form that will be
 /// properly accepted back as a definition.
 static void PrintMacroDefinition(const IdentifierInfo &II, const MacroInfo &MI,
                                  Preprocessor &PP, raw_ostream &OS) {
   OS << "#define " << II.getName();
 
if (Reason == PPCallbacks::EnterFile) {
     // extra blank line after the directive to avoid making all following lines
     // off by one. We can do better by simply incrementing NewLine here.
     NewLine += 1;
   }
 
   CurLine = NewLine;
 
   CurFilename.clear();
-  CurFilename += UserLoc.getFilename();
+
+  SmallString<128> AbsolutePath;
+  CurFilename +=
+      TextDiagnostic::canonicalizePath(UserLoc.getFilename(), SM, AbsolutePath);
+
   FileType = NewFileType;
 
   if (DisableLineMarkers) {
     startNewLineIfNeeded(/*ShouldUpdateCurrentLine=*/false);
     return;
   }
 
   if (!Initialized) {

lib/Frontend/TextDiagnostic.cpp

else {
     assert(Normal && "Formatting should have returned to normal");
   }
 
   if (ShowColors)
     OS.resetColor();
   OS << '\n';
 }
 
-void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
-  SmallVector<char, 128> AbsoluteFilename;
-  if (DiagOpts->AbsolutePath) {
-    const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
-        llvm::sys::path::parent_path(Filename));
-    if (Dir) {
-      // We want to print a simplified absolute path, i. e. without "dots".
-      //
-      // The hardest part here are the paths like "<part1>/<link>/../<part2>".
-      // On Unix-like systems, we cannot just collapse "<link>/..", because
-      // paths are resolved sequentially, and, thereby, the path
-      // "<part1>/<part2>" may point to a different location. That is why
-      // we use FileManager::getCanonicalName(), which expands all indirections
-      // with llvm::sys::fs::real_path() and caches the result.
-      //
-      // On the other hand, it would be better to preserve as much of the
-      // original path as possible, because that helps a user to recognize it.
-      // real_path() expands all links, which sometimes too much. Luckily,
-      // on Windows we can just use llvm::sys::path::remove_dots(), because,
-      // on that system, both aforementioned paths point to the same place.
+StringRef
+TextDiagnostic::canonicalizePath(StringRef Filename, const SourceManager &SM,
+                                 SmallVectorImpl<char> &AbsolutePath) {
+  DiagnosticOptions &Opts = SM.getDiagnostics().getDiagnosticOptions();
+  if (!Opts.AbsolutePath)
+    return Filename;
+
+  SmallString<4096> Path(Filename);
+  SM.getFileManager().makeAbsolutePath(Path);
+
+  const DirectoryEntry *Dir =
+      SM.getFileManager().getDirectory(llvm::sys::path::parent_path(Path));
+  if (!Dir)
+    return Filename;
+
+  // FIXME: convert to an enum
+  if (Filename == "<command line>" || Filename == "<scratch space>" ||
+      Filename == "<built-in>")
+    return Filename;
+
 #ifdef _WIN32
-      SmallString<4096> DirName = Dir->getName();
-      llvm::sys::fs::make_absolute(DirName);
-      llvm::sys::path::native(DirName);
-      llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
 #else
-      StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
 #endif
-      llvm::sys::path::append(AbsoluteFilename, DirName,
-                              llvm::sys::path::filename(Filename));
-      Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
-    }
-  }
+  llvm::sys::path::append(AbsolutePath, DirName,
+  return StringRef(AbsolutePath.data(), AbsolutePath.size());
+
+std::string TextDiagnostic::emitFilename(StringRef Filename,
+                                         const SourceManager &SM) {
+  SmallString<128> AbsolutePath;
+  return canonicalizePath(Filename, SM, AbsolutePath);
 
-  OS << Filename;
+std::string TextDiagnostic::emitFilename(FullSourceLoc Loc, PresumedLoc PLoc) {
+  SmallString<128> AbsolutePath;
+  return canonicalizePath(PLoc.getFilename(), Loc.getManager(), AbsolutePath);
 }
 
 /// Print out the file/line/column information and include trace.
 ///
-/// This method handlen the emission of the diagnostic location information.
+/// This method handles the emission of the diagnostic location information.
 /// This includes extracting as much location information as is present for
 /// the diagnostic and printing it, as well as any include stack or source
 /// ranges necessary.
 void TextDiagnostic::emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
                                        DiagnosticsEngine::Level Level,
                                        ArrayRef<CharSourceRange> Ranges) {
   if (PLoc.isInvalid()) {
     // At least print the file name if available:
     FileID FID = Loc.getFileID();
     if (FID.isValid()) {
       const FileEntry *FE = Loc.getFileEntry();
       if (FE && FE->isValid()) {
-        emitFilename(FE->getName(), Loc.getManager());
+        OS << emitFilename(FE->getName(), Loc.getManager());
         OS << ": ";
       }
     }
     return;
   }
   unsigned LineNo = PLoc.getLine();
 
   if (!DiagOpts->ShowLocation)
     return;
 
   if (DiagOpts->ShowColors)
     OS.changeColor(savedColor, true);
 
-  emitFilename(PLoc.getFilename(), Loc.getManager());
+  OS << emitFilename(Loc, PLoc);
   switch (DiagOpts->getFormat()) {
   case DiagnosticOptions::Clang: OS << ':'  << LineNo; break;
   case DiagnosticOptions::MSVC:  OS << '('  << LineNo; break;
   case DiagnosticOptions::Vi:    OS << " +" << LineNo; break;
   }
 
   if (DiagOpts->ShowColumn)
     // Compute the column number.
if (DiagOpts->ShowSourceRanges && !Ranges.empty()) {
     if (PrintedRange)
       OS << ':';
   }
   OS << ' ';
 }
 
 void TextDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) {
   if (DiagOpts->ShowLocation && PLoc.isValid())
-    OS << "In file included from " << PLoc.getFilename() << ':'
+    OS << "In file included from "
+       << emitFilename(Loc, PLoc) << ':'
        << PLoc.getLine() << ":\n";
   else
     OS << "In included file:\n";
 }
 
 void TextDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc,
                                         StringRef ModuleName) {
   if (DiagOpts->ShowLocation && PLoc.isValid())
     OS << "In module '" << ModuleName << "' imported from "
-       << PLoc.getFilename() << ':' << PLoc.getLine() << ":\n";
+       << emitFilename(Loc, PLoc) << ':'
+       << PLoc.getLine() << ":\n";
   else
     OS << "In module '" << ModuleName << "':\n";
 }
 
 void TextDiagnostic::emitBuildingModuleLocation(FullSourceLoc Loc,
                                                 PresumedLoc PLoc,
                                                 StringRef ModuleName) {
   if (DiagOpts->ShowLocation && PLoc.isValid())
     OS << "While building module '" << ModuleName << "' imported from "
-      << PLoc.getFilename() << ':' << PLoc.getLine() << ":\n";
+       << emitFilename(Loc, PLoc) << ':'
+       << PLoc.getLine() << ":\n";
   else
     OS << "While building module '" << ModuleName << "':\n";
 }
 
 /// Find the suitable set of lines to show to include a set of ranges.
 static llvm::Optional<std::pair<unsigned, unsigned>>
 findLinesForRange(const CharSourceRange &R, FileID FID,
                   const SourceManager &SM) {
for (ArrayRef<FixItHint>::iterator I = Hints.begin(), E = Hints.end();
 
     // We specifically do not do word-wrapping or tab-expansion here,
     // because this is supposed to be easy to parse.
     PresumedLoc PLoc = SM.getPresumedLoc(BLoc);
     if (PLoc.isInvalid())
       break;
 
     OS << "fix-it:\"";
-    OS.write_escaped(PLoc.getFilename());
+    OS.write_escaped(emitFilename(PLoc.getFilename(), SM));
     OS << "\":{" << SM.getLineNumber(BInfo.first, BInfo.second)
       << ':' << SM.getColumnNumber(BInfo.first, BInfo.second)
       << '-' << SM.getLineNumber(EInfo.first, EInfo.second)
       << ':' << SM.getColumnNumber(EInfo.first, EInfo.second)
       << "}:\"";
     OS.write_escaped(I->CodeToInsert);
     OS << "\"\n";
   }
 }

test/Frontend/absolute-paths.c

 // CHECK: warning: control reaches end of non-void function
 
 
 // For files which don't exist, just print the filename.
 #line 123 "non-existant.c"
 int g() {}
 // NORMAL: non-existant.c:123:10: warning: control reaches end of non-void function
 // ABSOLUTE: non-existant.c:123:10: warning: control reaches end of non-void function
+
+// Also test with relative include paths
+// RUN: cd %S
+// RUN: %clang_cc1 -fsyntax-only -I Inputs --show-includes absolute-paths.c 2>&1 | FileCheck -check-prefix=RELATIVE %s
+
+// RELATIVE: Note: including file: Inputs{{/|\\}}absolute-paths.h
+// RELATIVE-NEXT: In file included from absolute-paths.c:4:
+// RELATIVE-NEXT: Inputs{{/|\\}}absolute-paths.h:3:1: warning: control reaches end of non-void function
+// RELATIVE: non-existant.c:123:10: warning: control reaches end of non-void function
+
+// RUN: %clang_cc1 -fsyntax-only -I Inputs --show-includes -fdiagnostics-absolute-paths absolute-paths.c 2>&1 | FileCheck -check-prefix=RELATIVE2 %s
+
+// RELATIVE2: Note: including file: {{.+(/|\\)test(/|\\)Frontend(/|\\)Inputs(/|\\)}}absolute-paths.h
+// RELATIVE2-NEXT: In file included from {{.+(/|\\)test(/|\\)Frontend(/|\\)}}absolute-paths.c:4:
+// RELATIVE2-NEXT: {{.+(/|\\)test(/|\\)Frontend(/|\\)Inputs(/|\\)}}absolute-paths.h:3:1: warning: control reaches end of non-void function
+// RELATIVE2: {{.+(/|\\)test(/|\\)Frontend(/|\\)}}non-existant.c:123:10: warning: control reaches end of non-void function

test/Modules/build-fail-notes.m

 // CHECK-SDIAG: Module.h:9:13: error: expected ';' after top level declarator
 // CHECK-SDIAG: build-fail-notes.m:4:9: note: while building module 'DependsOnModule' imported from
 // CHECK-SDIAG: DependsOnModule.h:1:10: note: while building module 'Module' imported from
 // CHECK-SDIAG: note: expanded from here
 // CHECK-SDIAG: warning: umbrella header for module 'Module' does not include header 'NotInModule.h' [-Wincomplete-umbrella]
 // CHECK-SDIAG: DependsOnModule.h:1:10: fatal: could not build module 'Module'
 // CHECK-SDIAG: build-fail-notes.m:4:9: note: while building module 'DependsOnModule' imported from
 
+// Also test path canonicalization 
+// RUN: cd %S
+// RUN: not %clang_cc1 -Wincomplete-umbrella -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F Inputs -DgetModuleVersion="epic fail" -serialize-diagnostic-file %t/tmp.diag build-fail-notes.m 2>&1 -fdiagnostics-show-note-include-stack | FileCheck -check-prefix=CHECK-RELATIVE %s
+// CHECK-RELATIVE: While building module 'DependsOnModule' imported from build-fail-notes.m:4:
+// CHECK-RELATIVE-NEXT: While building module 'Module' imported from Inputs{{/|\\}}DependsOnModule.framework{{/|\\}}Headers{{/|\\}}DependsOnModule.h:1:
+// CHECK-RELATIVE-NEXT: In file included from <module-includes>:1:
+// CHECK-RELATIVE-NEXT: Inputs{{/|\\}}Module.framework{{/|\\}}Headers{{/|\\}}Module.h:9:13: error: expected ';' after top level declarator
+
+// RUN: not %clang_cc1 -Wincomplete-umbrella -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F Inputs -DgetModuleVersion="epic fail" -serialize-diagnostic-file %t/tmp.diag build-fail-notes.m 2>&1 -fdiagnostics-show-note-include-stack -fdiagnostics-absolute-paths | FileCheck -check-prefix=CHECK-ABSOLUTE %s
+// CHECK-ABSOLUTE: While building module 'DependsOnModule' imported from {{.+(/|\\)test(/|\\)Modules(/|\\)}}build-fail-notes.m:4:
+// CHECK-ABSOLUTE-NEXT: While building module 'Module' imported from {{.+(/|\\)test(/|\\)Modules(/|\\)Inputs(/|\\)DependsOnModule.framework(/|\\)Headers(/|\\)}}DependsOnModule.h:1:
+// CHECK-ABSOLUTE-NEXT: In file included from {{.+(/|\\)test(/|\\)Modules(/|\\)}}<module-includes>:1:
+// CHECK-ABSOLUTE-NEXT: {{.+(/|\\)test(/|\\)Modules(/|\\)Inputs(/|\\)Module.framework(/|\\)Headers(/|\\)}}Module.h:9:13: error: expected ';' after top level declarator

test/Preprocessor/pp-modules.c

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c %s -F %S/../Modules/Inputs -E -o - | FileCheck %s
 
 // CHECK: int bar();
 int bar();
 // CHECK: #pragma clang module import Module /* clang -E: implicit import for #include <Module/Module.h> */{{$}}
 #include <Module/Module.h>
 // CHECK: int foo();
 int foo();
 // CHECK: #pragma clang module import Module /* clang -E: implicit import for #include <Module/Module.h> */{{$}}
 #include <Module/Module.h>
 
 #include "pp-modules.h" // CHECK: # 1 "{{.*}}pp-modules.h" 1
 // CHECK: #pragma clang module import Module /* clang -E: implicit import for #include <Module/Module.h> */{{$}}
 // CHECK: # 14 "{{.*}}pp-modules.c" 2
+
+// Also test path canonicalization 
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c pp-modules.c -F %S/../Modules/Inputs -E -o - | FileCheck -check-prefix=CHECK-RELATIVE %s
+
+// CHECK-RELATIVE: # 1 "pp-modules.c"
+// CHECK-RELATIVE-NEXT: # 1 "<built-in>" 1
+// CHECK-RELATIVE-NEXT: # 1 "<built-in>" 3
+// CHECK-RELATIVE-NEXT: # {{[0-9]+}} "<built-in>" 3
+// CHECK-RELATIVE-NEXT: # 1 "<command line>" 1
+// CHECK-RELATIVE-NEXT: # 1 "<built-in>" 2
+// CHECK-RELATIVE-NEXT: # 1 "pp-modules.c" 2
+// CHECK-RELATIVE: # 1 "./pp-modules.h" 1
+// CHECK-RELATIVE: # 14 "pp-modules.c" 2
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c pp-modules.c -F %S/../Modules/Inputs -E -o - -fdiagnostics-absolute-paths | FileCheck -check-prefix=CHECK-ABSOLUTE %s
+
+// CHECK-ABSOLUTE: # 1 "{{.+(/|\\\\)test(/|\\\\)Preprocessor(/|\\\\)}}pp-modules.c"
+// CHECK-ABSOLUTE-NEXT: # 1 "<built-in>" 1
+// CHECK-ABSOLUTE-NEXT: # 1 "<built-in>" 3
+// CHECK-ABSOLUTE-NEXT: # {{[0-9]+}} "<built-in>" 3
+// CHECK-ABSOLUTE-NEXT: # 1 "<command line>" 1
+// CHECK-ABSOLUTE-NEXT: # 1 "<built-in>" 2
+// CHECK-ABSOLUTE-NEXT: # 1 "{{.+(/|\\\\)test(/|\\\\)Preprocessor(/|\\\\)}}pp-modules.c" 2
+
+// CHECK-ABSOLUTE: # 1 "{{.+(/|\\\\)test(/|\\\\)Preprocessor(/|\\\\)}}pp-modules.h" 1
+// CHECK-ABSOLUTE: # 14 "{{.+(/|\\\\)test(/|\\\\)Preprocessor(/|\\\\)}}pp-modules.c" 2