public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: binutils@sourceware.org
Subject: [PATCH] libctf: adding CU mappings should be idempotent
Date: Mon, 13 Nov 2023 10:43:48 +0000	[thread overview]
Message-ID: <20231113104348.90526-1-nick.alcock@oracle.com> (raw)

When CTF finds conflicting types, it usually shoves each definition
into a CTF dictionary named after the compilation unit.

The intent of the obscure "cu-mapped link" feature is to allow you to
implement custom linkers that shove the definitions into other, more
coarse-grained units (say, one per kernel module, even if a module consists
of more than one compilation unit): conflicting types within one of these
larger components are hidden from name lookup so you can only look up (an
arbitrary one of) them by name, but can still be found by chasing type graph
links and are still fully deduplicated.

You do this by calling
ctf_link_add_cu_mapping (fp, "CU name", "bigger lump name"), repeatedly,
with different "CU name"s: the ctf_link() following that will put all
conflicting types found in "CU name"s sharing a "bigger lump name" into a
child dict in an archive member named "bigger lump name".

So it's clear enough what happens if you call it repeatedly with the same
"bigger lump name" more than once, because that's the whole point of it: but
what if you call it with the same "CU name" repeatedly?

ctf_link_add_cu_mapping (fp, "CU name", "bigger lump name");
ctf_link_add_cu_mapping (fp, "CU name", "other name");

This is meant to be the same as just doing the second of these, as if the
first was never called.  Alas, this isn't what happens, and what you get is
instead a bit of an inconsistent mess: more or less, the first takes
precedence, which is the exact opposite of what we wanted.

Fix this to work the right way round.

(I plan to add support for CU-mapped links to GNU ld, mainly so that we can
properly *test* this machinery.)

libctf/ChangeLog:

	* ctf-link.c (ctf_create_per_cu): Note the behaviour of
	repeatedly adding FROMs.
	(ctf_link_add_cu_mapping): Implement that behavour.
---
 libctf/ctf-link.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

All these little cu-mapping bugs that keep turning up are a definite sign that
we need a way to do cu-mapped links in ld, so we can properly test it if
nothing else -- but that means we need a way to communicate a many-source-
file-to-one-output-name mapping to ld.  No existing file really seems
suitable (certainly not a linker script).  To add extra fun, just like section
names, the source file names here are not filenames or archive member names
but the CU names written by the compiler into the CTF header.

I was thinking of accepting some new file, as an argument to a new option,
perhaps --ctf-map-cu-names=FILE, where the FILE is of the format

output-dict-name: input-1.o input-2.o input-3.o

(where input-{1,2,3}.o are object files in the link, and the usual format
for members of archives is also accepted -- so the linker at least ensures
that it's easy to get the cunames right).  This would cause the ctf_cuname()
of each member to be looked up in the inputs' CTF and
ctf_link_add_cu_mapping() to be called three times:

 ctf_link_cu_mapping (output, ctf_cuname (input-1-ctf), "output-dict-name");
 ctf_link_cu_mapping (output, ctf_cuname (input-2-ctf), "output-dict-name");
 ctf_link_cu_mapping (output, ctf_cuname (input-3-ctf), "output-dict-name");

That should be fairly easy to tie into the testsuite/ld-ctf tests.

diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 27d11c97893..ac330629fc8 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -361,7 +361,8 @@ ctf_create_per_cu (ctf_dict_t *fp, ctf_dict_t *input, const char *cu_name)
 
 /* Add a mapping directing that the CU named FROM should have its
    conflicting/non-duplicate types (depending on link mode) go into a dict
-   named TO.  Many FROMs can share a TO.
+   named TO.  Many FROMs can share a TO, but adding the same FROM with
+   a different TO will replace the old mapping.
 
    We forcibly add a dict named TO in every case, even though it may well
    wind up empty, because clients that use this facility usually expect to find
@@ -371,7 +372,7 @@ int
 ctf_link_add_cu_mapping (ctf_dict_t *fp, const char *from, const char *to)
 {
   int err;
-  char *f = NULL, *t = NULL;
+  char *f = NULL, *t = NULL, *existing;
   ctf_dynhash_t *one_out;
 
   /* Mappings cannot be set up if per-CU output dicts already exist.  */
@@ -393,6 +394,19 @@ ctf_link_add_cu_mapping (ctf_dict_t *fp, const char *from, const char *to)
   if (fp->ctf_link_out_cu_mapping == NULL)
     goto oom;
 
+  /* If this FROM already exists, remove the mapping from both the FROM->TO
+     and the TO->FROM lists: the user wants to change it.  */
+
+  if ((existing = ctf_dynhash_lookup (fp->ctf_link_in_cu_mapping, from)) != NULL)
+    {
+      one_out = ctf_dynhash_lookup (fp->ctf_link_out_cu_mapping, existing);
+      if (!ctf_assert (fp, one_out))
+	return -1;				/* errno is set for us.  */
+
+      ctf_dynhash_remove (one_out, from);
+      ctf_dynhash_remove (fp->ctf_link_in_cu_mapping, from);
+    }
+
   f = strdup (from);
   t = strdup (to);
   if (!f || !t)
-- 
2.42.0.271.g85384428f1


                 reply	other threads:[~2023-11-13 10:43 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231113104348.90526-1-nick.alcock@oracle.com \
    --to=nick.alcock@oracle.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).