libctf: propagate errors from parents correctly

CTF dicts have per-dict errno values: as with other errno values these
are set on error and left unchanged on success.  This means that all
errors *must* set the CTF errno: if a call leaves it unchanged, the
caller is apt to find a previous, lingering error and misinterpret
it as the real error.

There are many places in libctf where we carry out operations on parent
dicts as a result of carrying out other user-requested operations on
child dicts (e.g. looking up information on a pointer to a type will
look up the type as well: the pointer might well be in a child and the
type it's a pointer to in the parent).  Those operations on the parent
might fail; if they do, the error must be correctly reflected on the
child that the user-visible operation was carried out on.  In many
places this was not happening.

So, audit and fix all those places.  Add tests for as many of those
cases as possible so they don't regress.

libctf/
	* ctf-create.c (ctf_add_slice): Use the original dict.
	* ctf-lookup.c (ctf_lookup_variable): Propagate errors.
	(ctf_lookup_symbol_idx): Likewise.
	* ctf-types.c (ctf_member_next): Likewise.
	(ctf_type_resolve_unsliced): Likewise.
	(ctf_type_aname): Likewise.
	(ctf_member_info): Likewise.
	(ctf_type_rvisit): Likewise.
	(ctf_func_type_info): Set the error on the right dict.
	(ctf_type_encoding): Use the original dict.
	* testsuite/libctf-writable/error-propagation.*: New test.

libctf/ctf-create.c

@@ -628,8 +628,8 @@ ctf_add_slice (ctf_dict_t *fp, uint32_t flag, ctf_id_t ref,
      point to the unimplemented type, for now, because the compiler can emit
      such slices, though they're not very much use.  */
 
-  resolved_ref = ctf_type_resolve_unsliced (tmp, ref);
-  kind = ctf_type_kind_unsliced (tmp, resolved_ref);
+  resolved_ref = ctf_type_resolve_unsliced (fp, ref);
+  kind = ctf_type_kind_unsliced (fp, resolved_ref);
 
   if ((kind != CTF_K_INTEGER) && (kind != CTF_K_FLOAT) &&
       (kind != CTF_K_ENUM)

libctf/ctf-lookup.c

@@ -402,7 +402,13 @@ ctf_lookup_variable (ctf_dict_t *fp, const char *name)
   if (ent == NULL)
     {
       if (fp->ctf_parent != NULL)
-	return ctf_lookup_variable (fp->ctf_parent, name);
+        {
+          ctf_id_t ptype;
+
+          if ((ptype = ctf_lookup_variable (fp->ctf_parent, name)) != CTF_ERR)
+            return ptype;
+          return (ctf_set_errno (fp, ctf_errno (fp->ctf_parent)));
+        }
 
       return (ctf_set_errno (fp, ECTF_NOTYPEDAT));
     }
@@ -626,7 +632,16 @@ ctf_lookup_symbol_idx (ctf_dict_t *fp, const char *symname)
 
  try_parent:
   if (fp->ctf_parent)
-    return ctf_lookup_symbol_idx (fp->ctf_parent, symname);
+    {
+      unsigned long psym;
+
+      if ((psym = ctf_lookup_symbol_idx (fp->ctf_parent, symname))
+          != (unsigned long) -1)
+        return psym;
+
+      ctf_set_errno (fp, ctf_errno (fp->ctf_parent));
+      return (unsigned long) -1;
+    }
   else
     {
       ctf_set_errno (fp, err);

libctf/ctf-types.c

@@ -177,7 +177,7 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
 
       if (ctf_struct_member (fp, &memb, i->ctn_tp, i->u.ctn_vlen, i->ctn_size,
 			     i->ctn_n) < 0)
-	return -1;				/* errno is set for us.  */
+        return (ctf_set_errno (ofp, ctf_errno (fp)));
 
       membname = ctf_strptr (fp, memb.ctlm_name);
 
@@ -216,11 +216,12 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
 	  ctf_next_destroy (i);
 	  *it = NULL;
 	  i->ctn_type = 0;
-	  return ret;				/* errno is set for us.  */
+	  ctf_set_errno (ofp, ctf_errno (fp));
+	  return ret;
 	}
 
       if (!ctf_assert (fp, (i->ctn_next == NULL)))
-	return -1;				/* errno is set for us.  */
+        return (ctf_set_errno (ofp, ctf_errno (fp)));
 
       i->ctn_type = 0;
       /* This sub-struct has ended: on to the next real member.  */
@@ -597,6 +598,7 @@ ctf_type_resolve (ctf_dict_t *fp, ctf_id_t type)
 ctf_id_t
 ctf_type_resolve_unsliced (ctf_dict_t *fp, ctf_id_t type)
 {
+  ctf_dict_t *ofp = fp;
   const ctf_type_t *tp;
 
   if ((type = ctf_type_resolve (fp, type)) == CTF_ERR)
@@ -606,7 +608,13 @@ ctf_type_resolve_unsliced (ctf_dict_t *fp, ctf_id_t type)
     return CTF_ERR;		/* errno is set for us.  */
 
   if ((LCTF_INFO_KIND (fp, tp->ctt_info)) == CTF_K_SLICE)
-    return ctf_type_reference (fp, type);
+    {
+      ctf_id_t ret;
+
+      if ((ret = ctf_type_reference (fp, type)) == CTF_ERR)
+	return (ctf_set_errno (ofp, ctf_errno (fp)));
+      return ret;
+    }
   return type;
 }
 
@@ -767,6 +775,7 @@ ctf_type_aname (ctf_dict_t *fp, ctf_id_t type)
 		break;
 
 	      err:
+		ctf_set_errno (fp, ctf_errno (rfp));
 		free (argv);
 		ctf_decl_fini (&cd);
 		return NULL;
@@ -1216,8 +1225,8 @@ ctf_type_encoding (ctf_dict_t *fp, ctf_id_t type, ctf_encoding_t *ep)
 	ctf_id_t underlying;
 
 	slice = (ctf_slice_t *) vlen;
-	underlying = ctf_type_resolve (fp, slice->cts_type);
-	if (ctf_type_encoding (fp, underlying, &underlying_en) < 0)
+	underlying = ctf_type_resolve (ofp, slice->cts_type);
+	if (ctf_type_encoding (ofp, underlying, &underlying_en) < 0)
 	  return -1;				/* errno is set for us.  */
 
 	ep->cte_format = underlying_en.cte_format;
@@ -1409,7 +1418,7 @@ ctf_member_info (ctf_dict_t *fp, ctf_id_t type, const char *name,
       const char *membname;
 
       if (ctf_struct_member (fp, &memb, tp, vlen, vbytes, i) < 0)
-	return -1;				/* errno is set for us.  */
+        return (ctf_set_errno (ofp, ctf_errno (fp)));
 
       membname = ctf_strptr (fp, memb.ctlm_name);
 
@@ -1558,6 +1567,7 @@ ctf_enum_value (ctf_dict_t *fp, ctf_id_t type, const char *name, int *valp)
 int
 ctf_func_type_info (ctf_dict_t *fp, ctf_id_t type, ctf_funcinfo_t *fip)
 {
+  ctf_dict_t *ofp = fp;
   const ctf_type_t *tp;
   uint32_t kind;
   const uint32_t *args;
@@ -1574,7 +1584,7 @@ ctf_func_type_info (ctf_dict_t *fp, ctf_id_t type, ctf_funcinfo_t *fip)
   kind = LCTF_INFO_KIND (fp, tp->ctt_info);
 
   if (kind != CTF_K_FUNCTION)
-    return (ctf_set_errno (fp, ECTF_NOTFUNC));
+    return (ctf_set_errno (ofp, ECTF_NOTFUNC));
 
   fip->ctc_return = tp->ctt_type;
   fip->ctc_flags = 0;
@@ -1638,6 +1648,7 @@ static int
 ctf_type_rvisit (ctf_dict_t *fp, ctf_id_t type, ctf_visit_f *func,
 		 void *arg, const char *name, unsigned long offset, int depth)
 {
+  ctf_dict_t *ofp = fp;
   ctf_id_t otype = type;
   const ctf_type_t *tp;
   const ctf_dtdef_t *dtd;
@@ -1686,7 +1697,7 @@ ctf_type_rvisit (ctf_dict_t *fp, ctf_id_t type, ctf_visit_f *func,
       ctf_lmember_t memb;
 
       if (ctf_struct_member (fp, &memb, tp, vlen, vbytes, i) < 0)
-	return -1;				/* errno is set for us.  */
+        return (ctf_set_errno (ofp, ctf_errno (fp)));
 
       if ((rc = ctf_type_rvisit (fp, memb.ctlm_type,
 				 func, arg, ctf_strptr (fp, memb.ctlm_name),

libctf/testsuite/libctf-writable/error-propagation.c

@@ -0,0 +1,164 @@
+/* Make sure that errors are propagated properly from parent dicts to children
+   when errors are encountered in child functions that can recurse to parents.
+
+   We check specifically a subset of known-buggy functions.
+   Functions that require a buggy linker to expose, or that only fail on
+   assertion-failure-incurring corrupted dicts, are not tested. */
+
+#include <ctf-api.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static const char *desc;
+
+static void
+check_prop_err (ctf_dict_t *child, ctf_dict_t *parent, int expected)
+{
+  if (ctf_errno (child) == expected)
+    return;
+
+  if (ctf_errno (parent) == expected)
+    fprintf (stderr, "%s: error propagation failure: error \"%s\" not seen on child, "
+             "but instead on parent\n", desc, ctf_errmsg (ctf_errno (parent)));
+  else
+    fprintf (stderr, "%s: expected error is entirely lost: "
+             "\"%s\" seen on parent, \"%s\" on child\n", desc,
+             ctf_errmsg (ctf_errno (parent)),
+             ctf_errmsg (ctf_errno (child)));
+}
+
+static void
+no_prop_err (void)
+{
+  fprintf (stderr, "%s: expected error return not observed.\n", desc);
+}
+
+int main (void)
+{
+  ctf_dict_t *parent;
+  ctf_dict_t *blank;
+  ctf_dict_t *child;
+  ctf_id_t void_id;
+  ctf_id_t base;
+  ctf_id_t slice;
+  ctf_id_t function;
+  ctf_encoding_t long_encoding = { CTF_INT_SIGNED, 0, sizeof (long) };
+  ctf_encoding_t void_encoding = { CTF_INT_SIGNED, 0, 0 };
+  ctf_encoding_t foo;
+  ctf_funcinfo_t fi;
+  ctf_id_t bar;
+  char *funcname;
+  int err;
+
+  if ((parent = ctf_create (&err)) == NULL
+      || (child = ctf_create (&err)) == NULL
+      || (blank = ctf_create (&err)) == NULL)
+    {
+      fprintf (stderr, "Cannot create dicts: %s\n", ctf_errmsg (err));
+      return 1;
+    }
+
+  if ((ctf_import (child, parent)) < 0)
+    {
+      fprintf (stderr, "cannot import: %s\n", ctf_errmsg (ctf_errno (child)));
+      return 1;
+    }
+
+  if ((void_id = ctf_add_integer (parent, CTF_ADD_ROOT, "void", &void_encoding))
+      == CTF_ERR)
+    goto parent_err;
+
+  if ((base = ctf_add_integer (parent, CTF_ADD_ROOT, "long int", &long_encoding))
+      == CTF_ERR)
+    goto parent_err;
+
+  foo.cte_format = 0;
+  foo.cte_bits = 4;
+  foo.cte_offset = 4;
+  if ((slice = ctf_add_slice (child, CTF_ADD_ROOT, base, &foo)) == CTF_ERR)
+    goto parent_err;
+
+  if (ctf_add_variable (parent, "foo", base) < 0)
+    goto child_err;
+
+  fi.ctc_return = void_id;
+  fi.ctc_argc = 0;
+  fi.ctc_flags = 0;
+  if ((function = ctf_add_function (child, CTF_ADD_ROOT, &fi, NULL)) == CTF_ERR)
+    goto child_err;
+
+  desc = "func info lookup of non-function";
+  if ((ctf_func_type_info (child, base, &fi)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_NOTFUNC);
+
+  desc = "func args lookup of non-function";
+  if ((ctf_func_type_args (child, base, 0, &bar)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_NOTFUNC);
+
+  if ((ctf_import (child, blank)) < 0)
+    {
+      fprintf (stderr, "cannot reimport: %s\n", ctf_errmsg (ctf_errno (child)));
+      return 1;
+    }
+
+  /* This is testing ctf_type_resolve_unsliced(), which is called by the enum
+     functions (which are not themselves buggy).  This typea isn't an enum, but
+     that's OK: we're after an error, after all, and the type we're slicing is
+     not visible any longer, so nothing can tell it's not an enum.  */
+
+  desc = "child slice resolution";
+  if ((ctf_enum_value (child, slice, "foo", NULL)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_BADID);
+
+  desc = "child slice encoding lookup";
+  if ((ctf_type_encoding (child, slice, &foo)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_BADID);
+
+  desc = "func info lookup of non-function";
+  if ((ctf_func_type_info (child, base, &fi)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_BADID);
+
+  desc = "func args lookup of non-function";
+  if ((ctf_func_type_args (child, base, 0, &bar)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_BADID);
+
+  desc = "child slice addition";
+  if ((slice = ctf_add_slice (child, CTF_ADD_ROOT, base, &foo)) != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_BADID);
+
+  desc = "variable lookup";
+  if (ctf_lookup_variable (child, "foo") != CTF_ERR)
+    no_prop_err ();
+  check_prop_err (child, parent, ECTF_NOTYPEDAT);
+
+  desc = "function lookup via ctf_type_aname";
+  if ((funcname = ctf_type_aname (child, function)) != NULL)
+    {
+      no_prop_err ();
+      free (funcname);
+    }
+  check_prop_err (child, parent, ECTF_BADID);
+
+  ctf_dict_close (child);
+  ctf_dict_close (parent);
+  ctf_dict_close (blank);
+  fprintf (stderr, "All done.\n");
+  return 0;
+
+ parent_err:
+  fprintf (stderr, "cannot populate parent: %s\n", ctf_errmsg (ctf_errno (parent)));
+  return 1;
+
+ child_err:
+  fprintf (stderr, "cannot populate child: %s\n", ctf_errmsg (ctf_errno (parent)));
+  return 1;
+
+}

libctf/testsuite/libctf-writable/error-propagation.lk

@@ -0,0 +1 @@
+All done.