| Message ID | 20160505232654.GW21574@panther |
|---|---|
| State | New |
| Headers | show |
Eduard Sanou <eduardsanou@openmailbox.org> writes: > 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 This is bigger than INT_MAX, doesn't it trigger a warning that breaks bootstrap? Andreas.
On 16-05-06 08:26:07, Andreas Schwab wrote: > Eduard Sanou <eduardsanou@openmailbox.org> writes: > > > 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 > > This is bigger than INT_MAX, doesn't it trigger a warning that breaks > bootstrap? Sorry but I don't understand the issue. Is defining a macro to a integer bigger than INT_MAX invalid?
Dhole <dhole@openmailbox.org> writes: > On 16-05-06 08:26:07, Andreas Schwab wrote: >> Eduard Sanou <eduardsanou@openmailbox.org> writes: >> >> > 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 >> >> This is bigger than INT_MAX, doesn't it trigger a warning that breaks >> bootstrap? > > Sorry but I don't understand the issue. Is defining a macro to a > integer bigger than INT_MAX invalid? Did it survive bootstrap? Andreas.
On 05/06/2016 01:26 AM, Eduard Sanou wrote: > > 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); Not sure %ld is always correct here. See below. > + 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 I think Andreas' concern is that this might need to have a "ll" suffix. The simplest option might be to use the HOST_WIDE_INT_C macro, and then HOST_WIDE_INT_PRINT_DEC for the error message. > 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 This doc patch is ok and co in independently of the others. > 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); Best to just move the function before its use so you don't have to declare it here. > > /* 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 () Functions need a comment documenting what they do. Also, not thrilled about the name with the caps. Maybe set_source_date_envvar. > + /* 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); > +} Do we really need the whole dance with gmtime/strftime? I thought time_t is documented to hold the number we want, so convert that to long and print it. > 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); That looks like we could call the callback multiple times, which is inefficient and could get repeated error messages. Best to store the value once computed, and maybe add a testcase that the error is printed only once (once we have the dejagnu machinery). The callback could potentially be NULL, right, if this isn't called from one of the C frontends? Best to check for that as well. Bernd
On Tue, 10 May 2016, Bernd Schmidt wrote: > I think Andreas' concern is that this might need to have a "ll" suffix. The > simplest option might be to use the HOST_WIDE_INT_C macro, and then > HOST_WIDE_INT_PRINT_DEC for the error message. HOST_WIDE_INT_PRINT_DEC is a host printf format, not a format for GCC's diagnostic functions such as error_at. Formats such as %wd must be used with the latter. (Formats for GCC's diagnostic functions also mustn't depend on the host, or involve concatenation with macros, so that the messages can be translated.)
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); /* 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