[binutils-gdb] libctf: minor error-handling fixes

Nick Alcock nix@sourceware.org
Tue Mar 2 15:12:12 GMT 2021


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4659554b280bee894d117360f403d4073edd3bc9

commit 4659554b280bee894d117360f403d4073edd3bc9
Author: Nick Alcock <nick.alcock@oracle.com>
Date:   Tue Mar 2 15:10:05 2021 +0000

    libctf: minor error-handling fixes
    
    A transient bug in the preceding change (fixed before commit) exposed a
    new failure, of ld/testsuite/ld-ctf/diag-parname.d.  This attempts to
    ensure that if we link a dict with child type IDs but no attached
    parent, we get a suitable ECTF_NOPARENT error.  This was happening
    before this commit, but only by chance, because ctf_variable_iter and
    ctf_variable_next check to see if the dict they're passed is a child
    dict without an associated parent.  We forgot error-checking on the
    ctf_variable_next call, and as a result this was concealed -- and
    looking for the problem exposed a new bug.
    
    If any of the lookups beneath ctf_dedup_hash_type fail, the CTF link
    does *not* fail, but acts quite bizarrely, skipping the type but
    emitting an error to the CTF error/warning log -- so the linker will
    report an error, emit a partial CTF dict missing some types, and exit
    with exitcode 0 as if nothing went wrong.  Since ctf_dedup_hash_type is
    never expected to fail in normal operation, this is surely wrong:
    failures at emission time do not emit partial CTF dicts, so failures
    at hashing time should not either.
    
    So propagate the error back up.
    
    Also fix a couple of smaller bugs where we fail to properly free things
    and/or propagate error codes on various rare link-time errors and
    out-of-memory conditions.
    
    libctf/ChangeLog
    2021-03-02  Nick Alcock  <nick.alcock@oracle.com>
    
            * ctf-dedup.c (ctf_dedup): Pass on errors from ctf_dedup_hash_type.
            Call ctf_dedup_fini properly on other errors.
            (ctf_dedup_emit_type): Set the errno on dynhash insertion failure.
            * ctf-link.c (ctf_link_deduplicating_per_cu): Close outputs beyond
            output 0 when asserting because >1 output is found.
            (ctf_link_deduplicating): Likewise, when asserting because the
            shared output is not the same as the passed-in fp.

Diff:
---
 libctf/ChangeLog   | 10 ++++++++++
 libctf/ctf-dedup.c | 17 +++++++++++------
 libctf/ctf-link.c  | 13 +++++++++++--
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 97069191c48..767d48c3599 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,13 @@
+2021-03-02  Nick Alcock  <nick.alcock@oracle.com>
+
+	* ctf-dedup.c (ctf_dedup): Pass on errors from ctf_dedup_hash_type.
+	Call ctf_dedup_fini properly on other errors.
+	(ctf_dedup_emit_type): Set the errno on dynhash insertion failure.
+	* ctf-link.c (ctf_link_deduplicating_per_cu): Close outputs beyond
+	output 0 when asserting because >1 output is found.
+	(ctf_link_deduplicating): Likewise, when asserting because the
+	shared output is not the same as the passed-in fp.
+
 2021-03-02  Nick Alcock  <nick.alcock@oracle.com>
 
 	* ctf-impl.h (ctf_dict_t) <ctf_link_type_mapping>: No longer used
diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index 50da4ac5c11..ef8507a59f2 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -1924,15 +1924,17 @@ ctf_dedup (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
 
       while ((id = ctf_type_next (inputs[i], &it, NULL, 1)) != CTF_ERR)
 	{
-	  ctf_dedup_hash_type (output, inputs[i], inputs, parents,
-			       i, id, 0, 0, ctf_dedup_populate_mappings);
+	  if (ctf_dedup_hash_type (output, inputs[i], inputs,
+				   parents, i, id, 0, 0,
+				   ctf_dedup_populate_mappings) == NULL)
+	    goto err;				/* errno is set for us.  */
 	}
       if (ctf_errno (inputs[i]) != ECTF_NEXT_END)
 	{
 	  ctf_set_errno (output, ctf_errno (inputs[i]));
 	  ctf_err_warn (output, 0, 0, _("iteration failure "
 					"computing type hashes"));
-	  return -1;
+	  goto err;
 	}
     }
 
@@ -1943,7 +1945,7 @@ ctf_dedup (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
 
   ctf_dprintf ("Detecting type name ambiguity\n");
   if (ctf_dedup_detect_name_ambiguity (output, inputs) < 0)
-    return -1;					/* errno is set for us.  */
+      goto err;					/* errno is set for us.  */
 
   /* If the link mode is CTF_LINK_SHARE_DUPLICATED, we change any unconflicting
      types whose output mapping references only one input dict into a
@@ -1953,7 +1955,7 @@ ctf_dedup (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
     {
       ctf_dprintf ("Conflictifying unshared types\n");
       if (ctf_dedup_conflictify_unshared (output, inputs) < 0)
-	return -1;				/* errno is set for us.  */
+	goto err;				/* errno is set for us.  */
     }
   return 0;
 
@@ -2882,7 +2884,10 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
 		     id, out_id);
 	/* Record the need to emit the members of this structure later.  */
 	if (ctf_dynhash_insert (d->cd_emission_struct_members, id, out_id) < 0)
-	  goto err_target;
+	  {
+	    ctf_set_errno (target, errno);
+	    goto err_target;
+	  }
 	break;
       }
     default:
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index c0b0916f536..05733a0cb7d 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -1134,7 +1134,12 @@ ctf_link_deduplicating_per_cu (ctf_dict_t *fp)
 	  goto err_inputs;
 	}
       if (!ctf_assert (fp, noutputs == 1))
-	goto err_inputs_outputs;
+	{
+	  size_t j;
+	  for (j = 1; j < noutputs; j++)
+	    ctf_dict_close (outputs[j]);
+	  goto err_inputs_outputs;
+	}
 
       if (!(fp->ctf_link_flags & CTF_LINK_OMIT_VARIABLES_SECTION)
 	  && ctf_link_deduplicating_variables (out, inputs, ninputs, 1) < 0)
@@ -1263,7 +1268,11 @@ ctf_link_deduplicating (ctf_dict_t *fp)
     }
 
   if (!ctf_assert (fp, outputs[0] == fp))
-    goto err;
+    {
+      for (i = 1; i < noutputs; i++)
+	ctf_dict_close (outputs[i]);
+      goto err;
+    }
 
   for (i = 0; i < noutputs; i++)
     {


More information about the Binutils-cvs mailing list