[llvm-strip] Add support for -p/--preserve-dates

[rupprecht]

Summary

[llvm-strip] Preserve access/modification timestamps when -p is used.

[jhenderson]

test/tools/llvm-objcopy/strip-preserve-time.test

1

Have you run this test both on Windows and Linux to ensure the they work the same? I know a colleague had some issues with access times on Windows at one point, for example.

3

The comments in the file should end in a full stop, I think.

5

I'm slightly worried that this might lead to a flaky test, because there's a race condition here: if another process accesses the file some time between file creation and checking, then we'll presumably get a failure (and I could see a virus scanner doing just that...).

14

I'd prefer it if you use a different output file for each case. It'll make it easier to debug any issues with failing tests, because all of the different cases will be available.

tools/llvm-objcopy/llvm-objcopy.cpp

680

Should this be static?

[rupprecht]

• Use different objects in test/fix static-ness of stat restore method

[rupprecht]

test/tools/llvm-objcopy/strip-preserve-time.test

1

I don't have access to a Windows machine, but I'll ask a coworker to patch it and take a look.

I'd actually rather use stat if this is linux-only, but I assume that ls is more portable.

5

I'm not sure if there's a good solution for this. At the very least, I can watch build bots once this is submitted to see if this test causes flakiness. Very little time goes by between file creation and running llvm-strip, so it _probably_ won't cause issues, but I don't know how virus scanners work.

[jhenderson]

If I get a chance, I'll verify this on Windows. Otherwise, this LGTM.

test/tools/llvm-objcopy/strip-preserve-time.test

5

Yeah, I don't really know either. If it starts causing flakiness, we might just want to sacrifice that part of the test, and just verify it manually...

35

Nit: I don't think you need the 's' part of this command.

[rupprecht]

• Fix tests for windows compatibility

[rupprecht]

test/tools/llvm-objcopy/strip-preserve-time.test

5

So, apparently windows is fine with "ls -lu" etc., but barfs on the parens used to pipe several commands to FileCheck

$ "C:\src\clang\bin\llvm-strip.EXE" "-p" "C:\src\clang\test\tools\llvm-objcopy\Output\strip-preserve-time.test.tmp.1.o" "-o" "C:\src\clang\test\tools\llvm-objcopy\Output\strip-preserve-time.test.tmp-preserved.1.o"
$ ":" "RUN: at line 8"
$ "(ls" "-lu" "C:\src\clang\test\tools\llvm-objcopy\Output\strip-preserve-time.test.tmp-preserved.1.o"
# command stderr:
'(ls': command not found
error: command failed with exit status: 127

Splitting it into two with different access/modify time check-prefixes seems to work.

[rupprecht]

• crs->cr

[jhenderson]

llvm/trunk/test/tools/llvm-objcopy/strip-preserve-time.test

16–17 ↗

(On Diff #161082)

Great! I think what you've done here is fine, but for reference, you could have done it in an equally valid way using output redirection. I think either approach works though.

# RUN: ls -lu %t-preserved.2.o > %t.out.txt
# RUN: ls -l %t-preserved.2.o >> %t.out.txt
# RUN: FileCheck %s --input-file %t.out.txt --check-prefix=CHECK-PRESERVE

test/tools/llvm-objcopy/strip-preserve-time.test

+# Note: ls -lu prints the accessed timestamp, ls -l prints the modified timestamp
+
+# Preserve dates when stripping to an output file
+# RUN: yaml2obj %s > %t.o
+# RUN: touch -a -t 199505050555.55 %t.o
+# RUN: touch -m -t 199705050555.55 %t.o
+# RUN: llvm-strip -p %t.o -o %t-preserved.o
+# RUN: (ls -lu %t-preserved.o; ls -l %t-preserved.o) | FileCheck %s --check-prefix=CHECK-PRESERVE
+
+# Preserve dates available via objcopy interface as well
+# RUN: yaml2obj %s > %t.o
+# RUN: touch -a -t 199505050555.55 %t.o
+# RUN: touch -m -t 199705050555.55 %t.o
+# RUN: llvm-objcopy -p %t.o -o %t-preserved.o
+# RUN: (ls -lu %t-preserved.o; ls -l %t-preserved.o) | FileCheck %s --check-prefix=CHECK-PRESERVE
+
+# Preserve dates when stripping in place
+# RUN: yaml2obj %s > %t.o
+# RUN: touch -a -t 199505050555.55 %t.o
+# RUN: touch -m -t 199705050555.55 %t.o
+# RUN: llvm-strip -p %t.o
+# RUN: (ls -lu %t.o; ls -l %t.o) | FileCheck %s --check-prefix=CHECK-PRESERVE
+
+# Without -p set, don't preserve dates
+# RUN: yaml2obj %s > %t.o
+# RUN: touch -a -t 199505050555.55 %t.o
+# RUN: touch -m -t 199705050555.55 %t.o
+# RUN: llvm-strip %t.o
+# RUN: (ls -lu %t.o; ls -l %t.o) | FileCheck %s --check-prefix=CHECK-NO-PRESERVE
+
+# Preserve dates in archives
+# RUN: yaml2obj %s > %t.o
+# RUN: rm -f %t.a
+# RUN: llvm-ar crs %t.a %t.o
+# RUN: touch -a -t 199505050555.55 %t.a
+# RUN: touch -m -t 199705050555.55 %t.a
+# RUN: llvm-strip -p %t.a
+# RUN: (ls -lu %t.a; ls -l %t.a) | FileCheck %s --check-prefix=CHECK-PRESERVE
+
+# Preserve dates in split DWO files
+# RUN: cp %p/Inputs/dwarf.dwo %t-input.dwo
+# RUN: touch -a -t 199505050555.55 %t-input.dwo
+# RUN: touch -m -t 199705050555.55 %t-input.dwo
+# RUN: llvm-objcopy -p -split-dwo=%t-dwo %t-input.dwo %t-nondwo
+# RUN: (ls -lu %t-dwo; ls -l %t-dwo) | FileCheck %s --check-prefix=CHECK-PRESERVE
+# RUN: (ls -lu %t-nondwo; ls -l %t-nondwo) | FileCheck %s --check-prefix=CHECK-PRESERVE
+
+# CHECK-PRESERVE:         1995
+# CHECK-PRESERVE-NEXT:    1997
+
+# CHECK-NO-PRESERVE-NOT:  1995
+# CHECK-NO-PRESERVE-NOT:  1997
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]

tools/llvm-objcopy/ObjcopyOpts.td

 defm output_target : Eq<"output-target">,
                      HelpText<"Format of the output file">,
                      Values<"binary">;
 def O : JoinedOrSeparate<["-"], "O">,
         Alias<output_target>;
 defm split_dwo : Eq<"split-dwo">,
                  MetaVarName<"dwo-file">,
                  HelpText<"Equivalent to extract-dwo on the input file to <dwo-file>, then strip-dwo on the input file">;
+
+def preserve_dates : Flag<[ "-", "--" ], "preserve-dates">,
+                     HelpText<"Preserve access and modification timestamps">;
+
+def p : Flag<[ "-" ], "p">, Alias<preserve_dates>;
+
 defm add_gnu_debuglink : Eq<"add-gnu-debuglink">,
                          MetaVarName<"debug-file">,
                          HelpText<"Add a .gnu_debuglink for <debug-file>">;
 defm remove_section : Eq<"remove-section">,
                       MetaVarName<"section">,
                       HelpText<"Remove <section>">;
 defm rename_section
     : Eq<"rename-section">,

tools/llvm-objcopy/StripOpts.td

 include "llvm/Option/OptParser.td"
 
 multiclass Eq<string name> {
   def NAME: Separate<["--", "-"], name>;
   def NAME # _eq: Joined<["--", "-"], name # "=">, Alias<!cast<Separate>(NAME)>;
 }
 
 def help : Flag<["-", "--"], "help">;
 
 defm output : Eq<"o">,
               MetaVarName<"output">,
               HelpText<"Write output to <file>">;
 
+def preserve_dates : Flag<[ "-", "--" ], "preserve-dates">,
+                     HelpText<"Preserve access and modification timestamps">;
+
+def p : Flag<[ "-" ], "p">, Alias<preserve_dates>;
+
 def strip_all : Flag<["-", "--"], "strip-all">,
                 HelpText<"Remove non-allocated sections other than .gnu.warning* sections">;
 
 def strip_debug : Flag<["-", "--"], "strip-debug">,
                   HelpText<"Remove debugging symbols only">;
 
 def d : Flag<["-"], "d">,
         Alias<strip_debug>;

tools/llvm-objcopy/llvm-objcopy.cpp

 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdlib>
 #include <functional>
 #include <iterator>
 #include <memory>
struct CopyConfig {
   bool StripDWO = false;
   bool StripUnneeded = false;
   bool ExtractDWO = false;
   bool LocalizeHidden = false;
   bool Weaken = false;
   bool DiscardAll = false;
   bool OnlyKeepDebug = false;
   bool KeepFileSymbols = false;
+  bool PreserveDates = false;
 };
 
 using SectionPred = std::function<bool(const SectionBase &Sec)>;
 
 enum SectionFlag {
   SecNone = 0,
   SecAlloc = 1 << 0,
   SecLoad = 1 << 1,
static void executeElfObjcopyOnArchive(const CopyConfig &Config,
   if (Err)
     reportError(Config.InputFilename, std::move(Err));
   if (Error E =
           deepWriteArchive(Config.OutputFilename, NewArchiveMembers,
                            Ar.hasSymbolTable(), Ar.kind(), true, Ar.isThin()))
     reportError(Config.OutputFilename, std::move(E));
 }
 
+void restoreDateOnFile(StringRef Filename, const sys::fs::file_status &Stat) {
+  int FD;
+
+  if (auto EC = sys::fs::openFileForWrite(Filename, FD))
+    reportError(Filename, EC);
+
+  if (auto EC = sys::fs::setLastAccessAndModificationTime(
+          FD, Stat.getLastAccessedTime(), Stat.getLastModificationTime()))
+    reportError(Filename, EC);
+
+  if (auto EC = sys::Process::SafelyCloseFileDescriptor(FD))
+    reportError(Filename, EC);
+}
+
 static void executeElfObjcopy(const CopyConfig &Config) {
+  sys::fs::file_status Stat;
+  if (Config.PreserveDates)
+    if (auto EC = sys::fs::status(Config.InputFilename, Stat))
+      reportError(Config.InputFilename, EC);
+
   Expected<OwningBinary<llvm::object::Binary>> BinaryOrErr =
       createBinary(Config.InputFilename);
   if (!BinaryOrErr)
     reportError(Config.InputFilename, BinaryOrErr.takeError());
 
-  if (Archive *Ar = dyn_cast<Archive>(BinaryOrErr.get().getBinary()))
-    return executeElfObjcopyOnArchive(Config, *Ar);
-
+  if (Archive *Ar = dyn_cast<Archive>(BinaryOrErr.get().getBinary())) {
+    executeElfObjcopyOnArchive(Config, *Ar);
+  } else {
     FileBuffer FB(Config.OutputFilename);
     executeElfObjcopyOnBinary(Config, *BinaryOrErr.get().getBinary(), FB);
   }
 
+  if (Config.PreserveDates) {
+    restoreDateOnFile(Config.OutputFilename, Stat);
+    if (!Config.SplitDWO.empty())
+      restoreDateOnFile(Config.SplitDWO, Stat);
+  }
+}
+
 // ParseObjcopyOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseObjcopyOptions will print the help messege and
 // exit.
 static CopyConfig parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   ObjcopyOptTable T;
   unsigned MissingArgumentIndex, MissingArgumentCount;
   llvm::opt::InputArgList InputArgs =
       T.ParseArgs(ArgsArr, MissingArgumentIndex, MissingArgumentCount);
for (auto Arg : InputArgs.filtered(OBJCOPY_globalize_symbol))
     Config.SymbolsToGlobalize.push_back(Arg->getValue());
   for (auto Arg : InputArgs.filtered(OBJCOPY_weaken_symbol))
     Config.SymbolsToWeaken.push_back(Arg->getValue());
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbol))
     Config.SymbolsToRemove.push_back(Arg->getValue());
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_symbol))
     Config.SymbolsToKeep.push_back(Arg->getValue());
 
+  Config.PreserveDates = InputArgs.hasArg(OBJCOPY_preserve_dates);
+
   return Config;
 }
 
 // ParseStripOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseStripOptions will print the help messege and
 // exit.
 static CopyConfig parseStripOptions(ArrayRef<const char *> ArgsArr) {
   StripOptTable T;
if (!Config.StripDebug && !Config.StripUnneeded && !Config.DiscardAll)
     Config.StripAll = true;
 
   for (auto Arg : InputArgs.filtered(STRIP_remove_section))
     Config.ToRemove.push_back(Arg->getValue());
 
   for (auto Arg : InputArgs.filtered(STRIP_keep_symbol))
     Config.SymbolsToKeep.push_back(Arg->getValue());
 
+  Config.PreserveDates = InputArgs.hasArg(STRIP_preserve_dates);
+
   return Config;
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   ToolName = argv[0];
   CopyConfig Config;
   if (sys::path::stem(ToolName).endswith_lower("strip"))
     Config = parseStripOptions(makeArrayRef(argv + 1, argc));
   else
     Config = parseObjcopyOptions(makeArrayRef(argv + 1, argc));
   executeElfObjcopy(Config);
 }