[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    Re: [PATCHv3 6/6] gdb: native target invalid architecture detection
From:       Pedro Alves <pedro () palves ! net>
Date:       2022-06-30 11:44:20
Message-ID: 0b9bee5a-d2c6-33f6-c3b9-67b9457add22 () palves ! net
[Download RAW message or body]

On 2022-06-30 10:33, Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
> 
> > On 2022-06-27 17:27, Andrew Burgess wrote:
> > > Pedro Alves <pedro@palves.net> writes:
> > > > On 2022-06-13 17:15, Andrew Burgess via Gdb-patches wrote:
> > > > I have another question.  I'm confused on how if e.g., on x86-64, you
> > > > load a RISC-V binary into GDB, and the try to run it, we don't end up
> > > > with architecture of the target description instead.
> > > 
> > > The problem is that we end up trying to read the registers before we
> > > read the target description.  Which I suspect you will say sounds wrong,
> > > and I agree.
> > > 

...

> > Attaching will need some other fix, because in that case, the procedure is that \
> > the core requests an attach, and then the target reports an initial stop, and \
> > that stop makes infrun read the thread's PC, before setup_inferior is called.  It \
> > may be we need to call setup_inferior earlier.  Or maybe we can just add a \
> > target_find_description call in linux_nat_target::attach, like remote.c does:
> > 
> > void
> > extended_remote_target::attach (const char *args, int from_tty)
> > {
> > ...
> > /* Next, if the target can specify a description, read it.  We do
> > this before anything involving memory or registers.  */
> > target_find_description ();
> > 
> > I haven't looked at that in much detail, and I have to leave for tonight.
> 
> How about the patch below to solve the attach case?

I think this should be two patches.  The part about picking the architecture
of the target isn't specific to attaching.  We're counting on it for "run" too.

> No test as I don't know how I can reliably compile a binary for a
> different architecture as part of the testsuite.

We don't even need a binary to trigger badness.  Just need to manually
specify an incompatible architecture, with "set architecture", and then "run".
Like, with current master:

$ gdb -ex "set architecture riscv" ~/gdb/tests/main
GNU gdb (GDB) 13.0.50.20220630-git
...
Reading symbols from /home/pedro/gdb/tests/main...
The target architecture is set to "riscv".
(gdb) r
Starting program: /pedro/gdb/tests/main 
warning: Selected architecture riscv is not compatible with reported target \
                architecture i386:x86-64
warning: Architecture rejected target-supplied description
../../src/gdb/i387-tdep.c:957: internal-error: i387_supply_xsave: Assertion \
`tdep->st0_regnum >= I386_ST0_REGNUM' failed. A problem internal to GDB has been \
detected, further debugging may prove unreliable.
----- Backtrace -----
0x556325d5365b gdb_internal_backtrace_1
        ../../src/gdb/bt-utils.c:122
0x556325d53714 _Z22gdb_internal_backtracev
        ../../src/gdb/bt-utils.c:168
...

A slight difference here compared to actually having a binary of the wrong arch,
is that this isn't "set architecture auto", so by making \
choose_architecture_for_target chose the target architecture, we'd be overriding the \
explicit user selection.  Maybe that's OK, not sure.  Maybe the right thing to do in \
that case (when arch is not auto) is to error out (and kill/detach the inferior) \
instead of warning.


[prev in list] [next in list] [prev in thread] [next in thread]