From patchwork Thu May 12 00:36:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dhole X-Patchwork-Id: 621330 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3r4vHD6MtJz9sBc for ; Thu, 12 May 2016 10:38:30 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=CCPR/dSo; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=IjaOGDWNRyP5h03Fr IPqA6xWszGSOElw+noIxBXXYiRKdjWwOqj5IC24cNgyLtmI25zC568YhMYnDOf9m EFbn609St9mFAIVjeA4vud7n/vDs+VDbWNhkqiDDXQ8aB6tIKFM1exciaVuX6IyA RlHm7Hm6mpmCbu/Fxjpw53Wapk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=qO6bLC2fMx2jrllVXdt4Gri a7HM=; b=CCPR/dSoj71AZlvJ3FMsbAg43aHjnmc7F9KQA87DD1zkw6eStzqGHZ/ UrogU5ZSLPOSkJ+MsCuZqlKNgq0p37Yax7MHVUvBA7jeNZHoV0m2bA0TB+v3wbTo vXl0/E0L4Z3pSV3J61JRaKwd6IYKQfJEaP3/QgpTjfPF860ZOfFc= Received: (qmail 31955 invoked by alias); 12 May 2016 00:38:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 31929 invoked by uid 89); 12 May 2016 00:38:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=20160512, 2016-05-12, fcompare-debug, fcomparedebug X-HELO: mail.openmailbox.org Received: from mail.openmailbox.org (HELO mail.openmailbox.org) (62.4.1.34) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 12 May 2016 00:38:10 +0000 Received: by mail.openmailbox.org (Postfix, from userid 20002) id 25B3E2013E2; Thu, 12 May 2016 02:38:07 +0200 (CEST) Received: from localhost (10.203.22.95.dynamic.jazztel.es [95.22.203.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: dhole@openmailbox.org) by mail.openmailbox.org (Postfix) with ESMTPSA id 21B05203865; Thu, 12 May 2016 02:37:59 +0200 (CEST) Date: Thu, 12 May 2016 02:36:51 +0200 From: Dhole To: Bernd Schmidt Cc: Eduard Sanou , Jakub Jelinek , Matthias Klose , gcc-patches@gcc.gnu.org Subject: Re: Allow embedded timestamps by C/C++ macros to be set externally (3) Message-ID: <20160512003651.GB17273@panther> References: <5721D5DC.7060004@ubuntu.com> <20160428100815.GL26501@tucnak.zalov.cz> <5721E68C.20208@redhat.com> <20160428103506.GM26501@tucnak.zalov.cz> <57220BC2.7080901@redhat.com> <20160428131420.GO26501@tucnak.zalov.cz> <20160428182956.GG21574@panther> <20160429071744.GX26501@tucnak.zalov.cz> <20160505232654.GW21574@panther> <5731C6B3.3070903@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5731C6B3.3070903@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes Attaching the patch with all the issues addressed. On 16-05-10 13:32:03, Bernd Schmidt wrote: > Not sure %ld is always correct here. See below. Using %wd now > >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. done. > > > > /* 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. Added comment and changed to set_source_date_epoch_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. Using a single snprintf with a cast to unsigned long long now. > >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. I've added the pfile->source_date_epoch back to store the value once computed. It's meant to be initialized to -2 (not yet set), and then set to either -1 (SOURCE_DATE_EPOCH not set) or a non-negative value containing the value form SOURCE_DATE_EPOCH. I've also added the dg-set-compiler-env-var function in the test framework and written 2 tests: one to check the correct behaviour of SOURCE_DATE_EPOCH, and one to check that when SOURCE_DATE_EPOCH contains an invalid value the error is only reported once. Cheers, diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 665448c..0a35086 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -12794,8 +12794,9 @@ valid_array_size_p (location_t loc, tree type, tree name) /* 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. */ + time_t -get_source_date_epoch () +cb_get_source_date_epoch (cpp_reader *pfile ATTRIBUTE_UNUSED) { char *source_date_epoch; long long epoch; @@ -12807,19 +12808,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 %wd", + 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 0ee9f56..da18d27 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1088,6 +1088,16 @@ extern vec *make_tree_vector_copy (const vec *); 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 *); @@ -1482,9 +1492,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..308e355 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,8 @@ 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); + cpp_init_source_date_epoch (parse_in); 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 7bcf3b3..58a0b77 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -3547,6 +3547,32 @@ save_switch (const char *opt, size_t n_args, const char *const *args, n_switches++; } +/* Set the SOURCE_DATE_EPOCH environment variable to the current time if it is + not set already. */ + +static void +set_source_date_epoch_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); + + snprintf (source_date_epoch, 21, "%llu", (unsigned long long) tb); + /* 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); +} + /* Handle an option DECODED that is unknown to the option-processing machinery. */ @@ -3846,6 +3872,7 @@ driver_handle_option (struct gcc_options *opts, else compare_debug_opt = arg; save_switch (compare_debug_replacement_opt, 0, NULL, validated, true); + set_source_date_epoch_envvar (); return true; case OPT_fdiagnostics_color_: diff --git a/gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c b/gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c new file mode 100644 index 0000000..f6aa1a3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c @@ -0,0 +1,11 @@ +/* { dg-do run } */ +/* { dg-set-compiler-env-var SOURCE_DATE_EPOCH "630333296" } */ + +int +main(void) +{ + __builtin_printf ("%s %s\n", __DATE__, __TIME__); + return 0; +} + +/* { dg-output "^Dec 22 1989 12:34:56\n$" } */ diff --git a/gcc/testsuite/gcc.dg/cpp/source_date_epoch-2.c b/gcc/testsuite/gcc.dg/cpp/source_date_epoch-2.c new file mode 100644 index 0000000..4211552 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/source_date_epoch-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-set-compiler-env-var SOURCE_DATE_EPOCH "AAA" } */ + +int +main(void) +{ + __builtin_printf ("%s %s\n", __DATE__, __TIME__); /* { dg-error "environment variable SOURCE_DATE_EPOCH must expand to a non-negative integer less than or equal to 253402300799" "Invalid SOURCE_DATE_EPOCH not reported" } */ + __builtin_printf ("%s %s\n", __DATE__, __TIME__); /* { dg-bogus "environment variable SOURCE_DATE_EPOCH must expand to a non-negative integer less than or equal to 253402300799" "Invalid SOURCE_DATE_EPOCH reported twice" } */ + return 0; +} diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp index 39a7c20..c0c0cc0 100644 --- a/gcc/testsuite/lib/gcc-dg.exp +++ b/gcc/testsuite/lib/gcc-dg.exp @@ -451,6 +451,38 @@ proc restore-target-env-var { } { } } +proc dg-set-compiler-env-var { args } { + global set_compiler_env_var + global saved_compiler_env_var + if { [llength $args] != 3 } { + error "dg-set-compiler-env-var: need two arguments" + return + } + set var [lindex $args 1] + set value [lindex $args 2] + if [info exists ::env($var)] { + lappend saved_compiler_env_var [list $var 1 $::env($var)] + } else { + lappend saved_compiler_env_var [list $var 0] + } + setenv $var $value + lappend set_compiler_env_var [list $var $value] +} + +proc restore-compiler-env-var { } { + global saved_compiler_env_var + for { set env_vari [llength $saved_compiler_env_var] } { + [incr env_vari -1] >= 0 } {} { + set env_var [lindex $saved_compiler_env_var $env_vari] + set var [lindex $env_var 0] + if [lindex $env_var 1] { + setenv $var [lindex $env_var 2] + } else { + unsetenv $var + } + } +} + # Utility routines. # @@ -874,6 +906,10 @@ if { [info procs saved-dg-test] == [list] } { if [info exists set_target_env_var] { unset set_target_env_var } + if [info exists set_compiler_env_var] { + restore-compiler-env-var + unset set_compiler_env_var + } if [info exists keep_saved_temps_suffixes] { unset keep_saved_temps_suffixes } diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h index 4998b3a..12cad78 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,8 +787,8 @@ 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); +/* Initialize the source_date_epoch value to -2. */ +extern void cpp_init_source_date_epoch (cpp_reader *); /* This is called after options have been parsed, and partially processed. */ diff --git a/libcpp/init.c b/libcpp/init.c index f5ff85b..76ce7d6 100644 --- a/libcpp/init.c +++ b/libcpp/init.c @@ -535,9 +535,9 @@ cpp_init_builtins (cpp_reader *pfile, int hosted) /* Initialize the source_date_epoch value. */ void -cpp_init_source_date_epoch (cpp_reader *pfile, time_t source_date_epoch) +cpp_init_source_date_epoch (cpp_reader *pfile) { - pfile->source_date_epoch = source_date_epoch; + pfile->source_date_epoch = (time_t) -2; } /* Sanity-checks are dependent on command-line options, so it is diff --git a/libcpp/internal.h b/libcpp/internal.h index e3eb26b..cea32ec 100644 --- a/libcpp/internal.h +++ b/libcpp/internal.h @@ -503,7 +503,8 @@ struct cpp_reader const unsigned char *time; /* Externally set timestamp to replace current date and time useful for - reproducibility. */ + reproducibility. It should be initialized to -2 (not yet set) and + set to -1 to disable it or to a non-negative value to enable it. */ time_t source_date_epoch; /* EOF token, and a token forcing paste avoidance. */ diff --git a/libcpp/macro.c b/libcpp/macro.c index c2a8376..55e53bf 100644 --- a/libcpp/macro.c +++ b/libcpp/macro.c @@ -358,9 +358,13 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node, struct tm *tb = NULL; /* 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); + if SOURCE_DATE_EPOCH is defined. */ + if (pfile->source_date_epoch == (time_t) -2 + && pfile->cb.get_source_date_epoch != NULL) + pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile); + + if (pfile->source_date_epoch >= (time_t) 0) + tb = gmtime (&pfile->source_date_epoch); else { /* (time_t) -1 is a legitimate value for "number of seconds