From: Eduard Sanou <eduardsanou@openmailbox.org>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Dhole <dhole@openmailbox.org>,
Bernd Schmidt <bschmidt@redhat.com>,
Matthias Klose <doko@ubuntu.com>,
gcc-patches@gcc.gnu.org
Subject: Re: Allow embedded timestamps by C/C++ macros to be set externally (3)
Date: Thu, 05 May 2016 23:28:00 -0000 [thread overview]
Message-ID: <20160505232654.GW21574@panther> (raw)
In-Reply-To: <20160429071744.GX26501@tucnak.zalov.cz>
[-- Attachment #1.1: Type: text/plain, Size: 3024 bytes --]
Hi,
I've worked on applying the changes that have been commented in several
messages from this thread. Find the patch and changelog attached.
On 16-04-29 09:17:44, Jakub Jelinek wrote:
> First of all, using error instead of fatal_error achieves just that too,
> except that it allows also detecting other errors in the source.
> fatal_error is meant for cases where there is no way to go forward with the
> compilation, while here exists a perfectly reasonable way to go forward (assume
> the env var is not set, use a hardcoded timestamp, ...).
I've changed the use of fatal_error to error_at. Now the parsing will
return -1 if there's an error, and gcc will be able to output the line
where the __DATE__ or __TIME__ macro was used that caused the error.
> Doing this on the gcc/ side is of course reasonable, but can be done through
> callbacks, libcpp already has lots of other callbacks into the gcc/ code,
> look for e.g. cpp_get_callbacks in gcc/c-family/* and in libcpp/ for
> corresponding code.
I've added a callback that allows parsing the SOURCE_DATE_EPOCH env var
right at the point where the date macros are expanded. I've removed
some uneeded stuff from my previous patch.
> Also, as a follow-up, guess the driver should set this
> env var for the -fcompare-debug case if not already set, to something that
> matches the current date, so that __TIME__ macros expands the same in
> between both compilations, even when they don't compile both in the same
> second.
I've also patched the driver to set the SOURCE_DATE_EPOCH env var to the
current date when the -fcompare-debug is set. I've tested this building
llvm with such flag.
Markus could you verify that the patch indeed fixes the issue?
Martin Sebor wrote:
> I'm sorry I'm a little late but I have a couple of minor comments
> on the patch:
Thanks for the comments :)
> In most texts (e.g. the C and POSIX standards), the name of
> an environment variable doesn't include the dollar sign. In
> other diagnostic messages GCC doesn't print one. I suggest
> to follow the established practice and remove the dollar sign
> from this error message as well.
Change applied.
> I would also suggest to issue a single generic error message
> explaining what the valid value of the variable is instead of
> trying to describe what's wrong with it, for example as follows
> (note also the hyphen in "non-negative" which is the prevalent
> style used by other GCC messages and GNU documentation).
>
> "environment variable SOURCE_DATE_EPOCH must expand to a non-
> negative integer less than or equal to %qlli", LLONG_MAX
Applied as well.
> The +%s option to the date command is a non-standard extension
> that's not universally available. To avoid confusing users on
> systems that don't support it I would suggest to either avoid
> mentioning or to clarify that it's a Linux command.
I've added a comment to clarify this fact in the documentation.
Cheers,
Dhole
[-- Attachment #1.2: ChangeLog --]
[-- Type: text/plain, Size: 1350 bytes --]
gcc/c-family/ChangeLog:
2016-05-05 Eduard Sanou <dhole@openmailbox.org>
* c-common.c (get_source_date_epoch): Renamed to
cb_get_source_date_epoch.
* c-common.c (cb_get_source_date_epoch): Use a single generic erorr
message when the parsing fails. Use error_at instead of fatal_error.
* c-common.h (get_source_date_epoch): Renamed to
cb_get_source_date_epoch.
* c-common.h (cb_get_source_date_epoch): Prototype.
* c-common.h (MAX_SOURCE_DATE_EPOCH): Define.
* c-lex.c (init_c_lex): Set cb->get_source_date_epoch callback.
* c-lex.c (c_lex_with_flags): Remove initialization of
source_date_epoch.
gcc/ChangeLog:
2016-05-05 Eduard Sanou <dhole@openmailbox.org>
* doc/cppenv.texi: Note that the `%s` in `date` is a non-standard
extension.
* gcc.c (driver_handle_option): Call
setenv_SOURCE_DATE_EPOCH_current_time.
* gcc.c (setenv_SOURCE_DATE_EPOCH_current_time): New function, sets
the SOURCE_DATE_EPOCH environment variable to the current time.
libcpp/ChangeLog:
2016-05-05 Eduard Sanou <dhole@openmailbox.org>
* include/cpplib.h (cpp_callbacks): Add get_source_date_epoch
callback.
* include/cpplib.h (cpp_init_source_date_epoch): Remove.
* init.c (cpp_init_source_date_epoch): Remove.
* internal.h (cpp_reader): Remove source_date_epoch.
* macro.c (_cpp_builtin_macro_text): Use get_source_date_epoch
callback.
[-- Attachment #1.3: gcc-SOURCE_DATE_EPOCH-patch-2016_05_05.diff.txt --]
[-- Type: text/plain, Size: 9313 bytes --]
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index d45bf1b..22974c5 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12788,7 +12788,7 @@ valid_array_size_p (location_t loc, tree type, tree name)
timestamp to replace embedded current dates to get reproducible
results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
time_t
-get_source_date_epoch ()
+cb_get_source_date_epoch (cpp_reader *pfile ATTRIBUTE_UNUSED)
{
char *source_date_epoch;
long long epoch;
@@ -12800,19 +12800,14 @@ get_source_date_epoch ()
errno = 0;
epoch = strtoll (source_date_epoch, &endptr, 10);
- if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN))
- || (errno != 0 && epoch == 0))
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "strtoll: %s\n", xstrerror(errno));
- if (endptr == source_date_epoch)
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "no digits were found: %s\n", endptr);
- if (*endptr != '\0')
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "trailing garbage: %s\n", endptr);
- if (epoch < 0)
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "value must be nonnegative: %lld \n", epoch);
+ if (errno != 0 || endptr == source_date_epoch || *endptr != '\0'
+ || epoch < 0 || epoch > MAX_SOURCE_DATE_EPOCH)
+ {
+ error_at (input_location, "environment variable SOURCE_DATE_EPOCH must "
+ "expand to a non-negative integer less than or equal to %ld",
+ MAX_SOURCE_DATE_EPOCH);
+ return (time_t) -1;
+ }
return (time_t) epoch;
}
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 1714284..dea2900 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1086,6 +1086,16 @@ extern vec<tree, va_gc> *make_tree_vector_copy (const vec<tree, va_gc> *);
c_register_builtin_type. */
extern GTY(()) tree registered_builtin_types;
+/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
+ timestamp to replace embedded current dates to get reproducible
+ results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
+extern time_t cb_get_source_date_epoch (cpp_reader *pfile);
+
+/* The value (as a unix timestamp) corresponds to date
+ "Dec 31 9999 23:59:59 UTC", which is the latest date that __DATE__ and
+ __TIME__ can store. */
+#define MAX_SOURCE_DATE_EPOCH 253402300799
+
/* In c-gimplify.c */
extern void c_genericize (tree);
extern int c_gimplify_expr (tree *, gimple_seq *, gimple_seq *);
@@ -1480,9 +1490,4 @@ extern bool valid_array_size_p (location_t, tree, tree);
extern bool cilk_ignorable_spawn_rhs_op (tree);
extern bool cilk_recognize_spawn (tree, tree *);
-/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
- timestamp to replace embedded current dates to get reproducible
- results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
-extern time_t get_source_date_epoch (void);
-
#endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 38a428d..8f33d86 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -80,6 +80,7 @@ init_c_lex (void)
cb->valid_pch = c_common_valid_pch;
cb->read_pch = c_common_read_pch;
cb->has_attribute = c_common_has_attribute;
+ cb->get_source_date_epoch = cb_get_source_date_epoch;
/* Set the debug callbacks if we can use them. */
if ((debug_info_level == DINFO_LEVEL_VERBOSE
@@ -389,9 +390,6 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
enum cpp_ttype type;
unsigned char add_flags = 0;
enum overflow_type overflow = OT_NONE;
- time_t source_date_epoch = get_source_date_epoch ();
-
- cpp_init_source_date_epoch (parse_in, source_date_epoch);
timevar_push (TV_CPP);
retry:
diff --git a/gcc/doc/cppenv.texi b/gcc/doc/cppenv.texi
index e958e93..8cefd52 100644
--- a/gcc/doc/cppenv.texi
+++ b/gcc/doc/cppenv.texi
@@ -81,7 +81,6 @@ main input file is omitted.
@end ifclear
@item SOURCE_DATE_EPOCH
-
If this variable is set, its value specifies a UNIX timestamp to be
used in replacement of the current date and time in the @code{__DATE__}
and @code{__TIME__} macros, so that the embedded timestamps become
@@ -89,8 +88,9 @@ reproducible.
The value of @env{SOURCE_DATE_EPOCH} must be a UNIX timestamp,
defined as the number of seconds (excluding leap seconds) since
-01 Jan 1970 00:00:00 represented in ASCII, identical to the output of
-@samp{@command{date +%s}}.
+01 Jan 1970 00:00:00 represented in ASCII; identical to the output of
+@samp{@command{date +%s}} on GNU/Linux and other systems that support the
+@code{%s} extension in the @code{date} command.
The value should be a known timestamp such as the last modification
time of the source or package and it should be set by the build
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 1af5920..0b11cb5 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -403,6 +403,7 @@ static const char *pass_through_libs_spec_func (int, const char **);
static const char *replace_extension_spec_func (int, const char **);
static const char *greater_than_spec_func (int, const char **);
static char *convert_white_space (char *);
+static void setenv_SOURCE_DATE_EPOCH_current_time (void);
\f
/* The Specs Language
@@ -3837,6 +3838,7 @@ driver_handle_option (struct gcc_options *opts,
else
compare_debug_opt = arg;
save_switch (compare_debug_replacement_opt, 0, NULL, validated, true);
+ setenv_SOURCE_DATE_EPOCH_current_time ();
return true;
case OPT_fdiagnostics_color_:
@@ -9853,6 +9855,30 @@ path_prefix_reset (path_prefix *prefix)
prefix->max_len = 0;
}
+static void
+setenv_SOURCE_DATE_EPOCH_current_time ()
+{
+ /* Array size is 21 = ceil(log_10(2^64)) + 1 to hold string representations
+ of 64 bit integers. */
+ char source_date_epoch[21];
+ time_t tt;
+ struct tm *tb = NULL;
+
+ errno = 0;
+ tt = time (NULL);
+ if (tt < (time_t) 0 || errno != 0)
+ tt = (time_t) 0;
+
+ tb = gmtime (&tt);
+
+ if (!strftime (source_date_epoch, 21, "%s", tb))
+ snprintf (source_date_epoch, 21, "0");
+ /* Using setenv instead of xputenv because we want the variable to remain
+ after finalizing so that it's still set in the second run when using
+ -fcompare-debug. */
+ setenv ("SOURCE_DATE_EPOCH", source_date_epoch, 0);
+}
+
/* Restore all state within gcc.c to the initial state, so that the driver
code can be safely re-run in-process.
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 4998b3a..9d70cc8 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -594,6 +594,9 @@ struct cpp_callbacks
/* Callback that can change a user builtin into normal macro. */
bool (*user_builtin_macro) (cpp_reader *, cpp_hashnode *);
+
+ /* Callback to parse SOURCE_DATE_EPOCH from environment. */
+ time_t (*get_source_date_epoch) (cpp_reader *);
};
#ifdef VMS
@@ -784,9 +787,6 @@ extern void cpp_init_special_builtins (cpp_reader *);
/* Set up built-ins like __FILE__. */
extern void cpp_init_builtins (cpp_reader *, int);
-/* Initialize the source_date_epoch value. */
-extern void cpp_init_source_date_epoch (cpp_reader *, time_t);
-
/* This is called after options have been parsed, and partially
processed. */
extern void cpp_post_options (cpp_reader *);
diff --git a/libcpp/init.c b/libcpp/init.c
index f5ff85b..1970bc5 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -533,13 +533,6 @@ cpp_init_builtins (cpp_reader *pfile, int hosted)
_cpp_define_builtin (pfile, "__OBJC__ 1");
}
-/* Initialize the source_date_epoch value. */
-void
-cpp_init_source_date_epoch (cpp_reader *pfile, time_t source_date_epoch)
-{
- pfile->source_date_epoch = source_date_epoch;
-}
-
/* Sanity-checks are dependent on command-line options, so it is
called as a subroutine of cpp_read_main_file. */
#if CHECKING_P
diff --git a/libcpp/internal.h b/libcpp/internal.h
index e3eb26b..9ce8707 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -502,10 +502,6 @@ struct cpp_reader
const unsigned char *date;
const unsigned char *time;
- /* Externally set timestamp to replace current date and time useful for
- reproducibility. */
- time_t source_date_epoch;
-
/* EOF token, and a token forcing paste avoidance. */
cpp_token avoid_paste;
cpp_token eof;
diff --git a/libcpp/macro.c b/libcpp/macro.c
index c2a8376..5f7ffbd 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -359,8 +359,9 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
/* Set a reproducible timestamp for __DATE__ and __TIME__ macro
usage if SOURCE_DATE_EPOCH is defined. */
- if (pfile->source_date_epoch != (time_t) -1)
- tb = gmtime (&pfile->source_date_epoch);
+ tt = pfile->cb.get_source_date_epoch (pfile);
+ if (tt != (time_t) -1)
+ tb = gmtime (&tt);
else
{
/* (time_t) -1 is a legitimate value for "number of seconds
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
next prev parent reply other threads:[~2016-05-05 23:28 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-18 12:28 Dhole
2016-04-18 13:05 ` Markus Trippelsdorf
2016-05-03 14:44 ` Dhole
2016-05-03 14:53 ` Markus Trippelsdorf
2016-04-25 10:16 ` Bernd Schmidt
2016-04-26 21:29 ` Dhole
2016-04-26 23:33 ` Bernd Schmidt
2016-04-27 15:57 ` Dhole
2016-04-28 9:20 ` Matthias Klose
2016-04-28 9:22 ` Bernd Schmidt
2016-04-28 10:08 ` Jakub Jelinek
2016-04-28 10:31 ` Bernd Schmidt
2016-04-28 10:35 ` Jakub Jelinek
2016-04-28 13:10 ` Bernd Schmidt
2016-04-28 13:14 ` Jakub Jelinek
2016-04-28 18:31 ` Dhole
2016-04-29 7:17 ` Jakub Jelinek
2016-05-05 23:28 ` Eduard Sanou [this message]
2016-05-06 6:26 ` Andreas Schwab
2016-05-10 11:14 ` Dhole
2016-05-10 11:24 ` Andreas Schwab
2016-05-10 11:32 ` Bernd Schmidt
2016-05-10 15:51 ` Joseph Myers
2016-05-12 0:38 ` Dhole
2016-05-12 9:17 ` Bernd Schmidt
2016-05-13 17:11 ` Dhole
2016-05-23 23:00 ` Dhole
2016-05-24 16:45 ` Jeff Law
2016-06-01 16:29 ` Bernd Schmidt
2016-06-01 16:59 ` Matthias Klose
2016-06-02 13:01 ` Christophe Lyon
2016-06-02 13:05 ` Jakub Jelinek
2016-06-02 13:21 ` Christophe Lyon
2016-06-02 13:24 ` Jakub Jelinek
2016-06-02 14:50 ` Christophe Lyon
2016-06-02 15:04 ` Jakub Jelinek
2016-06-02 15:27 ` Bernd Schmidt
2016-06-02 15:30 ` Christophe Lyon
2016-06-02 12:19 ` Fix up dg-set-compiler-env-var Jakub Jelinek
2016-06-02 12:26 ` Bernd Schmidt
2016-06-02 12:33 ` Jakub Jelinek
2016-05-05 23:39 ` Allow embedded timestamps by C/C++ macros to be set externally (3) Dhole
2016-05-09 10:24 ` Bernd Schmidt
2016-05-09 10:38 ` Bernd Schmidt
2016-05-09 10:42 ` Jakub Jelinek
2016-05-09 11:01 ` Bernd Schmidt
2016-05-10 11:18 ` Dhole
2016-04-28 18:57 ` Martin Sebor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160505232654.GW21574@panther \
--to=eduardsanou@openmailbox.org \
--cc=bschmidt@redhat.com \
--cc=dhole@openmailbox.org \
--cc=doko@ubuntu.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).