mirror of
https://github.com/llvm/llvm-project.git
synced 2025-05-03 06:56:08 +00:00
02a3754783
The FileIndex values returned from GetFileInformationByHandle are considered stable and uniquely identifying a file, as long as the handle is open. When handles are closed, there are no guarantees for their stability or uniqueness. On some file systems (such as NTFS), the indices are documented to be stable even across handles. But with some file systems, in particular network mounts, file indices can be reused very soon after handles are closed. When such file indices are used for LLVM's UniqueID, files are considered duplicates as soon as the filesystem driver happens to have used the same file index for the handle used to inspect the file. This caused widespread, non-obvious (seemingly random) breakage. This can happen e.g. if running on a directory that is shared via Remote Desktop or VirtualBox. To avoid the issue, use a hash of the canonicalized path for the file as unique identifier, instead of using FileIndex. This fixes https://github.com/llvm/llvm-project/issues/61401 and https://github.com/llvm/llvm-project/issues/22079. Performance wise, this adds (usually) one extra call to GetFinalPathNameByHandleW for each call to getStatus(). A test cases such as running clang-scan-deps becomes around 1% slower by this, which is considered tolerable. Change the equivalent() function to use getUniqueID instead of checking individual file_status fields. The equivalent(Twine,Twine,bool& result) function calls status() on each path successively, without keeping the file handles open, which also is prone to such false positives. This also gets rid of checks of other superfluous fields in the equivalent(file_status, file_status) function - the unique ID of a file should be enough (that is what is done for Unix anyway). This comes with one known caveat: For hardlinks, each name for the file now gets a different UniqueID, and equivalent() considers them different. While that's not ideal, occasional false negatives for equivalent() is usually that fatal (the cases where we strictly do need to deduplicate files with different path names are quite rare) compared to the issues caused by false positives for equivalent() (where we'd deduplicate and omit totally distinct files). The FileIndex is documented to be stable on NTFS though, so ideally we could maybe have used it in the majority of cases. That would require a heuristic for whether we can rely on FileIndex or not. We considered using the existing function is_local_internal for that; however that caused an unacceptable performance regression (clang-scan-deps became 38% slower in one test, even more than that in another test). Differential Revision: https://reviews.llvm.org/D155579