public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Do not adjust mtime timezone on Windows
@ 2020-09-03 17:02 Tom Tromey
  2020-09-05 20:49 ` Joel Brobecker
  2020-09-08 16:28 ` Pedro Alves
  0 siblings, 2 replies; 5+ messages in thread
From: Tom Tromey @ 2020-09-03 17:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Do not adjust mtime timezone on Windows
  2020-09-03 17:02 [PATCH] Do not adjust mtime timezone on Windows Tom Tromey
@ 2020-09-05 20:49 ` Joel Brobecker
  2020-09-06  0:16   ` Simon Marchi
  2020-09-08 16:28 ` Pedro Alves
  1 sibling, 1 reply; 5+ messages in thread
From: Joel Brobecker @ 2020-09-05 20:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Do not adjust mtime timezone on Windows
  2020-09-05 20:49 ` Joel Brobecker
@ 2020-09-06  0:16   ` Simon Marchi
  2020-09-08 16:20     ` Tom Tromey
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2020-09-06  0:16 UTC (permalink / raw)
  To: Joel Brobecker, Tom Tromey; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Do not adjust mtime timezone on Windows
  2020-09-06  0:16   ` Simon Marchi
@ 2020-09-08 16:20     ` Tom Tromey
  0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2020-09-08 16:20 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Joel Brobecker, Tom Tromey, gdb-patches

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Do not adjust mtime timezone on Windows
  2020-09-03 17:02 [PATCH] Do not adjust mtime timezone on Windows Tom Tromey
  2020-09-05 20:49 ` Joel Brobecker
@ 2020-09-08 16:28 ` Pedro Alves
  1 sibling, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2020-09-08 16:28 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-08 16:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 17:02 [PATCH] Do not adjust mtime timezone on Windows Tom Tromey
2020-09-05 20:49 ` Joel Brobecker
2020-09-06  0:16   ` Simon Marchi
2020-09-08 16:20     ` Tom Tromey
2020-09-08 16:28 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).