public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: binutils@sourceware.org
Subject: [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful
Date: Mon, 11 Oct 2021 20:18:07 +0100	[thread overview]
Message-ID: <20211011191810.274535-1-nick.alcock@oracle.com> (raw)

This option has been present since the very early days of the
development of libctf as part of binutils, and it shows.  Back in the
earliest days, I thought we might handle ambiguous types by introducing
new ELF sections on the fly named things like .ctf.foo.c for ambiguous
types found only in foo.c, etc.  This turned out to be a terrible idea,
so we moved to using a CTF archive in the .ctf section which contained
all the CTF dictionaries -- but the --ctf-parent option in objdump and
readelf was never adjusted, and lingered as a mechanism to specify CTF
parent dictionaries in sections other than .ctf, even though the linker
has no way to produce parent dictionaries in different sections from
their children, libctf's ctf_open can't handle such split-up
parent/child dicts, and they are never found in the wild, emitted by GNU
ld or by any known third-party linking tool.

Meanwhile, the actually-useful ctf_link feature (albeit not used by ld)
which lets you remap the names of CTF archive members (so you can end up
with a parent archive member named something other than ".ctf", still
contained with all its children in a single .ctf section) had no support
in objdump or readelf: there was no way to tell them that these members
were parents, so all the types in the associated child dicts always
appeared corrupted, referencing nonexistent types from a parent objdump
couldn't find.

So adjust --ctf-parent so that rather than taking a section name it
takes a member name instead (if not specified, the name is ".ctf", which
is what GNU ld emits).  Because the option was always useless before
now, this is expected to have no backward-compatibility implications.

binutils/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* objdump.c (usage): --ctf-parent now takes a name, not a section.
	(dump_ctf): Don't open a separate section; use the parent_name in
	ctf_dict_open instead.
	(dump_ctf_archive_member): Import parents into children no matter
	what the parent's name, while still avoiding displaying the header
	for the common parent name of ".ctf".
	* readelf.c (usage): Adjust similarly.
	(dump_ctf_archive_member): Likewise.
	(dump_section_as_ctf): Likewise.
	* doc/ctf.options.texi: Adjust.
---
 binutils/doc/ctf.options.texi | 15 ++++++---
 binutils/objdump.c            | 52 ++++++++----------------------
 binutils/readelf.c            | 60 ++++++-----------------------------
 3 files changed, 34 insertions(+), 93 deletions(-)

diff --git a/binutils/doc/ctf.options.texi b/binutils/doc/ctf.options.texi
index bb9946a8008..34451f9221a 100644
--- a/binutils/doc/ctf.options.texi
+++ b/binutils/doc/ctf.options.texi
@@ -8,8 +8,15 @@
 Display the contents of the specified CTF section.  CTF sections themselves
 contain many subsections, all of which are displayed in order.
 
-@item --ctf-parent=@var{section}
 
-Specify the name of another section from which the CTF dictionary can inherit
-types.  (If none is specified, we assume the CTF dictionary inherits types
-from the default-named member of the archive contained within this section.)
+@item --ctf-parent=@var{member}
+
+If the CTF section contains ambiguously-defined types, it will consist
+of an archive of many CTF dictionaries, all inheriting from one
+dictionary containing unambiguous types.  This member is by default
+named @var{.ctf}, like the section containing it, but it is possible to
+change this name using the @code{ctf_link_set_memb_name_changer}
+function at link time.  When looking at CTF archives that have been
+created by a linker that uses the name changer to rename the parent
+archive member, @option{--ctf-parent} can be used to specify the name
+used for the parent.
diff --git a/binutils/objdump.c b/binutils/objdump.c
index fdffb8d5263..2edf0a31478 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -361,7 +361,7 @@ usage (FILE *stream, int status)
       --dwarf-check              Make additional dwarf consistency checks.\n"));
 #ifdef ENABLE_LIBCTF
       fprintf (stream, _("\
-      --ctf-parent=SECTION       Use SECTION as the CTF parent\n"));
+      --ctf-parent=NAME          Use CTF archive member NAME as the CTF parent\n"));
 #endif
       fprintf (stream, _("\
       --visualize-jumps          Visualize jumps by drawing ASCII art lines\n"));
@@ -4168,15 +4168,12 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 
   /* Only print out the name of non-default-named archive members.
      The name .ctf appears everywhere, even for things that aren't
-     really archives, so printing it out is liable to be confusing.
+     really archives, so printing it out is liable to be confusing.  */
 
-     The parent, if there is one, is the default-owned archive member:
-     avoid importing it into itself.  (This does no harm, but looks
-     confusing.)  */
-
-  if (strcmp (name, ".ctf") != 0)
+  if (ctf_parent_name (ctf) != NULL)
     {
-      printf (_("\nCTF archive member: %s:\n"), sanitize_string (name));
+      if (strcmp (name, ".ctf") != 0)
+	printf (_("\nCTF archive member: %s:\n"), sanitize_string (name));
       ctf_import (ctf, parent);
     }
 
@@ -4211,22 +4208,19 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 static void
 dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
 {
-  ctf_archive_t *ctfa, *parenta = NULL, *lookparent;
-  bfd_byte *ctfdata, *parentdata = NULL;
-  bfd_size_type ctfsize, parentsize;
+  ctf_archive_t *ctfa = NULL;
+  bfd_byte *ctfdata = NULL;
+  bfd_size_type ctfsize;
   ctf_sect_t ctfsect;
-  ctf_dict_t *parent = NULL;
+  ctf_dict_t *parent;
   int err;
 
-  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
-      bfd_fatal (bfd_get_filename (abfd));
 
-  if (parent_name
-      && (parentdata = read_section_stabs (abfd, parent_name, &parentsize,
-					   NULL)) == NULL)
+  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
       bfd_fatal (bfd_get_filename (abfd));
 
-  /* Load the CTF file and dump it.  */
+  /* Load the CTF file and dump it.  Preload the parent dict, since it will
+     need to be imported into every child in turn. */
 
   ctfsect = make_ctfsect (sect_name, ctfdata, ctfsize);
   if ((ctfa = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
@@ -4236,25 +4230,7 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
       bfd_fatal (bfd_get_filename (abfd));
     }
 
-  if (parentdata)
-    {
-      ctfsect = make_ctfsect (parent_name, parentdata, parentsize);
-      if ((parenta = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
-	{
-	  dump_ctf_errs (NULL);
-	  non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
-	  bfd_fatal (bfd_get_filename (abfd));
-	}
-
-      lookparent = parenta;
-    }
-  else
-    lookparent = ctfa;
-
-  /* Assume that the applicable parent archive member is the default one.
-     (This is what all known implementations are expected to do, if they
-     put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
+  if ((parent = ctf_dict_open (ctfa, parent_name, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
@@ -4271,8 +4247,6 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
     }
   ctf_dict_close (parent);
   ctf_close (ctfa);
-  ctf_close (parenta);
-  free (parentdata);
   free (ctfdata);
 }
 #else
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 3b6f1a3ab1d..e0bde5809df 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -4823,8 +4823,7 @@ usage (FILE * stream)
   fprintf (stream, _("\
   --ctf=<number|name>    Display CTF info from section <number|name>\n"));
   fprintf (stream, _("\
-  --ctf-parent=<number|name>\n\
-                         Use section <number|name> as the CTF parent\n"));
+  --ctf-parent=<name>    Use CTF archive member <name> as the CTF parent\n"));
   fprintf (stream, _("\
   --ctf-symbols=<number|name>\n\
                          Use section <number|name> as the CTF external symtab\n"));
@@ -15118,15 +15117,12 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 
   /* Only print out the name of non-default-named archive members.
      The name .ctf appears everywhere, even for things that aren't
-     really archives, so printing it out is liable to be confusing.
+     really archives, so printing it out is liable to be confusing.  */
 
-     The parent, if there is one, is the default-owned archive member:
-     avoid importing it into itself.  (This does no harm, but looks
-     confusing.)  */
-
-  if (strcmp (name, ".ctf") != 0)
+  if (ctf_parent_name (ctf) != NULL)
     {
-      printf (_("\nCTF archive member: %s:\n"), name);
+      if (strcmp (name, ".ctf") != 0)
+	printf (_("\nCTF archive member: %s:\n"), name);
       ctf_import (ctf, parent);
     }
 
@@ -15160,18 +15156,15 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 static bool
 dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
 {
-  Elf_Internal_Shdr *  parent_sec = NULL;
   Elf_Internal_Shdr *  symtab_sec = NULL;
   Elf_Internal_Shdr *  strtab_sec = NULL;
   void *	       data = NULL;
   void *	       symdata = NULL;
   void *	       strdata = NULL;
-  void *	       parentdata = NULL;
-  ctf_sect_t	       ctfsect, symsect, strsect, parentsect;
+  ctf_sect_t	       ctfsect, symsect, strsect;
   ctf_sect_t *	       symsectp = NULL;
   ctf_sect_t *	       strsectp = NULL;
   ctf_archive_t *      ctfa = NULL;
-  ctf_archive_t *      parenta = NULL, *lookparent;
   ctf_dict_t *         parent = NULL;
 
   int err;
@@ -15220,25 +15213,9 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
       strsect.cts_data = strdata;
     }
 
-  if (dump_ctf_parent_name)
-    {
-      if ((parent_sec = find_section (filedata, dump_ctf_parent_name)) == NULL)
-	{
-	  error (_("No CTF parent section named %s\n"), dump_ctf_parent_name);
-	  goto fail;
-	}
-      if ((parentdata = (void *) get_data (NULL, filedata,
-					   parent_sec->sh_offset, 1,
-					   parent_sec->sh_size,
-					   _("CTF parent"))) == NULL)
-	goto fail;
-      shdr_to_ctf_sect (&parentsect, parent_sec, filedata);
-      parentsect.cts_data = parentdata;
-    }
-
   /* Load the CTF file and dump it.  It may be a raw CTF section, or an archive:
      libctf papers over the difference, so we can pretend it is always an
-     archive.  Possibly open the parent as well, if one was specified.  */
+     archive.  */
 
   if ((ctfa = ctf_arc_bufopen (&ctfsect, symsectp, strsectp, &err)) == NULL)
     {
@@ -15250,24 +15227,9 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
   ctf_arc_symsect_endianness (ctfa, filedata->file_header.e_ident[EI_DATA]
 			      != ELFDATA2MSB);
 
-  if (parentdata)
-    {
-      if ((parenta = ctf_arc_bufopen (&parentsect, symsectp, strsectp,
-				      &err)) == NULL)
-	{
-	  dump_ctf_errs (NULL);
-	  error (_("CTF open failure: %s\n"), ctf_errmsg (err));
-	  goto fail;
-	}
-      lookparent = parenta;
-    }
-  else
-    lookparent = ctfa;
-
-  /* Assume that the applicable parent archive member is the default one.
-     (This is what all known implementations are expected to do, if they
-     put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
+  /* Preload the parent dict, since it will need to be imported into every
+     child in turn.  */
+  if ((parent = ctf_dict_open (ctfa, dump_ctf_parent_name, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       error (_("CTF open failure: %s\n"), ctf_errmsg (err));
@@ -15294,8 +15256,6 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
  fail:
   ctf_dict_close (parent);
   ctf_close (ctfa);
-  ctf_close (parenta);
-  free (parentdata);
   free (data);
   free (symdata);
   free (strdata);
-- 
2.33.0.256.gb827f06fa9


             reply	other threads:[~2021-10-11 19:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-11 19:18 Nick Alcock [this message]
2021-10-11 19:18 ` [PATCH 2/4] binutils, ld: make objdump --ctf's parameter optional Nick Alcock
2021-10-11 19:18 ` [PATCH 3/4] libctf: dump: do not stop dumping types on error Nick Alcock
2021-10-11 19:18 ` [PATCH 4/4] libctf, ld: handle nonrepresentable types better Nick Alcock
2021-10-12 13:05 ` [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
2021-10-20 22:22   ` Alan Modra
2021-10-21 20:22     ` Nick Alcock

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211011191810.274535-1-nick.alcock@oracle.com \
    --to=nick.alcock@oracle.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).