Provide -fsource-dir flag in Clang

[phosek]

Summary

This flag can be used to relativize source paths againts a given
directory. Compared to -f*-prefix-map flags, it avoids the absolute
path on the compiler command line analogously to -fdebug-compile-dir,
which is important distributed compilation scenarios.

[phosek]

This change is trying to address the issues raised in D83154. There are still some open questions:

• Is -fsource-dir the best name for this flag?
• I'm not sure if make_relative should be applied to all source paths, or only paths that start with SourceDir which would exclude system paths outside of the source directory (e.g. it's probably undesirable to relativize paths to /usr/include)?
• If we decide to exclude source paths outside of the source directory, should we support -fsource-dir to be specified more then once to handle multiple source directories?

[keith]

In D87928#2282609, @phosek wrote:

This change is trying to address the issues raised in D83154. There are still some open questions:

• Is -fsource-dir the best name for this flag?

I think this name works as long as its recommended usage doesn't end up including directories outside of the source root.

• I'm not sure if make_relative should be applied to all source paths, or only paths that start with SourceDir which would exclude system paths outside of the source directory (e.g. it's probably undesirable to relativize paths to /usr/include)?

This is one place where I think the *-prefix-map flags have a UX advantage. For example in the Apple toolchain, there are places where the Xcode paths are embedded in this data, and you may want to exclude them by doing -f*-prefix-map=/path/to/Xcode.app=STANDARD_XCODE_PATH and remapping that later as needed to increase reproducibility

• If we decide to exclude source paths outside of the source directory, should we support -fsource-dir to be specified more then once to handle multiple source directories?

I think this would be a bit confusing usage wise, so I would say it should act like -fdebug-compilation-dir where only 1 can be passed.

[phosek]

@rnk @vsk does this look good to you as well?

[phosek]

Ping? Is it OK to land this?

[dexonsmith]

I missed this before, and it's an interesting problem.

I have a few questions:

• I'm not clear on the expected interaction between the current working directory of the compiler and -fsource-dir. What happens if they're different?
• I'm curious if you've considered other solutions that tie the source directory more closely to the current working directory, such as a flag -frelative-to-working-dir, which makes things relative to the compiler's working directory. Is that too restrictive?
• Have you looked at the similar "MakeRelative" logic in ASTWriter.cpp? Can this logic be shared somehow?
• What's the expected behaviour when building modules implicitly with -fmodules? What value (if any) should they get for -fsource-dir?
• How should this affect the dependency output (e.g., clang-scan-deps and .d files), if at all?
• Have you considered having -fsource-dir add entries to the prefix maps, rather than duplicating logic?

I am a bit concerned about mixing the concepts of "relative base path" with "source directory", since the latter sounds to me like where pristine sources live and that's not always the right relative base path.

For example, consider a layout like this:

/working/dir/
             sources/
                     lib/a/a.c
                     include/a/a.h
                     include/a/a.td
             build/
                   include/lib/a/a.inc
                   lib/a/a.o
             toolchain/usr/
                           bin/clang
                           lib/clang/11.0/include/stdint.h
             sdk/usr/include/
                             stdint.h
                             c++/v1/
                                    vector
                                    stdint.h

It seems cleaner to have everything relative to /working/dir, not /working/dir/sources, since you can name all compiler inputs and outputs with the former. But the latter is conceptually the source directory.

WDYT?

clang/lib/Lex/PPMacroExpansion.cpp

1545–1547

What does this do with -fsource-dir /a/b for the file /a/b.c? (It doesn't seem quite right for it to turn it into .c or ../b.c...)

[phosek]

In D87928#2394913, @dexonsmith wrote:

I missed this before, and it's an interesting problem.

I have a few questions:

• I'm not clear on the expected interaction between the current working directory of the compiler and -fsource-dir. What happens if they're different?

They're completely independent, -fsource-dir is only used for coverage mapping and macros.

• I'm curious if you've considered other solutions that tie the source directory more closely to the current working directory, such as a flag -frelative-to-working-dir, which makes things relative to the compiler's working directory. Is that too restrictive?

I think it depends on the use cases and user expectations. For example, in our case we have layout like this:

/working/dir/
             sources/
                     lib/a/a.c
                     include/a/a.h
             out/
                     default/

Here out/default is the working directory so if we used -frelative-to-working-dir, we would end up with paths like ../../sources/lib/a/a.c in the coverage mapping or macro expansions like __FILE__. While this may be acceptable for macro expansions, it's definitely undesirable in coverage mapping.

I can think of some potential solutions. We use could use -fprofile-prefix-map=../../= to strip the ../../ prefix. Alternatively we could introduce a new flags in tools like llvm-cov and llvm-profdata to strip the prefix during post-processing.

• Have you looked at the similar "MakeRelative" logic in ASTWriter.cpp? Can this logic be shared somehow?

I wasn't aware of that bit but once we decide on the approach, I'll try to deduplicate the logic.

• What's the expected behaviour when building modules implicitly with -fmodules? What value (if any) should they get for -fsource-dir?

Right now this flag doesn't apply to modules at all so there's no effect.

• How should this affect the dependency output (e.g., clang-scan-deps and .d files), if at all?

The same as for modules. We could consider extending it to support modules and dependency files as well although I'd prefer to do it separately.

• Have you considered having -fsource-dir add entries to the prefix maps, rather than duplicating logic?

That might be an option if we land D83154 first. It wasn't clear if we're going to have both but it might make sense to introduce -fprofile-prefix-map and then build -fsource-dir on top.

I am a bit concerned about mixing the concepts of "relative base path" with "source directory", since the latter sounds to me like where pristine sources live and that's not always the right relative base path.

For example, consider a layout like this:

/working/dir/
             sources/
                     lib/a/a.c
                     include/a/a.h
                     include/a/a.td
             build/
                   include/lib/a/a.inc
                   lib/a/a.o
             toolchain/usr/
                           bin/clang
                           lib/clang/11.0/include/stdint.h
             sdk/usr/include/
                             stdint.h
                             c++/v1/
                                    vector
                                    stdint.h

It seems cleaner to have everything relative to /working/dir, not /working/dir/sources, since you can name all compiler inputs and outputs with the former. But the latter is conceptually the source directory.

Yes, I agree. Ultimately it's up to the user to decide what output they expect. I guess this is one advantage of using the -frelative-to-working-dir approach where it's unambiguous and user can use other flags to do additional rewriting if needed.

WDYT?

Thanks for the detailed feedback!

clang/lib/Lex/PPMacroExpansion.cpp

1545–1547

That's definitely undesirable. We could introduce a new function like llvm::sys::path::has_prefix to compare path components instead pure string comparison to avoid this issue.

[dexonsmith]

In D87928#2396448, @phosek wrote:

-fsource-dir is only used for coverage mapping and macros.

Oh, interesting; it would be nice to consistently make all paths relative paths.

Here out/default is the working directory so if we used -frelative-to-working-dir, we would end up with paths like ../../sources/lib/a/a.c in the coverage mapping or macro expansions like __FILE__. While this may be acceptable for macro expansions, it's definitely undesirable in coverage mapping.

I can think of some potential solutions. We use could use -fprofile-prefix-map=../../= to strip the ../../ prefix.

This solution makes sense to me.

• What's the expected behaviour when building modules implicitly with -fmodules? What value (if any) should they get for -fsource-dir?

Right now this flag doesn't apply to modules at all so there's no effect.

I'm not quite following this. I don't see logic to avoid this flag applying to modules, although I may be missing something. I believe modules built explicitly via the command-line will happily accept and apply this option, and, similarly, I think modules built implicitly by cloning a CompilerInvocation will pick up the same value as the importing CompilerInstance.

It seems cleaner to have everything relative to /working/dir, not /working/dir/sources, since you can name all compiler inputs and outputs with the former. But the latter is conceptually the source directory.

Yes, I agree. Ultimately it's up to the user to decide what output they expect. I guess this is one advantage of using the -frelative-to-working-dir approach where it's unambiguous and user can use other flags to do additional rewriting if needed.

If we want to handle it consistently everywhere, I wonder if we'd want a feature in the FileManager to do this, potentially handling many other cases. E.g., instead of canonicalizing to an absolute path, it would canonicalize to a path relative to working directory. Then callers just need to worry about the prefix map. I'm not sure though.

[phosek]

I'm picking up this change again after D83154 landed. I did some tracing and it looks like FileManager already returns a relative path when relative path was passed to the compiler as you'd expect so I don't think we need to change anything there. It's CoverageMappingModuleGen::normalizeFilename that makes the path absolute so perhaps if we restrict this change only to coverage mapping, perhaps we should just introduce a flag to disable that behavior, for example -f[no]-normalize-coverage-mapping, or to normalize paths as relative against a given directory, for example -fcoverage-mapping-dir=[DIR]. What do you think?

[phosek]

No longer needed with -fcoverage-compilation-dir.

clang/include/clang/Basic/CodeGenOptions.h

public:
   std::string ProfileFilterFiles;
 
   /// Regexes separated by a semi-colon to filter the files to not instrument.
   std::string ProfileExcludeFiles;
 
   /// The version string to put into coverage files.
   char CoverageVersion[4];
 
+  /// The directory all source files are relative to.
+  std::string SourceDir;
+
   /// Enable additional debugging information.
   std::string DebugPass;
 
   /// The string to embed in debug information as the current working directory.
   std::string DebugCompilationDir;
 
   /// The string to embed in the debug information for the compile unit, if
   /// non-empty.

clang/include/clang/Driver/Options.td

def fprofile_exclude_files_EQ : Joined<["-"], "fprofile-exclude-files=">,
     HelpText<"Instrument only functions from files where names don't match all the regexes separated by a semi-colon">;
 def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
     Group<f_Group>, Flags<[CC1Option, CoreOption]>, Values<"atomic,prefer-atomic,single">,
     MetaVarName<"<method>">, HelpText<"Set update method of profile counters (atomic,prefer-atomic,single)">;
 def forder_file_instrumentation : Flag<["-"], "forder-file-instrumentation">,
     Group<f_Group>, Flags<[CC1Option, CoreOption]>,
     HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 
+def fsource_dir : Separate<["-"], "fsource-dir">,
+    Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
+    HelpText<"The directory all source files are relative to.">;
+def fsource_dir_EQ : Joined<["-"], "fsource-dir=">,
+    Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
+    Alias<fsource_dir>;
+
 defm addrsig : OptInFFlag<"addrsig", "Emit", "Don't emit", " an address-significance table", [CoreOption]>;
 defm blocks : OptInFFlag<"blocks", "Enable the 'blocks' language feature", "", "", [CoreOption]>;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group<f_Group>;
 def fborland_extensions : Flag<["-"], "fborland-extensions">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Accept non-standard constructs supported by the Borland compiler">;
 def fbuiltin : Flag<["-"], "fbuiltin">, Group<f_Group>, Flags<[CoreOption]>;
 def fbuiltin_module_map : Flag <["-"], "fbuiltin-module-map">, Group<f_Group>,
   Flags<[NoXarchOption]>, HelpText<"Load the clang builtins module map file.">;

clang/include/clang/Lex/PreprocessorOptions.h

public:
   /// The set of modules that failed to build.
   ///
   /// This pointer will be shared among all of the compiler instances created
   /// to (re)build modules, so that once a module fails to build anywhere,
   /// other instances will see that the module has failed and won't try to
   /// build it again.
   std::shared_ptr<FailedModulesSet> FailedModules;
 
+  /// The directory all source files are relative to.
+  std::string SourceDir;
+
   /// A prefix map for __FILE__ and __BASE_FILE__.
   std::map<std::string, std::string, std::greater<std::string>> MacroPrefixMap;
 
   /// Contains the currently active skipped range mappings for skipping excluded
   /// conditional directives.
   ///
   /// The pointer is passed to the Preprocessor when it's constructed. The
   /// pointer is unowned, the client is responsible for its lifetime.

clang/lib/CodeGen/CoverageMappingGen.h

class CoverageMappingModuleGen {
   };
 
   CodeGenModule &CGM;
   CoverageSourceInfo &SourceInfo;
   llvm::SmallDenseMap<const FileEntry *, unsigned, 8> FileEntries;
   std::vector<llvm::Constant *> FunctionNames;
   std::vector<FunctionInfo> FunctionRecords;
 
+  std::string normalizeFilename(StringRef Filename);
+
   /// Emit a function record.
   void emitFunctionMappingRecord(const FunctionInfo &Info,
                                  uint64_t FilenamesRef);
 
 public:
   static CoverageSourceInfo *setUpCoverageCallbacks(Preprocessor &PP);
 
   CoverageMappingModuleGen(CodeGenModule &CGM, CoverageSourceInfo &SourceInfo)

clang/lib/CodeGen/CoverageMappingGen.cpp

struct CounterCoverageMappingBuilder
   }
 
   void VisitLambdaExpr(const LambdaExpr *LE) {
     // Lambdas are treated as their own functions for now, so we shouldn't
     // propagate counts into them.
   }
 };
 
-std::string normalizeFilename(StringRef Filename) {
-  llvm::SmallString<256> Path(Filename);
-  llvm::sys::fs::make_absolute(Path);
-  llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
-  return std::string(Path);
-}
-
 } // end anonymous namespace
 
 static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
                  ArrayRef<CounterExpression> Expressions,
                  ArrayRef<CounterMappingRegion> Regions) {
   OS << FunctionName << ":\n";
   CounterMappingContext Ctx(Expressions);
   for (const auto &R : Regions) {

 }
 
 static std::string getInstrProfSection(const CodeGenModule &CGM,
                                        llvm::InstrProfSectKind SK) {
   return llvm::getInstrProfSectionName(
       SK, CGM.getContext().getTargetInfo().getTriple().getObjectFormat());
 }
 
+std::string CoverageMappingModuleGen::normalizeFilename(StringRef Filename) {
+  llvm::SmallString<256> Path(Filename);
+  llvm::sys::fs::make_absolute(Path);
+  llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
+  StringRef SourceDir = CGM.getCodeGenOpts().SourceDir;
+  if (!SourceDir.empty() && Path.startswith(SourceDir))
+    llvm::sys::path::make_relative(Path, SourceDir, Path);
+  return std::string(Path);
+}
+
 void CoverageMappingModuleGen::emitFunctionMappingRecord(
     const FunctionInfo &Info, uint64_t FilenamesRef) {
   llvm::LLVMContext &Ctx = CGM.getLLVMContext();
 
   // Assign a name to the function record. This is used to merge duplicates.
   std::string FuncRecordName = "__covrec_" + llvm::utohexstr(Info.NameHash);
 
   // A dummy description for a function included-but-not-used in a TU can be

clang/lib/Driver/ToolChains/Clang.cpp

#endif
   }
 
   if (ShouldDisableDwarfDirectory(Args, TC))
     CmdArgs.push_back("-fno-dwarf-directory-asm");
 
   if (!ShouldEnableAutolink(Args, TC, JA))
     CmdArgs.push_back("-fno-autolink");
 
+  if (Arg *A = Args.getLastArg(options::OPT_fsource_dir)) {
+    CmdArgs.push_back("-fsource-dir");
+    CmdArgs.push_back(A->getValue());
+  }
+
   // Add in -fdebug-compilation-dir if necessary.
   addDebugCompDirArg(Args, CmdArgs, D.getVFS());
 
   addDebugPrefixMapArg(D, Args, CmdArgs);
 
   if (Arg *A = Args.getLastArg(options::OPT_ftemplate_depth_,
                                options::OPT_ftemplate_depth_EQ)) {
     CmdArgs.push_back("-ftemplate-depth");

clang/lib/Frontend/CompilerInvocation.cpp

auto DCT = llvm::StringSwitch<llvm::DebugCompressionType>(A->getValue())
                    .Case("none", llvm::DebugCompressionType::None)
                    .Case("zlib", llvm::DebugCompressionType::Z)
                    .Case("zlib-gnu", llvm::DebugCompressionType::GNU)
                    .Default(llvm::DebugCompressionType::None);
     Opts.setCompressDebugSections(DCT);
   }
 
   Opts.RelaxELFRelocations = Args.hasArg(OPT_mrelax_relocations);
+
+  SmallString<128> P(Args.getLastArgValue(OPT_fsource_dir));
+  if (!P.empty() && !llvm::sys::path::is_absolute(P))
+    llvm::sys::fs::make_absolute(P);
+  llvm::sys::path::remove_dots(P, /*remove_dot_dot=*/true);
+  Opts.SourceDir = std::string(P.str());
+
   Opts.DebugCompilationDir =
       std::string(Args.getLastArgValue(OPT_fdebug_compilation_dir));
+
   for (auto *A :
        Args.filtered(OPT_mlink_bitcode_file, OPT_mlink_builtin_bitcode)) {
     CodeGenOptions::BitcodeFileToLink F;
     F.Filename = A->getValue();
     if (A->getOption().matches(OPT_mlink_builtin_bitcode)) {
       F.LinkFlags = llvm::Linker::Flags::LinkOnlyNeeded;
       // When linking CUDA bitcode, propagate function attributes so that
       // e.g. libdevice gets fast-math attrs if we're building with fast-math.
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
   Opts.DetailedRecord = Args.hasArg(OPT_detailed_preprocessing_record);
   Opts.DisablePCHValidation = Args.hasArg(OPT_fno_validate_pch);
   Opts.AllowPCHWithCompilerErrors = Args.hasArg(OPT_fallow_pch_with_errors);
 
   Opts.DumpDeserializedPCHDecls = Args.hasArg(OPT_dump_deserialized_pch_decls);
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
     Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
+  SmallString<128> P(Args.getLastArgValue(OPT_fsource_dir));
+  if (!P.empty() && !llvm::sys::path::is_absolute(P))
+    llvm::sys::fs::make_absolute(P);
+  llvm::sys::path::remove_dots(P, /*remove_dot_dot=*/true);
+  Opts.SourceDir = std::string(P.str());
+
   for (const auto &A : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
     auto Split = StringRef(A).split('=');
     Opts.MacroPrefixMap.insert(
         {std::string(Split.first), std::string(Split.second)});
   }
 
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
     StringRef Value(A->getValue());

clang/lib/Lex/PPMacroExpansion.cpp

if (PLoc.isValid()) {
         if (PLFileName != "")
           FN += PLFileName;
         else
           FN += PLoc.getFilename();
       } else {
         FN += PLoc.getFilename();
       }
       remapMacroPath(FN, PPOpts->MacroPrefixMap);
+      StringRef SourceDir = PPOpts->SourceDir;
+      if (!SourceDir.empty() && FN.startswith(SourceDir))
+        llvm::sys::path::make_relative(FN, SourceDir, FN);
       Lexer::Stringify(FN);
       OS << '"' << FN << '"';
     }
     Tok.setKind(tok::string_literal);
   } else if (II == Ident__DATE__) {
     Diag(Tok.getLocation(), diag::warn_pp_date_time);
     if (!DATELoc.isValid())
       ComputeDATE_TIME(DATELoc, TIMELoc, *this);

clang/test/CoverageMapping/source_dir.c

+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm %s -o - | FileCheck -check-prefix=ABSPATH %s
+
+// ABSPATH: @__llvm_coverage_mapping = {{.*}}"\01
+// ABSPATH: {{[/\\].*(/|\\\\)test(/|\\\\)CoverageMapping(/|\\\\)source_dir}}.c
+// ABSPATH: "
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -fsource-dir %S %s -o - | FileCheck -check-prefix=RELPATH %s
+
+// RELPATH: @__llvm_coverage_mapping = {{.*}}"\01
+// RELPATH: source_dir.c
+// RELPATH: "
+
+void f1() {}

clang/test/Preprocessor/file_test.c

 // UNSUPPORTED: system-windows
 // RUN: %clang -E -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s
 // RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s
 // RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
 // RUN: %clang -E -fmacro-prefix-map=%p/= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fsource-dir %S -c -o - %s | FileCheck %s --check-prefix CHECK-RELATIVE
 
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
 
 // CHECK: filename: "/UNLIKELY_PATH/empty/file_test.c"
 // CHECK: filename: "/UNLIKELY_PATH/empty/Inputs/include-file-test/file_test.h"
 // CHECK: basefile: "/UNLIKELY_PATH/empty/file_test.c"
 // CHECK-NOT: filename:
 
 // CHECK-EVIL: filename: "/UNLIKELY_PATH=empty/file_test.c"
 // CHECK-EVIL: filename: "/UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
 // CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty/file_test.c"
 // CHECK-EVIL-NOT: filename:
 
 // CHECK-REMOVE: filename: "file_test.c"
 // CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test.c"
 // CHECK-REMOVE-NOT: filename:
+
+// CHECK-RELATIVE: filename: "file_test.c"
+// CHECK-RELATIVE: filename: "Inputs/include-file-test/file_test.h"
+// CHECK-RELATIVE: basefile: "file_test.c"
+// CHECK-RELATIVE-NOT: filename:

llvm/include/llvm/Support/Path.h

 ///
 /// @param path processed path
 /// @param remove_dot_dot specify if '../' (except for leading "../") should be
 /// removed
 /// @result True if path was changed
 bool remove_dots(SmallVectorImpl<char> &path, bool remove_dot_dot = false,
                  Style style = Style::native);
 
+/// Return a path that when appeneded to from resolves to the same as to.
+void make_relative(StringRef from, StringRef to, SmallVectorImpl<char> &result,
+                   Style style = Style::native);
+
 } // end namespace path
 } // end namespace sys
 } // end namespace llvm
 
 #endif

llvm/lib/Support/Path.cpp

for (StringRef C : makeArrayRef(components).drop_front()) {
       buffer += preferred_separator(style);
       buffer += C;
     }
   }
   the_path.swap(buffer);
   return true;
 }
 
+void make_relative(StringRef from, StringRef to, SmallVectorImpl<char> &result,
+                   Style style) {
+  SmallString<256> buffer;
+  auto fromIt = begin(from, style), fromE = end(from);
+  auto toIt = begin(to, style), toE = end(to);
+  // Find a common base.
+  for (; fromIt != fromE && toIt != toE && *fromIt == *toIt; ++fromIt, ++toIt)
+    ;
+  // Navigate backwards to the base.
+  for (; toIt != toE; ++toIt)
+    if (*toIt != ".")
+      append(buffer, style, "..");
+  append(buffer, fromIt, fromE, style);
+  result.swap(buffer);
+}
+
 } // end namespace path
 
 namespace fs {
 
 std::error_code getUniqueID(const Twine Path, UniqueID &Result) {
   file_status Status;
   std::error_code EC = status(Path, Status);
   if (EC)

llvm/unittests/Support/Path.cpp

TEST(Support, ReplacePathPrefix) {
   EXPECT_EQ(Path, "/newoo\\bar");
   Path = Path4;
   Found = path::replace_path_prefix(Path, OldPrefixWin, NewPrefix,
                                     path::Style::posix);
   EXPECT_FALSE(Found);
   EXPECT_EQ(Path, "C:\\old/foo\\bar");
 }
 
+static std::string make_relative(StringRef from, StringRef to,
+                                 path::Style style) {
+  SmallString<256> buffer;
+  path::make_relative(from, to, buffer, style);
+  return std::string(buffer.str());
+}
+
+TEST(Support, MakeRelative) {
+  EXPECT_EQ("", make_relative("/a/b/c/d", "/a/b/c/d", path::Style::posix));
+  EXPECT_EQ("d", make_relative("/a/b/c/d", "/a/b/c", path::Style::posix));
+  EXPECT_EQ("c/d", make_relative("/a/b/c/d", "/a/b", path::Style::posix));
+  EXPECT_EQ("../../c/d",
+            make_relative("/a/b/c/d", "/a/b/e/f", path::Style::posix));
+  EXPECT_EQ("../../../../a/b/c/d",
+            make_relative("/a/b/c/d", "/e/f/g/h", path::Style::posix));
+
+  EXPECT_EQ("", make_relative("C:\\a\\b\\c\\d", "C:\\a\\b\\c\\d",
+                              path::Style::windows));
+  EXPECT_EQ("d", make_relative("C:\\a\\b\\c\\d", "C:\\a\\b\\c",
+                               path::Style::windows));
+  EXPECT_EQ("c\\d",
+            make_relative("C:\\a\\b\\c\\d", "C:\\a\\b", path::Style::windows));
+  EXPECT_EQ("..\\..\\c\\d", make_relative("C:\\a\\b\\c\\d", "C:\\a\\b\\e\\f",
+                                          path::Style::windows));
+  EXPECT_EQ(
+      "..\\..\\..\\..\\a\\b\\c\\d",
+      make_relative("C:\\a\\b\\c\\d", "C:\\e\\f\\g\\h", path::Style::windows));
+}
+
 TEST_F(FileSystemTest, OpenFileForRead) {
   // Create a temp file.
   int FileDescriptor;
   SmallString<64> TempPath;
   ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
   FileRemover Cleanup(TempPath);