[llvm] [Support] Use all_read | all_write for createTemporaryFile (PR #83360)
Jonas Devlieghere via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 28 16:34:18 PST 2024
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/83360
In a04879ce7dd6, 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
>From f1e00105edc6eb9fb4812e1b82416b93ce978f1e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 28 Feb 2024 16:23:13 -0800
Subject: [PATCH] [Support] Use all_read | all_write for createTemporaryFile
In a04879ce7dd6, 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
---
llvm/lib/Support/Path.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
More information about the llvm-commits
mailing list