Prechádzať zdrojové kódy

Ports/gdb: Implement wait and mourn_inferior overrides for our target

While troubleshooting why gdb wasn't working when attempting to debug
serenity programs I noticed two things:

 - The contract of serenity's `waitpid(..)` appears to be slightly
   different than the generic ptrace target expects. We need to make
   sure we pass `WSTOPPED`, and it can return different errno values
   that we would want to re-try on.

-  The contract of serenity's `ptrace(..)` implementation appears to
   diverge as well, as we are expected to call `PT_ATTACH` before we
   call `PT_CONTINUE`, otherwise `ptrace(..)` will just error out.

These two patches fix the behavior of wait and mourn_inferior so that
they work as expected on serenity and allow us to attach and then wait
for a process to exit while running under gdb.
Brian Gianforcaro 3 rokov pred
rodič
commit
e56262caed

+ 1 - 1
Ports/gdb/patches/0001-gdb-Disable-xmalloc-for-alternate_signal_stack-for-s.patch

@@ -1,7 +1,7 @@
 From 32bfc7a161e6bf10decbf29b31c7b547cf250d3a Mon Sep 17 00:00:00 2001
 From 32bfc7a161e6bf10decbf29b31c7b547cf250d3a Mon Sep 17 00:00:00 2001
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 Date: Sat, 19 Feb 2022 19:46:06 -0800
 Date: Sat, 19 Feb 2022 19:46:06 -0800
-Subject: [PATCH 1/4] gdb: Disable xmalloc for alternate_signal_stack for
+Subject: [PATCH 1/6] gdb: Disable xmalloc for alternate_signal_stack for
  serenity
  serenity
 
 
 ---
 ---

+ 1 - 1
Ports/gdb/patches/0002-serenity-Add-basic-ptrace-based-native-target-for-Se.patch

@@ -1,7 +1,7 @@
 From 883a25f5ed8fd8f13b8e30aed3a25001839d892c Mon Sep 17 00:00:00 2001
 From 883a25f5ed8fd8f13b8e30aed3a25001839d892c Mon Sep 17 00:00:00 2001
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 Date: Tue, 28 Dec 2021 04:35:47 -0800
 Date: Tue, 28 Dec 2021 04:35:47 -0800
-Subject: [PATCH 2/4] serenity: Add basic ptrace based native target for
+Subject: [PATCH 2/6] serenity: Add basic ptrace based native target for
  SerenityOS/i386
  SerenityOS/i386
 
 
 ---
 ---

+ 1 - 1
Ports/gdb/patches/0003-gdb-Add-build-support-for-SerenityOS.patch

@@ -1,7 +1,7 @@
 From e87fd74df2c7fcb4b146eb09b5b710a45003999a Mon Sep 17 00:00:00 2001
 From e87fd74df2c7fcb4b146eb09b5b710a45003999a Mon Sep 17 00:00:00 2001
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 Date: Sat, 19 Feb 2022 19:47:42 -0800
 Date: Sat, 19 Feb 2022 19:47:42 -0800
-Subject: [PATCH 3/4] gdb: Add build support for SerenityOS
+Subject: [PATCH 3/6] gdb: Add build support for SerenityOS
 
 
 ---
 ---
  bfd/config.bfd       | 9 +++++++++
  bfd/config.bfd       | 9 +++++++++

+ 1 - 1
Ports/gdb/patches/0004-serenity-Fix-compiler-fpermissive-warnings-from-usin.patch

@@ -1,7 +1,7 @@
 From 7a5e11d2e9cbe98af96faa4835592686bf261a23 Mon Sep 17 00:00:00 2001
 From 7a5e11d2e9cbe98af96faa4835592686bf261a23 Mon Sep 17 00:00:00 2001
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 From: Brian Gianforcaro <b.gianfo@gmail.com>
 Date: Tue, 28 Dec 2021 04:39:25 -0800
 Date: Tue, 28 Dec 2021 04:39:25 -0800
-Subject: [PATCH 4/4] serenity: Fix compiler -fpermissive warnings from using
+Subject: [PATCH 4/6] serenity: Fix compiler -fpermissive warnings from using
  latest GCC
  latest GCC
 
 
 ---
 ---

+ 114 - 0
Ports/gdb/patches/0005-serenity-Implement-custom-wait-override-for-the-sere.patch

@@ -0,0 +1,114 @@
+From a80fc99e8f2e1c5ac1620a8c6c26d65daa98204e Mon Sep 17 00:00:00 2001
+From: Brian Gianforcaro <b.gianfo@gmail.com>
+Date: Sun, 20 Feb 2022 00:32:51 -0800
+Subject: [PATCH 5/6] serenity: Implement custom wait override for the
+ serenity_nat_target
+
+While troubleshooting why gdb wasn't working when attempting to debug
+serenity programs I noticed two things:
+
+ - The contract of serenity's `waitpid(..)` appears to be slightly
+   different than the generic ptrace target expects. We need to make
+   sure we pass `WSTOPPED`, and it can return different errno values
+   that we would want to re-try on.
+
+-  The contract of serenity's `ptrace(..)` implementation appears to
+   diverge as well, as we are expected to call `PT_ATTACH` before we
+   call `PT_CONTINUE`, otherwise `ptrace(..)` will just error out.
+
+Allow gdb to understand these differences, I've overloaded the
+serenity_nat_target::wait(..) method and added the logic there.
+---
+ gdb/serenity-nat.c | 62 +++++++++++++++++++++++++++++++++++++++++++++-
+ gdb/serenity-nat.h |  4 +++
+ 2 files changed, 65 insertions(+), 1 deletion(-)
+
+diff --git a/gdb/serenity-nat.c b/gdb/serenity-nat.c
+index ff740d4..b1d39d4 100644
+--- a/gdb/serenity-nat.c
++++ b/gdb/serenity-nat.c
+@@ -10,4 +10,64 @@
+ #include "gdbsupport/gdb_wait.h"
+ 
+ #include "inf-child.h"
+-#include "serenity-nat.h"
+\ No newline at end of file
++#include "serenity-nat.h"
++
++ptid_t serenity_nat_target::wait (ptid_t ptid, struct target_waitstatus* ourstatus, target_wait_flags)
++{
++    int pid;
++    int status, save_errno;
++
++    do
++    {
++        set_sigint_trap ();
++
++        do
++        {
++            errno = 0;
++            pid = waitpid (ptid.pid (), &status, WSTOPPED);
++            if (errno != 0)
++            {
++                save_errno = errno;
++                perror_with_name (("waitpid"));
++            }
++        }
++        while (pid == -1 && (save_errno == EINTR || save_errno == EAGAIN));
++
++        clear_sigint_trap ();
++
++        if (pid == -1)
++        {
++            fprintf_unfiltered (gdb_stderr,
++                    _("Child process unexpectedly missing: %s.\n"),
++                    safe_strerror (save_errno));
++
++            /* Claim it exited with unknown signal.  */
++            ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
++            ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
++            return inferior_ptid;
++        }
++
++        /* Ignore terminated detached child processes.  */
++        if (!WIFSTOPPED (status) && find_inferior_pid (this, pid) == nullptr)
++            pid = -1;
++    }
++    while (pid == -1);
++
++    store_waitstatus (ourstatus, status);
++
++    /* Serenity requires us to PT_ATTACH before we PT_CONTINUE, however GDB doesn't
++     * provide a hook for us to do that before we issue the PT_CONTINUE, so we are
++     * forced to do it here.
++     */
++    if (!m_attach_before_continue_called) {
++        errno = 0;
++        ptrace (PT_ATTACH, pid, (PTRACE_TYPE_ARG3)0, 0);
++        if (errno != 0) {
++            save_errno = errno;
++            printf_unfiltered (_("PT_ATTACH failed: %d - %s \n"), save_errno, safe_strerror (save_errno));
++        }
++        m_attach_before_continue_called = true;
++    }
++
++    return ptid_t (pid);
++}
+diff --git a/gdb/serenity-nat.h b/gdb/serenity-nat.h
+index dcd24ce..9ae3b87 100644
+--- a/gdb/serenity-nat.h
++++ b/gdb/serenity-nat.h
+@@ -11,6 +11,10 @@
+ 
+ class serenity_nat_target : public inf_ptrace_target
+ {
++  ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
++
++private:
++  bool m_attach_before_continue_called { false };
+ };
+ 
+ #endif /* serenity-nat.h */
+-- 
+2.32.0
+

+ 51 - 0
Ports/gdb/patches/0006-serenity-Implement-mourn_inferior-override-for-the-s.patch

@@ -0,0 +1,51 @@
+From 1285f88fcd5a8eabf5536e5af8015777f2a64117 Mon Sep 17 00:00:00 2001
+From: Brian Gianforcaro <b.gianfo@gmail.com>
+Date: Sun, 20 Feb 2022 00:44:08 -0800
+Subject: [PATCH 6/6] serenity: Implement mourn_inferior override for the
+ serenity_nat_target
+
+We need to pass `WNOHANG` to our `waitpid(..)` call on SerenityOS,
+otherwise we will wait forever.
+---
+ gdb/serenity-nat.c | 14 ++++++++++++++
+ gdb/serenity-nat.h |  2 ++
+ 2 files changed, 16 insertions(+)
+
+diff --git a/gdb/serenity-nat.c b/gdb/serenity-nat.c
+index b1d39d4..7789950 100644
+--- a/gdb/serenity-nat.c
++++ b/gdb/serenity-nat.c
+@@ -71,3 +71,17 @@ ptid_t serenity_nat_target::wait (ptid_t ptid, struct target_waitstatus* ourstat
+ 
+     return ptid_t (pid);
+ }
++
++void serenity_nat_target::mourn_inferior ()
++{
++    int status;
++
++    /* Wait just one more time to collect the inferior's exit status.
++     * Do not check whether this succeeds though, since we may be
++     * dealing with a process that we attached to. Such a process will
++     * only report its exit status to its original parent.
++     */
++    waitpid (inferior_ptid.pid (), &status, WNOHANG);
++
++    inf_child_target::mourn_inferior ();
++}
+diff --git a/gdb/serenity-nat.h b/gdb/serenity-nat.h
+index 9ae3b87..35d7ffc 100644
+--- a/gdb/serenity-nat.h
++++ b/gdb/serenity-nat.h
+@@ -13,6 +13,8 @@ class serenity_nat_target : public inf_ptrace_target
+ {
+   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
+ 
++  void mourn_inferior ();
++
+ private:
+   bool m_attach_before_continue_called { false };
+ };
+-- 
+2.32.0
+

+ 26 - 0
Ports/gdb/patches/ReadMe.md

@@ -20,3 +20,29 @@ gdb: Add build support for SerenityOS
 serenity: Fix compiler -fpermissive warnings from using latest GCC
 serenity: Fix compiler -fpermissive warnings from using latest GCC
 
 
 
 
+## `0005-serenity-Implement-custom-wait-override-for-the-sere.patch`
+
+serenity: Implement custom wait override for the serenity_nat_target
+
+While troubleshooting why gdb wasn't working when attempting to debug
+serenity programs I noticed two things:
+
+ - The contract of serenity's `waitpid(..)` appears to be slightly
+   different than the generic ptrace target expects. We need to make
+   sure we pass `WSTOPPED`, and it can return different errno values
+   that we would want to re-try on.
+
+-  The contract of serenity's `ptrace(..)` implementation appears to
+   diverge as well, as we are expected to call `PT_ATTACH` before we
+   call `PT_CONTINUE`, otherwise `ptrace(..)` will just error out.
+
+Allow gdb to understand these differences, I've overloaded the
+serenity_nat_target::wait(..) method and added the logic there.
+
+## `0006-serenity-Implement-mourn_inferior-override-for-the-s.patch`
+
+serenity: Implement mourn_inferior override for the serenity_nat_target
+
+We need to pass `WNOHANG` to our `waitpid(..)` call on SerenityOS,
+otherwise we will wait forever.
+