[v2,1/2] gdb: Implement native dumpcore function

Message ID 20200728154602.3824-1-n54@gmx.com
State Superseded
Headers show
Series
  • [v2,1/2] gdb: Implement native dumpcore function
Related show

Commit Message

Kamil Rytarowski July 28, 2020, 3:46 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.

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.
       * gcore.c (gcore_command): Use target_supports_native_dumpcore ()
       and target_native_dumpcore ().
---
 gdb/ChangeLog          | 10 ++++++++
 gdb/gcore.c            | 21 ++++++++++------
 gdb/target-delegates.c | 55 ++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h           | 18 ++++++++++++++
 4 files changed, 96 insertions(+), 8 deletions(-)

--
2.26.2

Comments

Eli Zaretskii via Gdb-patches July 28, 2020, 6:56 p.m. | #1
On Tue, Jul 28, 2020 at 10:47 AM Kamil Rytarowski <n54@gmx.com> wrote:
> +++ b/gdb/target.h

> +/* Does this target support dumpcore API?  */

> +

> +#define target_supports_dumpcore() \

> +  (current_top_target ()->supports_dumpcore) ()

> +

> +/* Generate the core file with target API.  */

> +

> +#define target_dumpcore(x) \

> +  (current_top_target ()->dumpcore (x))


These macros don't seem that useful to me, and they hide a global
variable access... would it be possible to remove them?

Christian
Kamil Rytarowski July 28, 2020, 8:39 p.m. | #2
On 28.07.2020 20:56, Christian Biesinger wrote:
> On Tue, Jul 28, 2020 at 10:47 AM Kamil Rytarowski <n54@gmx.com> wrote:

>> +++ b/gdb/target.h

>> +/* Does this target support dumpcore API?  */

>> +

>> +#define target_supports_dumpcore() \

>> +  (current_top_target ()->supports_dumpcore) ()

>> +

>> +/* Generate the core file with target API.  */

>> +

>> +#define target_dumpcore(x) \

>> +  (current_top_target ()->dumpcore (x))

> 

> These macros don't seem that useful to me, and they hide a global

> variable access... would it be possible to remove them?

> 


I have got no preference myself, but preexisting code uses the macros.
If you want to remove them, please do it as an independent patch.

> Christian

>
Tom Tromey July 29, 2020, 3:45 p.m. | #3
>> These macros don't seem that useful to me, and they hide a global

>> variable access... would it be possible to remove them?


Kamil> I have got no preference myself, but preexisting code uses the macros.
Kamil> If you want to remove them, please do it as an independent patch.

I didn't read your patch, but I think it's fine to follow the existing
code here.

Most (maybe all?) of the target_* functions and function-like macros
implicitly use the current target.

Tom
Tom Tromey Aug. 5, 2020, 4:23 p.m. | #4
>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:


Kamil> Add new API for systems with native kernel support for dumping
Kamil> a process on demand. Wire it into the gdb's gcore functionality.

Kamil> gdb/ChangeLog:

Kamil>        * target.h (supports_native_dumpcore, native_dumpcore): New
Kamil>        function declarations.
Kamil>        * target.c (supports_native_dumpcore, native_dumpcore): New
Kamil>        functions.
Kamil>        * target-delegates.c: Rebuild.
Kamil>        * gcore.c (gcore_command): Use target_supports_native_dumpcore ()
Kamil>        and target_native_dumpcore ().

Thank you for the patch.

Kamil> +    /* Generate the core file with native target API.  */
Kamil> +    virtual bool dumpcore (const char *filename)
Kamil> +      TARGET_DEFAULT_RETURN (false);

There doesn't seem to be a reason to have a bool return here.
The value isn't documented or used.

Tom
Kamil Rytarowski Aug. 13, 2020, 5:15 p.m. | #5
On 05.08.2020 18:23, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

>

> Kamil> Add new API for systems with native kernel support for dumping

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

>

> Kamil> gdb/ChangeLog:

>

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

> Kamil>        function declarations.

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

> Kamil>        functions.

> Kamil>        * target-delegates.c: Rebuild.

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

> Kamil>        and target_native_dumpcore ().

>

> Thank you for the patch.

>

> Kamil> +    /* Generate the core file with native target API.  */

> Kamil> +    virtual bool dumpcore (const char *filename)

> Kamil> +      TARGET_DEFAULT_RETURN (false);

>

> There doesn't seem to be a reason to have a bool return here.

> The value isn't documented or used.

>


I've decided to remove the value in the follow up patchset.

> Tom

>

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index defca83c263..118bb4f89a2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@ 
+2020-07-28  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.
+	* gcore.c (gcore_command): Use target_supports_native_dumpcore ()
+	and target_native_dumpcore ().
+
 2020-07-28  H.J. Lu  <hongjiu.lu@intel.com>

 	PR binutils/26301
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 7b653fb74e3..d0e36b1a708 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_dumpcore ())
+    target_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/target-delegates.c b/gdb/target-delegates.c
index c28af097183..c7e0811caf2 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_dumpcore () override;
+  bool dumpcore (const 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_dumpcore () override;
+  bool dumpcore (const 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_dumpcore ()
+{
+  return this->beneath ()->supports_dumpcore ();
+}
+
+bool
+dummy_target::supports_dumpcore ()
+{
+  return false;
+}
+
+bool
+debug_target::supports_dumpcore ()
+{
+  bool result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->supports_dumpcore (...)\n", this->beneath ()->shortname ());
+  result = this->beneath ()->supports_dumpcore ();
+  fprintf_unfiltered (gdb_stdlog, "<- %s->supports_dumpcore (", this->beneath ()->shortname ());
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_bool (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
+bool
+target_ops::dumpcore (const char *arg0)
+{
+  return this->beneath ()->dumpcore (arg0);
+}
+
+bool
+dummy_target::dumpcore (const char *arg0)
+{
+  return false;
+}
+
+bool
+debug_target::dumpcore (const char *arg0)
+{
+  bool result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->dumpcore (...)\n", this->beneath ()->shortname ());
+  result = this->beneath ()->dumpcore (arg0);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->dumpcore (", this->beneath ()->shortname ());
+  target_debug_print_const_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..85a03ed5be8 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_dumpcore ()
+      TARGET_DEFAULT_RETURN (false);
+
+    /* Generate the core file with native target API.  */
+    virtual bool dumpcore (const 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 dumpcore API?  */
+
+#define target_supports_dumpcore() \
+  (current_top_target ()->supports_dumpcore) ()
+
+/* Generate the core file with target API.  */
+
+#define target_dumpcore(x) \
+  (current_top_target ()->dumpcore (x))
+
 /* Returns true if this target can handle breakpoint commands
    on its end.  */