[Reproducible-builds] GCC patch to honour SOURCE_DATE_EPOCH, comments?

Dhole dhole at openmailbox.org
Mon Sep 7 18:42:23 UTC 2015


Hi Matthias and everyone,

During DebConf we talked about a patch for GCC to honour the
SOURCE_DATE_EPOCH env var when using the __DATE__ and __TIME__ macros.

I sent a patch that did this from libcpp/macro.c in the gcc-patches
mailing list. There was a reply from Joseph Myers <joseph at
codesourcery dot com> suggesting that it may be better to put the patch
in some file in the gcc/ directory [0].

I checked the call trace until reaching the macro processing for both
gcc and g++ and found a common function in gcc/c-family/c-lex.c, so I
moved part of the patch there.

Now the patch consists on the following:
- the struct cpp_reader (defined in libcpp/internal.h) now has a new
field: source_date_epoch, with a default value of -1
- the function c_lex_with_flags from gcc/c-family/c-lex.c will check if
SOURCE_DATE_EPOCH is defined in the environment and if so, store the
value in the cpp_reader struct (parse_in), to pass it around the other
functions
- the function _cpp_builtin_macro_text from _cpp_builtin_macro_text will
use the source_date_epoch field from the cpp_reader struct instead of
the value returned by time(NULL) and use gmtime to get a representation
of the timestamp, if source_date_epoch != -1 (that is, it has been defined)

This way, the getenv call happens in the gcc/ directory.

There is one thing that I haven't managed to get nice, as I'm not that
familiar with the gcc source code: the ouptut of the error handling when
parsing SOURCE_DATE_EPOCH in gcc/c-family/c-lex.c doesn't look too good:

"""
$ export SOURCE_DATE_EPOCH="1234a"
$../gcc_build/bin/gcc test_c.c -o test_c
In file included from <command-line>:0:0:
/usr/include/stdc-predef.h:1:0: error: Environment variable
$SOURCE_DATE_EPOCH: Trailing garbage: a

 /* Copyright (C) 1991-2014 Free Software Foundation, Inc.
 ^
"""

The error function prints the line of the file it was parsing when the
error happened.

What do you think about this patch? I would like you to take a look at
it before sending it to the gcc-patches mailing list. Do you think it's
good to read the SOURCE_DATE_EPOCH env var in gcc/c-family/c-lex.c and
pass it to the macro processing function through the cpp_reader struct?
Or do you think there's a better place?

As a reminder, we now have a spec describing the usage of
SOURCE_DATE_EPOCH as a distribution-agnostic standard for build systems
to exchange a timestamp [1], I'll add the link when sending the patch to
the gcc-patches mailing list.

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

Regards,
-- 
Dhole
-------------- next part --------------
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index bb55be8..265195f 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -44,6 +44,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "debug.h"
 #include "target.h"
 #include "wide-int.h"
+#include "../libcpp/internal.h"
+#include <errno.h>		/* For parsing SOURCE_DATE_EPOCH */
+#include <limits.h>		/* For parsing SOURCE_DATE_EPOCH */
 
 #include "attribs.h"
 
@@ -388,6 +391,53 @@ c_common_has_attribute (cpp_reader *pfile)
 
   return result;
 }
+
+/* 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)
+    {
+      errno = 0;
+      epoch = strtoull (source_date_epoch, &endptr, 10);
+      if ((errno == ERANGE && (epoch == ULLONG_MAX || epoch == 0))
+	  || (errno != 0 && epoch == 0))
+	{
+	  error ("Environment variable $SOURCE_DATE_EPOCH: strtoull: \
+%s\n", xstrerror(errno));
+	  exit (EXIT_FAILURE);
+	}
+      if (endptr == source_date_epoch)
+	{
+	  error ("Environment variable $SOURCE_DATE_EPOCH: \
+No digits were found: %s\n", endptr);
+	  exit (EXIT_FAILURE);
+	}
+      if (*endptr != '\0')
+	{
+	  error ("Environment variable $SOURCE_DATE_EPOCH: \
+Trailing garbage: %s\n", endptr);
+	  exit (EXIT_FAILURE);
+        }
+      if (epoch > ULONG_MAX)
+	{
+	  error ("Environment variable $SOURCE_DATE_EPOCH: \
+value must be smaller than or equal to: %lu but was found to be: \
+%llu \n", ULONG_MAX, epoch);
+	  exit (EXIT_FAILURE);
+	}
+      return (long long) epoch;
+    }
+  else
+    return -1;
+}
 

 /* Read a token and return its type.  Fill *VALUE with its value, if
    applicable.  Fill *CPP_FLAGS with the token's flags, if it is
@@ -403,6 +453,7 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
   unsigned char add_flags = 0;
   enum overflow_type overflow = OT_NONE;
 
+  parse_in->source_date_epoch = get_source_date_epoch();
   timevar_push (TV_CPP);
  retry:
   tok = cpp_get_token_with_location (parse_in, loc);
diff --git a/libcpp/internal.h b/libcpp/internal.h
index c2d0816..cc80c9c 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 1e0a0b5..adb5309 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -350,13 +350,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)
 	    {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/reproducible-builds/attachments/20150907/011e76d8/attachment.sig>


More information about the Reproducible-builds mailing list