[PATCH 0/4] Re: gdb/riscv: Add read_description method for riscv_linux_nat_target

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Jim Wilson <jimw@sifive.com> [2018-11-28 18:05:21 -0800]:

> On Wed, Nov 28, 2018 at 2:49 PM Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> > Adds riscv_linux_nat_target::read_description method to find a
> > suitable target description for the native linux target we are running
> > on.
> 
> I tested this on my HiFive Unleashed running a 4.15 kernel with
> patches for gcc, gdb, and glibc support.  It looks good.  It correctly
> detects the FP register support.  I get 3 extra failures
> FAIL: gdb.base/solib-display.exp: NO: continue
> FAIL: gdb.base/solib-display.exp: IN: continue
> FAIL: gdb.base/solib-display.exp: SEP: continue
> which looks a little odd since there is no obvious connection to the
> target description support, but this is repeatable so something is
> going on here.  Anyways, I'm OK with this for now, we can worry about
> debugging this problem later.

Thanks for looking at this Jim.

I dug a little deeper, and now understand what is going on here.  We
(or I) currently create a new target description on every call into
`riscv_create_target_description', I (incorrectly) thought that
`riscv_gdbarch_init' would sort it all out.  However, it turns out GDB
relies on identical target descriptions being the exact same object.

This new set of patches does a little prep-work, and then adds a cache
so that calls to `riscv_create_target_description' with the same
feature set will return the exact same target description object.

With this done, the tests listed above now pass.

But why I hear you ask?

Well, in the test we we set up a 'display' of a function local static
variable.  When the 'display' is setup GDB evaluates the expression
and captures the block and symbol.  Then, later, when we stop we can
display the value of the symbol even though (technically) its out of
scope (as we have the block cached).

In the test in question we actually set up the display, and the
restart GDB, however, we still manage to keep displaying the static
variable as we have the block cached, and (I guess) as we are running
the same program everything is still valid (I hope).

Anyway, GDB does have one safety check built in - has the gdbarch
changed.  If it has then GDB ditches the cached block and symbol, and
tries to reevaluate the 'display' string.  However, when this happens
the function local static variable is out of scope, and the display
expression gets deleted.

Conclusion, we need to make sure we don't create new gdbarch objects
when we don't need to (well obviously) and to do this we need to make
sure that we reuse target descriptions.

This is still running through testing at my end, so far its looking
good, but I thought I'd share in case you also wanted to test.

Thanks,
Andrew


---

Andrew Burgess (4):
  gdb/riscv: Make some target description functions constant
  gdb/riscv: Add equality operators to riscv_gdb_features
  gdb/riscv: Create each unique target description only once
  gdb/riscv: Add read_description method for riscv_linux_nat_target

 gdb/ChangeLog         | 27 +++++++++++++++++++++++
 gdb/arch/riscv.c      | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/arch/riscv.h      | 15 ++++++++++++-
 gdb/riscv-linux-nat.c | 38 +++++++++++++++++++++++++++++++++
 gdb/riscv-tdep.c      |  6 ++----
 5 files changed, 139 insertions(+), 6 deletions(-)

-- 
2.14.5