[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-16 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D97850#2629464 , @ilyakuteev wrote: > If a fix will be in ModuleManager and only for ModuleCache the problem with > symlinks and path will not affect it as ModuleCache is managed by clang and > we can rely on it. > I agree

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-16 Thread Ilya Kuteev via Phabricator via cfe-commits
ilyakuteev added a comment. If a fix will be in ModuleManager and only for ModuleCache the problem with symlinks and path will not affect it as ModuleCache is managed by clang and we can rely on it. I agree that using `FileMgr.getBypassFile` is not the best way to solve this problem, we need

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-15 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Agreed that the right fix is to remove/replace the inode-based cache, but it's not safe to just drop it. Consider the following filesystem layout: /dir/file.h

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-09 Thread Robert Widmann via Phabricator via cfe-commits
CodaFi added a comment. I believe removing inode numbers is the correct fix, yes. The workaround I applied, and the one here, are both insufficient in the general case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97850/new/

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-08 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. It seems we already hit this issue before and decided to add a workaround: https://reviews.llvm.org/D86823 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97850/new/ https://reviews.llvm.org/D97850

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-05 Thread Ilya Kuteev via Phabricator via cfe-commits
ilyakuteev added a comment. @teemperor 's test shows the problem correctly. In my case I am working on a dist-compilation system (similar to distcc) for objective-c with `-fmodules`. Our previous generation used tmpfs for module cache and was ephemeral (Unique temp module cache per

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. If I understand this correctly, then the inode caching logic in FileManager isn't just breaking PCMs but all file operations: TEST_F(FileManagerTest, InodeReuse) { { std::ofstream myfile; myfile.open("a.cpp"); myfile << "content\n"; }

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-03 Thread Ilya Kuteev via Phabricator via cfe-commits
ilyakuteev created this revision. ilyakuteev requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D97850 Files: clang/lib/Serialization/ModuleManager.cpp Index: