[PATCH] gdb: Implement native dumpcore function

[PATCH] gdb: Implement native dumpcore function

[Kamil Rytarowski]

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(-)

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.  */

--
2.26.2

Re: [PATCH] gdb: Implement native dumpcore function

[John Baldwin]

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

Re: [PATCH] gdb: Implement native dumpcore function

[Tom Tromey]

>>>>> "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 v2 1/2] gdb: Implement native dumpcore function

[Kamil Rytarowski]

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(-)

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.  */

--
2.26.2

[PATCH v2 2/2] gdb: Implement native dumpcore function for NetBSD

[Kamil Rytarowski]

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

gdb/ChangeLog:

       * 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.
---
 gdb/ChangeLog  |  7 +++++++
 gdb/nbsd-nat.c | 19 +++++++++++++++++++
 gdb/nbsd-nat.h |  2 ++
 3 files changed, 28 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 118bb4f89a2..75b9a0bd13d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2020-07-28  Kamil Rytarowski  <n54@gmx.com>
+
+	* 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.
+
 2020-07-28  Kamil Rytarowski  <n54@gmx.com>

 	* target.h (supports_native_dumpcore, native_dumpcore): New
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index a9405ebf862..c893c5498bb 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -845,3 +845,22 @@ nbsd_nat_target::supports_multi_process ()
 {
   return true;
 }
+
+/* Implement the "supports_dumpcore" target_ops method.  */
+
+bool
+nbsd_nat_target::supports_dumpcore ()
+{
+  return true;
+}
+
+/* Implement the "dumpcore" target_ops method.  */
+
+bool
+nbsd_nat_target::dumpcore (const char *filename)
+{
+  pid_t pid = inferior_ptid.pid ();
+
+  return ptrace (PT_DUMPCORE, pid, const_cast<char *>(filename),
+		 strlen (filename)) != -1;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index 0a7048ecf35..550ea6a5d8d 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_dumpcore () override;
+  bool dumpcore (const char* filename) override;
 };

 #endif /* nbsd-nat.h */
--
2.26.2

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

[Christian Biesinger]

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

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

[Kamil Rytarowski]

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
> 

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

[Tom Tromey]

>> 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

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

[Tom Tromey]

>>>>> "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

Re: [PATCH v2 2/2] gdb: Implement native dumpcore function for NetBSD

[Tom Tromey]

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> Define supports_dumpcore and dumpcore for NetBSD, that wraps
Kamil> the ptrace(2) call with the PT_DUMPCORE operation.

Kamil> +  return ptrace (PT_DUMPCORE, pid, const_cast<char *>(filename),
Kamil> +		 strlen (filename)) != -1;

If this can fail it is probably better to call perror_with_name or
error or something along those lines.

Tom

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

[Kamil Rytarowski]

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
>

Re: [PATCH v2 2/2] gdb: Implement native dumpcore function for NetBSD

[Kamil Rytarowski]

On 05.08.2020 18:24, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
>
> Kamil> Define supports_dumpcore and dumpcore for NetBSD, that wraps
> Kamil> the ptrace(2) call with the PT_DUMPCORE operation.
>
> Kamil> +  return ptrace (PT_DUMPCORE, pid, const_cast<char *>(filename),
> Kamil> +		 strlen (filename)) != -1;
>
> If this can fail it is probably better to call perror_with_name or
> error or something along those lines.
>

I've switched to perror_with_name in v3 and I have checked that it can
fail and be handled gracefully.

> Tom
>