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

[Pedro Alves <palves at redhat dot com>]

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