libctf, bfd: fix ctf_bfdopen_ctfsect opening symbol and string sections

The code in ctf_bfdopen_ctfsect (which is the ultimate place where you
end up if you use ctf_open to open a CTF file and pull in the ELF string
and symbol tables) was written before it was possible to actually test
it, since the linker was not written.  Now it is, it turns out that the
previous code was completely nonfunctional: it assumed that you could
load the symbol table via bfd_section_from_elf_index (...,elf_onesymtab())
and the string table via bfd_section_from_elf_index on the sh_link.

Unfortunately BFD loads neither of these sections in the conventional
fashion it uses for most others: the symbol table is immediately
converted into internal form (which is useless for our purposes, since
we also have to work in the absence of BFD for readelf, etc) and the
string table is loaded specially via bfd_elf_get_str_section which is
private to bfd/elf.c.

So make this function public, export it in elf-bfd.h, and use it from
libctf, which does something similar to what bfd_elf_sym_name and
bfd_elf_string_from_elf_section do.  Similarly, load the symbol table
manually using bfd_elf_get_elf_syms and throw away the internal form
it generates for us (we never use it).

BFD allocates the strtab for us via bfd_alloc, so we can leave BFD to
deallocate it: we allocate the symbol table ourselves before calling
bfd_elf_get_elf_syms, so we still have to free it.

Also change the rules around what you are allowed to provide: It is
useful to provide a string section but no symbol table, because CTF
sections can legitimately have no function info or data object sections
while relying on the ELF strtab for some of their strings.  So allow
that combination.

v4: adjust to upstream changes.  ctf_bfdopen_ctfsect's first parameter
    is potentially unused again (if BFD is not in use for this link
    due to not supporting an ELF target).
v5: fix tabdamage.

bfd/
	* elf-bfd.h (bfd_elf_get_str_section): Add.
	* elf.c (bfd_elf_get_str_section): No longer static.

libctf/
	* ctf-open-bfd.c: Add <assert.h>.
	(ctf_bfdopen_ctfsect): Open string and symbol tables using
	techniques borrowed from bfd_elf_sym_name.
	(ctf_new_archive_internal): Improve comment.
	* ctf-archive.c (ctf_arc_close): Do not free the ctfi_strsect.
	* ctf-open.c (ctf_bufopen): Allow opening with a string section but
	no symbol section, but not vice versa.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8496cb484f9d06cbb9ef18131017193967a809f0..42300c111a636d5392d89e89f902da6c1abdf2d8 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,8 @@
+2019-07-13  Nick Alcock  <nick.alcock@oracle.com>
+
+       * elf-bfd.h (bfd_elf_get_str_section): Add.
+       * elf.c (bfd_elf_get_str_section): No longer static.
+
 2019-09-26  Alan Modra  <amodra@gmail.com>
 
        PR 24262

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 0a83c173327c23b8078c124a890512d08ac00ab7..a9e2d3eeaf14301b526b6a84a5ec516be46531f6 100644 (file)
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2051,6 +2051,7 @@ extern char *bfd_elf_string_from_elf_section
 extern Elf_Internal_Sym *bfd_elf_get_elf_syms
   (bfd *, Elf_Internal_Shdr *, size_t, size_t, Elf_Internal_Sym *, void *,
    Elf_External_Sym_Shndx *);
+extern char * bfd_elf_get_str_section (bfd *, unsigned int);
 extern const char *bfd_elf_sym_name
   (bfd *, Elf_Internal_Shdr *, Elf_Internal_Sym *, asection *);
 

diff --git a/bfd/elf.c b/bfd/elf.c
index 664eae5c66f6beeec4d6f64ea66b8ef20214cc31..bb994b5aaa380a23df8745412e970585dbfc2c0a 100644 (file)
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -275,7 +275,7 @@ bfd_elf_mkcorefile (bfd *abfd)
   return elf_tdata (abfd)->core != NULL;
 }
 
-static char *
+char *
 bfd_elf_get_str_section (bfd *abfd, unsigned int shindex)
 {
   Elf_Internal_Shdr **i_shdrp;

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index d0d0d6785c155f0e5c9fb6f3c7ca18c3863f2c6c..7fa9fc04771c42476b5a13ef0fe9379c2a327f49 100644 (file)
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,13 @@
+2019-07-13  Nick Alcock  <nick.alcock@oracle.com>
+
+       * ctf-open-bfd.c: Add <assert.h>.
+       (ctf_bfdopen_ctfsect): Open string and symbol tables using
+       techniques borrowed from bfd_elf_sym_name.
+       (ctf_new_archive_internal): Improve comment.
+       * ctf-archive.c (ctf_arc_close): Do not free the ctfi_strsect.
+       * ctf-open.c (ctf_bufopen): Allow opening with a string section but
+       no symbol section, but not vice versa.
+
 2019-07-08  Nick Alcock  <nick.alcock@oracle.com>
 
        * ctf-impl.h (ctf_file_t): New field ctf_openflags.

diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index 5c1692219e892f2f32cd54ce5878f8e26bedf884..a13bac8cd6ae44402cf8669e3c645ddc326cba81 100644 (file)
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -405,7 +405,7 @@ ctf_arc_close (ctf_archive_t *arc)
   else
     ctf_file_close (arc->ctfi_file);
   free ((void *) arc->ctfi_symsect.cts_data);
-  free ((void *) arc->ctfi_strsect.cts_data);
+  /* Do not free the ctfi_strsect: it is bound to the bfd.  */
   free (arc->ctfi_data);
   free (arc);
 }

diff --git a/libctf/ctf-open-bfd.c b/libctf/ctf-open-bfd.c
index 6a0c15531ef7cb6fea83c49a1595f433f7c88565..6fbbde88529b333fe2abe01602d9795893a126b7 100644 (file)
--- a/libctf/ctf-open-bfd.c
+++ b/libctf/ctf-open-bfd.c
@@ -19,6 +19,7 @@
 
 #include <ctf-impl.h>
 #include <stddef.h>
+#include <assert.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <errno.h>
@@ -32,8 +33,9 @@
 #include "elf-bfd.h"
 
 /* Make a new struct ctf_archive_internal wrapper for a ctf_archive or a
-   ctf_file.  Closes ARC and/or FP on error.  Arrange to free the SYMSECT and
-   STRSECT interior on close.  */
+   ctf_file.  Closes ARC and/or FP on error.  Arrange to free the SYMSECT or
+   STRSECT, as needed, on close (though the STRSECT interior is bound to the bfd
+   * and is not actually freed by this machinery).  */
 
 static struct ctf_archive_internal *
 ctf_new_archive_internal (int is_archive, struct ctf_archive *arc,
@@ -130,50 +132,63 @@ ctf_bfdopen_ctfsect (struct bfd *abfd _libctf_unused_,
   int is_archive;
 
 #ifdef HAVE_BFD_ELF
-  asection *sym_asect;
   ctf_sect_t symsect, strsect;
+  Elf_Internal_Shdr *strhdr;
+  Elf_Internal_Shdr *symhdr = &elf_symtab_hdr (abfd);
+  size_t symcount = symhdr->sh_size / symhdr->sh_entsize;
+  Elf_Internal_Sym *isymbuf;
+  bfd_byte *symtab;
+  const char *strtab = NULL;
   /* TODO: handle SYMTAB_SHNDX.  */
 
-  if ((sym_asect = bfd_section_from_elf_index (abfd,
-                                              elf_onesymtab (abfd))) != NULL)
+  if ((symtab = malloc (symhdr->sh_size)) == NULL)
     {
-      Elf_Internal_Shdr *symhdr = &elf_symtab_hdr (abfd);
-      asection *str_asect = NULL;
-      bfd_byte *contents;
-
-      if (symhdr->sh_link != SHN_UNDEF &&
-         symhdr->sh_link <= elf_numsections (abfd))
-       str_asect = bfd_section_from_elf_index (abfd, symhdr->sh_link);
+      bfderrstr = "Cannot malloc symbol table";
+      goto err;
+    }
 
-      Elf_Internal_Shdr *strhdr = elf_elfsections (abfd)[symhdr->sh_link];
+  isymbuf = bfd_elf_get_elf_syms (abfd, symhdr, symcount, 0,
+                                 NULL, symtab, NULL);
+  free (isymbuf);
+  if (isymbuf == NULL)
+    {
+      bfderrstr = "Cannot read symbol table";
+      goto err_free_sym;
+    }
 
-      if (sym_asect && str_asect)
+  if (elf_elfsections (abfd) != NULL
+      && symhdr->sh_link < elf_numsections (abfd))
+    {
+      strhdr = elf_elfsections (abfd)[symhdr->sh_link];
+      if (strhdr->contents == NULL)
        {
-         if (!bfd_malloc_and_get_section (abfd, str_asect, &contents))
-           {
-             bfderrstr = "Cannot malloc string table";
-             free (contents);
-             goto err;
-           }
-         strsect.cts_data = contents;
-         strsect.cts_name = (char *) strsect.cts_data + strhdr->sh_name;
-         strsect.cts_size = bfd_section_size (str_asect);
-         strsect.cts_entsize = strhdr->sh_size;
-         strsectp = &strsect;
-
-         if (!bfd_malloc_and_get_section (abfd, sym_asect, &contents))
+         if ((strtab = bfd_elf_get_str_section (abfd, symhdr->sh_link)) == NULL)
            {
-             bfderrstr = "Cannot malloc symbol table";
-             free (contents);
-             goto err_free_str;
+             bfderrstr = "Cannot read string table";
+             goto err_free_sym;
            }
-
-         symsect.cts_name = (char *) strsect.cts_data + symhdr->sh_name;
-         symsect.cts_entsize = symhdr->sh_size;
-         symsect.cts_size = bfd_section_size (sym_asect);
-         symsect.cts_data = contents;
-         symsectp = &symsect;
        }
+      else
+       strtab = (const char *) strhdr->contents;
+    }
+
+  if (strtab)
+    {
+      /* The names here are more or less arbitrary, but there is no point
+        thrashing around digging the name out of the shstrtab given that we don't
+        use it for anything but debugging.  */
+
+      strsect.cts_data = strtab;
+      strsect.cts_name = ".strtab";
+      strsect.cts_size = strhdr->sh_size;
+      strsectp = &strsect;
+
+      assert (symhdr->sh_entsize == get_elf_backend_data (abfd)->s->sizeof_sym);
+      symsect.cts_name = ".symtab";
+      symsect.cts_entsize = symhdr->sh_entsize;
+      symsect.cts_size = symhdr->sh_size;
+      symsect.cts_data = symtab;
+      symsectp = &symsect;
     }
 #endif
 
@@ -183,7 +198,7 @@ ctf_bfdopen_ctfsect (struct bfd *abfd _libctf_unused_,
       is_archive = 1;
       if ((arc = ctf_arc_bufopen ((void *) ctfsect->cts_data,
                                  ctfsect->cts_size, errp)) == NULL)
-       goto err_free_sym;
+       goto err_free_str;
     }
   else
     {
@@ -192,7 +207,7 @@ ctf_bfdopen_ctfsect (struct bfd *abfd _libctf_unused_,
        {
          ctf_dprintf ("ctf_internal_open(): cannot open CTF: %s\n",
                       ctf_errmsg (*errp));
-         goto err_free_sym;
+         goto err_free_str;
        }
     }
   arci = ctf_new_archive_internal (is_archive, arc, fp, symsectp, strsectp,
@@ -200,11 +215,10 @@ ctf_bfdopen_ctfsect (struct bfd *abfd _libctf_unused_,
 
   if (arci)
     return arci;
- err_free_sym:
+ err_free_str: ;
 #ifdef HAVE_BFD_ELF
-  free ((void *) symsect.cts_data);
-err_free_str:
-  free ((void *) strsect.cts_data);
+ err_free_sym:
+  free (symtab);
 #endif
 err: _libctf_unused_;
   if (bfderrstr)

diff --git a/libctf/ctf-open.c b/libctf/ctf-open.c
index 2979ef8d287b747e58bf5841c96cb3f748aa30d5..51f9edcc3a085076fbe1071b15a8056fe40396b6 100644 (file)
--- a/libctf/ctf-open.c
+++ b/libctf/ctf-open.c
@@ -1244,7 +1244,7 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
 
   libctf_init_debug();
 
-  if (ctfsect == NULL || ((symsect == NULL) != (strsect == NULL)))
+  if ((ctfsect == NULL) || ((symsect != NULL) && (strsect == NULL)))
     return (ctf_set_open_errno (errp, EINVAL));
 
   if (symsect != NULL && symsect->cts_entsize != sizeof (Elf32_Sym) &&