Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLD] [COFF] Pick timestamps from the SOURCE_DATE_EPOCH variable #81326

Merged
merged 2 commits into from
Feb 10, 2024

Conversation

mstorsjo
Copy link
Member

@mstorsjo mstorsjo commented Feb 9, 2024

The SOURCE_DATE_EPOCH environment variable can be set in order to get reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the current time, unless either the /timestamp: or /Brepro option is set. If neither of them is set, check the SOURCE_DATE_EPOCH variable, before resorting to using the actual current date and time.

The SOURCE_DATE_EPOCH environment variable can be set in order
to get reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is
set to the current time, unless either the /timestamp: or
/Brepro option is set. If neither of them is set, check the
SOURCE_DATE_EPOCH variable, before resorting to using the
actual current date and time.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 9, 2024

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-platform-windows

Author: Martin Storsjö (mstorsjo)

Changes

The SOURCE_DATE_EPOCH environment variable can be set in order to get reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the current time, unless either the /timestamp: or /Brepro option is set. If neither of them is set, check the SOURCE_DATE_EPOCH variable, before resorting to using the actual current date and time.


Full diff: https://github.com/llvm/llvm-project/pull/81326.diff

2 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+9-1)
  • (modified) lld/test/COFF/timestamp.test (+2)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index e0afb6b18805b2..22ee2f133be98a 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1825,7 +1825,15 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     }
   } else {
     config->repro = false;
-    config->timestamp = time(nullptr);
+    if (std::optional<std::string> epoch =
+            Process::GetEnv("SOURCE_DATE_EPOCH")) {
+      StringRef value(*epoch);
+      if (value.getAsInteger(0, config->timestamp))
+        fatal(Twine("invalid SOURCE_DATE_EPOCH timestamp: ") + value +
+              ".  Expected 32-bit integer");
+    } else {
+      config->timestamp = time(nullptr);
+    }
   }
 
   // Handle /alternatename
diff --git a/lld/test/COFF/timestamp.test b/lld/test/COFF/timestamp.test
index fbdc5788a33a55..f94eeee8793f01 100644
--- a/lld/test/COFF/timestamp.test
+++ b/lld/test/COFF/timestamp.test
@@ -3,9 +3,11 @@ RUN: yaml2obj %p/Inputs/generic.yaml -o %t.obj
 RUN: lld-link %t.obj /debug /Brepro /entry:main /nodefaultlib /out:%t.1.exe
 RUN: lld-link %t.obj /debug /Brepro /entry:main /nodefaultlib /out:%t.2.exe
 RUN: lld-link %t.obj /debug /timestamp:0 /entry:main /nodefaultlib /out:%t.3.exe
+RUN: env SOURCE_DATE_EPOCH=0 lld-link %t.obj /debug /entry:main /nodefaultlib /out:%t.4.exe
 RUN: llvm-readobj --file-headers --coff-debug-directory %t.1.exe | FileCheck %s --check-prefix=HASH
 RUN: llvm-readobj --file-headers --coff-debug-directory %t.2.exe | FileCheck %s --check-prefix=HASH
 RUN: llvm-readobj --file-headers --coff-debug-directory %t.3.exe | FileCheck %s --check-prefix=ZERO
+RUN: llvm-readobj --file-headers --coff-debug-directory %t.4.exe | FileCheck %s --check-prefix=ZERO
 
 HASH: ImageFileHeader {
 HASH: TimeDateStamp: [[STAMP:.*]]

@@ -3,9 +3,11 @@ RUN: yaml2obj %p/Inputs/generic.yaml -o %t.obj
RUN: lld-link %t.obj /debug /Brepro /entry:main /nodefaultlib /out:%t.1.exe
RUN: lld-link %t.obj /debug /Brepro /entry:main /nodefaultlib /out:%t.2.exe
RUN: lld-link %t.obj /debug /timestamp:0 /entry:main /nodefaultlib /out:%t.3.exe
RUN: env SOURCE_DATE_EPOCH=0 lld-link %t.obj /debug /entry:main /nodefaultlib /out:%t.4.exe
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be useful to test another value like clang/test/Preprocessor/SOURCE_DATE_EPOCH.c

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I added more tests for more interesting cases.

What do you think of backporting this to 18.x? It’s not a bug fix per se, but a pretty safe improvement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for a backport to improve reproducibility!

https://reproducible-builds.org/

@aganea
Copy link
Member

aganea commented Feb 10, 2024

There’s not much info online about this. Do you know where it was first introduced?

@aganea
Copy link
Member

aganea commented Feb 10, 2024

@mstorsjo mstorsjo merged commit 0df8aed into llvm:main Feb 10, 2024
4 checks passed
@mstorsjo mstorsjo deleted the lld-source-date-epoch branch February 10, 2024 21:57
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 10, 2024
…m#81326)

The SOURCE_DATE_EPOCH environment variable can be set in order to get
reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the
current time, unless either the /timestamp: or /Brepro option is set. If
neither of them is set, check the SOURCE_DATE_EPOCH variable, before
resorting to using the actual current date and time.

See https://reproducible-builds.org/docs/source-date-epoch/ for reference
on the use of this variable.

(cherry picked from commit 0df8aed)
@aganea
Copy link
Member

aganea commented Feb 10, 2024

Does this deserve an update of the release notes for 18.0?

@mstorsjo
Copy link
Member Author

Does this deserve an update of the release notes for 18.0?

Good point - yeah I think it does; I can make a separate PR to update the notes on the branch.

@MaskRay
Copy link
Member

MaskRay commented Feb 10, 2024

Does this deserve an update of the release notes for 18.0?

Good point - yeah I think it does; I can make a separate PR to update the notes on the branch.

The release notes for non-ELF ports are a barebone. @mstorsjo Do you want to find all interesting commits using git log origin/release/17.x..origin/release/18.x -- lld/COFF?

You may create a feature request with the base branch set to release/18.x and then add the release 18.x label.

@mstorsjo
Copy link
Member Author

The release notes for non-ELF ports are a barebone. @mstorsjo Do you want to find all interesting commits using git log origin/release/17.x..origin/release/18.x -- lld/COFF?

I did something similar already - filling in the stuff which is already there - but I only looked at the commits that I have been involved in myself, that I'm more familiar with. But I guess it would be useful to have a look through all of them - it's just a little bit more work. I'll see if I can get to it in the next few days.

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 11, 2024
…m#81326)

The SOURCE_DATE_EPOCH environment variable can be set in order to get
reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the
current time, unless either the /timestamp: or /Brepro option is set. If
neither of them is set, check the SOURCE_DATE_EPOCH variable, before
resorting to using the actual current date and time.

See https://reproducible-builds.org/docs/source-date-epoch/ for reference
on the use of this variable.

(cherry picked from commit 0df8aed)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 11, 2024
…m#81326)

The SOURCE_DATE_EPOCH environment variable can be set in order to get
reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the
current time, unless either the /timestamp: or /Brepro option is set. If
neither of them is set, check the SOURCE_DATE_EPOCH variable, before
resorting to using the actual current date and time.

See https://reproducible-builds.org/docs/source-date-epoch/ for reference
on the use of this variable.

(cherry picked from commit 0df8aed)
@DavidSpickett
Copy link
Collaborator

@mstorsjo
Copy link
Member Author

FYI: https://lab.llvm.org/buildbot/#/builders/178/builds/6797/steps/7/logs/FAIL__lld__timestamp_test

Only happens on the 32 bit bot.

Oops, sorry about that, and sorry for not noticing the buildbot mail earlier.

It looks like the code itself is right, but llvm-readobj might not be able to print the value correctly on hosts with a 32 bit time_t.

Although, https://learn.microsoft.com/en-us/windows/win32/debug/pe-format doesn't seem to say anything explicitly about the signedness of this field either, but treating it as an unsigned seems reasonable. It does say that the value 0xFFFFFFFF that we're using in the test doesn't represent a real timestamp. So perhaps we should test with a different value.

I guess the simplest way forward is to test with 0x7FFFFFFF which should work safely everywhere. (And potentially test with 0xFFFFFFFE if we know the host isn't limited to a 32 bit time_t?)

tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…m#81326)

The SOURCE_DATE_EPOCH environment variable can be set in order to get
reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the
current time, unless either the /timestamp: or /Brepro option is set. If
neither of them is set, check the SOURCE_DATE_EPOCH variable, before
resorting to using the actual current date and time.

See https://reproducible-builds.org/docs/source-date-epoch/ for reference
on the use of this variable.

(cherry picked from commit 0df8aed)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…m#81326)

The SOURCE_DATE_EPOCH environment variable can be set in order to get
reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the
current time, unless either the /timestamp: or /Brepro option is set. If
neither of them is set, check the SOURCE_DATE_EPOCH variable, before
resorting to using the actual current date and time.

See https://reproducible-builds.org/docs/source-date-epoch/ for reference
on the use of this variable.

(cherry picked from commit 0df8aed)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…m#81326)

The SOURCE_DATE_EPOCH environment variable can be set in order to get
reproducible build.

When linking PE/COFF modules with LLD, the timestamp field is set to the
current time, unless either the /timestamp: or /Brepro option is set. If
neither of them is set, check the SOURCE_DATE_EPOCH variable, before
resorting to using the actual current date and time.

See https://reproducible-builds.org/docs/source-date-epoch/ for reference
on the use of this variable.

(cherry picked from commit 0df8aed)
vszakats added a commit to curl/curl-for-win that referenced this pull request Mar 11, 2024
lld from llvm 18 respects it. We are doing our own PE cleaning for now,
but this could eventually take over (once `debian:testing` switches to
18 and possibly `ld` also implements it.)

https://releases.llvm.org/18.1.0/tools/lld/docs/ReleaseNotes.html#coff-improvements
llvm/llvm-project#81326
vszakats added a commit to curl/curl-for-win that referenced this pull request Mar 11, 2024
lld from llvm 18 respects it. We are doing our own PE cleaning for now,
but this could eventually take over (once `debian:testing` switches to
18 and possibly `ld` also implements it.)

https://releases.llvm.org/18.1.0/tools/lld/docs/ReleaseNotes.html#coff-improvements
llvm/llvm-project#81326
@pointhex pointhex mentioned this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants