Re: [PATCH 1/6] parse-datetime, posixtm: avoid uninit access

[Pádraig Brady]

On 25/09/17 18:29, Paul Eggert wrote:
> * lib/parse-datetime.y (parse_datetime2):
> * lib/posixtm.c (posixtime):
> Do not access uninitialized storage, even though the resulting
> value is never used.
> ---
>  ChangeLog            |  8 ++++++++
>  lib/parse-datetime.y | 16 ++++++++++++++--
>  lib/posixtm.c        |  7 ++++++-
>  3 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 386986ee7..9c6d73f72 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2017-09-25  Paul Eggert  <address@hidden>
> +
> +     parse-datetime, posixtm: avoid uninit access
> +     * lib/parse-datetime.y (parse_datetime2):
> +     * lib/posixtm.c (posixtime):
> +     Do not access uninitialized storage, even though the resulting
> +     value is never used.
> +
>  2017-09-25  Bruno Haible  <address@hidden>
>  
>       vma-iter: Improvements for BSD platforms.
> diff --git a/lib/parse-datetime.y b/lib/parse-datetime.y
> index 9eff2dc3b..f8da02d3f 100644
> --- a/lib/parse-datetime.y
> +++ b/lib/parse-datetime.y
> @@ -2034,7 +2034,13 @@ parse_datetime2 (struct timespec *result, char const 
> *p,
>        if (pc.local_zones_seen)
>          tm.tm_isdst = pc.local_isdst;
>  
> -      tm0 = tm;
> +      tm0.tm_sec = tm.tm_sec;
> +      tm0.tm_min = tm.tm_min;
> +      tm0.tm_hour = tm.tm_hour;
> +      tm0.tm_mday = tm.tm_mday;
> +      tm0.tm_mon = tm.tm_mon;
> +      tm0.tm_year = tm.tm_year;
> +      tm0.tm_isdst = tm.tm_isdst;
>  
>        Start = mktime_z (tz, &tm);
>  
> @@ -2064,7 +2070,13 @@ parse_datetime2 (struct timespec *result, char const 
> *p,
>                      dbg_printf (_("error: tzalloc (\"%s\") failed\n"), 
> tz2buf);
>                    goto fail;
>                  }
> -              tm = tm0;
> +              tm.tm_sec = tm0.tm_sec;
> +              tm.tm_min = tm0.tm_min;
> +              tm.tm_hour = tm0.tm_hour;
> +              tm.tm_mday = tm0.tm_mday;
> +              tm.tm_mon = tm0.tm_mon;
> +              tm.tm_year = tm0.tm_year;
> +              tm.tm_isdst = tm0.tm_isdst;
>                Start = mktime_z (tz2, &tm);
>                repaired = mktime_ok (tz2, &tm0, &tm, Start);
>                tzfree (tz2);
> diff --git a/lib/posixtm.c b/lib/posixtm.c
> index 26a35dd3f..030f704f0 100644
> --- a/lib/posixtm.c
> +++ b/lib/posixtm.c
> @@ -182,7 +182,12 @@ posixtime (time_t *p, const char *s, unsigned int 
> syntax_bits)
>    if (! posix_time_parse (&tm0, s, syntax_bits))
>      return false;
>  
> -  tm1 = tm0;
> +  tm1.tm_sec = tm0.tm_sec;
> +  tm1.tm_min = tm0.tm_min;
> +  tm1.tm_hour = tm0.tm_hour;
> +  tm1.tm_mday = tm0.tm_mday;
> +  tm1.tm_mon = tm0.tm_mon;
> +  tm1.tm_year = tm0.tm_year;
>    tm1.tm_isdst = -1;
>    t = mktime (&tm1);
>  
> 

This triggers the following warning with gcc-6.3

  lib/posixtm.c:214:20: error: '*((void *)&tm0+20)' may be used
  uninitialized in this function [-Werror=maybe-uninitialized]
     if ((tm0.tm_year ^ tm->tm_year)
         ~~~~~~~~~~~~~^~~~~~~~~~~~~~

It's not introducing any new issue I think, but
seems to be triggering the compiler warning due
to tm_year being explicitly set?

How about the attached to ensure tm_year is set?

cheers,
Pádraig

posixtm-warnings.patch
Description: Text Data