Move default module cache from system temporary directory

[davezarzycki]

Summary

1. Shared writable directories like /tmp are a security problem.
2. Systems provide dedicated cache directories these days anyway.

As a followup, I hope to deprecate LLVM's llvm::sys::path::system_temp_directory(). See: https://reviews.llvm.org/D82259

[sammccall]

Can you update the commit message to also reflect the change in cache_directory behavior? (This is going to move clangd index files on macOS, for instance... I think that's OK, but let's not bury it)

clang/lib/Driver/ToolChains/Clang.cpp

3251

what happens in the else case?

llvm/lib/Support/Unix/Path.inc

1163

nit: /*TempDir=*/false is conventional and e.g. clang-tidy will verify it

[davezarzycki]

clang/lib/Driver/ToolChains/Clang.cpp

3251

No module caching.

[compnerd]

LGTM

[sammccall]

clang/lib/Driver/ToolChains/Clang.cpp

3251

This seems reasonable (though a bit surprising that it happens silently). Please add a comment.

[thakis]

Looks like this breaks tests on Mac: http://45.33.8.238/mac/16222/step_11.txt

Please take a look and revert for now if this takes a while to fix.

[thakis]

Reverted for now in 4d5c4489435dc1cb3d4989614e96b157c74afdea

clang/docs/ReleaseNotes.rst

 
 - -ffp-exception-behavior={ignore,maytrap,strict} allows the user to specify
   the floating-point exception behavior. The default setting is ``ignore``.
 
 - -ffp-model={precise,strict,fast} provides the user an umbrella option to
   simplify access to the many single purpose floating point options. The default
   setting is ``precise``.
 
+- The default module cache has moved from /tmp to a per-user cache directory.
+  By default, this is ~/.cache but on some platforms or installations, this
+  might be elsewhere. The -fmodules-cache-path=... flag continues to work.
+
 Deprecated Compiler Flags
 -------------------------
 
 The following options are deprecated and ignored. They will be removed in
 future versions of Clang.
 
 - ...
 

clang/include/clang/Driver/Driver.h

public:
   /// the parsed numbers. This method assumes that the max number of
   /// digits to look for is equal to Digits.size().
   ///
   /// \return True if the entire string was parsed and there are
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
                                 MutableArrayRef<unsigned> Digits);
   /// Compute the default -fmodule-cache-path.
-  static void getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
+  /// \return True if the system provides a default cache directory.
+  static bool getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.
 /// And False otherwise.
 bool isOptimizationLevelFast(const llvm::opt::ArgList &Args);
 
 /// \return True if the argument combination will end up generating remarks.
 bool willEmitRemarks(const llvm::opt::ArgList &Args);
 
 } // end namespace driver
 } // end namespace clang
 
 #endif

clang/lib/Driver/ToolChains/Clang.cpp

else {
     default:
       ClangType = types::getTypeName(Input.getType());
       break;
     }
     CmdArgs.push_back(ClangType);
   }
 }
 
-static void appendUserToPath(SmallVectorImpl<char> &Result) {
-#ifdef LLVM_ON_UNIX
-  const char *Username = getenv("LOGNAME");
-#else
-  const char *Username = getenv("USERNAME");
-#endif
-  if (Username) {
-    // Validate that LoginName can be used in a path, and get its length.
-    size_t Len = 0;
-    for (const char *P = Username; *P; ++P, ++Len) {
-      if (!clang::isAlphanumeric(*P) && *P != '_') {
-        Username = nullptr;
-        break;
-      }
-    }
-
-    if (Username && Len > 0) {
-      Result.append(Username, Username + Len);
-      return;
-    }
-  }
-
-// Fallback to user id.
-#ifdef LLVM_ON_UNIX
-  std::string UID = llvm::utostr(getuid());
-#else
-  // FIXME: Windows seems to have an 'SID' that might work.
-  std::string UID = "9999";
-#endif
-  Result.append(UID.begin(), UID.end());
-}
-
 static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
                                    const Driver &D, const InputInfo &Output,
                                    const ArgList &Args,
                                    ArgStringList &CmdArgs) {
 
   auto *PGOGenerateArg = Args.getLastArg(options::OPT_fprofile_generate,
                                          options::OPT_fprofile_generate_EQ,
                                          options::OPT_fno_profile_generate);
static void RenderBuiltinOptions(const ToolChain &TC, const llvm::Triple &T,
 
   // le32-specific flags:
   //  -fno-math-builtin: clang should not convert math builtins to intrinsics
   //                     by default.
   if (TC.getArch() == llvm::Triple::le32)
     CmdArgs.push_back("-fno-math-builtin");
 }
 
-void Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
-  llvm::sys::path::append(Result, "org.llvm.clang.");
-  appendUserToPath(Result);
+bool Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
+  if (llvm::sys::path::cache_directory(Result)) {
+    llvm::sys::path::append(Result, "clang");
     llvm::sys::path::append(Result, "ModuleCache");
+    return true;
+  }
+  return false;
 }
 
 static void RenderModulesOptions(Compilation &C, const Driver &D,
                                  const ArgList &Args, const InputInfo &Input,
                                  const InputInfo &Output,
                                  ArgStringList &CmdArgs, bool &HaveModules) {
   // -fmodules enables the use of precompiled modules (off by default).
   // Users can pass -fno-cxx-modules to turn off modules support for
static void RenderModulesOptions(Compilation &C, const Driver &D,
   } else if (HaveModules) {
     ImplicitModules = true;
     // -fmodule-cache-path specifies where our implicitly-built module files
     // should be written.
     SmallString<128> Path;
     if (Arg *A = Args.getLastArg(options::OPT_fmodules_cache_path))
       Path = A->getValue();
 
+    bool HasPath = true;
     if (C.isForDiagnostics()) {
       // When generating crash reports, we want to emit the modules along with
       // the reproduction sources, so we ignore any provided module path.
       Path = Output.getFilename();
       llvm::sys::path::replace_extension(Path, ".cache");
       llvm::sys::path::append(Path, "modules");
     } else if (Path.empty()) {
       // No module path was provided: use the default.
-      Driver::getDefaultModuleCachePath(Path);
+      HasPath = Driver::getDefaultModuleCachePath(Path);
     }
 
+    // `HasPath` will only be false if getDefaultModuleCachePath() fails.
+    // That being said, that failure is unlikely and not caching is harmless.
+    if (HasPath) {
       const char Arg[] = "-fmodules-cache-path=";
       Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
       CmdArgs.push_back(Args.MakeArgString(Path));
     }
+  }
 
   if (HaveModules) {
     // -fprebuilt-module-path specifies where to load the prebuilt module files.
     for (const Arg *A : Args.filtered(options::OPT_fprebuilt_module_path)) {
       CmdArgs.push_back(Args.MakeArgString(
           std::string("-fprebuilt-module-path=") + A->getValue()));
       A->claim();
     }

clang/test/Driver/modules-cache-path.m

-// RUN: env USERNAME=asdf LOGNAME=asdf %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-SET
-// CHECK-SET: -fmodules-cache-path={{.*}}org.llvm.clang.asdf{{[/\\]+}}ModuleCache
-
 // RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
-// CHECK-DEFAULT: -fmodules-cache-path={{.*}}org.llvm.clang.{{[A-Za-z0-9_]*[/\\]+}}ModuleCache
+// CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache

clang/unittests/Driver/ModuleCacheTest.cpp

 using namespace clang::driver;
 
 namespace {
 
 TEST(ModuleCacheTest, GetTargetAndMode) {
   SmallString<128> Buf;
   Driver::getDefaultModuleCachePath(Buf);
   StringRef Path = Buf;
-  EXPECT_TRUE(Path.find("org.llvm.clang") != Path.npos);
+  EXPECT_TRUE(Path.find("clang") != Path.npos);
   EXPECT_TRUE(Path.endswith("ModuleCache"));  
 }
 } // end anonymous namespace.

llvm/lib/Support/Unix/Path.inc

bool home_directory(SmallVectorImpl<char> &result) {
   if (!RequestedDir)
     return false;
 
   result.clear();
   result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
   return true;
 }
 
-bool cache_directory(SmallVectorImpl<char> &result) {
-  if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
-    result.clear();
-    result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
-    return true;
-  }
-  if (!home_directory(result)) {
-    return false;
-  }
-  append(result, ".cache");
-  return true;
-}
-
 static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
   #if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)
   // On Darwin, use DARWIN_USER_TEMP_DIR or DARWIN_USER_CACHE_DIR.
   // macros defined in <unistd.h> on darwin >= 9
   int ConfName = TempDir ? _CS_DARWIN_USER_TEMP_DIR
                          : _CS_DARWIN_USER_CACHE_DIR;
   size_t ConfLen = confstr(ConfName, nullptr, 0);
   if (ConfLen > 0) {
if (ConfLen > 0) {
     }
 
     Result.clear();
   }
   #endif
   return false;
 }
 
+bool cache_directory(SmallVectorImpl<char> &result) {
+#ifdef __APPLE__
+  if (getDarwinConfDir(false/*tempDir*/, result)) {
+    return true;
+  }
+#else
+  // XDG_CACHE_HOME as defined in the XDG Base Directory Specification:
+  // http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
+  if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
+    result.clear();
+    result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
+    return true;
+  }
+#endif
+  if (!home_directory(result)) {
+    return false;
+  }
+  append(result, ".cache");
+  return true;
+}
+
 static const char *getEnvTempDir() {
   // Check whether the temporary directory is specified by an environment
   // variable.
   const char *EnvironmentVariables[] = {"TMPDIR", "TMP", "TEMP", "TEMPDIR"};
   for (const char *Env : EnvironmentVariables) {
     if (const char *Dir = std::getenv(Env))
       return Dir;
   }