Re: [patch] objcopy embeds the current time and ignores SOURCE_DATE_EPOCH making the output unreproducible

Re: [patch] objcopy embeds the current time and ignores SOURCE_DATE_EPOCH making the output unreproducible

[Nick Clifton]

Hi Johannes,

>>     * Updating the description of the --timestamp command line option in
>>       ld/ld.texi to mention that if SOURCE_DATE_EPOCH is defined in the
>>      environment then this will be used instead of the current time.
> 
> I'm a bit confused here. I'm fixing objcopy but am supposed to edit ld/ld.texi?

Ah - I think that we were both confused.  You see the code that you are patching
in bfd/peXXigen.c is used by both the linker and objcopy.  I was tracking down
where the timestamp field in the pe_data_type structure was set, and I got diverted
to ld/emultempl/pe.em, which is why I thought about updating the linker documentation.

So anyway, both the linker documentation *and* the objcopy documentation need to
be updated...

There is also the minor issue that there is no way to undo the effects of the
-p/--preserve-dates command line option.  This can be an issue for complex build
systems that like to build up command lines in stages, allowing later stages to
override the effects of earlier stages.  But that is not something that affects
the review of your patch.

> Also, that file does not contain docs for a --timestamp command line option. Do
> you mean the --insert-timestamp option?

I did.  Sorry.

> Also, there is another instance of something calling time(0) in ld/pe-dll.c. I
> don't know what that does. Does it need fixing as well?

Good catch.  Yes that will need the same kind of change as peXXigen.c.

If you could resubmit your patch (to the list) with these changes and accompanied
by a DCO then I will approve and apply it.

Cheers
   Nick

Re: [patch] objcopy embeds the current time and ignores SOURCE_DATE_EPOCH making the output unreproducible

[Johannes Schauer Marin Rodrigues]

[-- Attachment #1.1: Type: text/plain, Size: 1861 bytes --]

Hi,

Quoting Nick Clifton (2023-07-20 13:08:35)
> >>     * Updating the description of the --timestamp command line option in
> >>       ld/ld.texi to mention that if SOURCE_DATE_EPOCH is defined in the
> >>      environment then this will be used instead of the current time.
> > I'm a bit confused here. I'm fixing objcopy but am supposed to edit ld/ld.texi?
> Ah - I think that we were both confused.  You see the code that you are patching
> in bfd/peXXigen.c is used by both the linker and objcopy.  I was tracking down
> where the timestamp field in the pe_data_type structure was set, and I got diverted
> to ld/emultempl/pe.em, which is why I thought about updating the linker documentation.
> 
> So anyway, both the linker documentation *and* the objcopy documentation need to
> be updated...

where would you put it in the objcopy docs? I put it to the
--deterministic-libraries option because that's the option I was consulting
when I wanted to know how to make the timestamp reproducible.

> > Also, there is another instance of something calling time(0) in ld/pe-dll.c. I
> > don't know what that does. Does it need fixing as well?
> Good catch.  Yes that will need the same kind of change as peXXigen.c.

Done.

> If you could resubmit your patch (to the list)

Whoops, seems my last mail didn't have the list in CC.

> with these changes and accompanied
> by a DCO then I will approve and apply it.

With DCO you mean this, right? https://gcc.gnu.org/dco.html

I agree to the Developer's Certificate of Origin (version 1.1 or later) and
added a Signed-off-by to my commit accordingly.

If only minor problems remain with the patch, feel free to correct those,
depending on whether you'd spend less time with amending my patch or writing
me an email about the issues you found. :)

Thanks!

cheers, josch

[-- Attachment #1.2: 0001-bfd-peXXigen.c-respect-SOURCE_DATE_EPOCH-environment.patch --]
[-- Type: text/x-diff, Size: 3774 bytes --]

From bdbdaf4f9dace4ba60cb3debe9afc876a6567ffa Mon Sep 17 00:00:00 2001
From: Johannes Schauer Marin Rodrigues <josch@mister-muffin.de>
Date: Thu, 20 Jul 2023 07:11:44 +0200
Subject: [PATCH] bfd/peXXigen.c: respect SOURCE_DATE_EPOCH environment
 variable

Instead of obtaining the current time via time(0), use the seconds since
Unix epoch stored in the SOURCE_DATE_EPOCH environment variable to
create a reproducible timestamp.

Signed-off-by: Johannes Schauer Marin Rodrigues <josch@mister-muffin.de>
---
 bfd/peXXigen.c             | 11 ++++++++++-
 binutils/doc/binutils.texi |  4 ++++
 ld/ld.texi                 |  5 ++++-
 ld/pe-dll.c                | 11 ++++++++++-
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index da53f349dd0..7a5e5961162 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -838,7 +838,16 @@ _bfd_XXi_only_swap_filehdr_out (bfd * abfd, void * in, void * out)
   /* Use a real timestamp by default, unless the no-insert-timestamp
      option was chosen.  */
   if ((pe_data (abfd)->timestamp) == -1)
-    H_PUT_32 (abfd, time (0), filehdr_out->f_timdat);
+    {
+      time_t now;
+      char *source_date_epoch;
+      source_date_epoch = getenv("SOURCE_DATE_EPOCH");
+      if (source_date_epoch)
+	now = (time_t)strtoll(source_date_epoch, NULL, 10);
+      else
+	now = time(NULL);
+      H_PUT_32 (abfd, now, filehdr_out->f_timdat);
+    }
   else
     H_PUT_32 (abfd, pe_data (abfd)->timestamp, filehdr_out->f_timdat);
 
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 309bedf6110..ab974e1d304 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -4692,6 +4692,10 @@ When creating output libraries in response to either the
 the value of zero for any timestamps, user ids and group ids created
 (@option{--deterministic-libraries}) or the actual timestamps, user
 ids and group ids (@option{--non-deterministic-libraries}).
+Instead of inserting a zero value for the timestamp,
+an arbitrary reproducible timestamp can be inserted by setting the
+@code{SOURCE_DATE_EPOCH} environment variable to the desired number of
+seconds since Unix epoch.
 
 @item --export-all-symbols
 Treat all global and weak defined symbols found in the input object
diff --git a/ld/ld.texi b/ld/ld.texi
index 75e82eda004..02ace7778d9 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -3569,7 +3569,10 @@ will result in slightly different images being produced each time the
 same sources are linked.  The option @option{--no-insert-timestamp}
 can be used to insert a zero value for the timestamp, this ensuring
 that binaries produced from identical sources will compare
-identically.
+identically. Instead of inserting a zero value for the timestamp,
+an arbitrary reproducible timestamp can be inserted by setting the
+@code{SOURCE_DATE_EPOCH} environment variable to the desired number of
+seconds since Unix epoch.
 
 @kindex --enable-reloc-section
 @item --enable-reloc-section
diff --git a/ld/pe-dll.c b/ld/pe-dll.c
index 02e03d16948..e1465d4d115 100644
--- a/ld/pe-dll.c
+++ b/ld/pe-dll.c
@@ -1231,7 +1231,16 @@ fill_edata (bfd *abfd, struct bfd_link_info *info ATTRIBUTE_UNUSED)
   memset (edata_d, 0, edata_sz);
 
   if (pe_data (abfd)->timestamp == -1)
-    H_PUT_32 (abfd, time (0), edata_d + 4);
+    {
+      time_t now;
+      char *source_date_epoch;
+      source_date_epoch = getenv("SOURCE_DATE_EPOCH");
+      if (source_date_epoch)
+	now = (time_t)strtoll(source_date_epoch, NULL, 10);
+      else
+	now = time(NULL);
+      H_PUT_32 (abfd, now, edata_d + 4);
+    }
   else
     H_PUT_32 (abfd, pe_data (abfd)->timestamp, edata_d + 4);
 
-- 
2.40.0


[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEElFhU6KL81LF4wVq58sulx4+9g+EFAmS5v/YACgkQ8sulx4+9
g+EJLA/+IbEAZL6siT+skoitgEPhNiuUzIDp1ZjL7oYbGKhygbfCoNZ4JTFeqedh
9homL9b7BD8AAGjmjJ8q0bI3yVkmIl3s8ZrGvrjGQfx245u2Ej2+JDoxv1PqP+b+
md2FqmU0yT7pocwaihYwnHUSNGCSRW1jkV4jQ1yPgdn1h8i32l5ijghoMOft2FIi
+nG7teAOPmaMD+X//zKtbGlKL4XdOgfRwRx9EYpqwrdv6+X+n0l9/w9qWKB1gWiw
sC3jTO4c/WWclKG3Dcp2I2Jobzm4HNCocigas3E69kuLPmm0vXw9b8opP0dZFkpl
zWECHspvn+LuJQGAo56C/bMiSIxIYQ6uHIoMM6kO5zWHpLpQs7b913W67ZFWBS/T
uc+yNVD57XBKwvgghj9IX+ePYT7hFSPJPvuhuzcikBU06ZeWGg5iuUGs4o8cGbF4
DcUoC2stdhJ7IbR7Czo+6olqwNKhAAyqEFBL9MUHecCUgrw4xd6fjHi1J5bWETUP
2lhoe5QWQJPB6AeOsYVrFQXMo7dg4Rlf1zO+zmpJLvfIJxBEf3kE6N319zgF5Jvg
vGRR5iUmbWTOcWNRlos/JXVGRItlRGFxSE/baJPDxSTxCaqpHcTctBelVjMMnahZ
dS9RbiO3unxfU75KJIxiTeOymOBKt5mcQr99WTPwgZKr7COwaLg=
=a5wg
-----END PGP SIGNATURE-----

Re: [patch] objcopy embeds the current time and ignores SOURCE_DATE_EPOCH making the output unreproducible

[Nick Clifton]

Hi Johannes,

   Thanks very much for the revised patch.  I have gone ahead and applied
   it, with a few tweaks to the documentation part just to make things
   clearer (I hope...).

Cheers
   Nick

Re: [patch] objcopy embeds the current time and ignores SOURCE_DATE_EPOCH making the output unreproducible

[Nick Clifton]

Hi Matthias,

> This is forwarded from https://bugs.debian.org/1040450
> 
> is this suitable upstream, or should it be kept as a local patch? SOURCE_DATE_EPOCH is also respected in GCC upstream.

It is suitable for upstream.  (Plus I happen to know that Fedora would
like to have this functionality as well).

I would suggest a couple of small changes to the patch however:

   * Formatting fixes.  (Especially for the positioning of the curly braces).

   * Updating the description of the --timestamp command line option in
     ld/ld.texi to mention that if SOURCE_DATE_EPOCH is defined in the
    environment then this will be used instead of the current time.

Cheers
   Nick

[patch] objcopy embeds the current time and ignores SOURCE_DATE_EPOCH making the output unreproducible

[Matthias Klose]

This is forwarded from https://bugs.debian.org/1040450

is this suitable upstream, or should it be kept as a local patch? 
SOURCE_DATE_EPOCH is also respected in GCC upstream.


Hi,

steps to reproduce the unreproducibility:

     $ objcopy /usr/lib/systemd/boot/efi/linuxaa64.efi.stub bootaa64_1
     $ objcopy /usr/lib/systemd/boot/efi/linuxaa64.efi.stub bootaa64_2
     $ cmp bootaa64_1 bootaa64_2
     bootaa64_1 bootaa64_2 differ: byte 137, line 1

The resulting bootaa64.efi will have the local time embedded which makes it
unreproducible unless faketime is used. Setting SOURCE_DATE_EPOCH or adding
--enable-deterministic-archives does not make a difference.

I identified the part of the code that generates this timestamp and pasted a
preliminary patch fixing this issue to the end of this email. With that patch,
setting SOURCE_DATE_EPOCH=0 results in a file with:

$ objdump -x bootaa64.efi
[...]
Time/Date		Thu Jan  1 01:00:00 1970

Since building binutils takes 7.4 hours on my machine and since I have never
interacted with binutils upstream before, I'd appreciate if you could take it
from here and see that this problem gets fixed in binutils upstream the way
that its developers like to see this fixed.

Thanks!

cheers, josch

--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -847,9 +847,17 @@ _bfd_XXi_only_swap_filehdr_out (bfd * ab

    /* Use a real timestamp by default, unless the no-insert-timestamp
       option was chosen.  */
-  if ((pe_data (abfd)->timestamp) == -1)
-    H_PUT_32 (abfd, time (0), filehdr_out->f_timdat);
-  else
+  if ((pe_data (abfd)->timestamp) == -1) {
+    time_t now;
+    char *source_date_epoch;
+    source_date_epoch = getenv("SOURCE_DATE_EPOCH");
+    if (source_date_epoch) {
+      now = (time_t)strtoll(source_date_epoch, NULL, 10);
+    } else {
+      now = time(NULL);
+    }
+    H_PUT_32 (abfd, now, filehdr_out->f_timdat);
+  } else
      H_PUT_32 (abfd, pe_data (abfd)->timestamp, filehdr_out->f_timdat);

    PUT_FILEHDR_SYMPTR (abfd, filehdr_in->f_symptr,