libctf, archive, link: fix parent importing

We are about to move to a regime where there are very few things you can do
with most dicts before you ctf_import them.  So emit a warning if
ctf_archive_next()'s convenience ctf_import of parents fails.  Rip out the
buggy code in ctf_link_deduplicating_open_inputs which opened the parent by
hand (with a hardwired name), and instead rely on ctf_archive_next to do it
for us (which also means we don't end up opening it twice, once in
ctf_archive_next, once in ctf_link_deduplicating_open_inputs).

While we're there, arrange to close the inputs we already opened if opening
of some inputs fails, rather than leaking them.  (There are still some leaks
here, so add a comment to remind us to clean them up later.)

libctf/
	* ctf-archive.c (ctf_arc_import_parent): Emit a warning if importing
	fails.
	* ctf-link.c (ctf_link_deduplicating_open_inputs): Rely on the
        ctf_archive_next to open parent dicts.

diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index dc01e2e0b08cebc2057e6982a6f3440659286e06..739d0349fab522cbb0376179c96b8a9df95b9672 100644 (file)
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -769,7 +769,10 @@ ctf_arc_import_parent (const ctf_archive_t *arc, ctf_dict_t *fp, int *errp)
 
       if (parent)
        {
-         ctf_import (fp, parent);
+         if (ctf_import (fp, parent) < 0)
+           ctf_err_warn (NULL, 1, ctf_errno (fp),
+                         "ctf_arc_import_parent: cannot import: %s",
+                         ctf_errmsg (ctf_errno (fp)));
          ctf_dict_close (parent);
        }
       else if (err != ECTF_ARNNAME)

diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 2019c11ae9bc49fca6286f823030b71a60981bb8..b992fb1a33e58efd377ab64072be9a70e4f44992 100644 (file)
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -711,6 +711,10 @@ ctf_link_deduplicating_count_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
   return count;
 }
 
+static int
+ctf_link_deduplicating_close_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
+                                    ctf_dict_t **inputs, ssize_t ninputs);
+
 /* Allocate and populate an inputs array big enough for a given set of inputs:
    either a specific set of CU names (those from that set found in the
    ctf_link_inputs), or the entire ctf_link_inputs (if cu_names is not set).
@@ -763,9 +767,10 @@ ctf_link_deduplicating_open_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
       const char *one_name = (const char *) name;
       ctf_link_input_t *one_input;
       ctf_dict_t *one_fp;
-      ctf_dict_t *parent_fp = NULL;
-      uint32_t parent_i = 0;
       ctf_next_t *j = NULL;
+      ssize_t parent_i;
+      int parent_set = 0;
+      ctf_dict_t *parent_fp = NULL;
 
       /* If we are processing CU names, get the real input.  All the inputs
         will have been opened, if they contained any CTF at all.  */
@@ -777,7 +782,7 @@ ctf_link_deduplicating_open_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
       if (!one_input || (!one_input->clin_arc && !one_input->clin_fp))
        continue;
 
-      /* Short-circuit: if clin_fp is set, just use it.   */
+      /* Short-circuit: if clin_fp is set, just use it.  */
       if (one_input->clin_fp)
        {
          parents_[walk - dedup_inputs] = walk - dedup_inputs;
@@ -786,48 +791,73 @@ ctf_link_deduplicating_open_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
          continue;
        }
 
-      /* Get and insert the parent archive (if any), if this archive has
-        multiple members.  We assume, as elsewhere, that the parent is named
-        _CTF_SECTION.  */
+      if (one_input->clin_filename)
+       ctf_dprintf ("Opening %s\n", one_input->clin_filename);
 
-      if ((parent_fp = ctf_dict_open (one_input->clin_arc, _CTF_SECTION,
-                                     &err)) == NULL)
+      /* We disregard the input archive name: either it is the parent (which we can get
+        via ctf_parent_dict), or we want to put everything into one TU sharing the
+        cuname anyway (if this is a CU-mapped link), or this is the final phase of a
+        relink with CU-mapping off (i.e. ld -r) in which case the cuname is correctly
+        set regardless.  */
+
+      while ((one_fp = ctf_archive_next (one_input->clin_arc, &j, NULL,
+                                        0, &err)) != NULL)
        {
-         if (err != ECTF_NOMEMBNAM)
+         /* ctf_archive_next either auto-imports the parent, or this *is* the parent.
+            In both cases, we can set the parent up if it's not already set.
+
+            If parent importing failed, we must report an error.
+
+            We must do it first, so we can set later elements of the parents_ array to
+            the parent.  */
+
+         if (one_fp->ctf_flags & LCTF_CHILD && one_fp->ctf_parent == NULL)
            {
              ctf_next_destroy (i);
-             ctf_set_errno (fp, err);
-             goto err;
+             ctf_set_errno (fp, ECTF_NOPARENT);
+             ctf_err_warn (fp, 0, 0, _("cannot open linker inputs"));
+             goto reported_err;
            }
-       }
-      else
-       {
-         *walk = parent_fp;
-         parent_i = walk - dedup_inputs;
-         walk++;
-       }
 
-      /* We disregard the input archive name: either it is the parent (which we
-        already have), or we want to put everything into one TU sharing the
-        cuname anyway (if this is a CU-mapped link), or this is the final phase
-        of a relink with CU-mapping off (i.e. ld -r) in which case the cuname
-        is correctly set regardless.  */
-      while ((one_fp = ctf_archive_next (one_input->clin_arc, &j, NULL,
-                                        1, &err)) != NULL)
-       {
-         if (one_fp->ctf_flags & LCTF_CHILD)
+         if (!parent_set)
            {
-             /* The contents of the parents array for elements not
-                corresponding to children is undefined.  If there is no parent
-                (itself a sign of a likely linker bug or corrupt input), we set
-                it to itself.  */
-
-             ctf_import (one_fp, parent_fp);
-             if (parent_fp)
-               parents_[walk - dedup_inputs] = parent_i;
+             if (one_fp->ctf_flags & LCTF_CHILD)
+               parent_fp = one_fp->ctf_parent;
              else
-               parents_[walk - dedup_inputs] = walk - dedup_inputs;
+               parent_fp = one_fp;
+
+             *walk = parent_fp;
+             parent_i = walk - dedup_inputs;
+             parents_[parent_i] = parent_i;
+             walk++;
+
+             parent_set = 1;
+
+             /* If this is the parent, we don't need to process the child dict itself
+                (because there isn't one).  */
+
+             if (!(one_fp->ctf_flags & LCTF_CHILD))
+               continue;
+           }
+
+         if (!ctf_assert (fp, one_fp->ctf_parent == parent_fp))
+           {
+             ctf_next_destroy (i);
+             goto err;                         /* errno is set for us. */
            }
+
+         parents_[walk - dedup_inputs] = parent_i;
+
+         /* This should never happen, but if it *does*, we want to know, because it
+            breaks string lookup and thus eventually everything.  */
+         if (one_fp->ctf_flags & LCTF_NO_STR)
+           {
+             ctf_next_destroy (j);
+             ctf_next_destroy (i);
+             ctf_set_errno (fp, ECTF_NOPARENT);
+             goto err;
+           }
+
          *walk = one_fp;
          walk++;
        }
@@ -851,10 +881,14 @@ ctf_link_deduplicating_open_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
   ctf_set_errno (fp, err);
 
  err:
-  free (dedup_inputs);
-  free (parents_);
   ctf_err_warn (fp, 0, 0, _("error in deduplicating CTF link "
                            "input allocation"));
+ reported_err:
+  ctf_link_deduplicating_close_inputs (fp, cu_names, dedup_inputs, ninputs);
+  /* UPTODO XXX fix pre-existing leak of the parents on error.  */
+
+  free (dedup_inputs);
+  free (parents_);
   return NULL;
 }