From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82791 invoked by alias); 10 May 2016 11:32:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 82778 invoked by uid 89); 10 May 2016 11:32:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=caps, validated, (unknown), 889 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 10 May 2016 11:32:07 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 42D206034; Tue, 10 May 2016 11:32:06 +0000 (UTC) Received: from localhost.localdomain (vpn1-6-245.ams2.redhat.com [10.36.6.245]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4ABW3UG014448; Tue, 10 May 2016 07:32:04 -0400 Subject: Re: Allow embedded timestamps by C/C++ macros to be set externally (3) To: Eduard Sanou , Jakub Jelinek References: <571FFADB.3080209@redhat.com> <20160427155633.GA21574@panther> <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> Cc: Dhole , Matthias Klose , gcc-patches@gcc.gnu.org From: Bernd Schmidt Message-ID: <5731C6B3.3070903@redhat.com> Date: Tue, 10 May 2016 11:32:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <20160505232654.GW21574@panther> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-05/txt/msg00689.txt.bz2 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 *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 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