Re: [RFA/RFT PATCH 0/3] Add TDB regset support

Pedro Alves
2013-06-07T18:58:21Z

On 06/07/2013 07:15 PM, Andreas Arnez wrote:
> Pedro Alves <palves@redhat.com> writes:
>> You might not need to look at the auxv. Return the right tdesc
>> depending on presence of bfd sections. See ppc_linux_core_read_description.
>
> This doesn't work for the TDB, because the absence of the TDB register
> section doesn't mean that the core file target didn't support it. At
> least we'd break the gcore.exp test case.

Okay. How are the TDB registers presented to the user (info all-registers,
etc) in the current patchset when the target supports them, but the inferior
is presently interrupted outside a transaction, with live debugging?
(screenshot, please! :-))

>
>> If you connect a 7.6 GDB without this patch to a new GDBserver that
>> sends this extended regdat, I believe GDB will complain with
>> "Remote 'g' packet reply is too long", even if the target actually
>> doesn't support TDB.
>
> Hm, didn't realize that. But if the target does support TDB, we'd be in
> trouble either way, right?

Right.

>> You're extending the remote register buffer with an optional feature,
>> and in future when another alternative register set comes along that
>> replaces this one, we're in trouble, as we won't be able to tell
>> which is which (without ugly hacks).
>
> Well, the patch doesn't treat TDB as an optional feature, does it?

+ /* Transaction diagnostic block. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.tdb");
+ if (feature)

That makes it optional (as it should).

+ {
+ for (i = 0; i < ARRAY_SIZE (tdb_regs); i++)
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ S390_TDB_DWORD0_REGNUM + i,
+ tdb_regs[i]);
+ }
+

The new org.gnu.gdb.s390.tdb target feature needs to be documented, btw.

http://sourceware.org/gdb/wiki/ContributionChecklist#Documentation

TDB support should be mentioned in NEWS too.

> In the future the TDB would always be included in the target description.

That's really wrong. No reason for such a hack when we have a
proper mechanism in place.

> Actually representing n optional register sets with different tdescs
> would require 2**n tdescs, right?

With xi:include, that's really pretty manageable. Please do
take a look at ppc_linux_read_description and at features/rs6000/,
for example, though there are several others in the tree.
The x86 port has a complex selection of proper description/register
set too, given xsave.

--
Pedro Alves