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