This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 8/8] Ptrace support for Aarch64 SVE
- From: Alan Hayward <Alan dot Hayward at arm dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, nd <nd at arm dot com>
- Date: Mon, 4 Jun 2018 15:49:33 +0000
- Subject: Re: [PATCH 8/8] Ptrace support for Aarch64 SVE
- Nodisclaimer: True
- References: <20180511105256.27388-1-alan.hayward@arm.com> <20180511105256.27388-9-alan.hayward@arm.com> <752617dd-3221-7aa7-d626-b841fe13761c@ericsson.com> <22AC70D2-D24D-4DE9-939F-067BE65F02F3@arm.com> <0ff06fe2-396d-a9da-d95b-130f6bef2d2c@ericsson.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
>>>> diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
>>>> index 9381786fda..84c7a41f40 100644
>>>> --- a/gdb/nat/aarch64-sve-linux-ptrace.c
>>>> +++ b/gdb/nat/aarch64-sve-linux-ptrace.c
>>>> @@ -25,6 +25,13 @@
>>>> #include "aarch64-sve-linux-ptrace.h"
>>>> #include "arch/aarch64.h"
>>>>
>>>> +#ifndef GDBSERVER
>>>> +#include "defs.h"
>>>> +#endif
>>>> +#include "regcache.h"
>>>
>>> Hmm we try not add any more "#ifdef GDBSERVER" in the common code.
>>>
>>> https://sourceware.org/gdb/wiki/Common#Header_files_in_common_code_.28defs.h_vs_server.h.2C_etc..29
>>>
>>> Instead, we should try defining a common interface (probably in common/common-regcache.h?) that the
>>> common code will use, and that regcaches from GDB and GDBserver will implement.
>>
>> I tried using common-defs.h, but gdb/regcache.h requires defines from
>> defs.h - RequireLongest and maybe others.
>> Putting defs.h at the top of gdb/regcache.h then broke in a weird way.
>> A lot of fiddling later, and I hadn’t found a way to make it work.
>>
>> Creating common/common-regcache.h gets a bit odd because, the functions
>> I need for gdbserver (raw_supply, raw_collect and get_register_status)
>> on gdb come from:
>>
>>
>> class reg_buffer
>> ...
>> enum register_status get_register_status (int regnum) const;
>> ...
>>
>>
>> class readable_regcache : public reg_buffer
>> ...
>>
>>
>> class detached_regcache : public readable_regcache
>> ...
>> void raw_supply (int regnum, const void *buf);
>> ...
>>
>>
>> class regcache : public detached_regcache
>> ...
>> void raw_collect (int regnum, void *buf) const;
>> ...
>>
>>
>> I don’t think that this would work:
>> class regcache : public detached_regcache, common_regcache
>
> I did some quick hacking and it seems to work to have this in common/common-regcache.h
>
> struct reg_buffer_common
> {
> virtual ~reg_buffer_common () = default;
> virtual void raw_supply (int regnum, const void *buf) = 0;
> virtual void raw_collect (int regnum, void *buf) const = 0;
> virtual bool raw_compare (int regnum, const void *buf, int offset) const = 0;
> virtual register_status get_register_status (int regnum) const = 0;
> };
>
> and make your code in nat/ take a pointer to reg_buffer_common. gdb's reg_buffer
> and gdbserver's regcache should inherit from reg_buffer_common. I built
> it on other regcache refactor patches I have in the pipeline, so maybe some other
> changes in there are needed, maybe not. I pushed the result on this branch, it's
> a bit raw but it should be enough to show the idea.
>
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/simark/regcache-for-alan
Thanks for trying this out, however I tried adding your changes
into my build, but that results in the following:
../../src/binutils-gdb/gdb/linux-fork.c: In function ‘void fork_save_infrun_state(fork_info*, int)’:
../../src/binutils-gdb/gdb/linux-fork.c:298:75: error: invalid new-expression of abstract class type ‘readonly_detached_regcache’
fp->savedregs = new readonly_detached_regcache (*get_current_regcache ());
^
In file included from ../../src/binutils-gdb/gdb/gdbarch.h:70:0,
from ../../src/binutils-gdb/gdb/defs.h:531,
from ../../src/binutils-gdb/gdb/linux-fork.c:20:
../../src/binutils-gdb/gdb/regcache.h:367:7: note: because the following virtual functions are pure within ‘readonly_detached_regcache’:
class readonly_detached_regcache : public readable_regcache
^
In file included from ../../src/binutils-gdb/gdb/regcache.h:23:0,
from ../../src/binutils-gdb/gdb/gdbarch.h:70,
from ../../src/binutils-gdb/gdb/defs.h:531,
from ../../src/binutils-gdb/gdb/linux-fork.c:20:
../../src/binutils-gdb/gdb/common/common-regcache.h:68:16: note: virtual void reg_buffer_common::raw_supply(int, const void*)
virtual void raw_supply (int regnum, const void *buf) = 0;
^
../../src/binutils-gdb/gdb/common/common-regcache.h:69:16: note: virtual void reg_buffer_common::raw_collect(int, void*) const
virtual void raw_collect (int regnum, void *buf) const = 0;
And then similar errors trying to create a register_dump_reg_buffer.
I could then add the following to readonly_detached_regcache:
void raw_supply (int regnum, const void *buf) override
{ gdb_assert(false); }
void raw_collect (int regnum, void *buf) const override
{ gdb_assert(false); }
...Or even make the methods in reg_buffer_common have a
{ gdb_assert(false); } body.
But that feels wrong, as it’s breaking the nice regcache abstractions.
Any thoughts?
Alan.