[PATCH 0/4] Handle null inferiors in target::read_description

[PATCH 0/4] Handle null inferiors in target::read_description

[John Baldwin]

This is a set of changes split out from my larger XSAVE review that
will hopefully be easier to review as a smaller chunk.  Compared to
the version of these patches posted previously I have added extra
details to the log messages for patches 1 and 2 and added two new
patches.  Namely, patch 4 now includes a test for this case (and patch
3 is some refactoring in preparation for patch 4).

John Baldwin (4):
  *-fbsd-nat: Handle null inferior in read_description.
  *-linux-nat: Handle null inferior in read_description.
  Add a have_native_target helper function for use with require.
  Test that native targets can read a tdesc without a process attached.

 gdb/aarch64-fbsd-nat.c                        |  3 +++
 gdb/aarch64-linux-nat.c                       |  3 +++
 gdb/amd64-fbsd-nat.c                          |  3 +++
 gdb/arm-fbsd-nat.c                            |  3 +++
 gdb/arm-linux-nat.c                           |  3 +++
 gdb/i386-fbsd-nat.c                           |  3 +++
 gdb/mips-linux-nat.c                          |  3 +++
 gdb/ppc-linux-nat.c                           |  3 +++
 gdb/riscv-linux-nat.c                         |  3 +++
 gdb/s390-linux-nat.c                          |  3 +++
 .../gdb.base/auto-connect-native-target.exp   | 18 +------------
 .../gdb.base/native-target-noproc-tdesc.exp   | 27 +++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     | 17 ++++++++++++
 gdb/x86-linux-nat.c                           |  3 +++
 14 files changed, 78 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp

-- 
2.40.0

[PATCH 1/4] *-fbsd-nat: Handle null inferior in read_description.

[John Baldwin]

Don't invoke ptrace in the target read_description method if there is
not an active inferior to query via ptrace.  Instead, use the default
register set for the architecture.

Previously the native target could report an error from a failed
ptrace operation when fetching a tdesc without an attached process.
For example on FreeBSD/amd64:

(gdb) target native
Done.  Use the "run" command to start a process.
(gdb) unset tdesc filename
Couldn't get registers: Operation not permitted.
---
 gdb/aarch64-fbsd-nat.c | 3 +++
 gdb/amd64-fbsd-nat.c   | 3 +++
 gdb/arm-fbsd-nat.c     | 3 +++
 gdb/i386-fbsd-nat.c    | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/gdb/aarch64-fbsd-nat.c b/gdb/aarch64-fbsd-nat.c
index 709f5162ce0..29b58e5ff4a 100644
--- a/gdb/aarch64-fbsd-nat.c
+++ b/gdb/aarch64-fbsd-nat.c
@@ -120,6 +120,9 @@ aarch64_fbsd_nat_target::store_registers (struct regcache *regcache,
 const struct target_desc *
 aarch64_fbsd_nat_target::read_description ()
 {
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
   aarch64_features features;
   features.tls = have_regset (inferior_ptid, NT_ARM_TLS)? 1 : 0;
   return aarch64_read_description (features);
diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index bae267f230f..adbd85571a5 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -310,6 +310,9 @@ amd64_fbsd_nat_target::read_description ()
   struct reg regs;
   int is64;
 
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
   if (ptrace (PT_GETREGS, inferior_ptid.pid (),
 	      (PTRACE_TYPE_ARG3) &regs, 0) == -1)
     perror_with_name (_("Couldn't get registers"));
diff --git a/gdb/arm-fbsd-nat.c b/gdb/arm-fbsd-nat.c
index 5181281b27d..cf22fc6cce9 100644
--- a/gdb/arm-fbsd-nat.c
+++ b/gdb/arm-fbsd-nat.c
@@ -93,6 +93,9 @@ arm_fbsd_nat_target::read_description ()
   const struct target_desc *desc;
   bool tls = false;
 
+  if (inferior_ptid == null_ptid)
+    return this->beneath ()->read_description ();
+
 #ifdef PT_GETREGSET
   tls = have_regset (inferior_ptid, NT_ARM_TLS) != 0;
 #endif
diff --git a/gdb/i386-fbsd-nat.c b/gdb/i386-fbsd-nat.c
index 927771e8b20..26ae420949e 100644
--- a/gdb/i386-fbsd-nat.c
+++ b/gdb/i386-fbsd-nat.c
@@ -315,6 +315,9 @@ i386_fbsd_nat_target::read_description ()
 #endif
   static int xmm_probed;
 
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
 #ifdef PT_GETXSTATE_INFO
   if (!xsave_probed)
     {
-- 
2.40.0

[PATCH 2/4] *-linux-nat: Handle null inferior in read_description.

[John Baldwin]

Don't invoke ptrace in the target read_description method if there is
not an active inferior to query via ptrace.  Instead, use the default
register set for the architecture.

Previously the native target could report an error from a failed
ptrace operation when fetching a tdesc without an attached process.
For example on Linux x86-64:

(gdb) target native
Done.  Use the "run" command to start a process.
(gdb) unset tdesc filename
Couldn't get CS register: No such process.
---
 gdb/aarch64-linux-nat.c | 3 +++
 gdb/arm-linux-nat.c     | 3 +++
 gdb/mips-linux-nat.c    | 3 +++
 gdb/ppc-linux-nat.c     | 3 +++
 gdb/riscv-linux-nat.c   | 3 +++
 gdb/s390-linux-nat.c    | 3 +++
 gdb/x86-linux-nat.c     | 3 +++
 7 files changed, 21 insertions(+)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index ecb2eeb9540..62f8825b9c8 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -785,6 +785,9 @@ aarch64_linux_nat_target::read_description ()
   gdb_byte regbuf[ARM_VFP3_REGS_SIZE];
   struct iovec iovec;
 
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
   tid = inferior_ptid.pid ();
 
   iovec.iov_base = regbuf;
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index ef3fa008adf..70c6bc684fa 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -531,6 +531,9 @@ ps_get_thread_area (struct ps_prochandle *ph,
 const struct target_desc *
 arm_linux_nat_target::read_description ()
 {
+  if (inferior_ptid == null_ptid)
+    return this->beneath ()->read_description ();
+
   CORE_ADDR arm_hwcap = linux_get_hwcap ();
 
   if (have_ptrace_getregset == TRIBOOL_UNKNOWN)
diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
index 972b5db8e76..1fa0e8c479c 100644
--- a/gdb/mips-linux-nat.c
+++ b/gdb/mips-linux-nat.c
@@ -454,6 +454,9 @@ mips_linux_nat_target::register_u_offset (struct gdbarch *gdbarch,
 const struct target_desc *
 mips_linux_nat_target::read_description ()
 {
+  if (inferior_ptid == null_ptid)
+    return _MIPS_SIM == _ABIO32 ? tdesc_mips_linux : tdesc_mips64_linux;
+
   static int have_dsp = -1;
 
   if (have_dsp < 0)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 42885deb45e..2f4799aa73a 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1941,6 +1941,9 @@ ppc_linux_nat_target::auxv_parse (const gdb_byte **readptr,
 const struct target_desc *
 ppc_linux_nat_target::read_description ()
 {
+  if (inferior_ptid == null_ptid)
+    return ppc_linux_match_description (ppc_linux_no_features);
+
   int tid = inferior_ptid.pid ();
 
   if (have_ptrace_getsetevrregs)
diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
index 8be4a5ac3e5..5d325e633da 100644
--- a/gdb/riscv-linux-nat.c
+++ b/gdb/riscv-linux-nat.c
@@ -201,6 +201,9 @@ fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
 const struct target_desc *
 riscv_linux_nat_target::read_description ()
 {
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
   const struct riscv_gdbarch_features features
     = riscv_linux_read_features (inferior_ptid.pid ());
   return riscv_lookup_target_description (features);
diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
index fc3917d30be..7d3b3cfe78b 100644
--- a/gdb/s390-linux-nat.c
+++ b/gdb/s390-linux-nat.c
@@ -987,6 +987,9 @@ s390_linux_nat_target::auxv_parse (const gdb_byte **readptr,
 const struct target_desc *
 s390_linux_nat_target::read_description ()
 {
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
   int tid = inferior_ptid.pid ();
 
   have_regset_last_break
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index fd2145244cc..87862b89eab 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -115,6 +115,9 @@ x86_linux_nat_target::read_description ()
   static uint64_t xcr0;
   uint64_t xcr0_features_bits;
 
+  if (inferior_ptid == null_ptid)
+    return nullptr;
+
   tid = inferior_ptid.pid ();
 
 #ifdef __x86_64__
-- 
2.40.0

[PATCH 3/4] Add a have_native_target helper function for use with require.

[John Baldwin]

Move logic from auto-connect-native-target.exp into this helper.
---
 .../gdb.base/auto-connect-native-target.exp    | 18 +-----------------
 gdb/testsuite/lib/gdb.exp                      | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/gdb/testsuite/gdb.base/auto-connect-native-target.exp b/gdb/testsuite/gdb.base/auto-connect-native-target.exp
index 002a6d61126..0586cd4baf4 100644
--- a/gdb/testsuite/gdb.base/auto-connect-native-target.exp
+++ b/gdb/testsuite/gdb.base/auto-connect-native-target.exp
@@ -22,23 +22,7 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
     return -1
 }
 
-# Whether this GDB is configured with a "native" target.
-set have_native 0
-
-set test "help target native"
-gdb_test_multiple $test $test {
-    -re "Undefined target command.*$gdb_prompt $" {
-	set have_native 0
-    }
-    -re "Native process.*$gdb_prompt $" {
-	set have_native 1
-    }
-}
-
-if { !$have_native } {
-    unsupported "no \"target native\" support."
-    return
-}
+require have_native_target
 
 # Returns the topmost target pushed on the target stack.  TEST is used
 # as test message.
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 133d914aff8..128c0779b69 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -9739,6 +9739,23 @@ gdb_caching_proc have_compile_and_link_flag { flag } {
 		additional_flags=$flag]
 }
 
+# Return 1 if this GDB is configured with a "native" target.
+
+gdb_caching_proc have_native_target {} {
+    global gdb_prompt
+
+    set test "help target native"
+    gdb_test_multiple $test $test {
+	-re "Undefined target command.*$gdb_prompt $" {
+	    return 0
+	}
+	-re "Native process.*$gdb_prompt $" {
+	    return 1
+	}
+    }
+    return 0
+}
+
 # Handle include file $srcdir/$subdir/FILE.
 
 proc include_file { file } {
-- 
2.40.0

[PATCH 4/4] Test that native targets can read a tdesc without a process attached.

[John Baldwin]

This ensures that 'unset tdesc filename' does not generate any output
on a "bare" native target inferior without an attached process.
---
 .../gdb.base/native-target-noproc-tdesc.exp   | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp

diff --git a/gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp b/gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp
new file mode 100644
index 00000000000..820d9941c71
--- /dev/null
+++ b/gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp
@@ -0,0 +1,27 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test "unset tdesc filename" without an attached process on native
+# targets.
+
+clean_restart
+
+require have_native_target
+
+# Manually attach the native target
+gdb_test "target native" "Done.  Use the \"run\" command to start a process."
+
+# Load a default target description
+gdb_test_no_output "unset tdesc filename"
-- 
2.40.0

[PING] [PATCH 0/4] Handle null inferiors in target::read_description

[John Baldwin]

On 5/26/23 10:57 AM, John Baldwin wrote:
> This is a set of changes split out from my larger XSAVE review that
> will hopefully be easier to review as a smaller chunk.  Compared to
> the version of these patches posted previously I have added extra
> details to the log messages for patches 1 and 2 and added two new
> patches.  Namely, patch 4 now includes a test for this case (and patch
> 3 is some refactoring in preparation for patch 4).
> 
> John Baldwin (4):
>    *-fbsd-nat: Handle null inferior in read_description.
>    *-linux-nat: Handle null inferior in read_description.
>    Add a have_native_target helper function for use with require.
>    Test that native targets can read a tdesc without a process attached.
> 
>   gdb/aarch64-fbsd-nat.c                        |  3 +++
>   gdb/aarch64-linux-nat.c                       |  3 +++
>   gdb/amd64-fbsd-nat.c                          |  3 +++
>   gdb/arm-fbsd-nat.c                            |  3 +++
>   gdb/arm-linux-nat.c                           |  3 +++
>   gdb/i386-fbsd-nat.c                           |  3 +++
>   gdb/mips-linux-nat.c                          |  3 +++
>   gdb/ppc-linux-nat.c                           |  3 +++
>   gdb/riscv-linux-nat.c                         |  3 +++
>   gdb/s390-linux-nat.c                          |  3 +++
>   .../gdb.base/auto-connect-native-target.exp   | 18 +------------
>   .../gdb.base/native-target-noproc-tdesc.exp   | 27 +++++++++++++++++++
>   gdb/testsuite/lib/gdb.exp                     | 17 ++++++++++++
>   gdb/x86-linux-nat.c                           |  3 +++
>   14 files changed, 78 insertions(+), 17 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp

Ping.

-- 
John Baldwin

Re: [PATCH 2/4] *-linux-nat: Handle null inferior in read_description.

[Luis Machado]

Hi John,

Do you have an example of when this would happen? So, having no inferior and invoking ptrace?

I know this can happen for extended remote mode, but it isn't clear for native gdb.

On 5/26/23 18:57, John Baldwin wrote:
> Don't invoke ptrace in the target read_description method if there is
> not an active inferior to query via ptrace.  Instead, use the default
> register set for the architecture.
> 
> Previously the native target could report an error from a failed
> ptrace operation when fetching a tdesc without an attached process.
> For example on Linux x86-64:
> 
> (gdb) target native
> Done.  Use the "run" command to start a process.
> (gdb) unset tdesc filename
> Couldn't get CS register: No such process.
> ---
>  gdb/aarch64-linux-nat.c | 3 +++
>  gdb/arm-linux-nat.c     | 3 +++
>  gdb/mips-linux-nat.c    | 3 +++
>  gdb/ppc-linux-nat.c     | 3 +++
>  gdb/riscv-linux-nat.c   | 3 +++
>  gdb/s390-linux-nat.c    | 3 +++
>  gdb/x86-linux-nat.c     | 3 +++
>  7 files changed, 21 insertions(+)
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index ecb2eeb9540..62f8825b9c8 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -785,6 +785,9 @@ aarch64_linux_nat_target::read_description ()
>    gdb_byte regbuf[ARM_VFP3_REGS_SIZE];
>    struct iovec iovec;
>  
> +  if (inferior_ptid == null_ptid)
> +    return nullptr;
> +
>    tid = inferior_ptid.pid ();
>  
>    iovec.iov_base = regbuf;
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index ef3fa008adf..70c6bc684fa 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -531,6 +531,9 @@ ps_get_thread_area (struct ps_prochandle *ph,
>  const struct target_desc *
>  arm_linux_nat_target::read_description ()
>  {
> +  if (inferior_ptid == null_ptid)
> +    return this->beneath ()->read_description ();
> +
>    CORE_ADDR arm_hwcap = linux_get_hwcap ();
>  
>    if (have_ptrace_getregset == TRIBOOL_UNKNOWN)
> diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
> index 972b5db8e76..1fa0e8c479c 100644
> --- a/gdb/mips-linux-nat.c
> +++ b/gdb/mips-linux-nat.c
> @@ -454,6 +454,9 @@ mips_linux_nat_target::register_u_offset (struct gdbarch *gdbarch,
>  const struct target_desc *
>  mips_linux_nat_target::read_description ()
>  {
> +  if (inferior_ptid == null_ptid)
> +    return _MIPS_SIM == _ABIO32 ? tdesc_mips_linux : tdesc_mips64_linux;
> +
>    static int have_dsp = -1;
>  
>    if (have_dsp < 0)
> diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
> index 42885deb45e..2f4799aa73a 100644
> --- a/gdb/ppc-linux-nat.c
> +++ b/gdb/ppc-linux-nat.c
> @@ -1941,6 +1941,9 @@ ppc_linux_nat_target::auxv_parse (const gdb_byte **readptr,
>  const struct target_desc *
>  ppc_linux_nat_target::read_description ()
>  {
> +  if (inferior_ptid == null_ptid)
> +    return ppc_linux_match_description (ppc_linux_no_features);
> +
>    int tid = inferior_ptid.pid ();
>  
>    if (have_ptrace_getsetevrregs)
> diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
> index 8be4a5ac3e5..5d325e633da 100644
> --- a/gdb/riscv-linux-nat.c
> +++ b/gdb/riscv-linux-nat.c
> @@ -201,6 +201,9 @@ fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
>  const struct target_desc *
>  riscv_linux_nat_target::read_description ()
>  {
> +  if (inferior_ptid == null_ptid)
> +    return nullptr;
> +
>    const struct riscv_gdbarch_features features
>      = riscv_linux_read_features (inferior_ptid.pid ());
>    return riscv_lookup_target_description (features);
> diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
> index fc3917d30be..7d3b3cfe78b 100644
> --- a/gdb/s390-linux-nat.c
> +++ b/gdb/s390-linux-nat.c
> @@ -987,6 +987,9 @@ s390_linux_nat_target::auxv_parse (const gdb_byte **readptr,
>  const struct target_desc *
>  s390_linux_nat_target::read_description ()
>  {
> +  if (inferior_ptid == null_ptid)
> +    return nullptr;
> +
>    int tid = inferior_ptid.pid ();
>  
>    have_regset_last_break
> diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
> index fd2145244cc..87862b89eab 100644
> --- a/gdb/x86-linux-nat.c
> +++ b/gdb/x86-linux-nat.c
> @@ -115,6 +115,9 @@ x86_linux_nat_target::read_description ()
>    static uint64_t xcr0;
>    uint64_t xcr0_features_bits;
>  
> +  if (inferior_ptid == null_ptid)
> +    return nullptr;
> +
>    tid = inferior_ptid.pid ();
>  
>  #ifdef __x86_64__

Re: [PATCH 2/4] *-linux-nat: Handle null inferior in read_description.

[John Baldwin]

On 6/12/23 1:56 AM, Luis Machado wrote:
> Hi John,
> 
> Do you have an example of when this would happen? So, having no inferior and invoking ptrace?

The example given in the log message provokes this reliably, that is running
"target native" followed by "unset tdesc filename".  It's more that you have an
inferior, it just doesn't have an attached process.

-- 
John Baldwin

Re: [PING] [PATCH 0/4] Handle null inferiors in target::read_description

[John Baldwin]

On 6/9/23 9:55 AM, John Baldwin wrote:
> On 5/26/23 10:57 AM, John Baldwin wrote:
>> This is a set of changes split out from my larger XSAVE review that
>> will hopefully be easier to review as a smaller chunk.  Compared to
>> the version of these patches posted previously I have added extra
>> details to the log messages for patches 1 and 2 and added two new
>> patches.  Namely, patch 4 now includes a test for this case (and patch
>> 3 is some refactoring in preparation for patch 4).
>>
>> John Baldwin (4):
>>     *-fbsd-nat: Handle null inferior in read_description.
>>     *-linux-nat: Handle null inferior in read_description.
>>     Add a have_native_target helper function for use with require.
>>     Test that native targets can read a tdesc without a process attached.
>>
>>    gdb/aarch64-fbsd-nat.c                        |  3 +++
>>    gdb/aarch64-linux-nat.c                       |  3 +++
>>    gdb/amd64-fbsd-nat.c                          |  3 +++
>>    gdb/arm-fbsd-nat.c                            |  3 +++
>>    gdb/arm-linux-nat.c                           |  3 +++
>>    gdb/i386-fbsd-nat.c                           |  3 +++
>>    gdb/mips-linux-nat.c                          |  3 +++
>>    gdb/ppc-linux-nat.c                           |  3 +++
>>    gdb/riscv-linux-nat.c                         |  3 +++
>>    gdb/s390-linux-nat.c                          |  3 +++
>>    .../gdb.base/auto-connect-native-target.exp   | 18 +------------
>>    .../gdb.base/native-target-noproc-tdesc.exp   | 27 +++++++++++++++++++
>>    gdb/testsuite/lib/gdb.exp                     | 17 ++++++++++++
>>    gdb/x86-linux-nat.c                           |  3 +++
>>    14 files changed, 78 insertions(+), 17 deletions(-)
>>    create mode 100644 gdb/testsuite/gdb.base/native-target-noproc-tdesc.exp
> 
> Ping.
> 

-- 
John Baldwin

Re: [PATCH 1/4] *-fbsd-nat: Handle null inferior in read_description.

[Simon Marchi]

On 5/26/23 13:57, John Baldwin wrote:
> Don't invoke ptrace in the target read_description method if there is
> not an active inferior to query via ptrace.  Instead, use the default
> register set for the architecture.
> 
> Previously the native target could report an error from a failed
> ptrace operation when fetching a tdesc without an attached process.
> For example on FreeBSD/amd64:
> 
> (gdb) target native
> Done.  Use the "run" command to start a process.
> (gdb) unset tdesc filename
> Couldn't get registers: Operation not permitted.
> ---
>  gdb/aarch64-fbsd-nat.c | 3 +++
>  gdb/amd64-fbsd-nat.c   | 3 +++
>  gdb/arm-fbsd-nat.c     | 3 +++
>  gdb/i386-fbsd-nat.c    | 3 +++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/gdb/aarch64-fbsd-nat.c b/gdb/aarch64-fbsd-nat.c
> index 709f5162ce0..29b58e5ff4a 100644
> --- a/gdb/aarch64-fbsd-nat.c
> +++ b/gdb/aarch64-fbsd-nat.c
> @@ -120,6 +120,9 @@ aarch64_fbsd_nat_target::store_registers (struct regcache *regcache,
>  const struct target_desc *
>  aarch64_fbsd_nat_target::read_description ()
>  {
> +  if (inferior_ptid == null_ptid)
> +    return nullptr;
> +
>    aarch64_features features;
>    features.tls = have_regset (inferior_ptid, NT_ARM_TLS)? 1 : 0;
>    return aarch64_read_description (features);
> diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
> index bae267f230f..adbd85571a5 100644
> --- a/gdb/amd64-fbsd-nat.c
> +++ b/gdb/amd64-fbsd-nat.c
> @@ -310,6 +310,9 @@ amd64_fbsd_nat_target::read_description ()
>    struct reg regs;
>    int is64;
>  
> +  if (inferior_ptid == null_ptid)
> +    return nullptr;

Why do you return nullptr here, instead of calling the beneath target,
as done in arm-fbsd-nat.c?  Calling the beneath target seems like the
right thing to do.  In practice, it will likely reach
dummy_target::read_description, which returns nullptr.

Simon

Re: [PATCH 3/4] Add a have_native_target helper function for use with require.

[Simon Marchi]

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 133d914aff8..128c0779b69 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -9739,6 +9739,23 @@ gdb_caching_proc have_compile_and_link_flag { flag } {
>  		additional_flags=$flag]
>  }
>  
> +# Return 1 if this GDB is configured with a "native" target.
> +
> +gdb_caching_proc have_native_target {} {
> +    global gdb_prompt
> +
> +    set test "help target native"
> +    gdb_test_multiple $test $test {
> +	-re "Undefined target command.*$gdb_prompt $" {
> +	    return 0
> +	}
> +	-re "Native process.*$gdb_prompt $" {
> +	    return 1
> +	}
> +    }
> +    return 0
> +}
Touching this code would be an opportunity to modernize it.  I think
that if you do:

  gdb_test_multiple "help target native"  "" {

the command is automatically used as the test name (so, no need for the
test variable).

Then, instead of having "$gdb_prompt $", you can probably use -wrap:

  -re -wrap "Undefined target command.*"

  -re -wrap "Native process.*"

Simon

Re: [PATCH 1/4] *-fbsd-nat: Handle null inferior in read_description.

[John Baldwin]

On 7/6/23 9:18 AM, Simon Marchi wrote:
> On 5/26/23 13:57, John Baldwin wrote:
>> Don't invoke ptrace in the target read_description method if there is
>> not an active inferior to query via ptrace.  Instead, use the default
>> register set for the architecture.
>>
>> Previously the native target could report an error from a failed
>> ptrace operation when fetching a tdesc without an attached process.
>> For example on FreeBSD/amd64:
>>
>> (gdb) target native
>> Done.  Use the "run" command to start a process.
>> (gdb) unset tdesc filename
>> Couldn't get registers: Operation not permitted.
>> ---
>>   gdb/aarch64-fbsd-nat.c | 3 +++
>>   gdb/amd64-fbsd-nat.c   | 3 +++
>>   gdb/arm-fbsd-nat.c     | 3 +++
>>   gdb/i386-fbsd-nat.c    | 3 +++
>>   4 files changed, 12 insertions(+)
>>
>> diff --git a/gdb/aarch64-fbsd-nat.c b/gdb/aarch64-fbsd-nat.c
>> index 709f5162ce0..29b58e5ff4a 100644
>> --- a/gdb/aarch64-fbsd-nat.c
>> +++ b/gdb/aarch64-fbsd-nat.c
>> @@ -120,6 +120,9 @@ aarch64_fbsd_nat_target::store_registers (struct regcache *regcache,
>>   const struct target_desc *
>>   aarch64_fbsd_nat_target::read_description ()
>>   {
>> +  if (inferior_ptid == null_ptid)
>> +    return nullptr;
>> +
>>     aarch64_features features;
>>     features.tls = have_regset (inferior_ptid, NT_ARM_TLS)? 1 : 0;
>>     return aarch64_read_description (features);
>> diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
>> index bae267f230f..adbd85571a5 100644
>> --- a/gdb/amd64-fbsd-nat.c
>> +++ b/gdb/amd64-fbsd-nat.c
>> @@ -310,6 +310,9 @@ amd64_fbsd_nat_target::read_description ()
>>     struct reg regs;
>>     int is64;
>>   
>> +  if (inferior_ptid == null_ptid)
>> +    return nullptr;
> 
> Why do you return nullptr here, instead of calling the beneath target,
> as done in arm-fbsd-nat.c?  Calling the beneath target seems like the
> right thing to do.  In practice, it will likely reach
> dummy_target::read_description, which returns nullptr.

I think you are right, and I will change both this and patch 2 for the
Linux targets to call the beneath one.  My thinking was that in general
returning nullptr causes the caller code to pick a "default" tdesc, but
that for arm-fbsd-nat.c I was trying to match the existing code
(copied from arm-linux-nat.c) which calls the beneath target for its fallback
instead of returning nullptr.

-- 
John Baldwin

Re: [PATCH 3/4] Add a have_native_target helper function for use with require.

[John Baldwin]

On 7/6/23 9:24 AM, Simon Marchi wrote:
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 133d914aff8..128c0779b69 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -9739,6 +9739,23 @@ gdb_caching_proc have_compile_and_link_flag { flag } {
>>   		additional_flags=$flag]
>>   }
>>   
>> +# Return 1 if this GDB is configured with a "native" target.
>> +
>> +gdb_caching_proc have_native_target {} {
>> +    global gdb_prompt
>> +
>> +    set test "help target native"
>> +    gdb_test_multiple $test $test {
>> +	-re "Undefined target command.*$gdb_prompt $" {
>> +	    return 0
>> +	}
>> +	-re "Native process.*$gdb_prompt $" {
>> +	    return 1
>> +	}
>> +    }
>> +    return 0
>> +}
> Touching this code would be an opportunity to modernize it.  I think
> that if you do:
> 
>    gdb_test_multiple "help target native"  "" {
> 
> the command is automatically used as the test name (so, no need for the
> test variable).
> 
> Then, instead of having "$gdb_prompt $", you can probably use -wrap:
> 
>    -re -wrap "Undefined target command.*"
> 
>    -re -wrap "Native process.*"

Thanks, those all worked and did make it a bit simpler.

-- 
John Baldwin