[PATCH 1/3] [fix] Unused struct

[PATCH 1/3] [fix] Unused struct

[Dominique Quatravaux via Gdb-patches]

---
 gdb/darwin-nat.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 0d7b028e39c..3ec881d3a76 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -903,8 +903,6 @@ darwin_suspend_inferior_threads (struct inferior *inf)
 void
 darwin_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
 {
-  struct target_waitstatus status;
-
   int nsignal;
 
   inferior_debug
-- 
2.31.1

[PATCH 2/3] [delete] Not-so-harmless spurious call to `wait4`

[Dominique Quatravaux via Gdb-patches]

As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069
---
 gdb/darwin-nat.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 3ec881d3a76..9c6423ceb02 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1106,9 +1106,6 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
 	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
 			      res_pid, wstatus);
 
-	      /* Looks necessary on Leopard and harmless...  */
-	      wait4 (inf->pid, &wstatus, 0, NULL);
-
 	      return ptid_t (inf->pid);
 	    }
 	  else
-- 
2.31.1

Re: [PATCH 2/3] [delete] Not-so-harmless spurious call to `wait4`

[Simon Marchi via Gdb-patches]

On 2021-04-08 3:14 p.m., Dominique Quatravaux via Gdb-patches wrote:
> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069

Hi Dominique,

We try to make the git commit messages somewhat self-contained, so that
it's possible to understand the change even if bugzilla becomes
inaccessible.  Can you please explain in the commit message what harm
that wait4 call does and why we want to remove it?

It's still a good idea to link to the Bugzilla entry, if people want to
dig more.

Thanks,

Simon

> ---
>  gdb/darwin-nat.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 3ec881d3a76..9c6423ceb02 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1106,9 +1106,6 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>  	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
>  			      res_pid, wstatus);
>  
> -	      /* Looks necessary on Leopard and harmless...  */
> -	      wait4 (inf->pid, &wstatus, 0, NULL);
> -
>  	      return ptid_t (inf->pid);
>  	    }
>  	  else
> 

[PATCH] was Re: [PATCH 2/3] [delete] Not-so-harmless spurious call to `wait4`

[Dominique Quatravaux via Gdb-patches]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]



> Le 8 avr. 2021 à 21:26, Simon Marchi <simon.marchi@polymtl.ca> a écrit :
> 
>   Can you please explain in the commit message what harm
> that wait4 call does and why we want to remove it?


In trying to make a better case in the commit message, I found that swapping the two patches around made more sense. Here is an updated, rebased, single patch (working under the assumption that the other two will be merged first):

From 5c3756e9eb0342b1a5a23bcb54d5b8317743ce0d Mon Sep 17 00:00:00 2001
From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
Date: Thu, 8 Apr 2021 21:35:57 +0200
Subject: [PATCH] [delete] not-so-harmless spurious call to `wait4`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 this
code will typically wait4() a second time on the same process that was
already wait4()'d a few lines above. While this used to be
harmless/idempotent (when we assumed that the process already exited),
this now causes a deadlock in the WIFSTOPPED case.

The early (~2019) history of bug #24069 cautiously suggests to use
WNOHANG instead of outright deleting the call. However, tests on the
current version of Darwin (Big Sur) demonstrate that gdb runs just
fine without a redundant call to wait4(), as would be expected.
Notwithstanding the debatable value of conserving bug compatibility
with an OS release that is more than a decade old, there is scant
evidence of what that double-wait4() was supposed to achieve in the
first place - A cursory investigation with `git blame` pinpoints
commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
fails to answer the “why” question conclusively.
---
 gdb/darwin-nat.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index d3edcdf3a74..ac19d330ffb 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1113,9 +1113,6 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
 			      res_pid, status->value.sig);
 		}
 
-	      /* Looks necessary on Leopard and harmless...  */
-	      wait4 (inf->pid, &wstatus, 0, NULL);
-
 	      return ptid_t (inf->pid);
 	    }
 	  else
-- 
2.31.1



— 
  Dominique Quatravaux
  +41 21 69 35624






[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4254 bytes --]

Re: [PATCH] was Re: [PATCH 2/3] [delete] Not-so-harmless spurious call to `wait4`

[Simon Marchi via Gdb-patches]

On 2021-04-08 4:25 p.m., Dominique Quatravaux wrote:> 
> 
>> Le 8 avr. 2021 à 21:26, Simon Marchi <simon.marchi@polymtl.ca <mailto:simon.marchi@polymtl.ca>> a écrit :
>>
>>   Can you please explain in the commit message what harm
>> that wait4 call does and why we want to remove it?
> 
> In trying to make a better case in the commit message, I found that swapping the two patches around made more sense. Here is an updated, rebased, single patch (working under the assumption that the other two will be merged first):
> 
> From 5c3756e9eb0342b1a5a23bcb54d5b8317743ce0d Mon Sep 17 00:00:00 2001
> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch <mailto:dominique.quatravaux@epfl.ch>>
> Date: Thu, 8 Apr 2021 21:35:57 +0200
> Subject: [PATCH] [delete] not-so-harmless spurious call to `wait4`
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 <https://sourceware.org/bugzilla/show_bug.cgi?id=24069> this
> code will typically wait4() a second time on the same process that was
> already wait4()'d a few lines above. While this used to be
> harmless/idempotent (when we assumed that the process already exited),
> this now causes a deadlock in the WIFSTOPPED case.
> 
> The early (~2019) history of bug #24069 cautiously suggests to use
> WNOHANG instead of outright deleting the call. However, tests on the
> current version of Darwin (Big Sur) demonstrate that gdb runs just
> fine without a redundant call to wait4(), as would be expected.
> Notwithstanding the debatable value of conserving bug compatibility
> with an OS release that is more than a decade old, there is scant
> evidence of what that double-wait4() was supposed to achieve in the
> first place

Thanks, this looks good.  If we had more contributors for macOS,
especially some that cared about old macOS versions, we could aim at
supporting many versions back.  But given the current state, it's
perfectly reasonable to aim at making GDB work well for the latest
version.

 - A cursory investigation with `git blame` pinpoints
> commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
> fails to answer the “why” question conclusively.

Yes, the commits that predate the usage of git don't have the patch
message / rationale that people sent along with the patch, it just
wasn't recorded in CVS.  You can always dig into into the mailing
list.

a80b95ba67e2: https://pi.simark.ca/gdb-patches/F1826DD8-CC0F-4FF1-BC47-3F2ACBB42909@adacore.com/
bb00b29d7802: https://pi.simark.ca/gdb-patches/20090319141746.GA81236@ulanbator.act-europe.fr/

Although there isn't more details about the bit you are removing.

So let's just wait until we hear back from the FSF about your copyright
assignment, and then we can merge all of this.

Note that we currently require contributions to include ChangeLog
entries:

    https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog

I don't mind for this time, the patches are small enough that I can
write them when merging.  We are also discussing whether we keep using
those, so it may or may not apply in the future.

Simon

Re: [PATCH] was Re: [PATCH 2/3] [delete] Not-so-harmless spurious call to `wait4`

[Simon Marchi via Gdb-patches]

>> From 5c3756e9eb0342b1a5a23bcb54d5b8317743ce0d Mon Sep 17 00:00:00 2001
>> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch <mailto:dominique.quatravaux@epfl.ch>>
>> Date: Thu, 8 Apr 2021 21:35:57 +0200
>> Subject: [PATCH] [delete] not-so-harmless spurious call to `wait4`
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 <https://sourceware.org/bugzilla/show_bug.cgi?id=24069> this
>> code will typically wait4() a second time on the same process that was
>> already wait4()'d a few lines above. While this used to be
>> harmless/idempotent (when we assumed that the process already exited),
>> this now causes a deadlock in the WIFSTOPPED case.
>>
>> The early (~2019) history of bug #24069 cautiously suggests to use
>> WNOHANG instead of outright deleting the call. However, tests on the
>> current version of Darwin (Big Sur) demonstrate that gdb runs just
>> fine without a redundant call to wait4(), as would be expected.
>> Notwithstanding the debatable value of conserving bug compatibility
>> with an OS release that is more than a decade old, there is scant
>> evidence of what that double-wait4() was supposed to achieve in the
>> first place
> 
> Thanks, this looks good.  If we had more contributors for macOS,
> especially some that cared about old macOS versions, we could aim at
> supporting many versions back.  But given the current state, it's
> perfectly reasonable to aim at making GDB work well for the latest
> version.
> 
>  - A cursory investigation with `git blame` pinpoints
>> commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
>> fails to answer the “why” question conclusively.
> 
> Yes, the commits that predate the usage of git don't have the patch
> message / rationale that people sent along with the patch, it just
> wasn't recorded in CVS.  You can always dig into into the mailing
> list.
> 
> a80b95ba67e2: https://pi.simark.ca/gdb-patches/F1826DD8-CC0F-4FF1-BC47-3F2ACBB42909@adacore.com/
> bb00b29d7802: https://pi.simark.ca/gdb-patches/20090319141746.GA81236@ulanbator.act-europe.fr/
> 
> Although there isn't more details about the bit you are removing.
> 
> So let's just wait until we hear back from the FSF about your copyright
> assignment, and then we can merge all of this.
> 
> Note that we currently require contributions to include ChangeLog
> entries:
> 
>     https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog
> 
> I don't mind for this time, the patches are small enough that I can
> write them when merging.  We are also discussing whether we keep using
> those, so it may or may not apply in the future.
> 
> Simon
> 

Hi Dominique,

Now that your copyright assignment is done, I'd like to merge your
patches, but I'm a bit lost and the patches don't apply because they got
messed up in the process of sending them.  Would you mind sending a
clean v2, rebased on today's master (I don't think it will change much,
darwin-nat.c hasn't had other changes)?  And if you could use
git-send-email, that would make sure the patches arrive correctly
formatted.

Thanks,

Simon

[PATCH 3/3] [fix] Skip over WIFSTOPPED wait4 status

[Dominique Quatravaux via Gdb-patches]

On modern Darwin's, there appears to be a new circumstance in which a
MACH_NOTIFY_DEAD_NAME message can be received, and which was not
previously accounted for: to signal the WIFSTOPPED condition in the
debuggee. In that case the debuggee is not dead yet (and in fact,
counting it as dead would cause a zombie leak - A process in such a
state reparents to PID 1, but cannot be killed).

- Read and ignore such messages (counting on the next exception
message to let us know of the inferior's new state again)
- Refactor logging so as to clearly distinguish between the three
MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal)
---
 gdb/darwin-nat.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 9c6423ceb02..cbe36eba626 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1053,7 +1053,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
     }
   else if (hdr->msgh_id == 0x48)
     {
-      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
+      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED.  */
       int res;
 
       res = darwin_decode_notify_message (hdr, &inf);
@@ -1096,16 +1096,23 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
 		{
 		  status->kind = TARGET_WAITKIND_EXITED;
 		  status->value.integer = WEXITSTATUS (wstatus);
+		  inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
+				  res_pid, wstatus);
+		}
+	      else if (WIFSTOPPED (wstatus))
+		{
+		  status->kind = TARGET_WAITKIND_IGNORE;
+		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"), res_pid);
+		  return minus_one_ptid;
 		}
 	      else
 		{
 		  status->kind = TARGET_WAITKIND_SIGNALLED;
 		  status->value.sig = gdb_signal_from_host (WTERMSIG (wstatus));
+		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
+			      res_pid, status->value.sig);
 		}
 
-	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
-			      res_pid, wstatus);
-
 	      return ptid_t (inf->pid);
 	    }
 	  else
-- 
2.31.1

Re: [PATCH 3/3] [fix] Skip over WIFSTOPPED wait4 status

[Simon Marchi via Gdb-patches]

On 2021-04-08 3:14 p.m., Dominique Quatravaux via Gdb-patches wrote:
> On modern Darwin's, there appears to be a new circumstance in which a
> MACH_NOTIFY_DEAD_NAME message can be received, and which was not
> previously accounted for: to signal the WIFSTOPPED condition in the
> debuggee. In that case the debuggee is not dead yet (and in fact,
> counting it as dead would cause a zombie leak - A process in such a
> state reparents to PID 1, but cannot be killed).
> 
> - Read and ignore such messages (counting on the next exception
> message to let us know of the inferior's new state again)
> - Refactor logging so as to clearly distinguish between the three
> MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal)
> ---
>  gdb/darwin-nat.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 9c6423ceb02..cbe36eba626 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1053,7 +1053,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>      }
>    else if (hdr->msgh_id == 0x48)
>      {
> -      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
> +      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED.  */
>        int res;
>  
>        res = darwin_decode_notify_message (hdr, &inf);
> @@ -1096,16 +1096,23 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>  		{
>  		  status->kind = TARGET_WAITKIND_EXITED;
>  		  status->value.integer = WEXITSTATUS (wstatus);
> +		  inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
> +				  res_pid, wstatus);
> +		}
> +	      else if (WIFSTOPPED (wstatus))
> +		{
> +		  status->kind = TARGET_WAITKIND_IGNORE;
> +		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"), res_pid);
> +		  return minus_one_ptid;
>  		}
>  	      else
>  		{
>  		  status->kind = TARGET_WAITKIND_SIGNALLED;
>  		  status->value.sig = gdb_signal_from_host (WTERMSIG (wstatus));
> +		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
> +			      res_pid, status->value.sig);
>  		}
>  
> -	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
> -			      res_pid, wstatus);
> -
>  	      return ptid_t (inf->pid);
>  	    }
>  	  else
> 

Thanks, that LGTM.  Let's just way until your copyright assigment is complete.

Simon

Re: [PATCH 1/3] [fix] Unused struct

[Simon Marchi via Gdb-patches]

On 2021-04-08 3:14 p.m., Dominique Quatravaux via Gdb-patches wrote:
> ---
>  gdb/darwin-nat.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 0d7b028e39c..3ec881d3a76 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -903,8 +903,6 @@ darwin_suspend_inferior_threads (struct inferior *inf)
>  void
>  darwin_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
>  {
> -  struct target_waitstatus status;
> -
>    int nsignal;
>  
>    inferior_debug
> 

Thanks, I pushed it right away since it's obvious.

Simon

[RFC][PATCH v2 0/2][PR gdb/24069] Fix GDB hang on macOS 10.14 and later (PR gdb/24609)

[Philippe Blain via Gdb-patches]

Hello, this is a rebased version of Dominique Quatravaux's patches from April
2021 [1] (also at [2]) relating to PR gdb/24609.

The first patch of that series was commited in 83a559f7b93 (Remove unused
variable un darwin_nat_target::resume, 2021-04-08), so it's omitted here.

1/2 is thus Dominique's 2nd patch, ans 2/2 Dominique's 3rd.

1/2 is unchanged; 2/2 is mostly the re-worked version that Louis posted as an
attachment on Bugzilla recently [3], [4]. I've kept Dominique as author
and added a "Co-authored-by" trailer for Louis and myself, I hope this is OK
with everyone (and in line with the project's conventions).

Louis, tell me if you'd preferred a different name+email identity be recorded
in the commit message trailer.

The date used in the Changelog entries is current, but I kept the commit dates
to those of Dominiques's v1. Let me know if one or the other should be changed.

The patches are marked RFC because I can't yet show testsuite results. When
manually running GDB compiled with these patches on macOS 10.15, it seems to
work (not hang), although only with 'set startup-with-shell off'. However,
when I run the testsuite, GDB hangs systematically in the 'wait4' call at
gdb/darwin-nat.c:1097, i.e.

    res_pid = wait4 (inf->pid, &wstatus, 0, NULL);

as can be seen by attaching LLDB, and so tests timeout. An exemple run on
GitHub Actions can be found at [5]. On my macOS 10.11 system, this hang does
not happen, with or without the patches. I'm not sure how to go about debugging
this hang when running the test suite... I did try "set debug infrun 1" and
"set debug darwin 12", as Simon suggested in [6], but it did not seem to help
me understand.

A range-diff against v1 is included below.

[1] https://pi.simark.ca/gdb-patches/20210408191449.27434-1-dominique.quatravaux@epfl.ch/t/#u
[2] https://sourceware.org/pipermail/gdb-patches/2021-April/177598.html
[3] https://sourceware.org/bugzilla/attachment.cgi?id=13953
[4] https://pi.simark.ca/gdb-patches/CAE1H+iEug4oQF1ty9tMz9xJUD1kW=6e7ZUuBcXPfuSAxzhFwvA@mail.gmail.com/
[5] https://github.com/phil-blain/binutils-gdb/runs/5114288157?check_suite_focus=true
[6] https://pi.simark.ca/gdb-patches/4bf807c4-d78f-05a4-6cf5-4fa283bc7c1d@simark.ca/

Dominique Quatravaux (2):
  [delete] Not-so-harmless spurious call to `wait4`
  [fix] Skip over WIFSTOPPED wait4 status

 gdb/darwin-nat.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Range-diff against v1:
1:  faee0c55f49 < -:  ----------- [fix] Unused struct
2:  1aeab1c3e10 ! 1:  1dd0dc08936 [delete] Not-so-harmless spurious call to `wait4`
    @@ Commit message
         commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
         fails to answer the “why” question conclusively.
     
    +    ChangeLog:
    +
    +    2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>
    +
    +            PR gdb/24609
    +            * gdb/darwin-nat.c (darwin_nat_target::decode_message): Remove
    +            spurious call to wait4.
    +
      ## gdb/darwin-nat.c ##
     @@ gdb/darwin-nat.c: darwin_nat_target::decode_message (mach_msg_header_t *hdr,
      	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
3:  fa02ae8a550 ! 2:  8da6aefb32d [fix] Skip over WIFSTOPPED wait4 status
    @@ Commit message
         - Refactor logging so as to clearly distinguish between the three
         MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal)
     
    +    Co-authored-by: Louis-He <1726110778@qq.com>
    +    Co-authored-by: Philippe Blain <levraiphilippeblain@gmail.com>
    +
    +    ChangeLog:
    +
    +    2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>
    +
    +            PR gdb/24609
    +            * gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
    +            check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
    +
    +
    + ## Notes ##
    +    I hope I did not mess up the indentation here. I did not find any guidelines
    +    about tab width in the C/C++ coding standards [1].
    +
    +    [1] https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Whitespaces
    +
      ## gdb/darwin-nat.c ##
     @@ gdb/darwin-nat.c: darwin_nat_target::decode_message (mach_msg_header_t *hdr,
          }
        else if (hdr->msgh_id == 0x48)
          {
     -      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
    -+      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED.  */
    ++      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED. */
            int res;
      
            res = darwin_decode_notify_message (hdr, &inf);
     @@ gdb/darwin-nat.c: darwin_nat_target::decode_message (mach_msg_header_t *hdr,
    - 		{
    - 		  status->kind = TARGET_WAITKIND_EXITED;
    - 		  status->value.integer = WEXITSTATUS (wstatus);
    -+		  inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
    + 		  return minus_one_ptid;
    + 		}
    + 	      if (WIFEXITED (wstatus))
    +-		status->set_exited (WEXITSTATUS (wstatus));
    ++		{
    ++		  status->set_exited (WEXITSTATUS (wstatus));
    ++	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
     +				  res_pid, wstatus);
     +		}
     +	      else if (WIFSTOPPED (wstatus))
     +		{
    -+		  status->kind = TARGET_WAITKIND_IGNORE;
    -+		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"), res_pid);
    ++		  status->set_ignore ();
    ++		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
    ++				  res_pid);
     +		  return minus_one_ptid;
    - 		}
    ++		}
      	      else
      		{
    - 		  status->kind = TARGET_WAITKIND_SIGNALLED;
    - 		  status->value.sig = gdb_signal_from_host (WTERMSIG (wstatus));
    + 		  status->set_signalled
    + 		    (gdb_signal_from_host (WTERMSIG (wstatus)));
     +		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
    -+			      res_pid, status->value.sig);
    ++				  res_pid, status->sig());
      		}
      
     -	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),

base-commit: 0acf434a23768449cbb4b3732355f3f2febecaee
-- 
2.29.2

[RFC][PATCH v2 1/2][PR gdb/24069] [delete] Not-so-harmless spurious call to `wait4`

[Philippe Blain via Gdb-patches]

From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>

As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 this
code will typically wait4() a second time on the same process that was
already wait4()'d a few lines above. While this used to be
harmless/idempotent (when we assumed that the process already exited),
this now causes a deadlock in the WIFSTOPPED case.

The early (~2019) history of bug #24069 cautiously suggests to use
WNOHANG instead of outright deleting the call. However, tests on the
current version of Darwin (Big Sur) demonstrate that gdb runs just
fine without a redundant call to wait4(), as would be expected.
Notwithstanding the debatable value of conserving bug compatibility
with an OS release that is more than a decade old, there is scant
evidence of what that double-wait4() was supposed to achieve in the
first place - A cursory investigation with `git blame` pinpoints
commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
fails to answer the “why” question conclusively.

ChangeLog:

2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>

        PR gdb/24609
        * gdb/darwin-nat.c (darwin_nat_target::decode_message): Remove
	spurious call to wait4.
---
 gdb/darwin-nat.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index d96ce1a6c65..8b0ecfd5b77 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1113,9 +1113,6 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
 	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
 			      res_pid, wstatus);
 
-	      /* Looks necessary on Leopard and harmless...  */
-	      wait4 (inf->pid, &wstatus, 0, NULL);
-
 	      return ptid_t (inf->pid);
 	    }
 	  else
-- 
2.29.2

Re: [RFC][PATCH v2 1/2][PR gdb/24069] [delete] Not-so-harmless spurious call to `wait4`

[Simon Marchi via Gdb-patches]

On 2022-02-16 09:15, Philippe Blain wrote:
> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
> 
> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 this
> code will typically wait4() a second time on the same process that was
> already wait4()'d a few lines above. While this used to be
> harmless/idempotent (when we assumed that the process already exited),
> this now causes a deadlock in the WIFSTOPPED case.
> 
> The early (~2019) history of bug #24069 cautiously suggests to use
> WNOHANG instead of outright deleting the call. However, tests on the
> current version of Darwin (Big Sur) demonstrate that gdb runs just
> fine without a redundant call to wait4(), as would be expected.
> Notwithstanding the debatable value of conserving bug compatibility
> with an OS release that is more than a decade old, there is scant
> evidence of what that double-wait4() was supposed to achieve in the
> first place - A cursory investigation with `git blame` pinpoints
> commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
> fails to answer the “why” question conclusively.

Given that this additional wait does not seem logical at all and
empirical evidence shows that it's not right, I'm fine with merging
this one directly.  Do you have push access, or would you like me to do
it on your behalf?

Simon

Re: [RFC][PATCH v2 1/2][PR gdb/24069] [delete] Not-so-harmless spurious call to `wait4`

[Philippe Blain via Gdb-patches]

Hi Simon,

Le 2022-02-19 à 10:47, Simon Marchi a écrit :
> On 2022-02-16 09:15, Philippe Blain wrote:
>> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
>>
>> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 this
>> code will typically wait4() a second time on the same process that was
>> already wait4()'d a few lines above. While this used to be
>> harmless/idempotent (when we assumed that the process already exited),
>> this now causes a deadlock in the WIFSTOPPED case.
>>
>> The early (~2019) history of bug #24069 cautiously suggests to use
>> WNOHANG instead of outright deleting the call. However, tests on the
>> current version of Darwin (Big Sur) demonstrate that gdb runs just
>> fine without a redundant call to wait4(), as would be expected.
>> Notwithstanding the debatable value of conserving bug compatibility
>> with an OS release that is more than a decade old, there is scant
>> evidence of what that double-wait4() was supposed to achieve in the
>> first place - A cursory investigation with `git blame` pinpoints
>> commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
>> fails to answer the “why” question conclusively.
> 
> Given that this additional wait does not seem logical at all and
> empirical evidence shows that it's not right, I'm fine with merging
> this one directly.  Do you have push access, or would you like me to do
> it on your behalf?
> 
> Simon
> 

I do not have push access, so yes, if you think that it's a good idea to merge
this one directly, please go ahead. I think it won't solve all problems on its
own, but the bugzilla history indicates it would at least be a good start.

I did include a changelog entry, so that it could also be included in an eventual 
11.3 release if you think it makes sense.

Thanks!

Philippe.

Re: [RFC][PATCH v2 1/2][PR gdb/24069] [delete] Not-so-harmless spurious call to `wait4`

[Simon Marchi via Gdb-patches]

On 2022-02-19 10:57, Philippe Blain wrote:
> Hi Simon,
> 
> Le 2022-02-19 à 10:47, Simon Marchi a écrit :
>> On 2022-02-16 09:15, Philippe Blain wrote:
>>> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
>>>
>>> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 this
>>> code will typically wait4() a second time on the same process that was
>>> already wait4()'d a few lines above. While this used to be
>>> harmless/idempotent (when we assumed that the process already exited),
>>> this now causes a deadlock in the WIFSTOPPED case.
>>>
>>> The early (~2019) history of bug #24069 cautiously suggests to use
>>> WNOHANG instead of outright deleting the call. However, tests on the
>>> current version of Darwin (Big Sur) demonstrate that gdb runs just
>>> fine without a redundant call to wait4(), as would be expected.
>>> Notwithstanding the debatable value of conserving bug compatibility
>>> with an OS release that is more than a decade old, there is scant
>>> evidence of what that double-wait4() was supposed to achieve in the
>>> first place - A cursory investigation with `git blame` pinpoints
>>> commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
>>> fails to answer the “why” question conclusively.
>>
>> Given that this additional wait does not seem logical at all and
>> empirical evidence shows that it's not right, I'm fine with merging
>> this one directly.  Do you have push access, or would you like me to do
>> it on your behalf?
>>
>> Simon
>>
> 
> I do not have push access, so yes, if you think that it's a good idea to merge
> this one directly, please go ahead. I think it won't solve all problems on its
> own, but the bugzilla history indicates it would at least be a good start.

Thanks, will push.

> I did include a changelog entry, so that it could also be included in an eventual 
> 11.3 release if you think it makes sense.

There likely won't be a 11.3 release.  But if downstream ports are
building off the gdb-11-branch and it would be helpful for the fix to be
there, then I can always push it there, just let me know.

Simon

[RFC][PATCH v2 2/2][PR gdb/24069] [fix] Skip over WIFSTOPPED wait4 status

[Philippe Blain via Gdb-patches]

From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>

On modern Darwin's, there appears to be a new circumstance in which a
MACH_NOTIFY_DEAD_NAME message can be received, and which was not
previously accounted for: to signal the WIFSTOPPED condition in the
debuggee. In that case the debuggee is not dead yet (and in fact,
counting it as dead would cause a zombie leak - A process in such a
state reparents to PID 1, but cannot be killed).

- Read and ignore such messages (counting on the next exception
message to let us know of the inferior's new state again)
- Refactor logging so as to clearly distinguish between the three
MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal)

Co-authored-by: Louis-He <1726110778@qq.com>
Co-authored-by: Philippe Blain <levraiphilippeblain@gmail.com>

ChangeLog:

2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>

	PR gdb/24609
	* gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
	check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
---

Notes:
    I hope I did not mess up the indentation here. I did not find any guidelines
    about tab width in the C/C++ coding standards [1].
    
    [1] https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Whitespaces

 gdb/darwin-nat.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 8b0ecfd5b77..a3c6a978676 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1063,7 +1063,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
     }
   else if (hdr->msgh_id == 0x48)
     {
-      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
+      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED. */
       int res;
 
       res = darwin_decode_notify_message (hdr, &inf);
@@ -1103,16 +1103,26 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
 		  return minus_one_ptid;
 		}
 	      if (WIFEXITED (wstatus))
-		status->set_exited (WEXITSTATUS (wstatus));
+		{
+		  status->set_exited (WEXITSTATUS (wstatus));
+	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
+				  res_pid, wstatus);
+		}
+	      else if (WIFSTOPPED (wstatus))
+		{
+		  status->set_ignore ();
+		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
+				  res_pid);
+		  return minus_one_ptid;
+		}
 	      else
 		{
 		  status->set_signalled
 		    (gdb_signal_from_host (WTERMSIG (wstatus)));
+		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
+				  res_pid, status->sig());
 		}
 
-	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
-			      res_pid, wstatus);
-
 	      return ptid_t (inf->pid);
 	    }
 	  else
-- 
2.29.2

Re: [RFC][PATCH v2 2/2][PR gdb/24069] [fix] Skip over WIFSTOPPED wait4 status

[Simon Marchi via Gdb-patches]

> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 8b0ecfd5b77..a3c6a978676 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1063,7 +1063,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>      }
>    else if (hdr->msgh_id == 0x48)
>      {
> -      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
> +      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED. */

Two spaces after period.

>        int res;
>  
>        res = darwin_decode_notify_message (hdr, &inf);
> @@ -1103,16 +1103,26 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>  		  return minus_one_ptid;
>  		}
>  	      if (WIFEXITED (wstatus))
> -		status->set_exited (WEXITSTATUS (wstatus));
> +		{
> +		  status->set_exited (WEXITSTATUS (wstatus));
> +	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
> +				  res_pid, wstatus);
> +		}
> +	      else if (WIFSTOPPED (wstatus))
> +		{
> +		  status->set_ignore ();
> +		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
> +				  res_pid);
> +		  return minus_one_ptid;

Can you add a comment explaining that the stop will be handled by a
following exception?  It can be confusing why we ignore a WIFSTOPPED.  A
bit like you explained in your commit message:

 - Read and ignore such messages (counting on the next exception
 message to let us know of the inferior's new state again)



> +		}
>  	      else
>  		{
>  		  status->set_signalled
>  		    (gdb_signal_from_host (WTERMSIG (wstatus)));
> +		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
> +				  res_pid, status->sig());
>  		}

Can you replace the else with

  else if (WIFSIGNALED (wstatus))

and add an else with a warning like:

  warning (_("Unexpected wait status after MACH_NOTIFY_DEAD_NAME "
             "notification: 0x%x"), wstatus);

I think that having this warning would have made the bug you fix more
visible, so easier to find.  Perhaps it will help catch another similar
problem in the future.

The else should probably do status->set_ignore () and return
minus_one_ptid.

Simon

Re: [RFC][PATCH v2 2/2][PR gdb/24069] [fix] Skip over WIFSTOPPED wait4 status

[Philippe Blain via Gdb-patches]

Hi Simon,

Le 2022-02-19 à 10:59, Simon Marchi a écrit :
>> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
>> index 8b0ecfd5b77..a3c6a978676 100644
>> --- a/gdb/darwin-nat.c
>> +++ b/gdb/darwin-nat.c
>> @@ -1063,7 +1063,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>>      }
>>    else if (hdr->msgh_id == 0x48)
>>      {
>> -      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
>> +      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED. */
> 
> Two spaces after period.
> 
>>        int res;
>>  
>>        res = darwin_decode_notify_message (hdr, &inf);
>> @@ -1103,16 +1103,26 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>>  		  return minus_one_ptid;
>>  		}
>>  	      if (WIFEXITED (wstatus))
>> -		status->set_exited (WEXITSTATUS (wstatus));
>> +		{
>> +		  status->set_exited (WEXITSTATUS (wstatus));
>> +	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
>> +				  res_pid, wstatus);
>> +		}
>> +	      else if (WIFSTOPPED (wstatus))
>> +		{
>> +		  status->set_ignore ();
>> +		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
>> +				  res_pid);
>> +		  return minus_one_ptid;
> 
> Can you add a comment explaining that the stop will be handled by a
> following exception?  It can be confusing why we ignore a WIFSTOPPED.  A
> bit like you explained in your commit message:
> 
>  - Read and ignore such messages (counting on the next exception
>  message to let us know of the inferior's new state again)
> 
> 
> 
>> +		}
>>  	      else
>>  		{
>>  		  status->set_signalled
>>  		    (gdb_signal_from_host (WTERMSIG (wstatus)));
>> +		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
>> +				  res_pid, status->sig());
>>  		}
> 
> Can you replace the else with
> 
>   else if (WIFSIGNALED (wstatus))
> 
> and add an else with a warning like:
> 
>   warning (_("Unexpected wait status after MACH_NOTIFY_DEAD_NAME "
>              "notification: 0x%x"), wstatus);
> 
> I think that having this warning would have made the bug you fix more
> visible, so easier to find.  Perhaps it will help catch another similar
> problem in the future.
> 
> The else should probably do status->set_ignore () and return
> minus_one_ptid.
> 
> Simon
> 

I'll address your feedback and send a v3 for 2/2. Thanks!

Philippe.

Re: [RFC][PATCH v2 0/2][PR gdb/24069] Fix GDB hang on macOS 10.14 and later (PR gdb/24609)

[Philippe Blain via Gdb-patches]

It seems Samuel's email which I took in [1] bounces;
I'm re-CC-ing him here with what seems the right address
(from [2]).

[1] https://pi.simark.ca/gdb-patches/1E9C17B9-0F4B-42B9-B21C-C23FA8D9B4BE@me.com/
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=24069#c27

[RFC][PATCH v3 0/1][PR gdb/24069] Fix GDB hang on macOS 10.14 and later

[Philippe Blain via Gdb-patches]

Hello, here is v3 of the series.

1/2 of v2 was commited in 9cca177baec (gdb/darwin: remove not-so-harmless
spurious call to `wait4`, 2022-02-16), and so is ommited here.

So this patch is 2/2 of v2, tweaked following Simon's feedback.

Once again Louis, tell me if you'd preferred a different name+email identity be
recorded in the Co-authored-by commit message trailer.

The patches are still marked RFC because I can't yet show testsuite results
(see cover letter of v2 [1]).

A range-diff against v2 is included below.

[1] https://pi.simark.ca/gdb-patches/20220216141540.96514-1-levraiphilippeblain@gmail.com/#t

Dominique Quatravaux (1):
  gdb/darwin: skip over WIFSTOPPED wait4 status

 gdb/darwin-nat.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

Range-diff against v2:
1:  e57a887cace < -:  ----------- [deleted] Not-so-harmless spurious call to `wait4`
2:  f7819ff3e55 ! 1:  7e2e0fa614b [fix] Skip over WIFSTOPPED wait4 status
    @@ Metadata
     Author: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
     
      ## Commit message ##
    -    [fix] Skip over WIFSTOPPED wait4 status
    +    gdb/darwin: skip over WIFSTOPPED wait4 status
     
         On modern Darwin's, there appears to be a new circumstance in which a
         MACH_NOTIFY_DEAD_NAME message can be received, and which was not
    @@ Commit message
     
         - Read and ignore such messages (counting on the next exception
         message to let us know of the inferior's new state again)
    -    - Refactor logging so as to clearly distinguish between the three
    -    MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal)
    +    - Refactor logging so as to clearly distinguish between the
    +    MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal, or
    +    something else), and warn in the last case
     
         Co-authored-by: Louis-He <1726110778@qq.com>
         Co-authored-by: Philippe Blain <levraiphilippeblain@gmail.com>
    @@ Commit message
     
         2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>
     
    -    PR gdb/24609
    +            PR gdb/24609
                 * gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
                 check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
     
    @@ gdb/darwin-nat.c: darwin_nat_target::decode_message (mach_msg_header_t *hdr,
        else if (hdr->msgh_id == 0x48)
          {
     -      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
    -+      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED. */
    ++      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED.  */
            int res;
      
            res = darwin_decode_notify_message (hdr, &inf);
    @@ gdb/darwin-nat.c: darwin_nat_target::decode_message (mach_msg_header_t *hdr,
      		}
      	      if (WIFEXITED (wstatus))
     -		status->set_exited (WEXITSTATUS (wstatus));
    +-	      else
     +		{
     +		  status->set_exited (WEXITSTATUS (wstatus));
     +	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
    @@ gdb/darwin-nat.c: darwin_nat_target::decode_message (mach_msg_header_t *hdr,
     +		}
     +	      else if (WIFSTOPPED (wstatus))
     +		{
    ++		  /* Ignore stopped state, it will be handled by the next exception */
     +		  status->set_ignore ();
     +		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
     +				  res_pid);
     +		  return minus_one_ptid;
     +		}
    - 	      else
    ++	      else if (WIFSIGNALED (wstatus))
      		{
      		  status->set_signalled
      		    (gdb_signal_from_host (WTERMSIG (wstatus)));
     +		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
     +				  res_pid, status->sig());
    ++		}
    ++	      else
    ++		{
    ++		  status->set_ignore ();
    ++		  warning (_("Unexpected wait status after MACH_NOTIFY_DEAD_NAME "
    ++		             "notification: 0x%x"), wstatus);
    ++		  return minus_one_ptid;
      		}
    - 
    +-
     -	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
     -			      res_pid, wstatus);
    --
    - 	      return ptid_t (inf->pid);
    - 	    }
    - 	  else
    + 
    + 	      /* Looks necessary on Leopard and harmless...  */
    + 	      wait4 (inf->pid, &wstatus, 0, NULL);

base-commit: 0acf434a23768449cbb4b3732355f3f2febecaee
-- 
2.29.2

[RFC][PATCH v3 1/1][PR gdb/24069] gdb/darwin: skip over WIFSTOPPED wait4 status

[Philippe Blain via Gdb-patches]

From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>

On modern Darwin's, there appears to be a new circumstance in which a
MACH_NOTIFY_DEAD_NAME message can be received, and which was not
previously accounted for: to signal the WIFSTOPPED condition in the
debuggee. In that case the debuggee is not dead yet (and in fact,
counting it as dead would cause a zombie leak - A process in such a
state reparents to PID 1, but cannot be killed).

- Read and ignore such messages (counting on the next exception
message to let us know of the inferior's new state again)
- Refactor logging so as to clearly distinguish between the
MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal, or
something else), and warn in the last case

Co-authored-by: Louis-He <1726110778@qq.com>
Co-authored-by: Philippe Blain <levraiphilippeblain@gmail.com>

ChangeLog:

2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>

	PR gdb/24609
	* gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
	check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
---
 gdb/darwin-nat.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index d96ce1a6c65..1adc6bd8bff 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1063,7 +1063,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
     }
   else if (hdr->msgh_id == 0x48)
     {
-      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
+      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED.  */
       int res;
 
       res = darwin_decode_notify_message (hdr, &inf);
@@ -1103,15 +1103,33 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
 		  return minus_one_ptid;
 		}
 	      if (WIFEXITED (wstatus))
-		status->set_exited (WEXITSTATUS (wstatus));
-	      else
+		{
+		  status->set_exited (WEXITSTATUS (wstatus));
+	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
+				  res_pid, wstatus);
+		}
+	      else if (WIFSTOPPED (wstatus))
+		{
+		  /* Ignore stopped state, it will be handled by the next exception */
+		  status->set_ignore ();
+		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
+				  res_pid);
+		  return minus_one_ptid;
+		}
+	      else if (WIFSIGNALED (wstatus))
 		{
 		  status->set_signalled
 		    (gdb_signal_from_host (WTERMSIG (wstatus)));
+		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
+				  res_pid, status->sig());
+		}
+	      else
+		{
+		  status->set_ignore ();
+		  warning (_("Unexpected wait status after MACH_NOTIFY_DEAD_NAME "
+		             "notification: 0x%x"), wstatus);
+		  return minus_one_ptid;
 		}
-
-	      inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
-			      res_pid, wstatus);
 
 	      /* Looks necessary on Leopard and harmless...  */
 	      wait4 (inf->pid, &wstatus, 0, NULL);
-- 
2.29.2

Re: [RFC][PATCH v3 1/1][PR gdb/24069] gdb/darwin: skip over WIFSTOPPED wait4 status

[Simon Marchi via Gdb-patches]

On 2022-02-24 09:23, Philippe Blain wrote:
> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
> 
> On modern Darwin's, there appears to be a new circumstance in which a
> MACH_NOTIFY_DEAD_NAME message can be received, and which was not
> previously accounted for: to signal the WIFSTOPPED condition in the
> debuggee. In that case the debuggee is not dead yet (and in fact,
> counting it as dead would cause a zombie leak - A process in such a
> state reparents to PID 1, but cannot be killed).
> 
> - Read and ignore such messages (counting on the next exception
> message to let us know of the inferior's new state again)
> - Refactor logging so as to clearly distinguish between the
> MACH_NOTIFY_DEAD_NAME cases (WIFEXITED, WIFSTOPPED, signal, or
> something else), and warn in the last case
> 
> Co-authored-by: Louis-He <1726110778@qq.com>
> Co-authored-by: Philippe Blain <levraiphilippeblain@gmail.com>
> 
> ChangeLog:
> 
> 2022-02-05  Dominique Quatravaux <dominique.quatravaux@epfl.ch>
> 
> 	PR gdb/24609
> 	* gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
> 	check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.

Thanks, pushed.

Simon

Re: [RFC][PATCH v3 1/1][PR gdb/24069] gdb/darwin: skip over WIFSTOPPED wait4 status

[Philippe Blain via Gdb-patches]

Hi Simon,

Le 2022-02-24 à 10:49, Simon Marchi a écrit :
> 
> 
> On 2022-02-24 09:23, Philippe Blain wrote:
>> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
>>>
>>
>> 	PR gdb/24609
>> 	* gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
>> 	check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
> 
> Thanks, pushed.
> 
> Simon
> 

I noticed that the Bugzilla entry was not updated when you pushed
that commit, whereas it was for the previous one [1]. I'm not sure
of what is the automation that makes this work...

I thought it was because my changelog entry mistakenly used 24609 instead
of 24069, but that was the case also for v2 1/2, so I guess that's not
the cause...

Philippe.


[1] https://sourceware.org/bugzilla/show_bug.cgi?id=24069#c46

Re: [RFC][PATCH v3 1/1][PR gdb/24069] gdb/darwin: skip over WIFSTOPPED wait4 status

[Simon Marchi via Gdb-patches]

On 2022-02-28 12:52, Philippe Blain wrote:
> Hi Simon,
> 
> Le 2022-02-24 à 10:49, Simon Marchi a écrit :
>>
>>
>> On 2022-02-24 09:23, Philippe Blain wrote:
>>> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
>>>>
>>>
>>> 	PR gdb/24609
>>> 	* gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
>>> 	check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
>>
>> Thanks, pushed.
>>
>> Simon
>>
> 
> I noticed that the Bugzilla entry was not updated when you pushed
> that commit, whereas it was for the previous one [1]. I'm not sure
> of what is the automation that makes this work...
> 
> I thought it was because my changelog entry mistakenly used 24609 instead
> of 24069, but that was the case also for v2 1/2, so I guess that's not
> the cause...

Patch #1 had a link to the correct bugzilla ticket in its commit message,
so the connection was made.

Patch #2 didn't have any mention of the bugzilla ticket, so there was
no connection.

The correct thing to do nowadays, according to our standard procedures,
would have been to use the:

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24069

trailer.  But I often forget about it.

Simon

Re: [RFC][PATCH v3 1/1][PR gdb/24069] gdb/darwin: skip over WIFSTOPPED wait4 status

[Philippe Blain via Gdb-patches]

Le 2022-02-28 à 13:34, Simon Marchi a écrit :
> 
> 
> On 2022-02-28 12:52, Philippe Blain wrote:
>> Hi Simon,
>>
>> Le 2022-02-24 à 10:49, Simon Marchi a écrit :
>>>
>>>
>>> On 2022-02-24 09:23, Philippe Blain wrote:
>>>> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
>>>>>
>>>>
>>>> 	PR gdb/24609
>>>> 	* gdb/darwin-nat.c (darwin_nat_target::decode_message): Also
>>>> 	check for WIFSTOPPED upon MACH_NOTIFY_DEAD_NAME.
>>>
>>> Thanks, pushed.
>>>
>>> Simon
>>>
>>
>> I noticed that the Bugzilla entry was not updated when you pushed
>> that commit, whereas it was for the previous one [1]. I'm not sure
>> of what is the automation that makes this work...
>>
>> I thought it was because my changelog entry mistakenly used 24609 instead
>> of 24069, but that was the case also for v2 1/2, so I guess that's not
>> the cause...
> 
> Patch #1 had a link to the correct bugzilla ticket in its commit message,
> so the connection was made.
> 
> Patch #2 didn't have any mention of the bugzilla ticket, so there was
> no connection.
> 
> The correct thing to do nowadays, according to our standard procedures,
> would have been to use the:
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24069
> 
> trailer.  But I often forget about it.
> 
> Simon

Noted. I just checked the ContributionChecklist page and
see you've updated it, thanks :)

Philippe.

[1] https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages