[PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file

[Andrew Burgess]

* Luis Machado <luis.machado@linaro.org> [2020-12-02 15:21:49 -0300]:

> On 12/2/20 2:39 PM, Andrew Burgess wrote:
> > This commit lays the ground work for allowing GDB to write its target
> > description into a generated core file.
> > 
> > The goal of this work is to allow a user to connect to a remote
> > target, capture a core file from within GDB, then pass the executable
> > and core file to another user and have the user be able to examine the
> > state of the machine without needing to connect to a running target.
> > 
> > Different remote targets can have different register sets and this
> > information is communicated from the target to GDB in the target
> > description.
> > 
> > It is possible for a user to extract the target description from GDB
> > and pass this along with the core file so that when the core file is
> > used the target description can be fed back into GDB, however this is
> > not a great user experience.
> > 
> > It would be nicer, I think, if GDB could write the target description
> > directly into the core file, and then make use of this description
> > when loading a core file.
> > 
> > This commit performs the binutils/bfd side of this task, adding the
> > boiler plate functions to access the target description from within a
> > core file note, and reserving a new number for a note containing the
> > target description.
> > 
> > Later commits will extend GDB to make use of this.
> > 
> > bfd/ChangeLog:
> > 
> > 	* elf-bfd.h (elfcore_write_gdb_tdesc): Declare new function.
> > 	* elf.c (elfcore_grok_gdb_tdesc): New function.
> > 	(elfcore_grok_note): Handle NT_GDB_TDESC.
> > 	(elfcore_write_gdb_tdesc): New function.
> > 	(elfcore_write_register_note): Handle NT_GDB_TDESC.
> > 
> > binutils/ChangeLog:
> > 
> > 	* readelf.c (get_note_type): Handle NT_GDB_TDESC.
> > 
> > include/ChangeLog:
> > 
> > 	* elf/common.h (NT_GDB_TDESC): Define.
> > ---
> >   bfd/ChangeLog        |  9 +++++++++
> >   bfd/elf-bfd.h        |  2 ++
> >   bfd/elf.c            | 23 +++++++++++++++++++++++
> >   binutils/ChangeLog   |  5 +++++
> >   binutils/readelf.c   |  2 ++
> >   include/ChangeLog    |  5 +++++
> >   include/elf/common.h |  2 ++
> >   7 files changed, 48 insertions(+)
> > 
> > diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> > index e9c890f6f16..5ef69ab5b13 100644
> > --- a/bfd/elf-bfd.h
> > +++ b/bfd/elf-bfd.h
> > @@ -2796,6 +2796,8 @@ extern char *elfcore_write_aarch_pauth
> >     (bfd *, char *, int *, const void *, int);
> >   extern char *elfcore_write_arc_v2
> >     (bfd *, char *, int *, const void *, int);
> > +extern char *elfcore_write_gdb_tdesc
> > +  (bfd *, char *, int *, const void *, int);
> >   extern char *elfcore_write_lwpstatus
> >     (bfd *, char *, int *, long, int, const void *);
> >   extern char *elfcore_write_register_note
> > diff --git a/bfd/elf.c b/bfd/elf.c
> > index 419c5f4420c..bea5ab12773 100644
> > --- a/bfd/elf.c
> > +++ b/bfd/elf.c
> > @@ -9909,6 +9909,12 @@ elfcore_grok_arc_v2 (bfd *abfd, Elf_Internal_Note *note)
> >     return elfcore_make_note_pseudosection (abfd, ".reg-arc-v2", note);
> >   }
> > +static bfd_boolean
> > +elfcore_grok_gdb_tdesc (bfd *abfd, Elf_Internal_Note *note)
> > +{
> > +  return elfcore_make_note_pseudosection (abfd, ".gdb-tdesc", note);
> > +}
> > +
> >   #if defined (HAVE_PRPSINFO_T)
> >   typedef prpsinfo_t   elfcore_psinfo_t;
> >   #if defined (HAVE_PRPSINFO32_T)		/* Sparc64 cross Sparc32 */
> > @@ -10566,6 +10572,9 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
> >         else
> >   	return TRUE;
> > +    case NT_GDB_TDESC:
> > +      return elfcore_grok_gdb_tdesc (abfd, note);
> > +
> >       case NT_PRPSINFO:
> >       case NT_PSINFO:
> >         if (bed->elf_backend_grok_psinfo)
> > @@ -11947,6 +11956,18 @@ elfcore_write_arc_v2 (bfd *abfd,
> >   			     note_name, NT_ARC_V2, arc_v2, size);
> >   }
> > +char *
> > +elfcore_write_gdb_tdesc (bfd *abfd,
> > +			 char *buf,
> > +			 int *bufsiz,
> > +			 const void *tdesc,
> > +			 int size)
> > +{
> > +  const char *note_name = "CORE";
> > +  return elfcore_write_note (abfd, buf, bufsiz,
> > +                             note_name, NT_GDB_TDESC, tdesc, size);
> > +}
> > +
> >   char *
> >   elfcore_write_register_note (bfd *abfd,
> >   			     char *buf,
> > @@ -12031,6 +12052,8 @@ elfcore_write_register_note (bfd *abfd,
> >       return elfcore_write_aarch_pauth (abfd, buf, bufsiz, data, size);
> >     if (strcmp (section, ".reg-arc-v2") == 0)
> >       return elfcore_write_arc_v2 (abfd, buf, bufsiz, data, size);
> > +  if (strcmp (section, ".gdb-tdesc") == 0)
> > +    return elfcore_write_gdb_tdesc (abfd, buf, bufsiz, data, size);
> >     return NULL;
> >   }
> > diff --git a/binutils/readelf.c b/binutils/readelf.c
> > index 57e0f1de459..5b3871d3e5f 100644
> > --- a/binutils/readelf.c
> > +++ b/binutils/readelf.c
> > @@ -18246,6 +18246,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
> >   	return _("NT_PRPSINFO (prpsinfo structure)");
> >         case NT_TASKSTRUCT:
> >   	return _("NT_TASKSTRUCT (task structure)");
> > +      case NT_GDB_TDESC:
> > +        return _("NT_GDB_TDESC (GDB XML target description)");
> >         case NT_PRXFPREG:
> >   	return _("NT_PRXFPREG (user_xfpregs structure)");
> >         case NT_PPC_VMX:
> > diff --git a/include/elf/common.h b/include/elf/common.h
> > index 95a852f0cf5..1dbf0b11983 100644
> > --- a/include/elf/common.h
> > +++ b/include/elf/common.h
> > @@ -666,6 +666,8 @@
> >   #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
> >   #define NT_FILE		0x46494c45	/* Description of mapped files.  */
> > +#define NT_GDB_TDESC	0x54444553	/* Contains copy of GDB's target description XML.  */
> > +
> 
> How about a generic name without the tool's name on it? Other tools may want
> to use it as well, and that can be a little confusing. NT_XML_TDESC,
> maybe?

I originally did name the note NT_XML_TDESC, however, I moved away
from this because NT_GDB_TDESC describes what this actually is, and I
don't think switching back is a good choice.

If some other tool can process these descriptions that this is because
that tool has made the choice to accept GDB format target
descriptions, so I don't think having GDB in the name would be a
problem.

Further, if some other tool comes up with its own target description
specification then it's quite possible that it would also use XML then
we have to find a new name that (a) doesn't include the tool name, but
(b) doesn't clash with NT_XML_TDESC.

> 
> For the constant, I find the magic number amusing, but I don't think it does
> any good when you're trying to find out why that particular magic number was
> picked, and what the next number should be.
> 
> Should we go with the basic increasing ID instead? I think this would be 7,
> right after NT_AUXV.

The number certainly wasn't picked for amusement.

I didn't pick 7 for fear of clashing with a number that someone else
might already be using, if I understand Jim correctly then it seems
this was a good call.

The ASCII encoded string provided a very semi random large number
that was unlikely to clash, and was inline with how at least some
other notes were numbered.

I really have no strong feelings one way or the other on what the note
number should be, if someone wants to propose a scheme, then I'm more
than happy to adopt that.

I'll wait to see how the numbering conversation pans out and adapt
this patch to fit in.

Thanks,
Andrew