gdb: Implement native dumpcore function

Message ID 20200727144338.7033-1-n54@gmx.com
State New
Headers show
Series
  • gdb: Implement native dumpcore function
Related show

Commit Message

Kamil Rytarowski July 27, 2020, 2:43 p.m.
Add new API for systems with native kernel support for dumping
a process on demand. Wire it into the gdb's gcore functionality.

Define supports_native_dumpcore and native_dumpcore for NetBSD,
that wraps the ptrace(2) call with the PT_DUMPCORE operation.

gdb/ChangeLog:

       * target.h (supports_native_dumpcore, native_dumpcore): New
       function declarations.
       * target.c (supports_native_dumpcore, native_dumpcore): New
       functions.
       * target-delegates.c: Rebuild.
       * nbsd-nat.h (nbsd_nat_target::supports_native_dumpcore)
       (nbsd_nat_target::native_dumpcore): New declarations.
       * nbsd-nat.c (nbsd_nat_target::supports_native_dumpcore)
       (nbsd_nat_target::native_dumpcore): New functions.
       * gcore.c (gcore_command): Use target_supports_native_dumpcore ()
       and target_native_dumpcore ().
---
 gdb/ChangeLog          | 14 +++++++++++
 gdb/gcore.c            | 21 ++++++++++------
 gdb/nbsd-nat.c         | 18 ++++++++++++++
 gdb/nbsd-nat.h         |  2 ++
 gdb/target-delegates.c | 55 ++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h           | 18 ++++++++++++++
 6 files changed, 120 insertions(+), 8 deletions(-)

--
2.26.2

Comments

John Baldwin July 27, 2020, 3:47 p.m. | #1
On 7/27/20 7:43 AM, Kamil Rytarowski wrote:
> Add new API for systems with native kernel support for dumping

> a process on demand. Wire it into the gdb's gcore functionality.

> 

> Define supports_native_dumpcore and native_dumpcore for NetBSD,

> that wraps the ptrace(2) call with the PT_DUMPCORE operation.

> 

> gdb/ChangeLog:

> 

>        * target.h (supports_native_dumpcore, native_dumpcore): New

>        function declarations.

>        * target.c (supports_native_dumpcore, native_dumpcore): New

>        functions.

>        * target-delegates.c: Rebuild.

>        * nbsd-nat.h (nbsd_nat_target::supports_native_dumpcore)

>        (nbsd_nat_target::native_dumpcore): New declarations.

>        * nbsd-nat.c (nbsd_nat_target::supports_native_dumpcore)

>        (nbsd_nat_target::native_dumpcore): New functions.

>        * gcore.c (gcore_command): Use target_supports_native_dumpcore ()

>        and target_native_dumpcore ().


One suggestion I would have is to do this as a 2-patch series with the
first patch introducing the new target hook and using it in gcore and
the second patch adding the implementation for the NetBSD native target.

I also wonder if the new target hook requires "native" in the name, it
could perhaps just be "target_supports_dumpcore" and "target_dumpcore"?

-- 
John Baldwin
Tom Tromey July 27, 2020, 9:09 p.m. | #2
>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:


Kamil> +bool
Kamil> +nbsd_nat_target::native_dumpcore (char *filename)

Should be "const char *".

Kamil> +{
Kamil> +  pid_t pid = inferior_ptid.pid ();
Kamil> +
Kamil> +  return ptrace(PT_DUMPCORE, pid, filename, strlen(filename)) != -1;

Spaces before parens in the gdb style.


I wonder if it makes sense to just always defer to the target, and then
have the default target method be a function implementing the existing
approach.  I'm not sure either way, but I thought I'd throw the idea out
there.

Tom

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a768df0d76c..fac4c8b51f3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@ 
+2020-07-27  Kamil Rytarowski  <n54@gmx.com>
+
+	* target.h (supports_native_dumpcore, native_dumpcore): New
+	function declarations.
+	* target.c (supports_native_dumpcore, native_dumpcore): New
+	functions.
+	* target-delegates.c: Rebuild.
+	* nbsd-nat.h (nbsd_nat_target::supports_native_dumpcore)
+	(nbsd_nat_target::native_dumpcore): New declarations.
+	* nbsd-nat.c (nbsd_nat_target::supports_native_dumpcore)
+	(nbsd_nat_target::native_dumpcore): New functions.
+	* gcore.c (gcore_command): Use target_supports_native_dumpcore ()
+	and target_native_dumpcore ().
+
 2020-07-22  Simon Marchi  <simon.marchi@polymtl.ca>
 	    Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

diff --git a/gdb/gcore.c b/gdb/gcore.c
index 7b653fb74e3..7fdd147a779 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -145,17 +145,22 @@  gcore_command (const char *args, int from_tty)
 		      "Opening corefile '%s' for output.\n",
 		      corefilename.get ());

-  /* Open the output file.  */
-  gdb_bfd_ref_ptr obfd (create_gcore_bfd (corefilename.get ()));
+  if (target_supports_native_dumpcore ())
+    target_native_dumpcore (corefilename.get ());
+  else
+    {
+      /* Open the output file.  */
+      gdb_bfd_ref_ptr obfd (create_gcore_bfd (corefilename.get ()));

-  /* Arrange to unlink the file on failure.  */
-  gdb::unlinker unlink_file (corefilename.get ());
+      /* Arrange to unlink the file on failure.  */
+      gdb::unlinker unlink_file (corefilename.get ());

-  /* Call worker function.  */
-  write_gcore_file (obfd.get ());
+      /* Call worker function.  */
+      write_gcore_file (obfd.get ());

-  /* Succeeded.  */
-  unlink_file.keep ();
+      /* Succeeded.  */
+      unlink_file.keep ();
+    }

   fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename.get ());
 }
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index a9405ebf862..c95cd041497 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -845,3 +845,21 @@  nbsd_nat_target::supports_multi_process ()
 {
   return true;
 }
+
+/* Implement the "supports_native_dumpcore" target_ops method.  */
+
+bool
+nbsd_nat_target::supports_native_dumpcore ()
+{
+  return true;
+}
+
+/* Implement the "native_dumpcore" target_ops method.  */
+
+bool
+nbsd_nat_target::native_dumpcore (char *filename)
+{
+  pid_t pid = inferior_ptid.pid ();
+
+  return ptrace(PT_DUMPCORE, pid, filename, strlen(filename)) != -1;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index 0a7048ecf35..1348a0dda45 100644
--- a/gdb/nbsd-nat.h
+++ b/gdb/nbsd-nat.h
@@ -49,6 +49,8 @@  struct nbsd_nat_target : public inf_ptrace_target
     override;

   bool supports_multi_process () override;
+  bool supports_native_dumpcore () override;
+  bool native_dumpcore (char* filename) override;
 };

 #endif /* nbsd-nat.h */
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index c28af097183..a45b49b2bb1 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -108,6 +108,8 @@  struct dummy_target : public target_ops
   bool supports_disable_randomization () override;
   bool supports_string_tracing () override;
   bool supports_evaluation_of_breakpoint_conditions () override;
+  bool supports_native_dumpcore () override;
+  bool native_dumpcore (char *arg0) override;
   bool can_run_breakpoint_commands () override;
   struct gdbarch *thread_architecture (ptid_t arg0) override;
   struct address_space *thread_address_space (ptid_t arg0) override;
@@ -277,6 +279,8 @@  struct debug_target : public target_ops
   bool supports_disable_randomization () override;
   bool supports_string_tracing () override;
   bool supports_evaluation_of_breakpoint_conditions () override;
+  bool supports_native_dumpcore () override;
+  bool native_dumpcore (char *arg0) override;
   bool can_run_breakpoint_commands () override;
   struct gdbarch *thread_architecture (ptid_t arg0) override;
   struct address_space *thread_address_space (ptid_t arg0) override;
@@ -2825,6 +2829,57 @@  debug_target::supports_evaluation_of_breakpoint_conditions ()
   return result;
 }

+bool
+target_ops::supports_native_dumpcore ()
+{
+  return this->beneath ()->supports_native_dumpcore ();
+}
+
+bool
+dummy_target::supports_native_dumpcore ()
+{
+  return false;
+}
+
+bool
+debug_target::supports_native_dumpcore ()
+{
+  bool result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->supports_native_dumpcore (...)\n", this->beneath ()->shortname ());
+  result = this->beneath ()->supports_native_dumpcore ();
+  fprintf_unfiltered (gdb_stdlog, "<- %s->supports_native_dumpcore (", this->beneath ()->shortname ());
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_bool (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
+bool
+target_ops::native_dumpcore (char *arg0)
+{
+  return this->beneath ()->native_dumpcore (arg0);
+}
+
+bool
+dummy_target::native_dumpcore (char *arg0)
+{
+  return false;
+}
+
+bool
+debug_target::native_dumpcore (char *arg0)
+{
+  bool result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->native_dumpcore (...)\n", this->beneath ()->shortname ());
+  result = this->beneath ()->native_dumpcore (arg0);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->native_dumpcore (", this->beneath ()->shortname ());
+  target_debug_print_char_p (arg0);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_bool (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 bool
 target_ops::can_run_breakpoint_commands ()
 {
diff --git a/gdb/target.h b/gdb/target.h
index 4e8d4cccd5c..24f462dd38e 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -881,6 +881,14 @@  struct target_ops
     virtual bool supports_evaluation_of_breakpoint_conditions ()
       TARGET_DEFAULT_RETURN (false);

+    /* Does this target support native dumpcore API?  */
+    virtual bool supports_native_dumpcore ()
+      TARGET_DEFAULT_RETURN (false);
+
+    /* Generate the core file with native target API.  */
+    virtual bool native_dumpcore (char *filename)
+      TARGET_DEFAULT_RETURN (false);
+
     /* Does this target support evaluation of breakpoint commands on its
        end?  */
     virtual bool can_run_breakpoint_commands ()
@@ -1499,6 +1507,16 @@  int target_supports_disable_randomization (void);
 #define target_supports_evaluation_of_breakpoint_conditions() \
   (current_top_target ()->supports_evaluation_of_breakpoint_conditions) ()

+/* Does this target support native dumpcore API?  */
+
+#define target_supports_native_dumpcore() \
+  (current_top_target ()->supports_native_dumpcore) ()
+
+/* Generate the core file with native target API.  */
+
+#define target_native_dumpcore(x) \
+  (current_top_target ()->native_dumpcore (x))
+
 /* Returns true if this target can handle breakpoint commands
    on its end.  */