[Support] Use all_read | all_write for createTemporaryFile

[JDevlieghere]

In a04879c, dsymutil switched from using TempFile to createTemporaryFile. This caused a regression because the two use different permissions:

• TempFile opens the file as all_read | all_write
• createTemporaryFile opens the file as owner_read | owner_write

The latter turns out to be problematic for dsymutil because it either promotes the temporary to a proper output file, or it would pass it to lipo to create a universal binary and lipo preserves the permissions of the input files. Either way, this caused issues when the build system was run as a different user than the one ingesting the resulting binaries.

I did some version control archeology and I couldn't find evidence that these permissions were chosen purposely. Both could be considered reasonable default.

This patch changes the permissions to all read | all write to make the two consistent and match the one currently used by the higher level abstraction (TempFile).

rdar://123722848

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Jonas Devlieghere (JDevlieghere)

Changes

In a04879c, dsymutil switched from using TempFile to createTemporaryFile. This caused a regression because the two use different permissions:

• TempFile opens the file as all_read | all_write
• createTemporaryFile opens the file as owner_read | owner_write

The latter turns out to be problematic for dsymutil because it either promotes the temporary to a proper output file, or it would pass it to lipo to create a universal binary and lipo preserves the permissions of the input files. Either way, this caused issues when the build system was run as a different user than the one ingesting the resulting binaries.

I did some version control archeology and I couldn't find evidence that these permissions were chosen purposely. Both could be considered reasonable default.

This patch changes the permissions to all read | all write to make the two consistent and match the one currently used by the higher level abstraction (TempFile).

rdar://123722848

---

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

1 Files Affected:

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

diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp
index c8de2c0625aa26..acee228a0d0462 100644
--- a/llvm/lib/Support/Path.cpp
+++ b/llvm/lib/Support/Path.cpp
@@ -850,7 +850,7 @@ createTemporaryFile(const Twine &Model, int &ResultFD,
          "Model must be a simple filename.");
   // Use P.begin() so that createUniqueEntity doesn't need to recreate Storage.
   return createUniqueEntity(P.begin(), ResultFD, ResultPath, true, Type, Flags,
-                            owner_read | owner_write);
+                            all_read | all_write);
 }
 
 static std::error_code

[mstorsjo]

This looks reasonable. I guess it's possible, but unlikely, that it does affect some case somewhere, but we'll never know unless we roll it out. So LGTM.