Re: [PATCH 2/3] Consistently use BFD's time

[Eli Zaretskii <eliz@gnu.org>]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org,
>  Pedro Alves <palves@redhat.com> 
> Date: Thu, 18 Jun 2020 08:14:18 -0600
> 
> Eli> There's one more potential issue with Gnulib's replacement of 'fstat':
> Eli> it also replaces the definition of 'struct stat', and it does that in
> Eli> a way that might yield incompatibility between the definition on
> Eli> <sys/stat.h> the system header and Gnulib's sys/stat.h replacement.
> 
> I am looking into this issue again for the upcoming gdb 10 release.
> 
> Looking at the gnulib in the gdb tree, as far as I can tell, it doesn't
> actually override struct stat.  The code is all there:
> 
>     /* Optionally, override 'struct stat' on native Windows.  */
>     #if @GNULIB_OVERRIDES_STRUCT_STAT@
>     [...]
> 
> ... however, this is only ever set to 0 in our tree:
> 
>     $ git grep 'GNULIB_OVERRIDES_STRUCT_STAT.*=' -- gnulib/import/
>     gnulib/import/Makefile.in:GNULIB_OVERRIDES_STRUCT_STAT = @GNULIB_OVERRIDES_STRUCT_STAT@
>     gnulib/import/m4/sys_stat_h.m4:  GNULIB_OVERRIDES_STRUCT_STAT=0; AC_SUBST([GNULIB_OVERRIDES_STRUCT_STAT])
> 
> (I chose a tighter grep for the purposes of posting, but in reality I
> looked at all mentions of GNULIB_OVERRIDES_STRUCT_STAT.)
> 
> So, I think this is maybe not an issue.

I meant a different part of Gnulib's sys/stat.h:

  /* Large File Support on native Windows.  */
  #if @WINDOWS_64_BIT_ST_SIZE@
  # define stat _stati64
  #endif

This will also redirect 'struct stat' to 'struct _stati64', which has
a 64-bit st_size and a 64-bit st_mtime (and other times) members.  And
by default @WINDOWS_64_BIT_ST_SIZE@ evaluates to 1 in the MinGW build.

This might not be a problem when building with MinGW64, but my GDB
builds are 32-bit, and I want to use the default version of 'struct
stat', where both the st_size and the time-related members are 32-bit
wide.  Therefore, I had a problem with the above redirection.

In case you need to refresh your memory, I explained all that in more
detail here:

  https://sourceware.org/pipermail/gdb-patches/2020-January/164882.html

> The timezone thing, however, remains an issue.  It seems dangerous to me
> that we would have two implementations of stat that would give different
> answers.  It's too easy, IMO, to accidentally compare values from one
> with values from the other -- in fact, that's what lead to the patches
> in this thread I'm replying to.

I agree.

> Based on this, my inclination is to patch our copy of gnulib to avoid
> replacing stat on Windows.  I'm looking into how to do this.

AFAICT, gnulib/import/m4/stat.m4 does that for MinGW unconditionally:

  case "$host_os" in
    mingw*)
      dnl On this platform, the original stat() returns st_atime, st_mtime,
      dnl st_ctime values that are affected by the time zone.
      REPLACE_STAT=1
      ;;

So the way to avoid the replacement is to modify this test, right?

Thanks.