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

Eli Zaretskii
2020-06-18T15:04:18Z

> 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.