Page MenuHomePhabricator

Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__
Needs ReviewPublic

Authored by thakis on Aug 26 2016, 10:28 AM.

Details

Reviewers
rsmith
Summary

This is useful for creating reproducible builds. Defining these via -D triggers warnings, and can cause bugs (presently, e.g. PR29119). It makes it also impossible to override DATE and TIME and forget TIMESTAMP.

TIMESTAMP has slightly different semantics in that it's the timestamp of the current TU, not the time of compilation, but either meaning is bad for reproducible builds.

Diff Detail

Event Timeline

thakis retitled this revision from to Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.
thakis updated this object.
thakis added a reviewer: rsmith.
thakis added a subscriber: cfe-commits.
bruno added a subscriber: bruno.Aug 30 2016, 11:29 AM
ed added a subscriber: ed.Dec 28 2016, 4:23 AM

I'd be interested in seeing a feature like this appearing. Any chance this feature may be part of Clang 4.0?

In D23934#631656, @ed wrote:

I'd be interested in seeing a feature like this appearing.

I agree.

lib/Driver/Tools.cpp
4687

This seems like a fairly random place for this code. It is far removed from any other handling of preprocessor-related options.

5996

Somewhere around here would make more sense.

ed added a subscriber: emaste.Dec 29 2016, 3:10 AM

Please also accept SOURCE_DATE_EPOCH set in the environment -- see https://reproducible-builds.org/specs/source-date-epoch/

Also although I'm generally leery of options auto-detecting the argument format, I think we should be able to pass an epoch timestamp to -ffixed-date-time.

Please also accept SOURCE_DATE_EPOCH set in the environment -- see https://reproducible-builds.org/specs/source-date-epoch/

It looks like there's reasonable adoption of this: https://wiki.debian.org/ReproducibleBuilds/TimestampsProposal#Reading_the_variable and gcc >= 7 will support it.

Also although I'm generally leery of options auto-detecting the argument format, I think we should be able to pass an epoch timestamp to -ffixed-date-time.

I'd also find this convenient. The auto-detection should be unambiguous (if it is an integer, then it is an epoch).

bruno added a subscriber: vsapsai.Jan 2 2018, 2:52 PM

Cannot tell for sure from code but looks like we are still emitting -Wdate-time "expansion of date or time macro is not reproducible" with this flag. At least it's not covered by tests. And another similar warning is -Wpch-date-time.

efriedma added inline comments.
lib/Frontend/CompilerInvocation.cpp
2252

Does using mktime like this depend on the local timezone?

What is the status of this issue?

Alexander added inline comments.Wed, Oct 16, 9:24 AM
lib/Frontend/CompilerInvocation.cpp
2252

Since the return value of mktime() is ignored here, its dependence on the local timezone should not matter. The main effect of this call is to fill in tm_isdst flag with the best guess. This may result in unexpected results if fixed-date-time is set inside the "spring forward" hour for the local timezone, but this issue is inherent in the ISO 8601 format.