[PATCH] D46966: [lld] Use a real timestamp, with the option to use hash or explicit value

[Hans Wennborg via Phabricator via llvm-commits]

hans added inline comments.


================
Comment at: lld/COFF/Driver.cpp:1045
+      if (Value.getAsInteger(0, Config->Timestamp))
+        fatal("invalid timestamp: " + Value);
+    }
----------------
Maybe the error message could be clearer, indicating that it expects an integer?


================
Comment at: lld/COFF/Options.td:61
 def subsystem : P<"subsystem", "Specify subsystem">;
+def timestamp : P<"timestamp", "Specify the timestamp to be written to the PE header">;
 def version : P<"version", "Specify a version number in the PE header">;
----------------
Perhaps just the shorter "Specify the PE header timestamp" is enough?


================
Comment at: lld/test/COFF/timestamp.test:9
+RUN: llvm-readobj -file-headers -coff-debug-directory %t.3.exe | FileCheck %s --check-prefix=ZERO
+
+HASH: ImageFileHeader {
----------------
Should there be a test for the default case, without /Brepro or /timestamp?


https://reviews.llvm.org/D46966