This is an archive of the discontinued LLVM Phabricator instance.

Move default module cache from system temporary directory
ClosedPublic

Authored by davezarzycki on Jun 23 2020, 3:13 AM.

Details

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

Diff Detail

Event Timeline

davezarzycki created this revision.Jun 23 2020, 3:13 AM

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 marked an inline comment as done.Jun 23 2020, 4:42 AM
davezarzycki added inline comments.
clang/lib/Driver/ToolChains/Clang.cpp
3251

No module caching.

This revision is now accepted and ready to land.Jun 24 2020, 3:17 PM
sammccall accepted this revision.Jun 25 2020, 12:36 AM
sammccall added inline comments.
clang/lib/Driver/ToolChains/Clang.cpp
3251

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

This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Jun 26 2020, 7:34 AM

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.