From 80b56fad5c99a8c93004b0432dd2b363125de35a Mon Sep 17 00:00:00 2001 From: Nick Alcock Date: Mon, 25 Oct 2021 11:17:02 +0100 Subject: [PATCH] binutils: make objdump/readelf --ctf-parent actually useful 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. As part of this, we have to slightly adjust the code which skips the archive member name if redundant: right now it skips it if it's ".ctf", on the assumption that this name will almost always be at the start of the objdump output and thus we'll end up with a shared dump and then smaller, headed dumps for the per-TU child dicts; but if the parent name has been changed, that won't be true any more. So change the rules to "members named .ctf which appear first in the first have their member name skipped". Since we now need to count members, move from ctf_archive_iter (for which passing in extra parameters requires defining a new struct and is clumsy) to ctf_archive_next, allowing us to just *call* dump_ctf_archive_member and maintain a member count in the obvious way. In the process we fix a tiny difference between readelf and objdump: if a ctf_dump ever failed, readelf skipped every later member, while objdump tried to keep going as much as it could. For a dumping tool the former is clearly preferable. binutils/ChangeLog 2021-10-25 Nick Alcock * 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. Use ctf_archive_next, not ctf_archive_iter, so we can pass down a member count. (dump_ctf_archive_member): Add the member count; don't return anything. 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_section_as_ctf): Likewise. (dump_ctf_archive_member): Likewise. Never stop iterating over archive members, even if ctf_dump of one member fails. * doc/ctf.options.texi: Adjust. --- binutils/ChangeLog | 16 ++++++ binutils/doc/ctf.options.texi | 15 ++++-- binutils/objdump.c | 77 ++++++++++----------------- binutils/readelf.c | 99 +++++++++++------------------------ 4 files changed, 86 insertions(+), 121 deletions(-) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 192de3cda95..ea9cfe43146 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,19 @@ +2021-10-25 Nick Alcock + + * 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. Use ctf_archive_next, not ctf_archive_iter, + so we can pass down a member count. + (dump_ctf_archive_member): Add the member count; don't return + anything. 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_section_as_ctf): Likewise. + (dump_ctf_archive_member): Likewise. Never stop iterating over + archive members, even if ctf_dump of one member fails. + * doc/ctf.options.texi: Adjust. + 2021-10-19 Nick Clifton * nm.c (filter_symbols): Test for a NULL name before checking to 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 6dd3d8722d8..43472e19e25 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")); @@ -4156,29 +4156,27 @@ dump_ctf_errs (ctf_dict_t *fp) /* Dump one CTF archive member. */ -static int -dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg) +static void +dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, ctf_dict_t *parent, + size_t member) { - ctf_dict_t *parent = (ctf_dict_t *) arg; const char *things[] = {"Header", "Labels", "Data objects", "Function objects", "Variables", "Types", "Strings", ""}; const char **thing; size_t i; - /* 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. + /* Don't print out the name of the default-named archive member if it appears + first in the list. The name .ctf appears everywhere, even for things that + aren't really archives, so printing it out is liable to be confusing; also, + the common case by far is for only one archive member to exist, and hiding + it in that case seems worthwhile. */ - 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 || member != 0) + 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); - } + if (ctf_parent_name (ctf) != NULL) + ctf_import (ctf, parent); for (i = 0, thing = things; *thing[0]; thing++, i++) { @@ -4202,8 +4200,6 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg) } dump_ctf_errs (ctf); - - return 0; } /* Dump the CTF debugging information. */ @@ -4211,22 +4207,23 @@ 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; + ctf_dict_t *fp; + ctf_next_t *i = NULL; + const char *name; + size_t member = 0; 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 +4233,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)); @@ -4263,7 +4242,9 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name) printf (_("Contents of CTF section %s:\n"), sanitize_string (sect_name)); - if ((err = ctf_archive_iter (ctfa, dump_ctf_archive_member, parent)) != 0) + while ((fp = ctf_archive_next (ctfa, &i, &name, 0, &err)) != NULL) + dump_ctf_archive_member (fp, name, parent, member++); + if (err != ECTF_NEXT_END) { dump_ctf_errs (NULL); non_fatal (_("CTF archive member open failure: %s"), ctf_errmsg (err)); @@ -4271,8 +4252,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 fc17c043a9f..682eacdef14 100644 --- a/binutils/readelf.c +++ b/binutils/readelf.c @@ -4869,8 +4869,7 @@ usage (FILE * stream) fprintf (stream, _("\ --ctf= Display CTF info from section \n")); fprintf (stream, _("\ - --ctf-parent=\n\ - Use section as the CTF parent\n")); + --ctf-parent= Use CTF archive member as the CTF parent\n")); fprintf (stream, _("\ --ctf-symbols=\n\ Use section as the CTF external symtab\n")); @@ -15164,30 +15163,27 @@ dump_ctf_errs (ctf_dict_t *fp) /* Dump one CTF archive member. */ -static int -dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg) +static void +dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, ctf_dict_t *parent, + size_t member) { - ctf_dict_t *parent = (ctf_dict_t *) arg; const char *things[] = {"Header", "Labels", "Data objects", "Function objects", "Variables", "Types", "Strings", ""}; const char **thing; size_t i; - int err = 0; - /* 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. + /* Don't print out the name of the default-named archive member if it appears + first in the list. The name .ctf appears everywhere, even for things that + aren't really archives, so printing it out is liable to be confusing; also, + the common case by far is for only one archive member to exist, and hiding + it in that case seems worthwhile. */ - 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 || member != 0) + printf (_("\nCTF archive member: %s:\n"), name); - if (strcmp (name, ".ctf") != 0) - { - printf (_("\nCTF archive member: %s:\n"), name); - ctf_import (ctf, parent); - } + if (ctf_parent_name (ctf) != NULL) + ctf_import (ctf, parent); for (i = 0, thing = things; *thing[0]; thing++, i++) { @@ -15206,33 +15202,31 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg) { error (_("Iteration failed: %s, %s\n"), *thing, ctf_errmsg (ctf_errno (ctf))); - err = 1; - goto out; + break; } } - out: dump_ctf_errs (ctf); - return err; } 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; + ctf_dict_t * fp; + ctf_next_t *i = NULL; + const char *name; + size_t member = 0; int err; bool ret = false; @@ -15279,25 +15273,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) { @@ -15309,24 +15287,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)); @@ -15343,18 +15306,18 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata) printf (_("\nDump of CTF section '%s':\n"), printable_section_name (filedata, section)); - if ((err = ctf_archive_iter (ctfa, dump_ctf_archive_member, parent)) != 0) - { - dump_ctf_errs (NULL); - error (_("CTF member open failure: %s\n"), ctf_errmsg (err)); - ret = false; - } + while ((fp = ctf_archive_next (ctfa, &i, &name, 0, &err)) != NULL) + dump_ctf_archive_member (fp, name, parent, member++); + if (err != ECTF_NEXT_END) + { + dump_ctf_errs (NULL); + error (_("CTF member open failure: %s\n"), ctf_errmsg (err)); + ret = false; + } fail: ctf_dict_close (parent); ctf_close (ctfa); - ctf_close (parenta); - free (parentdata); free (data); free (symdata); free (strdata); -- 2.30.2