[0/2] Rework Cygwin signal handling

Message ID 20240220165605.563516-1-pedro@palves.net
Headers
Series Rework Cygwin signal handling |

Message

Pedro Alves Feb. 20, 2024, 4:56 p.m. UTC
  This upstreams a couple GDB patches that Cygwin has been carrying
downstream.

In my Windows non-stop work, I was having trouble with the
have_saved_context machinery, I couldn't get it to work properly.  The
Cygwin distro gdb was able to unwind signals properly, but my GDB did
not.  After some head banging, I realized that Cygwin gdb may have
some downstream patches.  And indeed it does.  One of those patches
eliminates the have_saved_context machinery...  I got signal handling
and non-stop working properly on top of that patch, which then means
that I need that change upstream as well, if I am to upstream my
non-stop changes...  So here we are.  That patch is patch #2 in this
series.

That have_saved_context patch depends on another downstream patch,
which is patch #1 here.  The version I show here is a polished,
modernized version compared to the downstream version, but the main
logic is the same.  While this is not perfect, it is certainly better
than what we have upstream, which just isn't able to unwind from a
signal handler at all.  Cygwin has been carrying these patches for
many years, so while we could think about improving all this, I see no
reason for holding back the patch as is.  We can always improve on
top, and we should be able to do that upstream.

Jon Turney (2):
  Teach gdb how to unwind cygwin _sigbe and sigdelayed frames
  Drop special way of getting inferior context after a Cygwin signal

 gdb/amd64-windows-tdep.c |  26 ++++++
 gdb/i386-windows-tdep.c  |  20 ++++
 gdb/windows-nat.c        |  52 +++--------
 gdb/windows-tdep.c       | 194 +++++++++++++++++++++++++++++++++++++++
 gdb/windows-tdep.h       |  20 ++++
 5 files changed, 275 insertions(+), 37 deletions(-)


base-commit: 94a75b0363b1e09416e9bd24cac72d98864688d8
  

Comments

Jon Turney Feb. 21, 2024, 1:25 p.m. UTC | #1
On 20/02/2024 16:56, Pedro Alves wrote:
> This upstreams a couple GDB patches that Cygwin has been carrying
> downstream.
> 
> In my Windows non-stop work, I was having trouble with the
> have_saved_context machinery, I couldn't get it to work properly.  The
> Cygwin distro gdb was able to unwind signals properly, but my GDB did
> not.  After some head banging, I realized that Cygwin gdb may have
> some downstream patches.  And indeed it does.  One of those patches
> eliminates the have_saved_context machinery...  I got signal handling
> and non-stop working properly on top of that patch, which then means
> that I need that change upstream as well, if I am to upstream my
> non-stop changes...  So here we are.  That patch is patch #2 in this
> series.
> 
> That have_saved_context patch depends on another downstream patch,
> which is patch #1 here.  The version I show here is a polished,
> modernized version compared to the downstream version, but the main
> logic is the same.  While this is not perfect, it is certainly better
> than what we have upstream, which just isn't able to unwind from a
> signal handler at all.  Cygwin has been carrying these patches for

Thanks very much for polishing and modernising this patch.

Just to note that the situation is even worse than noted here: as well 
as being unable to unwind in a signal handler, without this patch gdb 
also can't unwind from functions inside the cygwin DLL where the call 
stack passes through these wrappers (which is nearly all of them, which 
makes debugging problems there even more painful).

Obviously, I am very much in favour of this being applied. :)

> many years, so while we could think about improving all this, I see no
> reason for holding back the patch as is.  We can always improve on
> top, and we should be able to do that upstream.
> 
> Jon Turney (2):
>    Teach gdb how to unwind cygwin _sigbe and sigdelayed frames
>    Drop special way of getting inferior context after a Cygwin signal
> 
>   gdb/amd64-windows-tdep.c |  26 ++++++
>   gdb/i386-windows-tdep.c  |  20 ++++
>   gdb/windows-nat.c        |  52 +++--------
>   gdb/windows-tdep.c       | 194 +++++++++++++++++++++++++++++++++++++++
>   gdb/windows-tdep.h       |  20 ++++
>   5 files changed, 275 insertions(+), 37 deletions(-)
  
Tom Tromey Feb. 21, 2024, 9:14 p.m. UTC | #2
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This upstreams a couple GDB patches that Cygwin has been carrying
Pedro> downstream.

I don't know anything about Cygwin, but considering that these seem
fairly local to that code; and Jon reports their necessity; and they
remove some special handling from windows-nat.c, I'm in favor of you
putting them in.

thanks,
Tom
  
Pedro Alves Feb. 23, 2024, 4:24 p.m. UTC | #3
On 2024-02-21 13:25, Jon Turney wrote:

> Thanks very much for polishing and modernising this patch.
> 

Thanks for writing it in the first place.  :-)

> Just to note that the situation is even worse than noted here: as well as being unable to unwind in a signal handler, without this patch gdb also can't unwind from functions inside the cygwin DLL where the call stack passes through these wrappers (which is nearly all of them, which makes debugging problems there even more painful).

Oh, indeed.  I was too focused on the tree, missed the forest.

> Obviously, I am very much in favour of this being applied. :)

:-)
  
Pedro Alves Feb. 23, 2024, 4:25 p.m. UTC | #4
On 2024-02-21 21:14, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> This upstreams a couple GDB patches that Cygwin has been carrying
> Pedro> downstream.
> 
> I don't know anything about Cygwin, but considering that these seem
> fairly local to that code; and Jon reports their necessity; and they
> remove some special handling from windows-nat.c, I'm in favor of you
> putting them in.
> 

Great, thanks.  I've merged this now.

Pedro Alves