Allow embedded timestamps by C/C++ macros to be set externally (3)

Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1.1: Type: text/plain, Size: 2662 bytes --]

Hi!

A few months ago I submited a patch to allow the embedded timestamps by
C/C++ macros to be set externally [2], which was already an improvement
over [1].  I was told to wait until the GCC 7 stage 1 started to send
this patch again.

I'm quoting from the original emails:

> As a reminder for the motivation behind this patch, we are working on
> the reproducible builds project which aims to provide users with a way
> to reproduce bit-for-bit identical binary packages from the source and
> build environment. The project involves Debian as well as several other
> free software projects. See <https://reproducible-builds.org/> for more
> information.

> In order to do this, we need to make the build processes
> deterministic.  As you can imagine, gcc is quite involved in producing
> Debian packages.  One issue we encounter in many packages that fail to
> build reproducibly is the use of the __DATE__, __TIME__ C macros [3],
> right now we have 442 affected packages that would need patching
> (either removing the macros, or passing a known date externally).

> A solution for toolchain packages that embed timestamps during the
> build process has been proposed for anyone interested and it consists
> of the following: The build environment can export an environment
> variable called SOURCE_DATE_EPOCH with a known timestamp in Unix epoch
> format (In our case, we use the last date of the package's debian
> changelog). The toolchain package running during the build can check
> if the exported variable is set and if so, instead of embedding the
> local date/time, embed the date/time from SOURCE_DATE_EPOCH.

> The proposal to use SOURCE_DATE_EPOCH has now been gathered in a more
> formal specification [4], so that any project can adhere to it to
> achieve reproducible builds when dealing with timestamps.

> It would be very beneficial to our project (and other free software
> projects working on reproducible builds) if gcc supported this
> feature.

I'm attaching a patch for the svn/trunk GCC repository (now GCC 7) that
enables this feature: it modifies the behavior of the macros __DATE__
and __TIME__ when the environment variable SOURCE_DATE_EPOCH is
exported.

Documentation of the environment variable is also provided.

Note: I have already gone through the copyright assignment process :)


[1] https://gcc.gnu.org/ml/gcc-patches/2015-06/msg02210.html
[2] https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01890.html
[3] https://wiki.debian.org/ReproducibleBuilds/TimestampsFromCPPMacros
[4] https://reproducible-builds.org/specs/source-date-epoch/

Best regards,
-- 
Dhole

[-- Attachment #1.2: gcc-SOURCE_DATE_EPOCH-patch-2016_04_17.diff.txt --]
[-- Type: text/plain, Size: 6477 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f2846bb..3a83673 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12741,4 +12741,38 @@ valid_array_size_p (location_t loc, tree type, tree name)
   return true;
 }
 
+/* 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.  */
+long long
+get_source_date_epoch()
+{
+  char *source_date_epoch;
+  unsigned long long epoch;
+  char *endptr;
+
+  source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
+  if (!source_date_epoch)
+    return -1;
+
+  errno = 0;
+  epoch = strtoull (source_date_epoch, &endptr, 10);
+  if ((errno == ERANGE && (epoch == ULLONG_MAX || epoch == 0))
+      || (errno != 0 && epoch == 0))
+    fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
+		 "strtoull: %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 > ULONG_MAX)
+    fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
+		 "value must be smaller than or equal to: %lu but was found to "
+		 "be: %llu \n", ULONG_MAX, epoch);
+
+  return (long long) epoch;
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index fa3746c..b4d6afc 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1467,4 +1467,9 @@ extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);
 extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec<tree> **);
 extern bool valid_array_size_p (location_t, 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 long long get_source_date_epoch();
+
 #endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 96da4fc..2454c6f 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -385,6 +385,10 @@ 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;
+  long long source_date_epoch = -1;
+
+  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 22c8cb3..e958e93 100644
--- a/gcc/doc/cppenv.texi
+++ b/gcc/doc/cppenv.texi
@@ -79,4 +79,21 @@ main input file is omitted.
 @ifclear cppmanual
 @xref{Preprocessor Options}.
 @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
+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}}.
+
+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
+process.
+
 @end vtable
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 35b0375..4cd5170 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -784,6 +784,9 @@ 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 *, long long);
+
 /* 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 4343075..514282e 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -533,6 +533,13 @@ 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, long long 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 9ce8707..5fc41da 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -502,6 +502,10 @@ struct cpp_reader
   const unsigned char *date;
   const unsigned char *time;
 
+  /* Externally set timestamp to replace current date and time useful for
+     reproducibility.  */
+  long long 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 c251553..833f36b 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -357,13 +357,20 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
 	  time_t tt;
 	  struct tm *tb = NULL;
 
-	  /* (time_t) -1 is a legitimate value for "number of seconds
-	     since the Epoch", so we have to do a little dance to
-	     distinguish that from a genuine error.  */
-	  errno = 0;
-	  tt = time(NULL);
-	  if (tt != (time_t)-1 || errno == 0)
-	    tb = localtime (&tt);
+	  /* Set a reproducible timestamp for __DATE__ and __TIME__ macro
+	     usage if SOURCE_DATE_EPOCH is defined.  */
+	  if (pfile->source_date_epoch != -1)
+	     tb = gmtime ((time_t*) &pfile->source_date_epoch);
+	  else
+	    {
+	      /* (time_t) -1 is a legitimate value for "number of seconds
+		 since the Epoch", so we have to do a little dance to
+		 distinguish that from a genuine error.  */
+	      errno = 0;
+	      tt = time (NULL);
+	      if (tt != (time_t)-1 || errno == 0)
+		tb = localtime (&tt);
+	    }
 
 	  if (tb)
 	    {

[-- Attachment #1.3: ChangeLog --]
[-- Type: text/plain, Size: 1130 bytes --]

gcc/c-family/ChangeLog:

2016-04-18  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>
	* c-common.c (get_source_date_epoch): New function, gets the environment
	variable SOURCE_DATE_EPOCH and parses it as long long with error 
	handling.
	* c-common.h (get_source_date_epoch): Prototype.
	* c-lex.c (c_lex_with_flags): set parse_in->source_date_epoch.

gcc/ChangeLog:

2016-04-18  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>
	* doc/cppenv.texi: Document SOURCE_DATE_EPOCH environment variable.

libcpp/ChangeLog:

2016-04-18  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>
	* include/cpplib.h (cpp_init_source_date_epoch): Prototype.
	* init.c (cpp_init_source_date_epoch): New function.
	* internal.h: Added source_date_epoch variable to struct
	cpp_reader to store a reproducible date.
	* macro.c (_cpp_builtin_macro_text): Set pfile->date timestamp from 
	pfile->source_date_epoch instead of localtime if source_date_epoch is 
	set, to be used for __DATE__ and __TIME__ macros to help reproducible 
	builds.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Markus Trippelsdorf]

On 2016.04.18 at 14:26 +0200, Dhole wrote:
> Hi!
> 
> A few months ago I submited a patch to allow the embedded timestamps by
> C/C++ macros to be set externally [2], which was already an improvement
> over [1].  I was told to wait until the GCC 7 stage 1 started to send
> this patch again.

A nice follow-up patch would be to set SOURCE_DATE_EPOCH to the current
time during -fcompare-debug. This would avoid false positives like:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70679

-- 
Markus

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 04/18/2016 02:26 PM, Dhole wrote:
> A few months ago I submited a patch to allow the embedded timestamps by
> C/C++ macros to be set externally [2], which was already an improvement
> over [1].  I was told to wait until the GCC 7 stage 1 started to send
> this patch again.

> +/* 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.  */
> +long long
> +get_source_date_epoch()

Always have a space before open-paren. Maybe this should return time_t. 
See below.

> +/* 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 long long get_source_date_epoch();

Double space after the end of a sentence. Space before open paren.

> +  source_date_epoch = get_source_date_epoch();
> +  cpp_init_source_date_epoch(parse_in, source_date_epoch);

Spaces.

> +/* Initialize the source_date_epoch value.  */
> +extern void cpp_init_source_date_epoch (cpp_reader *, long long);

Also thinking we should be using time_t here.

>   /* Sanity-checks are dependent on command-line options, so it is
>      called as a subroutine of cpp_read_main_file ().  */

We don't write () to mark function names.

> +	     tb = gmtime ((time_t*) &pfile->source_date_epoch);

Space before the "*". But this cast looks ugly and unreliable (think 
big-endian). This is why I would prefer to move to a time_t 
representation sooner.

> 2016-04-18  Eduard Sanou<dhole@openmailbox.org>
> 	    Matthias Klose<doko@debian.org>
> 	* c-common.c (get_source_date_epoch): New function, gets the environment
> 	variable SOURCE_DATE_EPOCH and parses it as long long with error
> 	handling.
> 	* c-common.h (get_source_date_epoch): Prototype.
> 	* c-lex.c (c_lex_with_flags): set parse_in->source_date_epoch.

Add blank lines after the end of the names in ChangeLogs.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1.1: Type: text/plain, Size: 2733 bytes --]

Hi Bernd,

On 16-04-25 12:15:50, Bernd Schmidt wrote:
> On 04/18/2016 02:26 PM, Dhole wrote:
> >A few months ago I submited a patch to allow the embedded timestamps by
> >C/C++ macros to be set externally [2], which was already an improvement
> >over [1].  I was told to wait until the GCC 7 stage 1 started to send
> >this patch again.
> 
> >+/* 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.  */
> >+long long
> >+get_source_date_epoch()
> 
> Always have a space before open-paren. Maybe this should return time_t. See
> below.
> 
> >+/* 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 long long get_source_date_epoch();
> 
> Double space after the end of a sentence. Space before open paren.
> 
> >+  source_date_epoch = get_source_date_epoch();
> >+  cpp_init_source_date_epoch(parse_in, source_date_epoch);
> 
> Spaces.
> 
> >+/* Initialize the source_date_epoch value.  */
> >+extern void cpp_init_source_date_epoch (cpp_reader *, long long);
> 
> Also thinking we should be using time_t here.
> 
> >  /* Sanity-checks are dependent on command-line options, so it is
> >     called as a subroutine of cpp_read_main_file ().  */
> 
> We don't write () to mark function names.
> 
> >+	     tb = gmtime ((time_t*) &pfile->source_date_epoch);
> 
> Space before the "*". But this cast looks ugly and unreliable (think
> big-endian). This is why I would prefer to move to a time_t representation
> sooner.
> 
> >2016-04-18  Eduard Sanou<dhole@openmailbox.org>
> >	    Matthias Klose<doko@debian.org>
> >	* c-common.c (get_source_date_epoch): New function, gets the environment
> >	variable SOURCE_DATE_EPOCH and parses it as long long with error
> >	handling.
> >	* c-common.h (get_source_date_epoch): Prototype.
> >	* c-lex.c (c_lex_with_flags): set parse_in->source_date_epoch.
> 
> Add blank lines after the end of the names in ChangeLogs.

Thanks for the review!

I've fixed all the spaces issues.  I've also changed the "unsigned long long"
to "time_t" as you suggested.  Also, to improve reliability I'm now
using strtoll rather than strtoull, so that negative values can be
detected in SOURCE_DATE_EPOCH, which are treated as errors.  This way
the variable pfile->source_date_epoch can't be set to -1 (which is the
default value) when SOURCE_DATE_EPOCH is defined.

I'm attaching the improved patch with the ChangeLog.

Cheers,
-- 
Dhole

[-- Attachment #1.2: gcc-SOURCE_DATE_EPOCH-patch-2016_04_26.diff.txt --]
[-- Type: text/plain, Size: 6561 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f2846bb..6ef63b1 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12741,4 +12741,37 @@ valid_array_size_p (location_t loc, tree type, tree name)
   return true;
 }
 
+/* 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 ()
+{
+  char *source_date_epoch;
+  long long epoch;
+  char *endptr;
+
+  source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
+  if (!source_date_epoch)
+    return (time_t) -1;
+
+  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);
+
+  return (time_t) epoch;
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index fa3746c..656bc75 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1467,4 +1467,9 @@ extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);
 extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec<tree> **);
 extern bool valid_array_size_p (location_t, 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 96da4fc..753e8b8 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -385,6 +385,10 @@ 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 = (time_t) -1;
+
+  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 22c8cb3..e958e93 100644
--- a/gcc/doc/cppenv.texi
+++ b/gcc/doc/cppenv.texi
@@ -79,4 +79,21 @@ main input file is omitted.
 @ifclear cppmanual
 @xref{Preprocessor Options}.
 @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
+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}}.
+
+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
+process.
+
 @end vtable
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 35b0375..4998b3a 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -784,6 +784,9 @@ 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 4343075..f5ff85b 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -533,8 +533,15 @@ 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 ().  */
+   called as a subroutine of cpp_read_main_file.  */
 #if CHECKING_P
 static void sanity_checks (cpp_reader *);
 static void sanity_checks (cpp_reader *pfile)
diff --git a/libcpp/internal.h b/libcpp/internal.h
index 9ce8707..e3eb26b 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -502,6 +502,10 @@ 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 c251553..c2a8376 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -357,13 +357,20 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
 	  time_t tt;
 	  struct tm *tb = NULL;
 
-	  /* (time_t) -1 is a legitimate value for "number of seconds
-	     since the Epoch", so we have to do a little dance to
-	     distinguish that from a genuine error.  */
-	  errno = 0;
-	  tt = time(NULL);
-	  if (tt != (time_t)-1 || errno == 0)
-	    tb = localtime (&tt);
+	  /* 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);
+	  else
+	    {
+	      /* (time_t) -1 is a legitimate value for "number of seconds
+		 since the Epoch", so we have to do a little dance to
+		 distinguish that from a genuine error.  */
+	      errno = 0;
+	      tt = time (NULL);
+	      if (tt != (time_t)-1 || errno == 0)
+		tb = localtime (&tt);
+	    }
 
 	  if (tb)
 	    {

[-- Attachment #1.3: ChangeLog --]
[-- Type: text/plain, Size: 1136 bytes --]

gcc/c-family/ChangeLog:

2016-04-26  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>

	* c-common.c (get_source_date_epoch): New function, gets the environment
	variable SOURCE_DATE_EPOCH and parses it as long long with error 
	handling.
	* c-common.h (get_source_date_epoch): Prototype.
	* c-lex.c (c_lex_with_flags): set parse_in->source_date_epoch.

gcc/ChangeLog:

2016-04-26  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>

	* doc/cppenv.texi: Document SOURCE_DATE_EPOCH environment variable.

libcpp/ChangeLog:

2016-04-26  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>

	* include/cpplib.h (cpp_init_source_date_epoch): Prototype.
	* init.c (cpp_init_source_date_epoch): New function.
	* internal.h: Added source_date_epoch variable to struct
	cpp_reader to store a reproducible date.
	* macro.c (_cpp_builtin_macro_text): Set pfile->date timestamp from 
	pfile->source_date_epoch instead of localtime if source_date_epoch is 
	set, to be used for __DATE__ and __TIME__ macros to help reproducible 
	builds.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 04/26/2016 11:28 PM, Dhole wrote:
> I've fixed all the spaces issues.  I've also changed the "unsigned long long"
> to "time_t" as you suggested.  Also, to improve reliability I'm now
> using strtoll rather than strtoull, so that negative values can be
> detected in SOURCE_DATE_EPOCH, which are treated as errors.  This way
> the variable pfile->source_date_epoch can't be set to -1 (which is the
> default value) when SOURCE_DATE_EPOCH is defined.

Some minor issues remain, but then I think it'll be good to go.

> +  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);

These are somewhat unusual for error messages, but I think the general 
principle of no capitalization probably applies, so "No", "Trailing", 
and "Value" should be lowercase.

Not sure whether these should be fatal_errors or warnings. For now it's 
probably ok to leave it as-is. Also, I wonder whether zero might be a 
better value to use as an unset marker, but again it's probably fine for 
now.

> +  time_t source_date_epoch = (time_t) -1;
> +
> +  source_date_epoch = get_source_date_epoch ();

First initialization seems unnecessary. Might want to merge the 
declaration with the initialization.

Ok with these changes.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1.1: Type: text/plain, Size: 1389 bytes --]

Thanks again for the review Bernd,

On 16-04-27 01:33:47, Bernd Schmidt wrote:
> >+  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);
> 
> These are somewhat unusual for error messages, but I think the general
> principle of no capitalization probably applies, so "No", "Trailing", and
> "Value" should be lowercase.

Done.

> >+  time_t source_date_epoch = (time_t) -1;
> >+
> >+  source_date_epoch = get_source_date_epoch ();
> 
> First initialization seems unnecessary. Might want to merge the declaration
> with the initialization.

And done.

I'm attaching the updated patch with the two minor issues fixed.

Cheers,
-- 
Dhole

[-- Attachment #1.2: gcc-SOURCE_DATE_EPOCH-patch-2016_04_27.diff.txt --]
[-- Type: text/plain, Size: 6523 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f2846bb..5315475 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12741,4 +12741,37 @@ valid_array_size_p (location_t loc, tree type, tree name)
   return true;
 }
 
+/* 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 ()
+{
+  char *source_date_epoch;
+  long long epoch;
+  char *endptr;
+
+  source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
+  if (!source_date_epoch)
+    return (time_t) -1;
+
+  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);
+
+  return (time_t) epoch;
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index fa3746c..656bc75 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1467,4 +1467,9 @@ extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);
 extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec<tree> **);
 extern bool valid_array_size_p (location_t, 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 96da4fc..bf1db6c 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -385,6 +385,9 @@ 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 22c8cb3..e958e93 100644
--- a/gcc/doc/cppenv.texi
+++ b/gcc/doc/cppenv.texi
@@ -79,4 +79,21 @@ main input file is omitted.
 @ifclear cppmanual
 @xref{Preprocessor Options}.
 @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
+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}}.
+
+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
+process.
+
 @end vtable
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 35b0375..4998b3a 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -784,6 +784,9 @@ 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 4343075..f5ff85b 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -533,8 +533,15 @@ 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 ().  */
+   called as a subroutine of cpp_read_main_file.  */
 #if CHECKING_P
 static void sanity_checks (cpp_reader *);
 static void sanity_checks (cpp_reader *pfile)
diff --git a/libcpp/internal.h b/libcpp/internal.h
index 9ce8707..e3eb26b 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -502,6 +502,10 @@ 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 c251553..c2a8376 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -357,13 +357,20 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
 	  time_t tt;
 	  struct tm *tb = NULL;
 
-	  /* (time_t) -1 is a legitimate value for "number of seconds
-	     since the Epoch", so we have to do a little dance to
-	     distinguish that from a genuine error.  */
-	  errno = 0;
-	  tt = time(NULL);
-	  if (tt != (time_t)-1 || errno == 0)
-	    tb = localtime (&tt);
+	  /* 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);
+	  else
+	    {
+	      /* (time_t) -1 is a legitimate value for "number of seconds
+		 since the Epoch", so we have to do a little dance to
+		 distinguish that from a genuine error.  */
+	      errno = 0;
+	      tt = time (NULL);
+	      if (tt != (time_t)-1 || errno == 0)
+		tb = localtime (&tt);
+	    }
 
 	  if (tb)
 	    {

[-- Attachment #1.3: ChangeLog --]
[-- Type: text/plain, Size: 1136 bytes --]

gcc/c-family/ChangeLog:

2016-04-27  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>

	* c-common.c (get_source_date_epoch): New function, gets the environment
	variable SOURCE_DATE_EPOCH and parses it as long long with error 
	handling.
	* c-common.h (get_source_date_epoch): Prototype.
	* c-lex.c (c_lex_with_flags): set parse_in->source_date_epoch.

gcc/ChangeLog:

2016-04-27  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>

	* doc/cppenv.texi: Document SOURCE_DATE_EPOCH environment variable.

libcpp/ChangeLog:

2016-04-27  Eduard Sanou  <dhole@openmailbox.org>
	    Matthias Klose  <doko@debian.org>

	* include/cpplib.h (cpp_init_source_date_epoch): Prototype.
	* init.c (cpp_init_source_date_epoch): New function.
	* internal.h: Added source_date_epoch variable to struct
	cpp_reader to store a reproducible date.
	* macro.c (_cpp_builtin_macro_text): Set pfile->date timestamp from 
	pfile->source_date_epoch instead of localtime if source_date_epoch is 
	set, to be used for __DATE__ and __TIME__ macros to help reproducible 
	builds.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Matthias Klose]

On 27.04.2016 17:56, Dhole wrote:
> Thanks again for the review Bernd,
>
> On 16-04-27 01:33:47, Bernd Schmidt wrote:
>>> +  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);
>>
>> These are somewhat unusual for error messages, but I think the general
>> principle of no capitalization probably applies, so "No", "Trailing", and
>> "Value" should be lowercase.
>
> Done.
>
>>> +  time_t source_date_epoch = (time_t) -1;
>>> +
>>> +  source_date_epoch = get_source_date_epoch ();
>>
>> First initialization seems unnecessary. Might want to merge the declaration
>> with the initialization.
>
> And done.
>
> I'm attaching the updated patch with the two minor issues fixed.

committed.

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 04/28/2016 11:20 AM, Matthias Klose wrote:
> On 27.04.2016 17:56, Dhole wrote:
>>
>> I'm attaching the updated patch with the two minor issues fixed.
>
> committed.

Something else that occurred to me - could you please also work on a 
testcase?


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Apr 28, 2016 at 11:20:28AM +0200, Matthias Klose wrote:
> On 27.04.2016 17:56, Dhole wrote:
> >Thanks again for the review Bernd,
> >
> >On 16-04-27 01:33:47, Bernd Schmidt wrote:
> >>>+  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);
> >>
> >>These are somewhat unusual for error messages, but I think the general
> >>principle of no capitalization probably applies, so "No", "Trailing", and
> >>"Value" should be lowercase.
> >
> >Done.
> >
> >>>+  time_t source_date_epoch = (time_t) -1;
> >>>+
> >>>+  source_date_epoch = get_source_date_epoch ();
> >>
> >>First initialization seems unnecessary. Might want to merge the declaration
> >>with the initialization.
> >
> >And done.
> >
> >I'm attaching the updated patch with the two minor issues fixed.
> 
> committed.

BTW, I think fatal_error doesn't make sense, it isn't something that is not
recoverable, normal error or just a warning would be IMHO more than
sufficient.  The fallback would be just using current time, i.e. ignoring
the env var.

Additionally, I think it is a very bad idea to slow down the initialization
for something so rarely used - instead of initializing this always, IMNSHO
it should be only initialized when the first __TIME__ macro is expanded,
similarly how it only calls time/localtime etc. when the macro is expanded
for the first time.

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.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 04/28/2016 12:08 PM, Jakub Jelinek wrote:
> BTW, I think fatal_error doesn't make sense, it isn't something that is not
> recoverable, normal error or just a warning would be IMHO more than
> sufficient.  The fallback would be just using current time, i.e. ignoring
> the env var.

I thought about this, but we also error out for invalid arguments to 
options, and IMO this case is analogous.

> Additionally, I think it is a very bad idea to slow down the initialization
> for something so rarely used - instead of initializing this always, IMNSHO
> it should be only initialized when the first __TIME__ macro is expanded,
> similarly how it only calls time/localtime etc. when the macro is expanded
> for the first time.

I really don't see anything in that function that looks like a huge time 
sink, so I'm not that worried about it. I think it's likely to be buried 
way down in the noise.

> 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.

This sounds like a good idea. Maybe we could even have the bootstrap 
include an instance of __TIME__, with the env var set, and use the stage 
comparison as a test for this feature.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Apr 28, 2016 at 12:31:40PM +0200, Bernd Schmidt wrote:
> On 04/28/2016 12:08 PM, Jakub Jelinek wrote:
> >BTW, I think fatal_error doesn't make sense, it isn't something that is not
> >recoverable, normal error or just a warning would be IMHO more than
> >sufficient.  The fallback would be just using current time, i.e. ignoring
> >the env var.
> 
> I thought about this, but we also error out for invalid arguments to
> options, and IMO this case is analogous.
> 
> >Additionally, I think it is a very bad idea to slow down the initialization
> >for something so rarely used - instead of initializing this always, IMNSHO
> >it should be only initialized when the first __TIME__ macro is expanded,
> >similarly how it only calls time/localtime etc. when the macro is expanded
> >for the first time.
> 
> I really don't see anything in that function that looks like a huge time
> sink, so I'm not that worried about it. I think it's likely to be buried way
> down in the noise.

True, but the noise sums up, and the result is terrible speed of compiling
empty source files, something that e.g. Linux kernel or other packages
that have lots of small source files, care about a lot.
If initializing it early would buy us anything on code clarity etc., it
could be justified, but IMHO it doesn't, the code in libcpp already has the
delayed initialization anyway.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 04/28/2016 12:35 PM, Jakub Jelinek wrote:
> On Thu, Apr 28, 2016 at 12:31:40PM +0200, Bernd Schmidt wrote:
>> I really don't see anything in that function that looks like a huge time
>> sink, so I'm not that worried about it. I think it's likely to be buried way
>> down in the noise.
>
> True, but the noise sums up, and the result is terrible speed of compiling
> empty source files, something that e.g. Linux kernel or other packages
> that have lots of small source files, care about a lot.
> If initializing it early would buy us anything on code clarity etc., it
> could be justified, but IMHO it doesn't, the code in libcpp already has the
> delayed initialization anyway.

Well, it does buy us early (and reliable) error checks against the 
environment variable.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Apr 28, 2016 at 03:10:26PM +0200, Bernd Schmidt wrote:
> On 04/28/2016 12:35 PM, Jakub Jelinek wrote:
> >On Thu, Apr 28, 2016 at 12:31:40PM +0200, Bernd Schmidt wrote:
> >>I really don't see anything in that function that looks like a huge time
> >>sink, so I'm not that worried about it. I think it's likely to be buried way
> >>down in the noise.
> >
> >True, but the noise sums up, and the result is terrible speed of compiling
> >empty source files, something that e.g. Linux kernel or other packages
> >that have lots of small source files, care about a lot.
> >If initializing it early would buy us anything on code clarity etc., it
> >could be justified, but IMHO it doesn't, the code in libcpp already has the
> >delayed initialization anyway.
> 
> Well, it does buy us early (and reliable) error checks against the
> environment variable.

I'm not sure we really care about the env var unless it actually needs to be
used.  If we error only if it is used, people could e.g. use it in another
way, to verify their code doesn't contain any __TIME__ uses, compile with
the env var set to some invalid string and just compile everything with
that, it would diagnose any uses of __TIME__.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

On 16-04-28 15:14:20, Jakub Jelinek wrote:
> On Thu, Apr 28, 2016 at 03:10:26PM +0200, Bernd Schmidt wrote:
> > On 04/28/2016 12:35 PM, Jakub Jelinek wrote:
> > >On Thu, Apr 28, 2016 at 12:31:40PM +0200, Bernd Schmidt wrote:
> > >>I really don't see anything in that function that looks like a huge time
> > >>sink, so I'm not that worried about it. I think it's likely to be buried way
> > >>down in the noise.
> > >
> > >True, but the noise sums up, and the result is terrible speed of compiling
> > >empty source files, something that e.g. Linux kernel or other packages
> > >that have lots of small source files, care about a lot.
> > >If initializing it early would buy us anything on code clarity etc., it
> > >could be justified, but IMHO it doesn't, the code in libcpp already has the
> > >delayed initialization anyway.
> > 
> > Well, it does buy us early (and reliable) error checks against the
> > environment variable.
> 
> I'm not sure we really care about the env var unless it actually needs to be
> used.  If we error only if it is used, people could e.g. use it in another
> way, to verify their code doesn't contain any __TIME__ uses, compile with
> the env var set to some invalid string and just compile everything with
> that, it would diagnose any uses of __TIME__.

There is the Wdate-time flag, that warns on using __DATE__, __TIME__ and
__TIMESTAMP__.  Although that alone will not make the compilation fail
unless it's used with Werror.

The reason behind using fatal_error (rather than a warning) when
SOURCE_DATE_EPOCH contains an invalid value is due to the
SOURCE_DATE_EPOCH specification [1]:

  SOURCE_DATE_EPOCH
  (...)
  If the value is malformed, the build process SHOULD exit with a non-zero error code.

And the reason for reading and parsing the env var in gcc/ rather than
when the macro is expanded for the first time (in libcpp/) is from a
comment by Joseph Myers made the first time I submited this patch [2].
The most clean way to read the env var from gcc/ I found was to do it
during the initialization.  But if you think this should be done
different I'm open to change the implementation.


Bernd: I'll see if I can prepare a testcase; first I need to get
familiar with the testing framework and learn how to set environment
variables in tests.  Any tips on that will be really welcome!


Also, I'll take a look at the -fcompare-debug, see what's the best way
to get the same __TIME__ and __DATE__ with the help of
SOURCE_DATE_EPOCH.


[1] https://reproducible-builds.org/specs/source-date-epoch/
[2] https://gcc.gnu.org/ml/gcc-patches/2015-06/msg02270.html

Cheers,
-- 
Dhole

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Martin Sebor]

I'm sorry I'm a little late but I have a couple of minor comments
on the patch:

>>> +  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);

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.

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

One comment about the documentation:

 > +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}}.

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.

Martin

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Apr 28, 2016 at 08:29:56PM +0200, Dhole wrote:
> There is the Wdate-time flag, that warns on using __DATE__, __TIME__ and
> __TIMESTAMP__.  Although that alone will not make the compilation fail
> unless it's used with Werror.
> 
> The reason behind using fatal_error (rather than a warning) when
> SOURCE_DATE_EPOCH contains an invalid value is due to the
> SOURCE_DATE_EPOCH specification [1]:
> 
>   SOURCE_DATE_EPOCH
>   (...)
>   If the value is malformed, the build process SHOULD exit with a non-zero error code.

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, ...).

> And the reason for reading and parsing the env var in gcc/ rather than
> when the macro is expanded for the first time (in libcpp/) is from a
> comment by Joseph Myers made the first time I submited this patch [2].
> The most clean way to read the env var from gcc/ I found was to do it
> during the initialization.  But if you think this should be done
> different I'm open to change the implementation.

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.

> Bernd: I'll see if I can prepare a testcase; first I need to get
> familiar with the testing framework and learn how to set environment
> variables in tests.  Any tips on that will be really welcome!

grep for dg-set-target-env-var in various tests.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On 16-04-18 15:04:58, Markus Trippelsdorf wrote:
> A nice follow-up patch would be to set SOURCE_DATE_EPOCH to the current
> time during -fcompare-debug. This would avoid false positives like:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70679

I've been working on a patch to implement that, but I can't manage to
reproduce the false positive from the link.  Maybe the test code I'm
using compiles too fast.  I'm not familiar with -fcompare-debug either.

Could you provide me some code with instructions to reproduce this false
positive, to see if my patch is working as expected?

-- 
Dhole

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Markus Trippelsdorf]

On 2016.05.03 at 16:42 +0200, Dhole wrote:
> On 16-04-18 15:04:58, Markus Trippelsdorf wrote:
> > A nice follow-up patch would be to set SOURCE_DATE_EPOCH to the current
> > time during -fcompare-debug. This would avoid false positives like:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70679
> 
> I've been working on a patch to implement that, but I can't manage to
> reproduce the false positive from the link.  Maybe the test code I'm
> using compiles too fast.  I'm not familiar with -fcompare-debug either.
> 
> Could you provide me some code with instructions to reproduce this false
> positive, to see if my patch is working as expected?

Just building LLVM is enough:

markus@x4 llvm_build % time g++ -fcompare-debug -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -std=c++11 -O2 -pipe -Ilib/Support -I/home/trippels/llvm/lib/Support -Iinclude -I/home/trippels/llvm/include -UNDEBUG -fno-exceptions -fno-rtti -MMD -MT lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/CommandLine.cpp.o -c /home/markus/llvm/lib/Support/CommandLine.cpp 
g++: error: /home/markus/llvm/lib/Support/CommandLine.cpp: -fcompare-debug failure
g++ -fcompare-debug -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE    -fPIC  -Wall  5.71s user 0.26s system 101% cpu 5.904 total

-- 
Markus

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Eduard Sanou]

[-- 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 --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1.1: Type: text/plain, Size: 1045 bytes --]

On 16-04-29 09:17:44, Jakub Jelinek wrote:
> > Bernd: I'll see if I can prepare a testcase; first I need to get
> > familiar with the testing framework and learn how to set environment
> > variables in tests.  Any tips on that will be really welcome!
> 
> grep for dg-set-target-env-var in various tests.

I've been looking at how the test infrastructure works, and I'm having
some difficulties with setting the env var.

I've wrote a test case which fails (when it shouldn't) and I don't see
why.

I'm attaching the test file.

I'm running it with:
$ make check-gcc RUNTESTFLAGS=cpp.exp=source_date_epoch-1.c

What I find strange, however, is that if I set the env var from the
command line, it seems to pass:
$ SOURCE_DATE_EPOCH=123456 make check-gcc RUNTESTFLAGS=cpp.exp=source_date_epoch-1.c


P.S.: I've just sent another message to the thread with the patch
implementing the other mentioned issues.  I've mistakenly sent it from
another email account of mine: <eduardsanou@openmailbox.org>

Cheers,
-- 
Dhole

[-- Attachment #1.2: source_date_epoch-1.c --]
[-- Type: text/x-csrc, Size: 207 bytes --]

/* { dg-do run } */
/* { dg-set-target-env-var SOURCE_DATE_EPOCH "123456" } */

int
main(void)
{
  __builtin_printf ("%s %s\n", __DATE__, __TIME__);
  return 0;
}

/* { dg-output "Jan  2 1970 10:17:36" } */

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 05/06/2016 01:38 AM, Dhole wrote:
> On 16-04-29 09:17:44, Jakub Jelinek wrote:
>>> Bernd: I'll see if I can prepare a testcase; first I need to get
>>> familiar with the testing framework and learn how to set environment
>>> variables in tests.  Any tips on that will be really welcome!
>>
>> grep for dg-set-target-env-var in various tests.
>
> I've been looking at how the test infrastructure works, and I'm having
> some difficulties with setting the env var.
>
> I've wrote a test case which fails (when it shouldn't) and I don't see
> why.

I think it's setting the env var when executing the test, not when 
executing the compiler.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

On 05/09/2016 12:23 PM, Bernd Schmidt wrote:
> On 05/06/2016 01:38 AM, Dhole wrote:
>> I've wrote a test case which fails (when it shouldn't) and I don't see
>> why.
>
> I think it's setting the env var when executing the test, not when
> executing the compiler.

Here's something that seems to work (with an appropriately adjusted 
testcase). Cc'ing testsuite maintainers.


Bernd

[-- Attachment #2: sdate.diff --]
[-- Type: text/x-patch, Size: 2419 bytes --]

	* lib/gcc-dg.exp (orig_source_date_epoch_saved,
	orig_source_date_epoch_checked): Initialize to 0.
	(dg-set-source-date-epoch): New function.
	(restore_source_epoch_env_var): New function.
	(cleanup_after_saved_dg_test): Call it.

Index: gcc-dg.exp
===================================================================
--- gcc-dg.exp	(revision 236021)
+++ gcc-dg.exp	(working copy)
@@ -31,6 +31,9 @@ load_lib torture-options.exp
 load_lib fortran-modules.exp
 load_lib multiline.exp
 
+set orig_source_date_epoch_saved 0
+set orig_source_date_epoch_checked 0
+
 # We set LC_ALL and LANG to C so that we get the same error messages as expected.
 setenv LC_ALL C
 setenv LANG C
@@ -422,9 +425,24 @@ proc dg-set-target-env-var { args } {
     lappend set_target_env_var [list [lindex $args 1] [lindex $args 2]]
 }
 
+proc dg-set-source-date-epoch { args } {
+  global orig_source_date_epoch
+  global orig_source_date_epoch_saved
+  global orig_source_date_epoch_checked
+  if { $orig_source_date_epoch_checked == 0 } {
+    if [info exists env(SOURCE_DATE_EPOCH)] {
+      set orig_source_date_epoch "$env(SOURCE_DATE_EPOCH)"
+      set orig_source_date_epoch_saved 1
+    }
+    set orig_source_date_epoch_checked 1
+  }
+  setenv SOURCE_DATE_EPOCH [lindex $args 1]
+}
+
 proc set-target-env-var { } {
     global set_target_env_var
     upvar 1 saved_target_env_var saved_target_env_var
+    verbose "setting env var"
     foreach env_var $set_target_env_var {
 	set var [lindex $env_var 0]
 	set value [lindex $env_var 1]
@@ -840,6 +858,18 @@ proc output-exists-not { args } {
     }
 }
 
+proc restore_source_epoch_env_var { } {
+  global orig_source_date_epoch_saved
+  global orig_source_date_epoch
+  global env
+
+  if { $orig_source_date_epoch_saved } {
+    setenv SOURCE_DATE_EPOCH "$orig_source_date_epoch"
+  } elseif [info exists env(SOURCE_DATE_EPOCH)] {
+    unsetenv SOURCE_DATE_EPOCH
+  }
+}
+
 # We need to make sure that additional_* are cleared out after every
 # test.  It is not enough to clear them out *before* the next test run
 # because gcc-target-compile gets run directly from some .exp files
@@ -877,6 +907,7 @@ if { [info procs saved-dg-test] == [list
 	if [info exists keep_saved_temps_suffixes] {
 	    unset keep_saved_temps_suffixes
 	}
+	restore_source_epoch_env_var
 	unset_timeout_vars
 	if [info exists compiler_conditional_xfail_data] {
 	    unset compiler_conditional_xfail_data

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Mon, May 09, 2016 at 12:38:14PM +0200, Bernd Schmidt wrote:
> On 05/09/2016 12:23 PM, Bernd Schmidt wrote:
> >On 05/06/2016 01:38 AM, Dhole wrote:
> >>I've wrote a test case which fails (when it shouldn't) and I don't see
> >>why.
> >
> >I think it's setting the env var when executing the test, not when
> >executing the compiler.
> 
> Here's something that seems to work (with an appropriately adjusted
> testcase). Cc'ing testsuite maintainers.

Wouldn't it be better to add a more general directive, e.g. one that would
allow setting any env var for the compiler, rather than one specific,
similarly to how dg-set-target-env-var allows any env var to be set for the
execution?  dg-set-compiler-env-var ?

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 05/09/2016 12:42 PM, Jakub Jelinek wrote:
> On Mon, May 09, 2016 at 12:38:14PM +0200, Bernd Schmidt wrote:
>> On 05/09/2016 12:23 PM, Bernd Schmidt wrote:
>>> On 05/06/2016 01:38 AM, Dhole wrote:
>>>> I've wrote a test case which fails (when it shouldn't) and I don't see
>>>> why.
>>>
>>> I think it's setting the env var when executing the test, not when
>>> executing the compiler.
>>
>> Here's something that seems to work (with an appropriately adjusted
>> testcase). Cc'ing testsuite maintainers.
>
> Wouldn't it be better to add a more general directive, e.g. one that would
> allow setting any env var for the compiler, rather than one specific,
> similarly to how dg-set-target-env-var allows any env var to be set for the
> execution?  dg-set-compiler-env-var ?

Maybe. Eduard, can you look into that?


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

On 16-05-09 13:01:12, Bernd Schmidt wrote:
> >Wouldn't it be better to add a more general directive, e.g. one that would
> >allow setting any env var for the compiler, rather than one specific,
> >similarly to how dg-set-target-env-var allows any env var to be set for the
> >execution?  dg-set-compiler-env-var ?
> 
> Maybe. Eduard, can you look into that?

Yes!  I'll look into that and share it here once I have something :)

BTW: review on my patch addressig several comments from this tread is
very welcome:
https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00428.html

-- 
Dhole

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

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

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Joseph Myers]

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.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1.1: Type: text/plain, Size: 3958 bytes --]

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,
-- 
Dhole

[-- Attachment #1.2: ChangeLog --]
[-- Type: text/plain, Size: 2182 bytes --]

gcc/c-family/ChangeLog:

2016-05-12  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-common.h (c_omp_region_type): Remove trailing coma.
	* c-lex.c (init_c_lex): Set cb->get_source_date_epoch callback.
	* c-lex.c (c_lex_with_flags): Initialize source_date_epoch with the
	new cpp_init_source_date_epoch.

gcc/ChangeLog:

2016-05-12  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 set_source_date_epoch_envvar.
	* gcc.c (set_source_date_epoch_envvar): New function, sets
	the SOURCE_DATE_EPOCH environment variable to the current time.

gcc/testsuite/ChangeLog:

2016-05-12  Eduard Sanou  <dhole@openmailbox.org>

	* gcc.dg/cpp/source_date_epoch-1.c: New file, test the proper
	behaviour of the macros __DATE__ and __TIME__ when SOURCE_DATE_EPOCH
	env var is set.
	* gcc.dg/cpp/source_date_epoch-2.c: New file, test the error output
	when parsing the SOURCE_DATE_EPOCH env var, and make sure it is only
	shown once.
	* lib/gcc-dg.exp (dg-set-compiler-env-var): New function, set env vars
	during compilation.
	* lib/gcc-dg.exp (restore-compiler-env-var): New function, restore env
	vars set by dg-set-compiler-env-var.

libcpp/ChangeLog:

2016-05-12  Eduard Sanou  <dhole@openmailbox.org>

	* include/cpplib.h (cpp_callbacks): Add get_source_date_epoch
	callback.
	* include/cpplib.h (cpp_init_source_date_epoch): Modify prototype.
	* init.c (cpp_init_source_date_epoch): Set source_date_epoch to the
	initialization value -2.
	* internal.h (cpp_reader): Extend comment about source_date_epoch.
	* macro.c (_cpp_builtin_macro_text): Use get_source_date_epoch
	callback only once, read pfile->source_date_epoch on future passes.
	Check that get_source_date_epoch callback is not NULL.

[-- Attachment #1.3: gcc-SOURCE_DATE_EPOCH-patch-2016_05_12.diff.txt --]
[-- Type: text/plain, Size: 12353 bytes --]

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<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 *);
@@ -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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 05/12/2016 02:36 AM, Dhole wrote:
> +      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);

> +/* 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 should use HOST_WIDE_INT_C to make sure we match %wd in the error 
output, and to make sure we don't get any too large for an integer warnings.

> +  struct tm *tb = NULL;
[...]
> +  snprintf (source_date_epoch, 21, "%llu", (unsigned long long) tb);

That seems like the wrong thing to print.

> 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" } */

You can shorten the string you look for, like just "SOURCE_DATE_EPOCH 
must expand". People generally also skip the second arg to dg-error.

> +  __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" }  */

I would have expected no dg- directive at all on this line. Without one, 
any message should be reported as an excess error by the framework.

> @@ -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
> +	}

Shouldn't we also clear saved_compiler_env_var to keep that from growing?

> @@ -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:

I just spotted this - why is this initialization here and not in say 
init_c_lex? Or skip the call into libcpp and just put it in 
cpp_create_reader.

> 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);

Formatting.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1.1: Type: text/plain, Size: 3997 bytes --]

On 16-05-12 11:16:57, Bernd Schmidt wrote:
> On 05/12/2016 02:36 AM, Dhole wrote:
> >+      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);
> 
> >+/* 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 should use HOST_WIDE_INT_C to make sure we match %wd in the error
> output, and to make sure we don't get any too large for an integer warnings.

Done.

> >+  struct tm *tb = NULL;
> [...]
> >+  snprintf (source_date_epoch, 21, "%llu", (unsigned long long) tb);
> 
> That seems like the wrong thing to print.

Sorry about that.  I actually feel bad about it.  Fixed.

> >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" } */
> 
> You can shorten the string you look for, like just "SOURCE_DATE_EPOCH must
> expand". People generally also skip the second arg to dg-error.

Done.

> >+  __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" }  */
> 
> I would have expected no dg- directive at all on this line. Without one, any
> message should be reported as an excess error by the framework.

I wasn't sure about that, thanks for clarifying.  Done.

> >@@ -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
> >+	}
> 
> Shouldn't we also clear saved_compiler_env_var to keep that from growing?

You're right, done.

> >@@ -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:
> 
> I just spotted this - why is this initialization here and not in say
> init_c_lex? Or skip the call into libcpp and just put it in
> cpp_create_reader.

That makes more sense.  Moved the initialization to cpp_create_reader,
without the need to add a new function.

> >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);
> 
> Formatting.

Done.

Cheers,
-- 
Dhole

[-- Attachment #1.2: gcc-SOURCE_DATE_EPOCH-patch-2016_05_13.diff.txt --]
[-- Type: text/plain, Size: 12406 bytes --]

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..0654feb 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1088,6 +1088,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 HOST_WIDE_INT_C (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..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 7bcf3b3..ab11310 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3547,6 +3547,29 @@ 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;
+
+  errno = 0;
+  tt = time (NULL);
+  if (tt < (time_t) 0 || errno != 0)
+    tt = (time_t) 0;
+
+  snprintf (source_date_epoch, 21, "%llu", (unsigned long long) tt);
+  /* 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 +3869,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..ae18362
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/source_date_epoch-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-set-compiler-env-var SOURCE_DATE_EPOCH "AAA" } */
+
+/* Make sure that SOURCE_DATE_EPOCH is only parsed once */
+
+int
+main(void)
+{
+  __builtin_printf ("%s %s\n", __DATE__, __TIME__); /* { dg-error "SOURCE_DATE_EPOCH must expand" } */
+  __builtin_printf ("%s %s\n", __DATE__, __TIME__);
+  return 0;
+}
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 39a7c20..2304740 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,11 @@ 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
+	    unset saved_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..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..e78b320 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -257,6 +257,9 @@ cpp_create_reader (enum c_lang lang, cpp_hash_table *table,
   /* Do not force token locations by default.  */
   pfile->forced_token_location_p = NULL;
 
+  /* Initialize source_date_epoch to -2 (not yet set).  */
+  pfile->source_date_epoch = (time_t) -2;
+
   /* The expression parser stack.  */
   _cpp_expand_op_stack (pfile);
 
@@ -533,13 +536,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..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..ccd09b3 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

[-- Attachment #1.3: ChangeLog --]
[-- Type: text/plain, Size: 2191 bytes --]

gcc/c-family/ChangeLog:

2016-05-13  Eduard Sanou  <dhole@openmailbox.org>

	* c-common.c (get_source_date_epoch): Rename 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): Rename to
	cb_get_source_date_epoch.
	* c-common.h (cb_get_source_date_epoch): Prototype.
	* c-common.h (MAX_SOURCE_DATE_EPOCH): Define.
	* c-common.h (c_omp_region_type): Remove trailing coma.
	* c-lex.c (init_c_lex): Set cb->get_source_date_epoch callback.
	* c-lex.c (c_lex_with_flags): Remove initialization of
	pfile->source_date_epoch.

gcc/ChangeLog:

2016-05-13  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 set_source_date_epoch_envvar.
	* gcc.c (set_source_date_epoch_envvar): New function, sets
	the SOURCE_DATE_EPOCH environment variable to the current time.

gcc/testsuite/ChangeLog:

2016-05-13  Eduard Sanou  <dhole@openmailbox.org>

	* gcc.dg/cpp/source_date_epoch-1.c: New file, test the proper
	behaviour of the macros __DATE__ and __TIME__ when SOURCE_DATE_EPOCH
	env var is set.
	* gcc.dg/cpp/source_date_epoch-2.c: New file, test the error output
	when parsing the SOURCE_DATE_EPOCH env var, and make sure it is only
	shown once.
	* lib/gcc-dg.exp (dg-set-compiler-env-var): New function, set env vars
	during compilation.
	* lib/gcc-dg.exp (restore-compiler-env-var): New function, restore env
	vars set by dg-set-compiler-env-var.

libcpp/ChangeLog:

2016-05-13  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 prototype.
	* init.c (cpp_init_source_date_epoch): Remove function.
	* init.c (cpp_create_reader): Initialize pfile->source_date_epoch.
	* internal.h (cpp_reader): Extend comment about source_date_epoch.
	* macro.c (_cpp_builtin_macro_text): Use get_source_date_epoch
	callback only once, read pfile->source_date_epoch on future passes.
	Check that get_source_date_epoch callback is not NULL.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Dhole]

[-- Attachment #1: Type: text/plain, Size: 20 bytes --]

PING

-- 
Dhole

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jeff Law]

On 05/23/2016 04:59 PM, Dhole wrote:
> PING
Note Bernd is on PTO for another week or so.

jeff

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 05/13/2016 07:09 PM, Dhole wrote:
> +	    pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile);

Space before paren. Ok with that change.

> 	* c-common.h (c_omp_region_type): Remove trailing coma.

Also, comma.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Matthias Klose]

On 01.06.2016 18:29, Bernd Schmidt wrote:
> On 05/13/2016 07:09 PM, Dhole wrote:
>> +        pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile);
> 
> Space before paren. Ok with that change.
> 
>>     * c-common.h (c_omp_region_type): Remove trailing coma.
> 
> Also, comma.

committed with these changes.

Fix up dg-set-compiler-env-var

[Jakub Jelinek]

Hi!

On Fri, May 13, 2016 at 07:09:30PM +0200, Dhole wrote:
> --- 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 } {

I've noticed last night pr61861.c FAIL in i686-linux bootstrap,
easily reproduceable with short:
make check-gcc RUNTESTFLAGS='cpp.exp=source_date* dg.exp=pr61861.c'

The problem is that in cleanup-after-saved-dg-test, there are missing global
directives, so when it tests/uses the set_compiler_env_var and
saved_compiler_env_var vars to see if it should call
restore-compiler-env-var, it uses local (non-existing) vars instead and
never restores the env var.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok of
trunk?

2016-06-02  Jakub Jelinek  <jakub@redhat.com>

	* lib/gcc-dg.exp (cleanup-after-saved-dg-test): Add missing
	global set_compiler_env_var and global saved_compiler_env_var.

--- gcc/testsuite/lib/gcc-dg.exp.jj	2016-06-01 19:16:51.000000000 +0200
+++ gcc/testsuite/lib/gcc-dg.exp	2016-06-02 11:56:54.429137894 +0200
@@ -895,6 +895,8 @@ if { [info procs saved-dg-test] == [list
 	global shouldfail
 	global testname_with_flags
 	global set_target_env_var
+	global set_compiler_env_var
+	global saved_compiler_env_var
 	global keep_saved_temps_suffixes
 	global multiline_expected_outputs
 


	Jakub

Re: Fix up dg-set-compiler-env-var

[Bernd Schmidt]

On 06/02/2016 02:19 PM, Jakub Jelinek wrote:
> The problem is that in cleanup-after-saved-dg-test, there are missing global
> directives, so when it tests/uses the set_compiler_env_var and
> saved_compiler_env_var vars to see if it should call
> restore-compiler-env-var, it uses local (non-existing) vars instead and
> never restores the env var.

I'm puzzled how failing to restore the env var causes a failure, but 
your patch is (obviously) ok.


Bernd

Re: Fix up dg-set-compiler-env-var

[Jakub Jelinek]

On Thu, Jun 02, 2016 at 02:26:28PM +0200, Bernd Schmidt wrote:
> On 06/02/2016 02:19 PM, Jakub Jelinek wrote:
> >The problem is that in cleanup-after-saved-dg-test, there are missing global
> >directives, so when it tests/uses the set_compiler_env_var and
> >saved_compiler_env_var vars to see if it should call
> >restore-compiler-env-var, it uses local (non-existing) vars instead and
> >never restores the env var.
> 
> I'm puzzled how failing to restore the env var causes a failure, but your
> patch is (obviously) ok.

The second of the tests using the new dg-set-compiler-env-var directive is
doing
/* { dg-set-compiler-env-var SOURCE_DATE_EPOCH "AAA" } */
and then expecting an error.  If the env var isn't restored, then on any of
the following testcases that uses __DATE__ and similar macros the env var
contains still AAA and thus results in an error.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Christophe Lyon]

On 1 June 2016 at 18:59, Matthias Klose <doko@ubuntu.com> wrote:
> On 01.06.2016 18:29, Bernd Schmidt wrote:
>> On 05/13/2016 07:09 PM, Dhole wrote:
>>> +        pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile);
>>
>> Space before paren. Ok with that change.
>>
>>>     * c-common.h (c_omp_region_type): Remove trailing coma.
>>
>> Also, comma.
>
> committed with these changes.
>

Hi,

Since this was committed, I'm seeing:
- ~random failures of pr61861:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pr61861.c:22:3:
error: environment variable SOURCE_DATE_EPOCH must expand to a
non-negative integer less than or equal to 253402300799

I've noticed that HJ has reported it on gcc-regressions, too.

It would help if the error message showed the value of SOURCE_DATE_EPOCH :)

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Jun 02, 2016 at 03:01:07PM +0200, Christophe Lyon wrote:
> On 1 June 2016 at 18:59, Matthias Klose <doko@ubuntu.com> wrote:
> > On 01.06.2016 18:29, Bernd Schmidt wrote:
> >> On 05/13/2016 07:09 PM, Dhole wrote:
> >>> +        pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile);
> >>
> >> Space before paren. Ok with that change.
> >>
> >>>     * c-common.h (c_omp_region_type): Remove trailing coma.
> >>
> >> Also, comma.
> >
> > committed with these changes.
> >
> 
> Since this was committed, I'm seeing:
> - ~random failures of pr61861:
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pr61861.c:22:3:
> error: environment variable SOURCE_DATE_EPOCH must expand to a
> non-negative integer less than or equal to 253402300799
> 
> I've noticed that HJ has reported it on gcc-regressions, too.
> 
> It would help if the error message showed the value of SOURCE_DATE_EPOCH :)

I've fixed that already in r237035.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Christophe Lyon]

On 2 June 2016 at 15:05, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jun 02, 2016 at 03:01:07PM +0200, Christophe Lyon wrote:
>> On 1 June 2016 at 18:59, Matthias Klose <doko@ubuntu.com> wrote:
>> > On 01.06.2016 18:29, Bernd Schmidt wrote:
>> >> On 05/13/2016 07:09 PM, Dhole wrote:
>> >>> +        pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile);
>> >>
>> >> Space before paren. Ok with that change.
>> >>
>> >>>     * c-common.h (c_omp_region_type): Remove trailing coma.
>> >>
>> >> Also, comma.
>> >
>> > committed with these changes.
>> >
>>
>> Since this was committed, I'm seeing:
>> - ~random failures of pr61861:
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pr61861.c:22:3:
>> error: environment variable SOURCE_DATE_EPOCH must expand to a
>> non-negative integer less than or equal to 253402300799
>>
>> I've noticed that HJ has reported it on gcc-regressions, too.
>>
>> It would help if the error message showed the value of SOURCE_DATE_EPOCH :)
>
> I've fixed that already in r237035.
>

Ha, thanks, I had missed it.

But why do I see "random" failures?
Looking at your patch, I have the impression that the test should always fail?

Christophe

>         Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Jun 02, 2016 at 03:21:03PM +0200, Christophe Lyon wrote:
> Ha, thanks, I had missed it.
> 
> But why do I see "random" failures?
> Looking at your patch, I have the impression that the test should always fail?

Only with make -j1 -k check it should always FAIL.  With -j2 and above, it
really depends if the same runtest instance runs after the second of these
tests any test with __DATE__ etc. macros in it or not.

	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Christophe Lyon]

On 2 June 2016 at 15:01, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On 1 June 2016 at 18:59, Matthias Klose <doko@ubuntu.com> wrote:
>> On 01.06.2016 18:29, Bernd Schmidt wrote:
>>> On 05/13/2016 07:09 PM, Dhole wrote:
>>>> +        pfile->source_date_epoch = pfile->cb.get_source_date_epoch(pfile);
>>>
>>> Space before paren. Ok with that change.
>>>
>>>>     * c-common.h (c_omp_region_type): Remove trailing coma.
>>>
>>> Also, comma.
>>
>> committed with these changes.
>>
>
> Hi,
>
> Since this was committed, I'm seeing:
> - ~random failures of pr61861:
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pr61861.c:22:3:
> error: environment variable SOURCE_DATE_EPOCH must expand to a
> non-negative integer less than or equal to 253402300799
>
> I've noticed that HJ has reported it on gcc-regressions, too.
>
> It would help if the error message showed the value of SOURCE_DATE_EPOCH :)

I'm also seeing that the new gcc.dg/cpp/source_date_epoch-1.c fails because
the output pattern finishes with '\n' instead of the usual '(\n|\r\n|\r)'

Can I add this as obvious?

Thanks,

Christophe

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Jakub Jelinek]

On Thu, Jun 02, 2016 at 04:49:59PM +0200, Christophe Lyon wrote:
> I'm also seeing that the new gcc.dg/cpp/source_date_epoch-1.c fails because
> the output pattern finishes with '\n' instead of the usual '(\n|\r\n|\r)'
> 
> Can I add this as obvious?

Some remote test invocations even eat the final newline from what I vaguely
remember from various complains about the asan and ubsan testsuite.

So we could just drop that ^ and \n$ from dg-output, but then, what is the
point of using dg-output at all here?

Isn't following just better?  Tested on x86_64-linux, ok for trunk?

2016-06-02  Jakub Jelinek  <jakub@redhat.com>

	* gcc.dg/cpp/source_date_epoch-1.c (main): Test __DATE__ and
	__TIME__ strings with __builtin_strcmp instead of printf and
	dg-output.

--- gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c.jj	2016-06-01 21:26:27.000000000 +0200
+++ gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c	2016-06-02 17:02:06.164281564 +0200
@@ -2,10 +2,10 @@
 /* { dg-set-compiler-env-var SOURCE_DATE_EPOCH "630333296" } */
 
 int
-main(void)
+main ()
 {
-  __builtin_printf ("%s %s\n", __DATE__, __TIME__);
+  if (__builtin_strcmp (__DATE__, "Dec 22 1989") != 0
+      || __builtin_strcmp (__TIME__, "12:34:56") != 0)
+    __builtin_abort ();
   return 0;
 }
-
-/* { dg-output "^Dec 22 1989 12:34:56\n$" } */


	Jakub

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Bernd Schmidt]

On 06/02/2016 05:04 PM, Jakub Jelinek wrote:
> Isn't following just better?  Tested on x86_64-linux, ok for trunk?
>
> 2016-06-02  Jakub Jelinek  <jakub@redhat.com>
>
> 	* gcc.dg/cpp/source_date_epoch-1.c (main): Test __DATE__ and
> 	__TIME__ strings with __builtin_strcmp instead of printf and
> 	dg-output.
>

Sure.


Bernd

Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Christophe Lyon]

On 2 June 2016 at 17:04, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jun 02, 2016 at 04:49:59PM +0200, Christophe Lyon wrote:
>> I'm also seeing that the new gcc.dg/cpp/source_date_epoch-1.c fails because
>> the output pattern finishes with '\n' instead of the usual '(\n|\r\n|\r)'
>>
>> Can I add this as obvious?
>
> Some remote test invocations even eat the final newline from what I vaguely
> remember from various complains about the asan and ubsan testsuite.
>
Yes, that's a nightmare :)
Also when combined with qemu, in my configuration it thinks that stderr is
a tty so *san tests use the colorization codes.

And when it comes to cross-testing as we do it in Linaro, we have problems
with dejagnu eating the trailing \n in the remote execution functions.
But that's
another different problem.

> So we could just drop that ^ and \n$ from dg-output, but then, what is the
> point of using dg-output at all here?
>
> Isn't following just better?  Tested on x86_64-linux, ok for trunk?
LGTM

Thanks

> 2016-06-02  Jakub Jelinek  <jakub@redhat.com>
>
>         * gcc.dg/cpp/source_date_epoch-1.c (main): Test __DATE__ and
>         __TIME__ strings with __builtin_strcmp instead of printf and
>         dg-output.
>
> --- gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c.jj   2016-06-01 21:26:27.000000000 +0200
> +++ gcc/testsuite/gcc.dg/cpp/source_date_epoch-1.c      2016-06-02 17:02:06.164281564 +0200
> @@ -2,10 +2,10 @@
>  /* { dg-set-compiler-env-var SOURCE_DATE_EPOCH "630333296" } */
>
>  int
> -main(void)
> +main ()
>  {
> -  __builtin_printf ("%s %s\n", __DATE__, __TIME__);
> +  if (__builtin_strcmp (__DATE__, "Dec 22 1989") != 0
> +      || __builtin_strcmp (__TIME__, "12:34:56") != 0)
> +    __builtin_abort ();
>    return 0;
>  }
> -
> -/* { dg-output "^Dec 22 1989 12:34:56\n$" } */
>
>
>         Jakub