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