[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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365162 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test b/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
new file mode 100644 (file)
index 0000000..0c1f6b9
--- /dev/null
+++ b/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
@@ -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

diff --git a/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test b/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test
new file mode 100644 (file)
index 0000000..c35df34
--- /dev/null
+++ b/test/tools/llvm-objcopy/ELF/mirror-permissions-win.test
@@ -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

diff --git a/test/tools/llvm-objcopy/ELF/respect-umask.test b/test/tools/llvm-objcopy/ELF/respect-umask.test
new file mode 100644 (file)
index 0000000..4236c67
--- /dev/null
+++ b/test/tools/llvm-objcopy/ELF/respect-umask.test
@@ -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

diff --git a/tools/llvm-objcopy/llvm-objcopy.cpp b/tools/llvm-objcopy/llvm-objcopy.cpp
index 2ab77ea5c8684756152541d22f3111ccf92c7fc3..db5609ea2294c2d4142ffc1b4cd07a13b69d5ff8 100644 (file)
--- a/tools/llvm-objcopy/llvm-objcopy.cpp
+++ b/tools/llvm-objcopy/llvm-objcopy.cpp
@@ -196,16 +196,26 @@ static Error executeObjcopyOnArchive(const CopyConfig &Config,
                           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 Filename,
 /// 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 CopyConfig &Config) {
     }
   }
 
-  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();