public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] libctf, tests: do not assume host and target have identical field offsets
@ 2023-04-07 22:04 Nick Alcock
  2023-04-07 22:04 ` [PATCH 2/3] libctf: propagate errors from parents correctly Nick Alcock
  2023-04-07 22:04 ` [PATCH 3/3] libctf, link: fix CU-mapped links with CTF_LINK_EMPTY_CU_MAPPINGS Nick Alcock
  0 siblings, 2 replies; 3+ messages in thread
From: Nick Alcock @ 2023-04-07 22:04 UTC (permalink / raw)
  To: binutils

The newly-introduced libctf-lookup unnamed-field-info test checks
C compiler-observed field offsets against libctf-computed ones
by #including the testcase in the lookup runner as well as
generating CTF for it.  This only works if the host, on which
the lookup runner is compiled and executed, is the same architecture as
the target, for which the CTF is generated: when crossing, the trick
may fail.

So pass down an indication of whether this is a cross into the
testsuite, and add a new no_cross flag to .lk files that is used to
suppress test execution when a cross-compiler is being tested.

libctf/
	* Makefile.am (check_DEJAGNU): Pass down TEST_CROSS.
	* Makefile.in: Regenerated.
	* testsuite/lib/ctf-lib.exp (run_lookup_test): Use it to
	implement the new no_cross option.
	* testsuite/libctf-lookup/unnamed-field-info.lk: Mark as
	no_cross.
---
 libctf/Makefile.am                                   |  7 ++++++-
 libctf/Makefile.in                                   |  7 ++++++-
 libctf/testsuite/lib/ctf-lib.exp                     | 12 +++++++++++-
 libctf/testsuite/libctf-lookup/unnamed-field-info.lk |  1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/libctf/Makefile.am b/libctf/Makefile.am
index c959b09e492..b1dbc2f6ba4 100644
--- a/libctf/Makefile.am
+++ b/libctf/Makefile.am
@@ -92,10 +92,15 @@ check-DEJAGNU: site.exp development.exp
 	r=`pwd`; export r; \
 	LC_ALL=C; export LC_ALL; \
 	EXPECT=$(EXPECT); export EXPECT; \
+	if [ "@host@" = "@target@" ] ; then \
+	  TEST_CROSS=no; \
+	else \
+	  TEST_CROSS=yes; \
+	fi; \
 	runtest=$(RUNTEST); \
 	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
 	  $$runtest --tool $(DEJATOOL) --srcdir $${srcroot}/testsuite \
-		CC="$(CC)" CC_FOR_TARGET="$(CC_FOR_TARGET)" \
+		CC="$(CC)" CC_FOR_TARGET="$(CC_FOR_TARGET)" TEST_CROSS="$${TEST_CROSS}" \
 		CFLAGS="$(CFLAGS) -I$(INCDIR) -I$(srcdir) -I$(builddir) -I$(builddir)/../bfd $(ZLIBINC)" \
 		LIBS="$(libctf_nobfd_la_LIBADD) $(LIBS)" $(RUNTESTFLAGS); \
 	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
diff --git a/libctf/Makefile.in b/libctf/Makefile.in
index c6cb55c9768..3d2696bcc47 100644
--- a/libctf/Makefile.in
+++ b/libctf/Makefile.in
@@ -1678,10 +1678,15 @@ check-DEJAGNU: site.exp development.exp
 	r=`pwd`; export r; \
 	LC_ALL=C; export LC_ALL; \
 	EXPECT=$(EXPECT); export EXPECT; \
+	if [ "@host@" = "@target@" ] ; then \
+	  TEST_CROSS=no; \
+	else \
+	  TEST_CROSS=yes; \
+	fi; \
 	runtest=$(RUNTEST); \
 	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
 	  $$runtest --tool $(DEJATOOL) --srcdir $${srcroot}/testsuite \
-		CC="$(CC)" CC_FOR_TARGET="$(CC_FOR_TARGET)" \
+		CC="$(CC)" CC_FOR_TARGET="$(CC_FOR_TARGET)" TEST_CROSS="$${TEST_CROSS}" \
 		CFLAGS="$(CFLAGS) -I$(INCDIR) -I$(srcdir) -I$(builddir) -I$(builddir)/../bfd $(ZLIBINC)" \
 		LIBS="$(libctf_nobfd_la_LIBADD) $(LIBS)" $(RUNTESTFLAGS); \
 	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
diff --git a/libctf/testsuite/lib/ctf-lib.exp b/libctf/testsuite/lib/ctf-lib.exp
index 3ee82a00a33..73b67b45e34 100644
--- a/libctf/testsuite/lib/ctf-lib.exp
+++ b/libctf/testsuite/lib/ctf-lib.exp
@@ -103,6 +103,9 @@ proc compile_link_one_host_cc { src output additional_args } {
 #   xfail: GLOB|PROC ...
 #	This test is expected to fail on a specified list of targets.
 #
+#   no_cross:
+#       If set, do not run this test when host != target.
+#
 # Each option may occur at most once unless otherwise mentioned.
 #
 # After the option lines come regexp lines.  run_lookup_test calls
@@ -110,7 +113,7 @@ proc compile_link_one_host_cc { src output additional_args } {
 # regexps in FILE.d.
 #
 proc run_lookup_test { name } {
-    global CC_FOR_TARGET CFLAGS_FOR_TARGET LIBS
+    global CC_FOR_TARGET CFLAGS_FOR_TARGET LIBS TEST_CROSS
     global copyfile env runtests srcdir subdir verbose
 
     if ![runtest_file_p $runtests $name] then {
@@ -139,6 +142,7 @@ proc run_lookup_test { name } {
     set opts(name) {}
     set opts(source) {}
     set opts(xfail) {}
+    set opts(no_cross) {}
 
     foreach i $opt_array {
 	set opt_name [lindex $i 0]
@@ -156,6 +160,12 @@ proc run_lookup_test { name } {
 	set opts($opt_name) [concat $opts($opt_name) $opt_val]
     }
 
+    if { [llength $opts(no_cross)] != 0
+	 && "$TEST_CROSS" eq "yes" } {
+	untested "$subdir/$name not tested when cross-compiling"
+	return
+    }
+
     if { [llength $opts(lookup)] == 0 } {
 	set opts(lookup) "$file.c"
     } else {
diff --git a/libctf/testsuite/libctf-lookup/unnamed-field-info.lk b/libctf/testsuite/libctf-lookup/unnamed-field-info.lk
index eae6a517d50..680b8e0a4ca 100644
--- a/libctf/testsuite/libctf-lookup/unnamed-field-info.lk
+++ b/libctf/testsuite/libctf-lookup/unnamed-field-info.lk
@@ -1,2 +1,3 @@
 # source: unnamed-field-info-ctf.c
+# no_cross: yes
 Offset validation complete.
-- 
2.39.1.268.g9de2f9a303


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/3] libctf: propagate errors from parents correctly
  2023-04-07 22:04 [PATCH 1/3] libctf, tests: do not assume host and target have identical field offsets Nick Alcock
@ 2023-04-07 22:04 ` Nick Alcock
  2023-04-07 22:04 ` [PATCH 3/3] libctf, link: fix CU-mapped links with CTF_LINK_EMPTY_CU_MAPPINGS Nick Alcock
  1 sibling, 0 replies; 3+ messages in thread
From: Nick Alcock @ 2023-04-07 22:04 UTC (permalink / raw)
  To: binutils

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                           |   4 +-
 libctf/ctf-lookup.c                           |  19 +-
 libctf/ctf-types.c                            |  29 +++-
 .../libctf-writable/error-propagation.c       | 164 ++++++++++++++++++
 .../libctf-writable/error-propagation.lk      |   1 +
 5 files changed, 204 insertions(+), 13 deletions(-)
 create mode 100644 libctf/testsuite/libctf-writable/error-propagation.c
 create mode 100644 libctf/testsuite/libctf-writable/error-propagation.lk

diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index 7a3b3078dba..6b342dc64a2 100644
--- a/libctf/ctf-create.c
+++ b/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)
diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
index 950c0a809ac..c65849118cb 100644
--- a/libctf/ctf-lookup.c
+++ b/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);
diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
index dd82053e1d7..c20ff825d9a 100644
--- a/libctf/ctf-types.c
+++ b/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),
diff --git a/libctf/testsuite/libctf-writable/error-propagation.c b/libctf/testsuite/libctf-writable/error-propagation.c
new file mode 100644
index 00000000000..9d25e635c85
--- /dev/null
+++ b/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;
+
+}
diff --git a/libctf/testsuite/libctf-writable/error-propagation.lk b/libctf/testsuite/libctf-writable/error-propagation.lk
new file mode 100644
index 00000000000..b944f73d013
--- /dev/null
+++ b/libctf/testsuite/libctf-writable/error-propagation.lk
@@ -0,0 +1 @@
+All done.
-- 
2.39.1.268.g9de2f9a303


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 3/3] libctf, link: fix CU-mapped links with CTF_LINK_EMPTY_CU_MAPPINGS
  2023-04-07 22:04 [PATCH 1/3] libctf, tests: do not assume host and target have identical field offsets Nick Alcock
  2023-04-07 22:04 ` [PATCH 2/3] libctf: propagate errors from parents correctly Nick Alcock
@ 2023-04-07 22:04 ` Nick Alcock
  1 sibling, 0 replies; 3+ messages in thread
From: Nick Alcock @ 2023-04-07 22:04 UTC (permalink / raw)
  To: binutils

This is a bug in the intersection of two obscure options that cannot
even be invoked from ld with a feature added to stop ld of the
same input file repeatedly from crashing the linker.

The latter fix involved tracking input files (internally to libctf) not
just with their input CU name but with a version of their input CU name
that was augmented with a numeric prefix if their linker input file name
was changed, to prevent distinct CTF dicts with the same cuname from
overwriting each other. (We can't use just the linker input file name
because one linker input can contain many CU dicts, particularly under
ld -r).  If these inputs then produced conflicting types, those types
were emitted into similarly-named output dicts, so we needed similar
machinery to detect clashing output dicts and add a numeric prefix to
them as well.

This works fine, except that if you used the cu-mapping feature to force
double-linking of CTF (so that your CTF can be grouped into output dicts
larger than a single translation unit) and then also used
CTF_LINK_EMPTY_CU_MAPPINGS to force every possible output dict in the
mapping to be created (even if empty), we did the creation of empty dicts
first, and then all the actual content got considered to be a clash. So
you ended up with a pile of useless empty dicts and then all the content
was in full dicts with the same names suffixed with a #0.  This seems
likely to confuse consumers that use this facility.

Fixed by generating all the EMPTY_CU_MAPPINGS empty dicts after linking
is complete, not before it runs.

No impact on ld, which does not do cu-mapped links or pass
CTF_LINK_EMPTY_CU_MAPPINGS to ctf_link().

libctf/
	* ctf-link.c (ctf_create_per_cu): Don't create new dicts iff one
        already exists and we are making one for no input in particular.
        (ctf_link): Emit empty CTF dicts corresponding to no input in
        particular only after linkiing is complete.
---
 libctf/ctf-link.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 016bce5f6d6..9babec2aa37 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -321,12 +321,12 @@ ctf_create_per_cu (ctf_dict_t *fp, ctf_dict_t *input, const char *cu_name)
   if (ctf_name == NULL)
     ctf_name = cu_name;
 
-  /* Look up the per-CU dict.  If we don't know of one, or it is for
-     a different input CU which just happens to have the same name,
-     create a new one.  */
+  /* Look up the per-CU dict.  If we don't know of one, or it is for a different input
+     CU which just happens to have the same name, create a new one.  If we are creating
+     a dict with no input specified, anything will do.  */
 
   if ((cu_fp = ctf_dynhash_lookup (fp->ctf_link_outputs, ctf_name)) == NULL
-      || cu_fp->ctf_link_in_out != fp)
+      || (input && cu_fp->ctf_link_in_out != fp))
     {
       int err;
 
@@ -1505,11 +1505,17 @@ ctf_link (ctf_dict_t *fp, int flags)
   if (fp->ctf_link_outputs == NULL)
     return ctf_set_errno (fp, ENOMEM);
 
+  fp->ctf_flags |= LCTF_LINKING;
+  ctf_link_deduplicating (fp);
+  fp->ctf_flags &= ~LCTF_LINKING;
+
+  if ((ctf_errno (fp) != 0) && (ctf_errno (fp) != ECTF_NOCTFDATA))
+    return -1;
+
   /* Create empty CUs if requested.  We do not currently claim that multiple
      links in succession with CTF_LINK_EMPTY_CU_MAPPINGS set in some calls and
      not set in others will do anything especially sensible.  */
 
-  fp->ctf_flags |= LCTF_LINKING;
   if (fp->ctf_link_out_cu_mapping && (flags & CTF_LINK_EMPTY_CU_MAPPINGS))
     {
       ctf_next_t *i = NULL;
@@ -1535,11 +1541,6 @@ ctf_link (ctf_dict_t *fp, int flags)
 	}
     }
 
-  ctf_link_deduplicating (fp);
-
-  fp->ctf_flags &= ~LCTF_LINKING;
-  if ((ctf_errno (fp) != 0) && (ctf_errno (fp) != ECTF_NOCTFDATA))
-    return -1;
   return 0;
 }
 
-- 
2.39.1.268.g9de2f9a303


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-04-07 22:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-07 22:04 [PATCH 1/3] libctf, tests: do not assume host and target have identical field offsets Nick Alcock
2023-04-07 22:04 ` [PATCH 2/3] libctf: propagate errors from parents correctly Nick Alcock
2023-04-07 22:04 ` [PATCH 3/3] libctf, link: fix CU-mapped links with CTF_LINK_EMPTY_CU_MAPPINGS Nick Alcock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).