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

[Bernd Schmidt <bschmidt at redhat dot com>]

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