Canonicalize the path provided by -fmodules-cache-path.

This fixes lookup mismatches that could happen when the module cache path contained a '/./' component. <rdar://problem/30413458> Differential Revision: https://reviews.llvm.org/D30915 llvm-svn: 297790
llvm · Mar 14, 2017 · 0ebdeac · 0ebdeac
1 parent d3c22f0
commit 0ebdeac
Showing 8 changed files with 80 additions and 3 deletions.
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1419,7 +1419,8 @@ std::string CompilerInvocation::GetResourcesPath(const char *Argv0,
   return P.str();
 }
 
-static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args) {
+static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
+                                  const std::string &WorkingDir) {
   using namespace options;
   Opts.Sysroot = Args.getLastArgValue(OPT_isysroot, "/");
   Opts.Verbose = Args.hasArg(OPT_v);
@@ -1429,7 +1430,18 @@ static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args) {
   if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
     Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
   Opts.ResourceDir = Args.getLastArgValue(OPT_resource_dir);
-  Opts.ModuleCachePath = Args.getLastArgValue(OPT_fmodules_cache_path);
+
+  // Canonicalize -fmodules-cache-path before storing it.
+  SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
+  if (!(P.empty() || llvm::sys::path::is_absolute(P))) {
+    if (WorkingDir.empty())
+      llvm::sys::fs::make_absolute(P);
+    else
+      llvm::sys::fs::make_absolute(WorkingDir, P);
+  }
+  llvm::sys::path::remove_dots(P);
+  Opts.ModuleCachePath = P.str();
+
   Opts.ModuleUserBuildPath = Args.getLastArgValue(OPT_fmodules_user_build_path);
   for (const Arg *A : Args.filtered(OPT_fprebuilt_module_path))
     Opts.AddPrebuiltModulePath(A->getValue());
@@ -2496,7 +2508,8 @@ bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
                               Res.getTargetOpts());
-  ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args);
+  ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args,
+                        Res.getFileSystemOpts().WorkingDir);
   if (DashX == IK_AST || DashX == IK_LLVM_IR) {
     // ObjCAAutoRefCount and Sanitize LangOpts are used to setup the
     // PassManager in BackendUtil.cpp. They need to be initializd no matter

diff --git a/clang/test/Modules/Inputs/outofdate-rebuild/AppKit.h b/clang/test/Modules/Inputs/outofdate-rebuild/AppKit.h
@@ -0,0 +1,3 @@
+// AppKit
+#import "CoreVideo.h" // CoreVideo
+struct B { int i; };
diff --git a/clang/test/Modules/Inputs/outofdate-rebuild/Cocoa.h b/clang/test/Modules/Inputs/outofdate-rebuild/Cocoa.h
@@ -0,0 +1,5 @@
+// Cocoa
+#import "Foundation.h"
+#import "AppKit.h"
+
+struct A {  int i; };
diff --git a/clang/test/Modules/Inputs/outofdate-rebuild/CoreText.h b/clang/test/Modules/Inputs/outofdate-rebuild/CoreText.h
@@ -0,0 +1 @@
+struct C { int i; };
diff --git a/clang/test/Modules/Inputs/outofdate-rebuild/CoreVideo.h b/clang/test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
@@ -0,0 +1,3 @@
+// CoreVideo
+#import "Foundation.h" // Foundation
+struct E { int i; };
diff --git a/clang/test/Modules/Inputs/outofdate-rebuild/Foundation.h b/clang/test/Modules/Inputs/outofdate-rebuild/Foundation.h
@@ -0,0 +1,3 @@
+// Foundation
+#import "CoreText.h"
+struct D { int i; };
diff --git a/clang/test/Modules/Inputs/outofdate-rebuild/module.modulemap b/clang/test/Modules/Inputs/outofdate-rebuild/module.modulemap
@@ -0,0 +1,19 @@
+module Cocoa {
+  header "Cocoa.h"
+}
+
+module AppKit {
+  header "AppKit.h"
+}
+
+module CoreText {
+  header "CoreText.h"
+}
+
+module Foundation {
+  header "Foundation.h"
+}
+
+module CoreVideo {
+  header "CoreVideo.h"
+}
diff --git a/clang/test/Modules/modules-cache-path-canonicalization.m b/clang/test/Modules/modules-cache-path-canonicalization.m
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t/cache %T/rel
+
+// This testcase reproduces a use-after-free after looking up a PCM in
+// a non-canonical modules-cache-path.
+//
+// Prime the module cache (note the '.' in the path).
+// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   %s -fsyntax-only
+//
+// Force a module to be rebuilt by creating a conflict.
+// RUN: echo "@import CoreText;" > %t.m
+// RUN: %clang_cc1 -DMISMATCH -Werror -fdisable-module-hash \
+// RUN:   -fmodules-cache-path=%t/./cache -fmodules -fimplicit-module-maps \
+// RUN:   -I %S/Inputs/outofdate-rebuild %t.m -fsyntax-only
+//
+// Rebuild.
+// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   %s -fsyntax-only
+
+
+// Unrelated to the above: Check that a relative path is resolved correctly.
+//
+// RUN: %clang_cc1 -working-directory %T/rel -fmodules-cache-path=./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   -fdisable-module-hash %t.m -fsyntax-only -Rmodule-build 2>&1 \
+// RUN:   | FileCheck %s
+// CHECK: /rel/cache/CoreText.pcm
+@import Cocoa;