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

[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
  /dir/symlink.h -> file.h

the inode-based cache is what tells us that the following all point at the same file (given a working directory of `/dir`):

- `/dir/file.h`
- `/dir/./file.h`
- `../dir/file.h`
- `./file.h`
- `file.h`
- `symlink.h`
- `./symlink.h`
- `../dir/symlink.h`
- `/dir/symlink.h`
- `/dir/./symlink.h`

We can't drop the inode cache until there's another solution in place for this.

(One roundabout solution would be to keep this unchanged, and rely on the VFS to provide stable inodes. As it happens, I'm prototyping an on-disk caching `FileSystem` implementation that assigns stable/virtual inodes for each real path (hard links get distinct inodes); doesn't seem to have any overhead over the "real" filesystem (at least not at scale in clang-scan-deps). We could change the main path in Clang to use something like that, and change the PCM cache to use different logic (it's the only filemanager client that doesn't want/expect a stable view of the FS). I don't think it'd be too hard.)

Marking this as "request changes" since `getBypassFile()` is unsound and I'd prefer its use not be proliferated; I have some WIP to remove its current use.

Maybe there's a simpler solution though. If it's safe to use `getBypassFile()` every time a PCM file is opened, then we're not getting any value out of the FileManager. Maybe the module manager could skip the FileManager entirely...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97850/new/

https://reviews.llvm.org/D97850