[09/12] gdb_target_is_native -> gdb_protocol_is_native

  gdb_is_target_native uses "maint print target-stack", which is
unnecessary when checking whether gdb_protocol is empty would do.
Checking gdb_protocol is more efficient, and can be done before
starting GDB and running to main, unlike gdb_is_target_native.

This adds a new gdb_protocol_is_native procedure, and uses it in place
of gdb_is_target_native.

At first, I thought that we'd end up with a few testcases needing to
use gdb_is_target_native still, especially multi-target tests that
connect to targets different from the default board target, but no,
actually all uses of gdb_is_target_native could be converted.
gdb_is_target_native will be eliminated in a following patch.

In some spots, we no longer need to defer the check until after
starting GDB, so the patch adjusts accordingly.

Change-Id: Ia706232dbffac70f9d9740bcb89c609dbee5cee3
---
 gdb/testsuite/gdb.base/attach.exp                | 13 +++----------
 gdb/testsuite/gdb.base/foll-exec-mode.exp        |  4 +---
 gdb/testsuite/gdb.base/load-command.exp          | 11 +++++------
 gdb/testsuite/gdb.multi/stop-all-on-exit.exp     | 16 ++++++++--------
 gdb/testsuite/gdb.python/py-inferior.exp         |  2 +-
 gdb/testsuite/gdb.threads/threads-after-exec.exp |  2 +-
 gdb/testsuite/lib/gdb.exp                        | 15 +++++++++++++++
 7 files changed, 34 insertions(+), 29 deletions(-)
  

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> gdb_is_target_native uses "maint print target-stack", which is
Pedro> unnecessary when checking whether gdb_protocol is empty would do.
Pedro> Checking gdb_protocol is more efficient, and can be done before
Pedro> starting GDB and running to main, unlike gdb_is_target_native.

Pedro> This adds a new gdb_protocol_is_native procedure, and uses it in place
Pedro> of gdb_is_target_native.

Pedro> At first, I thought that we'd end up with a few testcases needing to
Pedro> use gdb_is_target_native still, especially multi-target tests that
Pedro> connect to targets different from the default board target, but no,
Pedro> actually all uses of gdb_is_target_native could be converted.
Pedro> gdb_is_target_native will be eliminated in a following patch.

Pedro> In some spots, we no longer need to defer the check until after
Pedro> starting GDB, so the patch adjusts accordingly.

Seems reasonable to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
  

On Friday, April 19, 2024 5:14 PM, Pedro Alves wrote:
> Subject: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native

In the title, "gdb_target_is_native" should be spelled "gdb_is_target_native".

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

On 2024-04-22 09:25, Aktemur, Tankut Baris wrote:
> On Friday, April 19, 2024 5:14 PM, Pedro Alves wrote:
>> Subject: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
> 
> In the title, "gdb_target_is_native" should be spelled "gdb_is_target_native".

Thanks Baris.  I've fixed that locally.
  

On 4/19/24 20:50, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> gdb_is_target_native uses "maint print target-stack", which is
> Pedro> unnecessary when checking whether gdb_protocol is empty would do.
> Pedro> Checking gdb_protocol is more efficient, and can be done before
> Pedro> starting GDB and running to main, unlike gdb_is_target_native.
> 
> Pedro> This adds a new gdb_protocol_is_native procedure, and uses it in place
> Pedro> of gdb_is_target_native.
> 
> Pedro> At first, I thought that we'd end up with a few testcases needing to
> Pedro> use gdb_is_target_native still, especially multi-target tests that
> Pedro> connect to targets different from the default board target, but no,
> Pedro> actually all uses of gdb_is_target_native could be converted.
> Pedro> gdb_is_target_native will be eliminated in a following patch.
> 
> Pedro> In some spots, we no longer need to defer the check until after
> Pedro> starting GDB, so the patch adjusts accordingly.
> 
> Seems reasonable to me.
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom
> 

Hi Pedro,

Sorry, for the late reply.

I am testing with riscv-unknown-elf target using simulator.
Before this patch I had:

PASS: gdb.base/load-command.exp: probe for target native
PASS: gdb.base/load-command.exp: check initial value of the_variable
PASS: gdb.base/load-command.exp: manually change the_variable
PASS: gdb.base/load-command.exp: check manually changed value of the_variable
PASS: gdb.base/load-command.exp: reload: re-load binary
PASS: gdb.base/load-command.exp: reload: check initial value of the_variable

and now

UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command

note: I use RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-sim"
but the test was working fine, because with this target
we use target sim, load, and can runto main, inspect variables, etc.


Regards
Bernd.
  

Hi Bernd,

On 2024-05-09 09:47, Bernd Edlinger wrote:

> Hi Pedro,
> 
> Sorry, for the late reply.
> 
> I am testing with riscv-unknown-elf target using simulator.
> Before this patch I had:
> 
> PASS: gdb.base/load-command.exp: probe for target native
> PASS: gdb.base/load-command.exp: check initial value of the_variable
> PASS: gdb.base/load-command.exp: manually change the_variable
> PASS: gdb.base/load-command.exp: check manually changed value of the_variable
> PASS: gdb.base/load-command.exp: reload: re-load binary
> PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
> 
> and now
> 
> UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
> 
> note: I use RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-sim"
> but the test was working fine, because with this target
> we use target sim, load, and can runto main, inspect variables, etc.
> 

I'm trying to understand how the sim board works, to figure out what is the best course
of action.

I see that /usr/share/dejagnu/config/sim.exp has:

 set_board_info protocol  "sim"

but evidently (from your output) nothing sets gdb_protocol.

Where is the code that does "target sim" to connect to the sim inside GDB?  I can't seem to find that.

Pedro Alves
  

Hi Pedro,

On 5/9/24 11:47, Pedro Alves wrote:
> Hi Bernd,
> 
> On 2024-05-09 09:47, Bernd Edlinger wrote:
> 
>> Hi Pedro,
>>
>> Sorry, for the late reply.
>>
>> I am testing with riscv-unknown-elf target using simulator.
>> Before this patch I had:
>>
>> PASS: gdb.base/load-command.exp: probe for target native
>> PASS: gdb.base/load-command.exp: check initial value of the_variable
>> PASS: gdb.base/load-command.exp: manually change the_variable
>> PASS: gdb.base/load-command.exp: check manually changed value of the_variable
>> PASS: gdb.base/load-command.exp: reload: re-load binary
>> PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
>>
>> and now
>>
>> UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
>>
>> note: I use RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-sim"
>> but the test was working fine, because with this target
>> we use target sim, load, and can runto main, inspect variables, etc.
>>
> 
> I'm trying to understand how the sim board works, to figure out what is the best course
> of action.
> 
> I see that /usr/share/dejagnu/config/sim.exp has:
> 
>  set_board_info protocol  "sim"
> 
> but evidently (from your output) nothing sets gdb_protocol.
> 
> Where is the code that does "target sim" to connect to the sim inside GDB?  I can't seem to find that.
> 

I think that file would be used when I run the gcc testsuite,
there I set the path to my riscv-toolchain,
and invoke this in the gcc-build directory:
make check -i RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-run"
or SIM=qemu-riscv32 for comparison.

But if I run the gdb testsuite I can do the same, but the SIM=parameter I could even
use SIM=true, is only a dummy, it must be executable, but it is not used in the end.
And I believe for gdb the following file is picked up instead:

binutils-gdb/gdb/testsuite/config/sim.exp

where I see:

proc gdb_target_sim { } {
    global gdb_prompt
     
    set target_sim_options "[board_info target gdb,target_sim_options]"

    send_gdb "target sim $target_sim_options\n"
    gdb_expect 60 {
        -re "Connected to the simulator.*$gdb_prompt $" {
            verbose "Set target to sim"
        }


and in the log file of this test case from my reference build which was
from commit 354f8d0a1f98a1ade9b669f9200a9c23d22349fe
this test case looked quite normal:

(gdb) target sim ^M
Connected to the simulator.^M
(gdb) load^M
Loading section .text, size 0x163c lma 10120^M
Loading section .eh_frame, size 0x4 lma 11760^M
Loading section .init_array, size 0x8 lma 12000^M
Loading section .fini_array, size 0x8 lma 12008^M
Loading section .data, size 0x9a0 lma 12010^M
Loading section .sdata, size 0x28 lma 129b0^M
Start address 10120^M
Transfer rate: 65728 bits in <1 sec.^M
(gdb) delete breakpoints^M
(gdb) info breakpoints^M
No breakpoints, watchpoints, tracepoints, or catchpoints.^M
(gdb) break -qualified main^M
warning: Can not parse XML memory map; XML support was disabled at compile time^M
Breakpoint 1 at 0x10200: file ..../binutils-gdb/gdb/testsuite/gdb.base/load-command.c, line 22.^M
(gdb) target sim ^M
Connected to the simulator.^M
(gdb) load^M
Loading section .text, size 0x163c lma 10120^M
Loading section .eh_frame, size 0x4 lma 11760^M
Loading section .init_array, size 0x8 lma 12000^M
Loading section .fini_array, size 0x8 lma 12008^M
Loading section .data, size 0x9a0 lma 12010^M
Loading section .sdata, size 0x28 lma 129b0^M
Start address 10120^M
Transfer rate: 65728 bits in <1 sec.^M
(gdb) run ^M
Starting program: ..../gdb/testsuite/outputs/gdb.base/load-command/load-command ^M
^M
Breakpoint 1, main () at ..../binutils-gdb/gdb/testsuite/gdb.base/load-command.c:22^M
22        return 0;^M
(gdb) maint print target-stack^M
The current target stack is:^M
  - sim (simulator)^M
  - exec (Local exec file)^M
  - None (None)^M
(gdb) PASS: gdb.base/load-command.exp: probe for target native
print/x the_variable^M
$1 = 0x1234^M
(gdb) PASS: gdb.base/load-command.exp: check initial value of the_variable
set the_variable = 0x5555^M
(gdb) PASS: gdb.base/load-command.exp: manually change the_variable
print/x the_variable^M
$2 = 0x5555^M
(gdb) PASS: gdb.base/load-command.exp: check manually changed value of the_variable
...

If you want I can help with step by step instructions
how to setting up a working riscv-cross-toolchain.


Thanks
Bernd.
  

On 2024-05-09 12:54, Bernd Edlinger wrote:
> On 5/9/24 11:47, Pedro Alves wrote:
> I think that file would be used when I run the gcc testsuite,
> there I set the path to my riscv-toolchain,
> and invoke this in the gcc-build directory:
> make check -i RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-run"
> or SIM=qemu-riscv32 for comparison.
> 
> But if I run the gdb testsuite I can do the same, but the SIM=parameter I could even
> use SIM=true, is only a dummy, it must be executable, but it is not used in the end.
> And I believe for gdb the following file is picked up instead:
> 
> binutils-gdb/gdb/testsuite/config/sim.exp
> 

Ah, OK, that makes a lot more sense.  I got confused with the "multi-sim" thing.

Since it's our file, it makes it makes easier.

I guess we should try sticking:

 set_board_info gdb_protocol "sim"

in that file.  Like the patchlet below.  Could you give it a try, please?

From 843a53be479e22926a3dbecc76c5de6160796af4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 9 May 2024 13:01:53 +0100
Subject: [PATCH] gdb_protocol sim

Change-Id: I48a7afed004a3517b90220674fe5bc856fe7d09a
---
 gdb/testsuite/config/sim.exp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/testsuite/config/sim.exp b/gdb/testsuite/config/sim.exp
index 81736d25814..da2780e1d47 100644
--- a/gdb/testsuite/config/sim.exp
+++ b/gdb/testsuite/config/sim.exp
@@ -14,6 +14,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# If we left this empty, the core of the testsuite would think we were
+# connecting to the native target.
+set_board_info gdb_protocol "sim"
+
 #
 # gdb_target_sim
 # Set gdb to target the simulator

base-commit: 5021daf303393722f58f4422d7ad53d526aa2d50
  

On 5/9/24 14:05, Pedro Alves wrote:
> On 2024-05-09 12:54, Bernd Edlinger wrote:
>> On 5/9/24 11:47, Pedro Alves wrote:
>> I think that file would be used when I run the gcc testsuite,
>> there I set the path to my riscv-toolchain,
>> and invoke this in the gcc-build directory:
>> make check -i RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-run"
>> or SIM=qemu-riscv32 for comparison.
>>
>> But if I run the gdb testsuite I can do the same, but the SIM=parameter I could even
>> use SIM=true, is only a dummy, it must be executable, but it is not used in the end.
>> And I believe for gdb the following file is picked up instead:
>>
>> binutils-gdb/gdb/testsuite/config/sim.exp
>>
> 
> Ah, OK, that makes a lot more sense.  I got confused with the "multi-sim" thing.
> 
> Since it's our file, it makes it makes easier.
> 
> I guess we should try sticking:
> 
>  set_board_info gdb_protocol "sim"
> 
> in that file.  Like the patchlet below.  Could you give it a try, please?
> 

Okay sure.

Here are the test results for rv32 and rv64 simulator,
- is from before your patch + with your patch.

rv32 test result differences:
-Test run by ed on Thu May  9 09:07:20 2024
+Test run by ed on Thu May  9 14:46:45 2024
 Target is riscv-unknown-elf
 Host   is x86_64-pc-linux-gnu
 
@@ -20210,7 +20210,11 @@
 PASS: gdb.base/list.exp: list around current line again
 PASS: gdb.base/list.exp: testing repeated invocations with GDB's auto-repeat
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/load-command.exp ...
-UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
+PASS: gdb.base/load-command.exp: check initial value of the_variable
+PASS: gdb.base/load-command.exp: manually change the_variable
+PASS: gdb.base/load-command.exp: check manually changed value of the_variable
+PASS: gdb.base/load-command.exp: reload: re-load binary
+PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/logical.exp ...
 PASS: gdb.base/logical.exp: evaluate x; variables x = 0; expecting 0
 PASS: gdb.base/logical.exp: evaluate !x; variables x = 0; expecting 1
@@ -20950,11 +20954,7 @@
 PASS: gdb.base/many-completions.exp: set max-completions unlimited
 PASS: gdb.base/many-completions.exp: duplicate check
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.exp ...
-gdb compile failed, ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.c:20:10: fatal error: sys/mman.h
: No such file or directory
-   20 | #include <sys/mman.h>
-      |          ^~~~~~~~~~~~
-compilation terminated.
-UNTESTED: gdb.base/many-headers.exp: failed to prepare
+UNSUPPORTED: gdb.base/many-headers.exp: not native
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/max-depth-c++.exp ...
 PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: set print max-depth 0
 PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: p s1
@@ -75717,10 +75717,7 @@
 collect2: error: ld returned 1 exit status
 UNTESTED: gdb.multi/start-inferior-specific.exp: start-inferior-specific.exp
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.exp ...
-gdb compile failed, ..../riscv-unknown-elf/lib/gcc/riscv-unknown-elf/14.1.0/../../../../riscv-unknown-elf/bin/ld: /home/ed/gnu/binutils-build-riscv/gdb/testsuite/outputs/gdb.multi/stop-all-on-exit/stop-all-on-exit0.o: in function `main':
-..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.c:25:(.text+0xe): undefined reference to `sleep'
-collect2: error: ld returned 1 exit status
-UNTESTED: gdb.multi/stop-all-on-exit.exp: failed to prepare
+UNTESTED: gdb.multi/stop-all-on-exit.exp: the test is aimed at a native target
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/tids-gid-reset.exp ...
 PASS: gdb.multi/tids-gid-reset.exp: successfully compiled posix threads test case
 PASS: gdb.multi/tids-gid-reset.exp: single-inferior: before restart: info threads -gid
@@ -76379,23 +76376,23 @@
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/watch-reverse.exp ...
 UNSUPPORTED: gdb.reverse/watch-reverse.exp: require failed: supports_reverse
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp ...
-UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-exec.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-fork.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-multi-inferiors.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/simple.exp ...
-UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/completion.exp ...
 UNSUPPORTED: gdb.rust/completion.exp: require failed: can_compile rust
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/dwindex.exp ...
@@ -77180,7 +77177,6 @@
       |   pthread_atfork
 UNTESTED: gdb.threads/break-while-running.exp: failed to prepare
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/check-libthread-db.exp ...
-UNSUPPORTED: gdb.threads/check-libthread-db.exp: require failed: istarget *-linux*
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-attach-detach.exp ...
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-new-thread-event.exp ...
 Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-thread_db.exp ...
@@ -78630,14 +78626,14 @@
 
                === gdb Summary ===
 
-# of expected passes           71715
+# of expected passes           71720
 # of unexpected failures       921
 # of expected failures         39
 # of unknown successes         4
 # of known failures            66
 # of unresolved testcases      257
-# of untested testcases                297
-# of unsupported tests         1231
+# of untested testcases                296
+# of unsupported tests         1230
 # of paths in test names       4
 # of duplicate test names      4
(END)

and here the rv64 test results:
@@ -1,4 +1,4 @@
-Test run by ed on Thu May  9 09:12:00 2024
+Test run by ed on Thu May  9 14:48:12 2024
 Target is riscv64-unknown-elf
 Host   is x86_64-pc-linux-gnu
 
@@ -20437,7 +20437,11 @@
 PASS: gdb.base/list.exp: list around current line again
 PASS: gdb.base/list.exp: testing repeated invocations with GDB's auto-repeat
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/load-command.exp ...
-UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
+PASS: gdb.base/load-command.exp: check initial value of the_variable
+PASS: gdb.base/load-command.exp: manually change the_variable
+PASS: gdb.base/load-command.exp: check manually changed value of the_variable
+PASS: gdb.base/load-command.exp: reload: re-load binary
+PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/logical.exp ...
 PASS: gdb.base/logical.exp: evaluate x; variables x = 0; expecting 0
 PASS: gdb.base/logical.exp: evaluate !x; variables x = 0; expecting 1
@@ -21177,11 +21181,7 @@
 PASS: gdb.base/many-completions.exp: set max-completions unlimited
 PASS: gdb.base/many-completions.exp: duplicate check
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.exp ...
-gdb compile failed, ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.c:20:10: fatal error: sys/mman.h: No such file or directory
-   20 | #include <sys/mman.h>
-      |          ^~~~~~~~~~~~
-compilation terminated.
-UNTESTED: gdb.base/many-headers.exp: failed to prepare
+UNSUPPORTED: gdb.base/many-headers.exp: not native
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/max-depth-c++.exp ...
 PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: set print max-depth 0
 PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: p s1
@@ -76019,10 +76019,7 @@
 collect2: error: ld returned 1 exit status
 UNTESTED: gdb.multi/start-inferior-specific.exp: start-inferior-specific.exp
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.exp ...
-gdb compile failed, ..../riscv64-unknown-elf/lib/gcc/riscv64-unknown-elf/14.1.0/../../../../riscv64-unknown-elf/bin/ld: ..../binutils-build-riscv64/gdb/testsuite/outputs/gdb.multi/stop-all-on-exit/stop-all-on-exit0.o: in function `main':
-..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.c:25:(.text+0xe): undefined reference to `sleep'
-collect2: error: ld returned 1 exit status
-UNTESTED: gdb.multi/stop-all-on-exit.exp: failed to prepare
+UNTESTED: gdb.multi/stop-all-on-exit.exp: the test is aimed at a native target
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/tids-gid-reset.exp ...
 PASS: gdb.multi/tids-gid-reset.exp: successfully compiled posix threads test case
 PASS: gdb.multi/tids-gid-reset.exp: single-inferior: before restart: info threads -gid
@@ -76681,23 +76678,23 @@
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/watch-reverse.exp ...
 UNSUPPORTED: gdb.reverse/watch-reverse.exp: require failed: supports_reverse
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp ...
-UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-exec.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-fork.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-multi-inferiors.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/simple.exp ...
-UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (remote debugging)
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/completion.exp ...
 UNSUPPORTED: gdb.rust/completion.exp: require failed: can_compile rust
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/dwindex.exp ...
@@ -77482,7 +77479,6 @@
       |   pthread_atfork
 UNTESTED: gdb.threads/break-while-running.exp: failed to prepare
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/check-libthread-db.exp ...
-UNSUPPORTED: gdb.threads/check-libthread-db.exp: require failed: istarget *-linux*
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-attach-detach.exp ...
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-new-thread-event.exp ...
 Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-thread_db.exp ...
@@ -78932,14 +78928,14 @@
 
                === gdb Summary ===
 
-# of expected passes           71993
+# of expected passes           71998
 # of unexpected failures       916
 # of expected failures         41
 # of unknown successes         4
 # of known failures            66
 # of unresolved testcases      257
-# of untested testcases                304
-# of unsupported tests         1226
+# of untested testcases                303
+# of unsupported tests         1225
 # of paths in test names       4
 # of duplicate test names      4
(END)

Hmm, okay, it is better than now, but the test casese that are affected, would probably
be broken by this change, if my target toolchain would either have the -linux in the name,
or the newlib would have a sleep and/or support the #include <sys/mman.h>.


Thanks
Bernd.
  

On 2024-05-09 14:19, Bernd Edlinger wrote:
> Hmm, okay, it is better than now, 

Indeed, seems strictly better.  Some tests that are meant for the native target are now properly
skipped.

> but the test casese that are affected, would probably
> be broken by this change, if my target toolchain would either have the -linux in the name,
> or the newlib would have a sleep and/or support the #include <sys/mman.h>.

I am afraid I don't know what you mean by this.  Why would they be broken?  Can you clarify?
  

On 5/9/24 15:31, Pedro Alves wrote:
> 
> On 2024-05-09 14:19, Bernd Edlinger wrote:
>> Hmm, okay, it is better than now, 
> 
> Indeed, seems strictly better.  Some tests that are meant for the native target are now properly
> skipped.
> 
>> but the test casese that are affected, would probably
>> be broken by this change, if my target toolchain would either have the -linux in the name,
>> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
> 
> I am afraid I don't know what you mean by this.  Why would they be broken?  Can you clarify?

What I mean is this:

there are in total 44 test cases failed to compile because of undefined reference to sleep, usleep or nanosleep
with your patch it is now one less.

But it would be piece of cake to implement some functions like
sleep(), usleep(), and nanosleep in the newlib, and then make the simulator simulate it.

Likewise there are currently 5 test cases failed to compile because of missing sys/mman.h header,
and with your patch it will be one less, but what if we want to add that to the
simulator, why cant we simulate a linux O/S?

What is so special in the one test case that changed the behavior, that
explains why the other 4 are good enough to try to include, and see if <sys/mman.h> works?


Bernd.
  

On 2024-05-09 16:01, Bernd Edlinger wrote:
> On 5/9/24 15:31, Pedro Alves wrote:
>>
>> On 2024-05-09 14:19, Bernd Edlinger wrote:
>>> Hmm, okay, it is better than now, 
>>
>> Indeed, seems strictly better.  Some tests that are meant for the native target are now properly
>> skipped.
>>
>>> but the test casese that are affected, would probably
>>> be broken by this change, if my target toolchain would either have the -linux in the name,
>>> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
>>
>> I am afraid I don't know what you mean by this.  Why would they be broken?  Can you clarify?
> 
> What I mean is this:
> 
> there are in total 44 test cases failed to compile because of undefined reference to sleep, usleep or nanosleep
> with your patch it is now one less.
> 
> But it would be piece of cake to implement some functions like
> sleep(), usleep(), and nanosleep in the newlib, and then make the simulator simulate it.
> 
> Likewise there are currently 5 test cases failed to compile because of missing sys/mman.h header,
> and with your patch it will be one less, but what if we want to add that to the
> simulator, why cant we simulate a linux O/S?
> 
> What is so special in the one test case that changed the behavior, that
> explains why the other 4 are good enough to try to include, and see if <sys/mman.h> works?
> 

The gdb.base/load-command.exp testcase just doesn't make sense to test with the native
target, since with the native target, you don't use the "load" command to load programs.
So the testcase checks for that:

if [gdb_protocol_is_native] {
    unsupported "the native target does not support the load command"
    return
}

Normally, a testcase that checks whether it is testing with the native target does so because it is
testing some feature of the native target (the ptrace support, etc.), and so such an early check
isn't that much about whether sleep or sys/mman.h exists (they don't exist on native Windows,
for example, and there are native-only tests that you'd still want to run there).

If a testcase should run against the sim target and would no longer run because we set
gdb_protocol, then that would be a problem with the testcase itself, for using the wrong
predicate to skip itself.  We've had plenty of cases of testcases using the wrong
predicate over the years, that is something that we're fixing pretty frequently, so
please don't be surprised if we need to do that after this change too.

Looking at gdb.base/many-headers.exp for example (the first in your previous message
that shows compile errors), it has:

if { [target_info gdb_protocol] != "" } {
    # Even though the feature under features being tested are supported by
    # gdbserver, the way this test is written doesn't make it easy with a
    # remote target.
    unsupported "not native"
    return
}

Note the comment -- it is saying to skip testing on remote, but then it skips
on any board that sets gdb_protocol to anything (meaning, not native).
If this truly should only be skipped with remote targets, then it should instead
do this:

require gdb_protocol_is_remote

which is equivalent to:

  if { [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote" } {
     return
  }

But, actually looking at the testcase, we see that it launches a program outside gdb, so that
the program crashes and generates a core (core_find), and then debugs the core under gdb,
with GDB stack limited to 4MB.  From the commit's log (31aceee86308), it's the equivalent
of doing:

    $ ulimit -s 4096
    $ gdb -batch ./a.out core.saved
    [New LWP 19379]
    Segmentation fault (core dumped)
    ...

So, I don't really understand what the "features being tested are supported by gdbserver"
remark is alluding to, since gdbserver does not support loading cores.  But then again,
the test doesn't even connect to the native target nor the remote target anyhow.  So
we could let the testcase run when testing against a gdbserver board anyhow.

Maybe what the comment is trying to say is, not easy to test with a "remote target"
in dejagnu sense, i.e., when you have to connect to a remote host board running on a
separate system, as in that scenario it's not easy to set ulimit.  Is so, a better
predicate to check would be "require !is_remote host" or "require isnative".

A similar analysis would have to be done to the other testcases.
  

On 5/9/24 17:49, Pedro Alves wrote:
> On 2024-05-09 16:01, Bernd Edlinger wrote:
>> On 5/9/24 15:31, Pedro Alves wrote:
>>>
>>> On 2024-05-09 14:19, Bernd Edlinger wrote:
>>>> Hmm, okay, it is better than now, 
>>>
>>> Indeed, seems strictly better.  Some tests that are meant for the native target are now properly
>>> skipped.
>>>
>>>> but the test casese that are affected, would probably
>>>> be broken by this change, if my target toolchain would either have the -linux in the name,
>>>> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
>>>
>>> I am afraid I don't know what you mean by this.  Why would they be broken?  Can you clarify?
>>
>> What I mean is this:
>>
>> there are in total 44 test cases failed to compile because of undefined reference to sleep, usleep or nanosleep
>> with your patch it is now one less.
>>
>> But it would be piece of cake to implement some functions like
>> sleep(), usleep(), and nanosleep in the newlib, and then make the simulator simulate it.
>>
>> Likewise there are currently 5 test cases failed to compile because of missing sys/mman.h header,
>> and with your patch it will be one less, but what if we want to add that to the
>> simulator, why cant we simulate a linux O/S?
>>
>> What is so special in the one test case that changed the behavior, that
>> explains why the other 4 are good enough to try to include, and see if <sys/mman.h> works?
>>
> 
> The gdb.base/load-command.exp testcase just doesn't make sense to test with the native
> target, since with the native target, you don't use the "load" command to load programs.
> So the testcase checks for that:
> 
> if [gdb_protocol_is_native] {
>     unsupported "the native target does not support the load command"
>     return
> }
> 
> Normally, a testcase that checks whether it is testing with the native target does so because it is
> testing some feature of the native target (the ptrace support, etc.), and so such an early check
> isn't that much about whether sleep or sys/mman.h exists (they don't exist on native Windows,
> for example, and there are native-only tests that you'd still want to run there).
> 
> If a testcase should run against the sim target and would no longer run because we set
> gdb_protocol, then that would be a problem with the testcase itself, for using the wrong
> predicate to skip itself.  We've had plenty of cases of testcases using the wrong
> predicate over the years, that is something that we're fixing pretty frequently, so
> please don't be surprised if we need to do that after this change too.
> 
> Looking at gdb.base/many-headers.exp for example (the first in your previous message
> that shows compile errors), it has:
> 
> if { [target_info gdb_protocol] != "" } {
>     # Even though the feature under features being tested are supported by
>     # gdbserver, the way this test is written doesn't make it easy with a
>     # remote target.
>     unsupported "not native"
>     return
> }
> 
> Note the comment -- it is saying to skip testing on remote, but then it skips
> on any board that sets gdb_protocol to anything (meaning, not native).
> If this truly should only be skipped with remote targets, then it should instead
> do this:
> 
> require gdb_protocol_is_remote
> 
> which is equivalent to:
> 
>   if { [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote" } {
>      return
>   }
> 
> But, actually looking at the testcase, we see that it launches a program outside gdb, so that
> the program crashes and generates a core (core_find), and then debugs the core under gdb,
> with GDB stack limited to 4MB.  From the commit's log (31aceee86308), it's the equivalent
> of doing:
> 
>     $ ulimit -s 4096
>     $ gdb -batch ./a.out core.saved
>     [New LWP 19379]
>     Segmentation fault (core dumped)
>     ...
> 
> So, I don't really understand what the "features being tested are supported by gdbserver"
> remark is alluding to, since gdbserver does not support loading cores.  But then again,
> the test doesn't even connect to the native target nor the remote target anyhow.  So
> we could let the testcase run when testing against a gdbserver board anyhow.
> 
> Maybe what the comment is trying to say is, not easy to test with a "remote target"
> in dejagnu sense, i.e., when you have to connect to a remote host board running on a
> separate system, as in that scenario it's not easy to set ulimit.  Is so, a better
> predicate to check would be "require !is_remote host" or "require isnative".
> 
> A similar analysis would have to be done to the other testcases.

Ah, okay, thanks for the explanation.

I was probably confused that sim looks an feels like a kind of native target,
but it supports the load command, and the test cases can read and write
local files, etc.  Therefore it is of course easy to fix things in the wrong way.
So I agree with your patch, and that any fallout can be fixed later
in the test cases when necessary.


Thanks
Bernd.