[llvm] r365162 - [llvm-objcopy] Change handling of output file permissions
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 22 14:58:49 PDT 2019
FWIW, a couple of these tests have been failing for me for a while now
(mirror-permissions-unix.test and respect-umask.test).
They use 'umask', and that doesn't seem to work for me:
# command stderr:
'umask': command not found
error: command failed with exit status: 127
I can fix this by adding "REQUIRES: shell" to these tests. Is that the
correct thing to do?
(I think this is because I'm using lit's pseudo-shell - but I forget how I
opted into it... )
On Thu, Jul 4, 2019 at 3:45 PM Alex Brachet via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: abrachet
> Date: Thu Jul 4 15:45:27 2019
> New Revision: 365162
>
> URL: http://llvm.org/viewvc/llvm-project?rev=365162&view=rev
> Log:
> [llvm-objcopy] Change handling of output file permissions
>
> Summary: Address bug [[ https://bugs.llvm.org/show_bug.cgi?id=42082 |
> 42082 ]] where files were always outputted with 0775 permissions. Now, the
> output file is given either 0666 or 0777 if the object is executable.
>
> Reviewers: espindola, alexshap, rupprecht, jhenderson, jakehehrlich,
> MaskRay
>
> Reviewed By: rupprecht, jhenderson, jakehehrlich, MaskRay
>
> Subscribers: emaste, arichardson, jakehehrlich, MaskRay, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D62718
>
> Added:
> llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
> llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test
> llvm/trunk/test/tools/llvm-objcopy/ELF/respect-umask.test
> Modified:
> llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp
>
> Added: llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test?rev=365162&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
> (added)
> +++ llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
> Thu Jul 4 15:45:27 2019
> @@ -0,0 +1,44 @@
> +## Test that permissions for ouput files are mirrored
> +## from their input files.
> +
> +## The Unix version of this test must use umask(1) because
> +## llvm-objcopy respects the umask in setting output permissions.
> +## Setting the umask to 0 ensures deterministic permissions across
> +## test environments.
> +# UNSUPPORTED: system-windows
> +
> +# RUN: touch %t
> +# RUN: chmod 0777 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0777
> +# RUN: chmod 0666 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0666
> +# RUN: chmod 0640 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0640
> +
> +## Set umask to be permissive of all permissions,
> +## only test mirroring of permissions.
> +# RUN: umask 0
> +
> +# RUN: yaml2obj %s -o %t
> +
> +# RUN: chmod 0777 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0777
> +
> +# RUN: chmod 0666 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0666
> +
> +# RUN: chmod 0640 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0640
> +
> +--- !ELF
> +FileHeader:
> + Class: ELFCLASS64
> + Data: ELFDATA2LSB
> + Type: ET_EXEC
> + Machine: EM_X86_64
>
> Added: llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test?rev=365162&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test
> (added)
> +++ llvm/trunk/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test Thu
> Jul 4 15:45:27 2019
> @@ -0,0 +1,39 @@
> +## Test that permissions for ouput files are mirrored
> +## from their input files.
> +
> +## The Unix version of this test is seperated because it needs
> +## to use umask(1). Windows has no umask, so it can be considered
> +## to be always 0, the required behavior.
> +# REQUIRES: system-windows
> +
> +# RUN: touch %t
> +# RUN: chmod 0777 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0777
> +# RUN: chmod 0666 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0666
> +# RUN: chmod 0640 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0640
> +
> +# RUN: yaml2obj %s -o %t
> +
> +# RUN: chmod 0777 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0777
> +
> +# RUN: chmod 0666 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0666
> +
> +# RUN: chmod 0640 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0640
> +
> +--- !ELF
> +FileHeader:
> + Class: ELFCLASS64
> + Data: ELFDATA2LSB
> + Type: ET_EXEC
> + Machine: EM_X86_64
>
> Added: llvm/trunk/test/tools/llvm-objcopy/ELF/respect-umask.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objcopy/ELF/respect-umask.test?rev=365162&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-objcopy/ELF/respect-umask.test (added)
> +++ llvm/trunk/test/tools/llvm-objcopy/ELF/respect-umask.test Thu Jul 4
> 15:45:27 2019
> @@ -0,0 +1,42 @@
> +## This tests that the umask is respected when
> +## assigning permissions of output files.
> +
> +## Windows has no umask so this test makes no sense, nor would
> +## it work because there is no umask(1) in a Windows environment
> +# UNSUPPORTED: system-windows
> +
> +# RUN: touch %t
> +# RUN: chmod 0755 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0755
> +# RUN: chmod 0500 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0500
> +# RUN: chmod 0555 %t
> +# RUN: ls -l %t | cut -f 1 -d ' ' > %t.0555
> +
> +# RUN: yaml2obj %s -o %t
> +
> +# RUN: umask 0022
> +# RUN: chmod 0777 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0755
> +
> +# RUN: umask 0237
> +# RUN: chmod 0707 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0500
> +
> +# RUN: umask 0222
> +# RUN: chmod 0777 %t
> +# RUN: llvm-objcopy %t %t1
> +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
> +# RUN: cmp %t1.perms %t.0555
> +
> +
> +--- !ELF
> +FileHeader:
> + Class: ELFCLASS64
> + Data: ELFDATA2LSB
> + Type: ET_EXEC
> + Machine: EM_X86_64
>
> Modified: llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp?rev=365162&r1=365161&r2=365162&view=diff
>
> ==============================================================================
> --- llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp (original)
> +++ llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp Thu Jul 4 15:45:27 2019
> @@ -196,16 +196,26 @@ static Error executeObjcopyOnArchive(con
> Config.DeterministicArchives, Ar.isThin());
> }
>
> -static Error restoreDateOnFile(StringRef Filename,
> - const sys::fs::file_status &Stat) {
> +static Error restoreStatOnFile(StringRef Filename,
> + const sys::fs::file_status &Stat,
> + bool PreserveDates) {
> int FD;
>
> + // Writing to stdout should not be treated as an error here, just
> + // do not set access/modification times or permissions.
> + if (Filename == "-")
> + return Error::success();
> +
> if (auto EC =
> sys::fs::openFileForWrite(Filename, FD,
> sys::fs::CD_OpenExisting))
> return createFileError(Filename, EC);
>
> - if (auto EC = sys::fs::setLastAccessAndModificationTime(
> - FD, Stat.getLastAccessedTime(), Stat.getLastModificationTime()))
> + if (PreserveDates)
> + if (auto EC = sys::fs::setLastAccessAndModificationTime(
> + FD, Stat.getLastAccessedTime(),
> Stat.getLastModificationTime()))
> + return createFileError(Filename, EC);
> +
> + if (auto EC = sys::fs::setPermissions(Filename, Stat.permissions()))
> return createFileError(Filename, EC);
>
> if (auto EC = sys::Process::SafelyCloseFileDescriptor(FD))
> @@ -219,9 +229,12 @@ static Error restoreDateOnFile(StringRef
> /// format-agnostic modifications, i.e. preserving dates.
> static Error executeObjcopy(const CopyConfig &Config) {
> sys::fs::file_status Stat;
> - if (Config.PreserveDates)
> + if (Config.InputFilename != "-") {
> if (auto EC = sys::fs::status(Config.InputFilename, Stat))
> return createFileError(Config.InputFilename, EC);
> + } else {
> + Stat.permissions(static_cast<sys::fs::perms>(0777));
> + }
>
> typedef Error (*ProcessRawFn)(const CopyConfig &, MemoryBuffer &,
> Buffer &);
> auto ProcessRaw = StringSwitch<ProcessRawFn>(Config.InputFormat)
> @@ -253,12 +266,15 @@ static Error executeObjcopy(const CopyCo
> }
> }
>
> - if (Config.PreserveDates) {
> - if (Error E = restoreDateOnFile(Config.OutputFilename, Stat))
> + if (Error E =
> + restoreStatOnFile(Config.OutputFilename, Stat,
> Config.PreserveDates))
> + return E;
> +
> + if (!Config.SplitDWO.empty()) {
> + Stat.permissions(static_cast<sys::fs::perms>(0666));
> + if (Error E =
> + restoreStatOnFile(Config.SplitDWO, Stat,
> Config.PreserveDates))
> return E;
> - if (!Config.SplitDWO.empty())
> - if (Error E = restoreDateOnFile(Config.SplitDWO, Stat))
> - return E;
> }
>
> return Error::success();
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190722/512da402/attachment.html>
More information about the llvm-commits
mailing list