[llvm] [Support][vfs] Remove can_write check when replacing file (PR #219540)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Michael Kruse (Meinersbur)

<details>
<summary>Changes</summary>

PR #<!-- -->113363 added a pre-check when preparing an output file that emits an "operation_not_permitted" error when the target files exists and is not writable.

This check is counterproductive:
 * It does not provide security because it does not inhibit TOCTOU attacks
 * It checks the wrong thing: The output file is not written to, but deleted and replaced with a different file. That is, the parent directory permissions are relevant, not of the file itself

This causes problem with e.g. ccache: With the `hard_link` option, ccache stores the output file with write permissions removed (actually opening the file for writing would also change the cache content). An incremental build that `can_write` check will fail although Clang will not actually write the file, but delete it[^1]. The result is 
```
error: unable to open output file 'lib/Support/CMakeFiles/FortranSupport.dir/Version.cpp.o': 'Operation not permitted'
1 error generated.
```

This PR removes the unnecessary check. It keeps the check for append mode, but I think "ask for forgiveness, not permission" should apply as well.

[^1]: There is a bit more nuance: in `hard_link` mode ccache itself deletes the output file before invoking the compiler. The error only appears when the ccache configuration is changed (e.g. by a sandbox environment, a different user, ccache removed in the build system, or invoked manually) and the write-protected file is still present. 

---
Full diff: https://github.com/llvm/llvm-project/pull/219540.diff


1 Files Affected:

- (modified) llvm/lib/Support/VirtualOutputBackends.cpp (+1-1) 


``````````diff
diff --git a/llvm/lib/Support/VirtualOutputBackends.cpp b/llvm/lib/Support/VirtualOutputBackends.cpp
index 0c6ce825d02d0..a90d9c076fc63 100644
--- a/llvm/lib/Support/VirtualOutputBackends.cpp
+++ b/llvm/lib/Support/VirtualOutputBackends.cpp
@@ -316,7 +316,7 @@ Error OnDiskOutputFile::initializeFile(std::optional<int> &FD) {
         Config.setNoAtomicWrite();
 
       // Fail now if we can't write to the final destination.
-      if (!sys::fs::can_write(OutputPath))
+      if (Config.getAppend() && !sys::fs::can_write(OutputPath))
         return make_error<OutputError>(
             OutputPath,
             std::make_error_code(std::errc::operation_not_permitted));

``````````

</details>


https://github.com/llvm/llvm-project/pull/219540