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

[Zachary Turner via llvm-commits]

On Thu, May 17, 2018 at 1:55 AM Hans Wennborg via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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?
>

I thought about it, but I'm not sure how to write a test case for that,
since the timestamp will be whatever the time is when the test is run.

I'll land it now to unblock the roll, and if anyone comes up with a good
way to write a test case for the default, I'll add that as a followup.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180517/2650c5c7/attachment.html>