[PATCH] Do not adjust mtime timezone on Windows

[PATCH] Do not adjust mtime timezone on Windows

[Tom Tromey]

PR win32/25302 notes that gdb will crash when trying to "run" even a
simple program on Windows.  The essential bug here is that the BFD
cache can easily be corrupted -- I have sent a separate patch for
that.

The particular reason that the cache is corrupted on Windows is that
gnulib overrides "stat" to make it do timezone adjustment -- but BFD
does not use this version of stat.  The difference here triggers the
latent cache bug, but can also cause other bugs as well; in particular
it can cause spurious warnings about source files being newer.

This patch simply removes the stat override on mingw, making gnulib
and BFD agree.

I tested this by backing out the local AdaCore changes to work around
this bug and then verifying that I could reproduce it.  Then, I
applied this patch and verified that "run" works again.

gnulib/ChangeLog
2020-09-03  Tom Tromey  <tromey@adacore.com>

	PR win32/25302:
	* update-gnulib.sh: Apply stat patch.
	* patches/0001-use-windows-stat: New file.
	* import/m4/stat.m4: Update.
	* configure: Rebuild.
---
 gnulib/ChangeLog                     |  8 ++++++++
 gnulib/configure                     |  3 +--
 gnulib/import/m4/stat.m4             |  2 +-
 gnulib/patches/0001-use-windows-stat | 13 +++++++++++++
 gnulib/update-gnulib.sh              |  2 ++
 5 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 gnulib/patches/0001-use-windows-stat

diff --git a/gnulib/configure b/gnulib/configure
index fa2feb5ea9a..5c6add6e371 100644
--- a/gnulib/configure
+++ b/gnulib/configure
@@ -26907,8 +26907,7 @@ $as_echo "#define ssize_t int" >>confdefs.h
 
   case "$host_os" in
     mingw*)
-                  REPLACE_STAT=1
-      ;;
+                        ;;
     *)
                         { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether stat handles trailing slashes on files" >&5
 $as_echo_n "checking whether stat handles trailing slashes on files... " >&6; }
diff --git a/gnulib/import/m4/stat.m4 b/gnulib/import/m4/stat.m4
index 46e9abceee7..8ef355f9407 100644
--- a/gnulib/import/m4/stat.m4
+++ b/gnulib/import/m4/stat.m4
@@ -15,7 +15,7 @@ AC_DEFUN([gl_FUNC_STAT],
     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
+      dnl REPLACE_STAT=1
       ;;
     *)
       dnl AIX 7.1, Solaris 9, mingw64 mistakenly succeed on stat("file/").
diff --git a/gnulib/patches/0001-use-windows-stat b/gnulib/patches/0001-use-windows-stat
new file mode 100644
index 00000000000..adf9638858d
--- /dev/null
+++ b/gnulib/patches/0001-use-windows-stat
@@ -0,0 +1,13 @@
+diff --git a/gnulib/import/m4/stat.m4 b/gnulib/import/m4/stat.m4
+index 46e9abceee7..8ef355f9407 100644
+--- a/gnulib/import/m4/stat.m4
++++ b/gnulib/import/m4/stat.m4
+@@ -15,7 +15,7 @@ AC_DEFUN([gl_FUNC_STAT],
+     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
++      dnl REPLACE_STAT=1
+       ;;
+     *)
+       dnl AIX 7.1, Solaris 9, mingw64 mistakenly succeed on stat("file/").
diff --git a/gnulib/update-gnulib.sh b/gnulib/update-gnulib.sh
index b9cc7d8f353..ac5e3d56a29 100755
--- a/gnulib/update-gnulib.sh
+++ b/gnulib/update-gnulib.sh
@@ -173,6 +173,8 @@ apply_patches ()
     fi
 }
 
+apply_patches "patches/0001-use-windows-stat"
+
 # Regenerate all necessary files...
 aclocal &&
 autoconf &&
-- 
2.26.2

Re: [PATCH] Do not adjust mtime timezone on Windows

[Joel Brobecker]

Hi Tom,

On Thu, Sep 03, 2020 at 11:02:44AM -0600, Tom Tromey wrote:
> PR win32/25302 notes that gdb will crash when trying to "run" even a
> simple program on Windows.  The essential bug here is that the BFD
> cache can easily be corrupted -- I have sent a separate patch for
> that.
> 
> The particular reason that the cache is corrupted on Windows is that
> gnulib overrides "stat" to make it do timezone adjustment -- but BFD
> does not use this version of stat.  The difference here triggers the
> latent cache bug, but can also cause other bugs as well; in particular
> it can cause spurious warnings about source files being newer.
> 
> This patch simply removes the stat override on mingw, making gnulib
> and BFD agree.
> 
> I tested this by backing out the local AdaCore changes to work around
> this bug and then verifying that I could reproduce it.  Then, I
> applied this patch and verified that "run" works again.
> 
> gnulib/ChangeLog
> 2020-09-03  Tom Tromey  <tromey@adacore.com>
> 
> 	PR win32/25302:
> 	* update-gnulib.sh: Apply stat patch.
> 	* patches/0001-use-windows-stat: New file.
> 	* import/m4/stat.m4: Update.
> 	* configure: Rebuild.

Thanks for helping us with this patch.

This looks good to me. We will want this patch in before
we create the gdb-10-branch, but let's give people a few
more days to comment if they'd like -- let's say, if by
Thursday, we haven't received comments, then we push it.

Thanks again!


> ---
>  gnulib/ChangeLog                     |  8 ++++++++
>  gnulib/configure                     |  3 +--
>  gnulib/import/m4/stat.m4             |  2 +-
>  gnulib/patches/0001-use-windows-stat | 13 +++++++++++++
>  gnulib/update-gnulib.sh              |  2 ++
>  5 files changed, 25 insertions(+), 3 deletions(-)
>  create mode 100644 gnulib/patches/0001-use-windows-stat
> 
> diff --git a/gnulib/configure b/gnulib/configure
> index fa2feb5ea9a..5c6add6e371 100644
> --- a/gnulib/configure
> +++ b/gnulib/configure
> @@ -26907,8 +26907,7 @@ $as_echo "#define ssize_t int" >>confdefs.h
>  
>    case "$host_os" in
>      mingw*)
> -                  REPLACE_STAT=1
> -      ;;
> +                        ;;
>      *)
>                          { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether stat handles trailing slashes on files" >&5
>  $as_echo_n "checking whether stat handles trailing slashes on files... " >&6; }
> diff --git a/gnulib/import/m4/stat.m4 b/gnulib/import/m4/stat.m4
> index 46e9abceee7..8ef355f9407 100644
> --- a/gnulib/import/m4/stat.m4
> +++ b/gnulib/import/m4/stat.m4
> @@ -15,7 +15,7 @@ AC_DEFUN([gl_FUNC_STAT],
>      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
> +      dnl REPLACE_STAT=1
>        ;;
>      *)
>        dnl AIX 7.1, Solaris 9, mingw64 mistakenly succeed on stat("file/").
> diff --git a/gnulib/patches/0001-use-windows-stat b/gnulib/patches/0001-use-windows-stat
> new file mode 100644
> index 00000000000..adf9638858d
> --- /dev/null
> +++ b/gnulib/patches/0001-use-windows-stat
> @@ -0,0 +1,13 @@
> +diff --git a/gnulib/import/m4/stat.m4 b/gnulib/import/m4/stat.m4
> +index 46e9abceee7..8ef355f9407 100644
> +--- a/gnulib/import/m4/stat.m4
> ++++ b/gnulib/import/m4/stat.m4
> +@@ -15,7 +15,7 @@ AC_DEFUN([gl_FUNC_STAT],
> +     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
> ++      dnl REPLACE_STAT=1
> +       ;;
> +     *)
> +       dnl AIX 7.1, Solaris 9, mingw64 mistakenly succeed on stat("file/").
> diff --git a/gnulib/update-gnulib.sh b/gnulib/update-gnulib.sh
> index b9cc7d8f353..ac5e3d56a29 100755
> --- a/gnulib/update-gnulib.sh
> +++ b/gnulib/update-gnulib.sh
> @@ -173,6 +173,8 @@ apply_patches ()
>      fi
>  }
>  
> +apply_patches "patches/0001-use-windows-stat"
> +
>  # Regenerate all necessary files...
>  aclocal &&
>  autoconf &&
> -- 
> 2.26.2

-- 
Joel

Re: [PATCH] Do not adjust mtime timezone on Windows

[Simon Marchi]

On 2020-09-05 4:49 p.m., Joel Brobecker wrote:
> Hi Tom,
> 
> On Thu, Sep 03, 2020 at 11:02:44AM -0600, Tom Tromey wrote:
>> PR win32/25302 notes that gdb will crash when trying to "run" even a
>> simple program on Windows.  The essential bug here is that the BFD
>> cache can easily be corrupted -- I have sent a separate patch for
>> that.
>>
>> The particular reason that the cache is corrupted on Windows is that
>> gnulib overrides "stat" to make it do timezone adjustment -- but BFD
>> does not use this version of stat.  The difference here triggers the
>> latent cache bug, but can also cause other bugs as well; in particular
>> it can cause spurious warnings about source files being newer.
>>
>> This patch simply removes the stat override on mingw, making gnulib
>> and BFD agree.
>>
>> I tested this by backing out the local AdaCore changes to work around
>> this bug and then verifying that I could reproduce it.  Then, I
>> applied this patch and verified that "run" works again.
>>
>> gnulib/ChangeLog
>> 2020-09-03  Tom Tromey  <tromey@adacore.com>
>>
>> 	PR win32/25302:
>> 	* update-gnulib.sh: Apply stat patch.
>> 	* patches/0001-use-windows-stat: New file.
>> 	* import/m4/stat.m4: Update.
>> 	* configure: Rebuild.
> 
> Thanks for helping us with this patch.
> 
> This looks good to me. We will want this patch in before
> we create the gdb-10-branch, but let's give people a few
> more days to comment if they'd like -- let's say, if by
> Thursday, we haven't received comments, then we push it.
> 
> Thanks again!

I wasn't involved in the discussions, so I'm not really aware of the various solutions that
were considered.  However, if that's the solution that was deemed the best, short term, then
the patch LGTM.

Simon

Re: [PATCH] Do not adjust mtime timezone on Windows

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I wasn't involved in the discussions, so I'm not really aware of
Simon> the various solutions that were considered.  However, if that's
Simon> the solution that was deemed the best, short term, then the patch
Simon> LGTM.

Thanks.  I am going to check it in now.

Tom

Re: [PATCH] Do not adjust mtime timezone on Windows

[Pedro Alves]

On 9/3/20 6:02 PM, Tom Tromey wrote:
> PR win32/25302 notes that gdb will crash when trying to "run" even a
> simple program on Windows.  The essential bug here is that the BFD
> cache can easily be corrupted -- I have sent a separate patch for
> that.
> 
> The particular reason that the cache is corrupted on Windows is that
> gnulib overrides "stat" to make it do timezone adjustment -- but BFD
> does not use this version of stat.  The difference here triggers the
> latent cache bug, but can also cause other bugs as well; in particular
> it can cause spurious warnings about source files being newer.
> 
> This patch simply removes the stat override on mingw, making gnulib
> and BFD agree.
> 
> I tested this by backing out the local AdaCore changes to work around
> this bug and then verifying that I could reproduce it.  Then, I
> applied this patch and verified that "run" works again.
> 
> gnulib/ChangeLog
> 2020-09-03  Tom Tromey  <tromey@adacore.com>
> 
> 	PR win32/25302:
> 	* update-gnulib.sh: Apply stat patch.
> 	* patches/0001-use-windows-stat: New file.
> 	* import/m4/stat.m4: Update.
> 	* configure: Rebuild.

I agree this is the best we can do right now.

LGTM.

Thanks,
Pedro Alves