* [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review)
@ 2021-02-27 13:29 Nick Alcock
2021-02-27 13:29 ` [PATCH 01/10] libctf: ctf_archive_next should set the parent name consistently Nick Alcock
` (10 more replies)
0 siblings, 11 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
The time has come to drop a bunch of code debt.
Implementing the deduplicating linker involved implementing replacements for
several existing pieces of libctf machinery. In most cases I left the old
machinery around so that its other users wouldn't be disrupted by any bugs and
because I wasn't confident that I had enough of a testing set to be sure that
the new stuff worked reliably.
Courtesy of rebuilding (at last count) almost a thousand Gentoo packages with
-gctf (eschewing only the kernel, which needs special support not in Gentoo
kernels: even glibc and ld.so got the CTF treatment), I think I can say that the
new stuff works well enough that the old stuff can go. This also reduces the
amount of code that peeks directly at the guts of CTF, which is useful
preparation for the upcoming CTF format v4.
So this patch series ditches most of the _iter iterators and reimplements them
in terms of the newer _next iterators, fixing one bug in ctf_archive_next in the
process, and removes the (extremely inefficient and slow) nondeduplicating CTF
linker, simplifying ctf-link.c quite a lot. (ld has no way to invoke the
nondeduplicating linker other than an undocumented environment variable for
debugging purposes; the ctf_link API has a CTF_LINK_NONDEDUP flag to enable it
which is now a NOP, though in future, if more expensive algorithms turn out to
be beneficial in deduplicating CTF linking, it might be repurposed to mean "save
time by deduplicating less".)
This lets us replace the type mapping machinery used to determine what types on
the link input translate to what types on the link output, moving from one
designed for the nondeduplicating linker and ctf_add_type to one that directly
queries the internal data structures of the deduplicator, saving about 20% of
link time with no loss of functionality.
There are a few bugfixes here that fell out of the Gentoo rebuild, of which one
is serious: a string refcounting bug which led to us sometimes emitting the type
for one symbol and attaching it to another. Sometimes that other symbol was of
a different st_type: *that* led to the CTF linker emitting a warning and the
symtypetab section becoming irretrievably corrupted (the names and thus types of
all symbols with an index >= the index in the warning would be wrongly
computed). That fix alone should probably go into 2.36, and might need a bz
bug: I'm not sure if libctf bugs count as user-visible before the CTF-generating
GCC gets in.
That last fix may need a tiny bit of review due to a few lines of bfd changes,
admittedly in code that only libctf uses. I can verify that every case where
this bug was known to appear is fixed by this (at least one binary in each of
twelve packages out of 860 in my Gentoo world rebuild).
(As ever, I'm happy for anyone to review anything I post.)
Nick Alcock (10):
libctf: ctf_archive_next should set the parent name consistently
libctf: reimplemement many _iter iterators in terms of _next
libctf: fix ChangeLog date
libctf, include: remove the nondeduplicating CTF linker
libctf: remove reference to "unconflicted link mode".
libctf: add a deduplicator-specific type mapping table
libctf: minor error-handling fixes
libctf: fix signed/unsigned comparison confusion
libctf: free ctf_dynsyms properly
bfd, ld, libctf: skip zero-refcount strings in CTF string reporting
bfd/ChangeLog | 4 +
bfd/elf-strtab.c | 2 +
include/ChangeLog | 5 +
include/ctf-api.h | 3 +-
ld/ChangeLog | 4 +
ld/ldelfgen.c | 16 +-
libctf/ChangeLog | 108 +++++++-
libctf/ctf-archive.c | 70 ++----
libctf/ctf-create.c | 111 ++++++++-
libctf/ctf-dedup.c | 215 ++++++++--------
libctf/ctf-dump.c | 4 +-
libctf/ctf-impl.h | 17 +-
libctf/ctf-link.c | 577 ++++++++++---------------------------------
libctf/ctf-types.c | 115 ++++-----
14 files changed, 550 insertions(+), 701 deletions(-)
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 01/10] libctf: ctf_archive_next should set the parent name consistently
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 02/10] libctf: reimplemement many _iter iterators in terms of _next Nick Alcock
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
The top level of CTF containers is a "CTF archive", which contains a
collection of named members (each a CTF dictionary). In the serialized
file format, this is optional and skipped if the archive would have only
one member, as when no ambiguous types are present: so it is commonplace
to have a simple ctf_dict_t written out, with no archive container
wrapped around it.
But, unlike ctf_archive_iter, ctf_archive_next didn't quite handle this
case right. It should set the name of this fake "member" to
_CTF_SECTION, i.e. ".ctf", but it was failing to do so, so callers got
an unintialized variable back instead and were understandably confused.
So set the name properly.
libctf/ChangeLog
2021-02-18 Nick Alcock <nick.alcock@oracle.com>
* ctf-archive.c (ctf_archive_next): Set the name of parents in
single-member archives.
---
libctf/ChangeLog | 5 +++++
libctf/ctf-archive.c | 2 ++
2 files changed, 7 insertions(+)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index da27a8fed6b..d8857301388 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-18 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-archive.c (ctf_archive_next): Set the name of parents in
+ single-member archives.
+
2021-02-26 Alan Modra <amodra@gmail.com>
* Makefile.in: Regenerate.
diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index 6d9c75c9013..6e1bf151f1a 100644
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -1165,6 +1165,8 @@ ctf_archive_next (const ctf_archive_t *wrapper, ctf_next_t **it, const char **na
if (!skip_parent)
{
wrapper->ctfi_dict->ctf_refcnt++;
+ if (name)
+ *name = _CTF_SECTION;
return wrapper->ctfi_dict;
}
}
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 02/10] libctf: reimplemement many _iter iterators in terms of _next
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
2021-02-27 13:29 ` [PATCH 01/10] libctf: ctf_archive_next should set the parent name consistently Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 03/10] libctf: fix ChangeLog date Nick Alcock
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
Ever since the generator-style _next iterators were introduced, there
have been separate implementations of the functional-style _iter
iterators that do the same thing as _next.
This is annoying and adds more dependencies on the internal guts of the
file format. Rip them all out and replace them with the corresponding
_next iterators. Only ctf_archive_raw_iter and ctf_label_iter survive,
the former because there is no access to the raw binary data of archives
via any _next iterator, and the latter because ctf_label_next hasn't
been implemented (because labels are currently not used for anything).
Tested by reverting the change (already applied) that reimplemented
ctf_member_iter in terms of ctf_member_next, then verifying that the
_iter and _next iterators produced the same results for every iterable
entity within a large type archive.
libctf/ChangeLog
2021-02-18 Nick Alcock <nick.alcock@oracle.com>
* ctf-types.c (ctf_member_iter): Move 'rc' to an inner scope.
(ctf_enum_iter): Reimplement in terms of ctf_enum_next.
(ctf_type_iter): Reimplement in terms of ctf_type_next.
(ctf_type_iter_all): Likewise.
(ctf_variable_iter): Reimplement in terms of ctf_variable_next.
* ctf-archive.c (ctf_archive_iter_internal): Remove.
(ctf_archive_iter): Reimplement in terms of ctf_archive_next.
---
libctf/ChangeLog | 10 ++++
libctf/ctf-archive.c | 68 ++++++-------------------
libctf/ctf-types.c | 115 +++++++++++++++++--------------------------
3 files changed, 69 insertions(+), 124 deletions(-)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index d8857301388..2d7b846fbb1 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,13 @@
+2021-02-18 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-types.c (ctf_member_iter): Move 'rc' to an inner scope.
+ (ctf_enum_iter): Reimplement in terms of ctf_enum_next.
+ (ctf_type_iter): Reimplement in terms of ctf_type_next.
+ (ctf_type_iter_all): Likewise.
+ (ctf_variable_iter): Reimplement in terms of ctf_variable_next.
+ * ctf-archive.c (ctf_archive_iter_internal): Remove.
+ (ctf_archive_iter): Reimplement in terms of ctf_archive_next.
+
2021-02-18 Nick Alcock <nick.alcock@oracle.com>
* ctf-archive.c (ctf_archive_next): Set the name of parents in
diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index 6e1bf151f1a..8b8e170241f 100644
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -1043,70 +1043,32 @@ ctf_archive_raw_iter (const ctf_archive_t *arc,
return -EINVAL; /* Not supported. */
}
-/* Iterate over all CTF files in an archive. We pass all CTF files in turn to
- the specified callback function. */
-static int
-ctf_archive_iter_internal (const ctf_archive_t *wrapper,
- const struct ctf_archive *arc,
- const ctf_sect_t *symsect,
- const ctf_sect_t *strsect,
- ctf_archive_member_f *func, void *data)
+/* Iterate over all CTF files in an archive: public entry point. We pass all
+ CTF files in turn to the specified callback function. */
+int
+ctf_archive_iter (const ctf_archive_t *arc, ctf_archive_member_f *func,
+ void *data)
{
- int rc;
- size_t i;
- ctf_dict_t *f;
- struct ctf_archive_modent *modent;
- const char *nametbl;
-
- modent = (ctf_archive_modent_t *) ((char *) arc
- + sizeof (struct ctf_archive));
- nametbl = (((const char *) arc) + le64toh (arc->ctfa_names));
+ ctf_next_t *i = NULL;
+ ctf_dict_t *fp;
+ const char *name;
+ int err;
- for (i = 0; i < le64toh (arc->ctfa_ndicts); i++)
+ while ((fp = ctf_archive_next (arc, &i, &name, 0, &err)) != NULL)
{
- const char *name;
+ int rc;
- name = &nametbl[le64toh (modent[i].name_offset)];
- if ((f = ctf_dict_open_internal (arc, symsect, strsect,
- name,
- wrapper->ctfi_symsect_little_endian,
- &rc)) == NULL)
- return rc;
-
- f->ctf_archive = (ctf_archive_t *) wrapper;
- ctf_arc_import_parent (wrapper, f);
- if ((rc = func (f, name, data)) != 0)
+ if ((rc = func (fp, name, data)) != 0)
{
- ctf_dict_close (f);
+ ctf_dict_close (fp);
+ ctf_next_destroy (i);
return rc;
}
-
- ctf_dict_close (f);
+ ctf_dict_close (fp);
}
return 0;
}
-/* Iterate over all CTF files in an archive: public entry point. We pass all
- CTF files in turn to the specified callback function. */
-int
-ctf_archive_iter (const ctf_archive_t *arc, ctf_archive_member_f *func,
- void *data)
-{
- const ctf_sect_t *symsect = &arc->ctfi_symsect;
- const ctf_sect_t *strsect = &arc->ctfi_strsect;
-
- if (symsect->cts_name == NULL)
- symsect = NULL;
- if (strsect->cts_name == NULL)
- strsect = NULL;
-
- if (arc->ctfi_is_archive)
- return ctf_archive_iter_internal (arc, arc->ctfi_archive, symsect, strsect,
- func, data);
-
- return func (arc->ctfi_dict, _CTF_SECTION, data);
-}
-
/* Iterate over all CTF files in an archive, returning each dict in turn as a
ctf_dict_t, and NULL on error or end of iteration. It is the caller's
responsibility to close it. Parent dicts may be skipped.
diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
index 57a284d82e7..28c5c7aa1e1 100644
--- a/libctf/ctf-types.c
+++ b/libctf/ctf-types.c
@@ -45,10 +45,10 @@ ctf_member_iter (ctf_dict_t *fp, ctf_id_t type, ctf_member_f *func, void *arg)
ssize_t offset;
const char *name;
ctf_id_t membtype;
- int rc;
while ((offset = ctf_member_next (fp, type, &i, &name, &membtype, 0)) >= 0)
{
+ int rc;
if ((rc = func (name, membtype, offset, arg)) != 0)
{
ctf_next_destroy (i);
@@ -255,47 +255,21 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
int
ctf_enum_iter (ctf_dict_t *fp, ctf_id_t type, ctf_enum_f *func, void *arg)
{
- ctf_dict_t *ofp = fp;
- const ctf_type_t *tp;
- const ctf_enum_t *ep;
- ctf_dtdef_t *dtd;
- ssize_t increment;
- uint32_t n;
- int rc;
-
- if ((type = ctf_type_resolve_unsliced (fp, type)) == CTF_ERR)
- return -1; /* errno is set for us. */
-
- if ((tp = ctf_lookup_by_id (&fp, type)) == NULL)
- return -1; /* errno is set for us. */
-
- if (LCTF_INFO_KIND (fp, tp->ctt_info) != CTF_K_ENUM)
- return (ctf_set_errno (ofp, ECTF_NOTENUM));
-
- (void) ctf_get_ctt_size (fp, tp, NULL, &increment);
+ ctf_next_t *i = NULL;
+ const char *name;
+ int val;
- if ((dtd = ctf_dynamic_type (ofp, type)) == NULL)
+ while ((name = ctf_enum_next (fp, type, &i, &val)) != NULL)
{
- ep = (const ctf_enum_t *) ((uintptr_t) tp + increment);
-
- for (n = LCTF_INFO_VLEN (fp, tp->ctt_info); n != 0; n--, ep++)
+ int rc;
+ if ((rc = func (name, val, arg)) != 0)
{
- const char *name = ctf_strptr (fp, ep->cte_name);
- if ((rc = func (name, ep->cte_value, arg)) != 0)
- return rc;
- }
- }
- else
- {
- ctf_dmdef_t *dmd;
-
- for (dmd = ctf_list_next (&dtd->dtd_u.dtu_members);
- dmd != NULL; dmd = ctf_list_next (dmd))
- {
- if ((rc = func (dmd->dmd_name, dmd->dmd_value, arg)) != 0)
- return rc;
+ ctf_next_destroy (i);
+ return rc;
}
}
+ if (ctf_errno (fp) != ECTF_NEXT_END)
+ return -1; /* errno is set for us. */
return 0;
}
@@ -424,16 +398,20 @@ ctf_enum_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
int
ctf_type_iter (ctf_dict_t *fp, ctf_type_f *func, void *arg)
{
- ctf_id_t id, max = fp->ctf_typemax;
- int rc, child = (fp->ctf_flags & LCTF_CHILD);
+ ctf_next_t *i = NULL;
+ ctf_id_t type;
- for (id = 1; id <= max; id++)
+ while ((type = ctf_type_next (fp, &i, NULL, 0)) != CTF_ERR)
{
- const ctf_type_t *tp = LCTF_INDEX_TO_TYPEPTR (fp, id);
- if (LCTF_INFO_ISROOT (fp, tp->ctt_info)
- && (rc = func (LCTF_INDEX_TO_TYPE (fp, id, child), arg)) != 0)
- return rc;
+ int rc;
+ if ((rc = func (type, arg)) != 0)
+ {
+ ctf_next_destroy (i);
+ return rc;
+ }
}
+ if (ctf_errno (fp) != ECTF_NEXT_END)
+ return -1; /* errno is set for us. */
return 0;
}
@@ -448,17 +426,21 @@ ctf_type_iter (ctf_dict_t *fp, ctf_type_f *func, void *arg)
int
ctf_type_iter_all (ctf_dict_t *fp, ctf_type_all_f *func, void *arg)
{
- ctf_id_t id, max = fp->ctf_typemax;
- int rc, child = (fp->ctf_flags & LCTF_CHILD);
+ ctf_next_t *i = NULL;
+ ctf_id_t type;
+ int flag;
- for (id = 1; id <= max; id++)
+ while ((type = ctf_type_next (fp, &i, &flag, 1)) != CTF_ERR)
{
- const ctf_type_t *tp = LCTF_INDEX_TO_TYPEPTR (fp, id);
- if ((rc = func (LCTF_INDEX_TO_TYPE (fp, id, child),
- LCTF_INFO_ISROOT(fp, tp->ctt_info)
- ? CTF_ADD_ROOT : CTF_ADD_NONROOT, arg) != 0))
- return rc;
+ int rc;
+ if ((rc = func (type, flag, arg)) != 0)
+ {
+ ctf_next_destroy (i);
+ return rc;
+ }
}
+ if (ctf_errno (fp) != ECTF_NEXT_END)
+ return -1; /* errno is set for us. */
return 0;
}
@@ -518,30 +500,21 @@ ctf_type_next (ctf_dict_t *fp, ctf_next_t **it, int *flag, int want_hidden)
int
ctf_variable_iter (ctf_dict_t *fp, ctf_variable_f *func, void *arg)
{
- int rc;
-
- if ((fp->ctf_flags & LCTF_CHILD) && (fp->ctf_parent == NULL))
- return (ctf_set_errno (fp, ECTF_NOPARENT));
+ ctf_next_t *i = NULL;
+ ctf_id_t type;
+ const char *name;
- if (!(fp->ctf_flags & LCTF_RDWR))
+ while ((type = ctf_variable_next (fp, &i, &name)) != CTF_ERR)
{
- unsigned long i;
- for (i = 0; i < fp->ctf_nvars; i++)
- if ((rc = func (ctf_strptr (fp, fp->ctf_vars[i].ctv_name),
- fp->ctf_vars[i].ctv_type, arg)) != 0)
- return rc;
- }
- else
- {
- ctf_dvdef_t *dvd;
-
- for (dvd = ctf_list_next (&fp->ctf_dvdefs); dvd != NULL;
- dvd = ctf_list_next (dvd))
+ int rc;
+ if ((rc = func (name, type, arg)) != 0)
{
- if ((rc = func (dvd->dvd_name, dvd->dvd_type, arg)) != 0)
- return rc;
+ ctf_next_destroy (i);
+ return rc;
}
}
+ if (ctf_errno (fp) != ECTF_NEXT_END)
+ return -1; /* errno is set for us. */
return 0;
}
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 03/10] libctf: fix ChangeLog date
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
2021-02-27 13:29 ` [PATCH 01/10] libctf: ctf_archive_next should set the parent name consistently Nick Alcock
2021-02-27 13:29 ` [PATCH 02/10] libctf: reimplemement many _iter iterators in terms of _next Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 04/10] libctf, include: remove the nondeduplicating CTF linker Nick Alcock
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
I pushed this change without fixing up the date by mistake.
---
libctf/ChangeLog | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 2d7b846fbb1..0fc8e3caafd 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -24,7 +24,7 @@
* configure: Regenerate.
* Makefile.in: Regenerate.
-2021-02-17 Nick Alcock <nick.alcock@oracle.com>
+2021-02-20 Nick Alcock <nick.alcock@oracle.com>
* ctf-impl.h (ctf_dict_t) <ctf_symhash>: New.
<ctf_symhash_latest>: Likewise.
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 04/10] libctf, include: remove the nondeduplicating CTF linker
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (2 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 03/10] libctf: fix ChangeLog date Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 05/10] libctf: remove reference to "unconflicted link mode" Nick Alcock
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
The nondeduplicating CTF linker was kept around when the deduplicating
one was added so that people had something to fall back to in case the
deduplicating linker turned out to be buggy. It's now much more stable
than the nondeduplicating linker, in addition to much faster, using much
less memory and producing much better output. In addition, while
libctf has a linker flag to invoke the nondeduplicating linker, ld does
not expose it: the only way to turn it on within ld is an intentionally-
undocumented environment variable. So we can remove it without any ABI
or user-visibility concerns (the only thing we leave around is the
CTF_LINK_NONDEDUP flag, which can easily be interpreted as "deduplicate
less", though right now it does nothing).
This lets us remove a lot of complexity associated with tracking
filenames and CU names separately (something the deduplcating linker
never bothered with, since the cunames are always reliable and ld never
hands us useful filenames anyway)
The biggest lacuna left behind is the ctf_type_mapping machinery, which
slows down deduplicating links quite a lot. We can't just ditch it
because ctf_add_type uses it: removing the slowdown from the
deduplicating linker is a job for another commit.
include/ChangeLog
2021-02-23 Nick Alcock <nick.alcock@oracle.com>
* ctf-api.h (CTF_LINK_SHARE_DUPLICATED): Note that this might
merely change how much deduplication is done.
libctf/ChangeLog
2021-02-23 Nick Alcock <nick.alcock@oracle.com>
* ctf-link.c (ctf_create_per_cu): Drop FILENAME now that it is
always identical to CUNAME.
(ctf_link_deduplicating_one_symtypetab): Adjust.
(ctf_link_one_type): Remove.
(ctf_link_one_input_archive_member): Likewise.
(ctf_link_close_one_input_archive): Likewise.
(ctf_link_one_input_archive): Likewise.
(ctf_link): No longer call it. Drop CTF_LINK_NONDEDUP path.
Improve header comment a bit (dicts, not files). Adjust
ctf_create_per_cu call.
(ctf_link_deduplicating_variables): Simplify.
(ctf_link_in_member_cb_arg_t) <cu_name>: Remove.
<in_input_cu_file>: Likewise.
<in_fp_parent>: Likewise.
<done_parent>: Likewise.
(ctf_link_one_variable): Turn uses of in_file_name to in_cuname.
---
include/ChangeLog | 5 +
include/ctf-api.h | 3 +-
libctf/ChangeLog | 19 ++++
libctf/ctf-link.c | 250 ++++------------------------------------------
4 files changed, 46 insertions(+), 231 deletions(-)
diff --git a/include/ChangeLog b/include/ChangeLog
index 616b923b53b..5f081bb3876 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-23 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-api.h (CTF_LINK_SHARE_DUPLICATED): Note that this might
+ merely change how much deduplication is done.
+
2021-02-21 Alan Modra <amodra@gmail.com>
* bfdlink.h (struct bfd_link_info): Add warn_multiple_definition.
diff --git a/include/ctf-api.h b/include/ctf-api.h
index ce764dff5e3..25dbe9ec533 100644
--- a/include/ctf-api.h
+++ b/include/ctf-api.h
@@ -94,7 +94,8 @@ typedef struct ctf_link_sym
/* Share only types that are used by multiple inputs. */
#define CTF_LINK_SHARE_DUPLICATED 0x1
-/* Do a nondeduplicating link. */
+/* Do a nondeduplicating link, or otherwise deduplicate "less hard", trading off
+ CTF output size for link time. */
#define CTF_LINK_NONDEDUP 0x2
/* Create empty outputs for all registered CU mappings even if no types are
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 0fc8e3caafd..0f84eea9533 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,22 @@
+2021-02-23 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-link.c (ctf_create_per_cu): Drop FILENAME now that it is
+ always identical to CUNAME.
+ (ctf_link_deduplicating_one_symtypetab): Adjust.
+ (ctf_link_one_type): Remove.
+ (ctf_link_one_input_archive_member): Likewise.
+ (ctf_link_close_one_input_archive): Likewise.
+ (ctf_link_one_input_archive): Likewise.
+ (ctf_link): No longer call it. Drop CTF_LINK_NONDEDUP path.
+ Improve header comment a bit (dicts, not files). Adjust
+ ctf_create_per_cu call.
+ (ctf_link_deduplicating_variables): Simplify.
+ (ctf_link_in_member_cb_arg_t) <cu_name>: Remove.
+ <in_input_cu_file>: Likewise.
+ <in_fp_parent>: Likewise.
+ <done_parent>: Likewise.
+ (ctf_link_one_variable): Turn uses of in_file_name to in_cuname.
+
2021-02-18 Nick Alcock <nick.alcock@oracle.com>
* ctf-types.c (ctf_member_iter): Move 'rc' to an inner scope.
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 7c342166b5c..5d813dbf8b3 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -280,29 +280,24 @@ ctf_link_add_ctf (ctf_dict_t *fp, ctf_archive_t *ctf, const char *name)
interning it if need be. */
static ctf_dict_t *
-ctf_create_per_cu (ctf_dict_t *fp, const char *filename, const char *cuname)
+ctf_create_per_cu (ctf_dict_t *fp, const char *cu_name)
{
ctf_dict_t *cu_fp;
const char *ctf_name = NULL;
char *dynname = NULL;
/* First, check the mapping table and translate the per-CU name we use
- accordingly. We check both the input filename and the CU name. Only if
- neither are set do we fall back to the input filename as the per-CU
- dictionary name. We prefer the filename because this is easier for likely
- callers to determine. */
+ accordingly. */
if (fp->ctf_link_in_cu_mapping)
{
- if (((ctf_name = ctf_dynhash_lookup (fp->ctf_link_in_cu_mapping,
- filename)) == NULL) &&
- ((ctf_name = ctf_dynhash_lookup (fp->ctf_link_in_cu_mapping,
- cuname)) == NULL))
- ctf_name = filename;
+ if ((ctf_name = ctf_dynhash_lookup (fp->ctf_link_in_cu_mapping,
+ cu_name)) == NULL)
+ ctf_name = cu_name;
}
if (ctf_name == NULL)
- ctf_name = filename;
+ ctf_name = cu_name;
if ((cu_fp = ctf_dynhash_lookup (fp->ctf_link_outputs, ctf_name)) == NULL)
{
@@ -311,8 +306,7 @@ ctf_create_per_cu (ctf_dict_t *fp, const char *filename, const char *cuname)
if ((cu_fp = ctf_create (&err)) == NULL)
{
ctf_err_warn (fp, 0, err, _("cannot create per-CU CTF archive for "
- "CU %s from input file %s"),
- cuname, filename);
+ "input CU %s"), cu_name);
ctf_set_errno (fp, err);
return NULL;
}
@@ -323,7 +317,7 @@ ctf_create_per_cu (ctf_dict_t *fp, const char *filename, const char *cuname)
goto oom;
ctf_import_unref (cu_fp, fp);
- ctf_cuname_set (cu_fp, cuname);
+ ctf_cuname_set (cu_fp, cu_name);
ctf_parent_name_set (cu_fp, _CTF_SECTION);
}
return cu_fp;
@@ -440,93 +434,16 @@ typedef struct ctf_link_in_member_cb_arg
/* The shared output dictionary. */
ctf_dict_t *out_fp;
- /* The filename of the input file, and an fp to each dictionary in that file
+ /* The cuname of the input file, and an fp to each dictionary in that file
in turn. */
- const char *in_file_name;
+ const char *in_cuname;
ctf_dict_t *in_fp;
- /* The CU name of the dict being processed. */
- const char *cu_name;
- int in_input_cu_file;
-
- /* The parent dictionary in the input, and whether it's been processed yet.
- Not needed by ctf_link_one_type / ctf_link_one_variable, only by higher
- layers. */
- ctf_dict_t *in_fp_parent;
- int done_parent;
-
/* If true, this is the CU-mapped portion of a deduplicating link: no child
dictionaries should be created. */
int cu_mapped;
} ctf_link_in_member_cb_arg_t;
-/* Link one type into the link. We rely on ctf_add_type() to detect
- duplicates. This is not terribly reliable yet (unnmamed types will be
- mindlessly duplicated), but will improve shortly. */
-
-static int
-ctf_link_one_type (ctf_id_t type, int isroot _libctf_unused_, void *arg_)
-{
- ctf_link_in_member_cb_arg_t *arg = (ctf_link_in_member_cb_arg_t *) arg_;
- ctf_dict_t *per_cu_out_fp;
- int err;
-
- if (arg->in_fp->ctf_link_flags != CTF_LINK_SHARE_UNCONFLICTED)
- {
- ctf_err_warn (arg->out_fp, 0, ECTF_NOTYET,
- _("share-duplicated mode not yet implemented"));
- return ctf_set_errno (arg->out_fp, ECTF_NOTYET);
- }
-
- /* Simply call ctf_add_type: if it reports a conflict and we're adding to the
- main CTF file, add to the per-CU archive member instead, creating it if
- necessary. If we got this type from a per-CU archive member, add it
- straight back to the corresponding member in the output. */
-
- if (!arg->in_input_cu_file)
- {
- if (ctf_add_type (arg->out_fp, arg->in_fp, type) != CTF_ERR)
- return 0;
-
- err = ctf_errno (arg->out_fp);
- if (err != ECTF_CONFLICT)
- {
- if (err != ECTF_NONREPRESENTABLE)
- ctf_err_warn (arg->out_fp, 1, 0,
- _("cannot link type %lx from input file %s, CU %s "
- "into output link"), type, arg->cu_name,
- arg->in_file_name);
- /* We must ignore this problem or we end up losing future types, then
- trying to link the variables in, then exploding. Better to link as
- much as possible. */
- return 0;
- }
- ctf_set_errno (arg->out_fp, 0);
- }
-
- if ((per_cu_out_fp = ctf_create_per_cu (arg->out_fp, arg->in_file_name,
- arg->cu_name)) == NULL)
- return -1; /* Errno is set for us. */
-
- if (ctf_add_type (per_cu_out_fp, arg->in_fp, type) != CTF_ERR)
- return 0;
-
- err = ctf_errno (per_cu_out_fp);
- if (err != ECTF_NONREPRESENTABLE)
- ctf_err_warn (arg->out_fp, 1, 0,
- _("cannot link type %lx from input file %s, CU %s "
- "into output per-CU CTF archive member %s: %s: skipped"),
- type, ctf_link_input_name (arg->in_fp), arg->in_file_name,
- ctf_link_input_name (per_cu_out_fp), ctf_errmsg (err));
- if (err == ECTF_CONFLICT)
- /* Conflicts are possible at this stage only if a non-ld user has combined
- multiple TUs into a single output dictionary. Even in this case we do not
- want to stop the link or propagate the error. */
- ctf_set_errno (arg->out_fp, 0);
-
- return 0; /* As above: do not lose types. */
-}
-
/* Set a function which is used to filter out unwanted variables from the link. */
int
ctf_link_set_variable_filter (ctf_dict_t *fp, ctf_link_variable_filter_f *filter,
@@ -615,13 +532,12 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
if (arg->cu_mapped)
{
ctf_dprintf ("Variable %s in input file %s depends on a type %lx hidden "
- "due to conflicts: skipped.\n", name, arg->in_file_name,
+ "due to conflicts: skipped.\n", name, arg->in_cuname,
type);
return 0;
}
- if ((per_cu_out_fp = ctf_create_per_cu (arg->out_fp, arg->in_file_name,
- arg->cu_name)) == NULL)
+ if ((per_cu_out_fp = ctf_create_per_cu (arg->out_fp, arg->in_cuname)) == NULL)
return -1; /* Errno is set for us. */
/* If the type was not found, check for it in the child too. */
@@ -635,7 +551,7 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
ctf_err_warn (arg->out_fp, 1, 0,
_("type %lx for variable %s in input file %s "
"not found: skipped"), type, name,
- arg->in_file_name);
+ arg->in_cuname);
/* Do not terminate the link: just skip the variable. */
return 0;
}
@@ -647,55 +563,6 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
return 0;
}
-/* Merge every type (and optionally, variable) in this archive member into the
- link, so we can relink things that have already had ld run on them. We use
- the archive member name, sans any leading '.ctf.', as the CU name for
- ambiguous types if there is one and it's not the default: otherwise, we use
- the name of the input file. */
-static int
-ctf_link_one_input_archive_member (ctf_dict_t *in_fp, const char *name, void *arg_)
-{
- ctf_link_in_member_cb_arg_t *arg = (ctf_link_in_member_cb_arg_t *) arg_;
- int err = 0;
-
- if (strcmp (name, _CTF_SECTION) == 0)
- {
- /* This file is the default member of this archive, and has already been
- explicitly processed.
-
- In the default sharing mode of CTF_LINK_SHARE_UNCONFLICTED, it does no
- harm to rescan an existing shared repo again: all the types will just
- end up in the same place. But in CTF_LINK_SHARE_DUPLICATED mode, this
- causes the system to erroneously conclude that all types are duplicated
- and should be shared, even if they are not. */
-
- if (arg->done_parent)
- return 0;
- }
- else
- {
- /* Get ambiguous types from our parent. */
- ctf_import (in_fp, arg->in_fp_parent);
- arg->in_input_cu_file = 1;
- }
-
- arg->cu_name = name;
- if (strncmp (arg->cu_name, ".ctf.", strlen (".ctf.")) == 0)
- arg->cu_name += strlen (".ctf.");
- arg->in_fp = in_fp;
-
- if ((err = ctf_type_iter_all (in_fp, ctf_link_one_type, arg)) > -1)
- if (!(in_fp->ctf_link_flags & CTF_LINK_OMIT_VARIABLES_SECTION))
- err = ctf_variable_iter (in_fp, ctf_link_one_variable, arg);
-
- arg->in_input_cu_file = 0;
-
- if (err < 0)
- return -1; /* Errno is set for us. */
-
- return 0;
-}
-
/* Dump the unnecessary link type mapping after one input file is processed. */
static void
empty_link_type_mapping (void *key _libctf_unused_, void *value,
@@ -753,73 +620,6 @@ ctf_link_lazy_open (ctf_dict_t *fp, ctf_link_input_t *input)
return (ssize_t) count;
}
-/* Close an input, as a ctf_dynhash_iter iterator. */
-static void
-ctf_link_close_one_input_archive (void *key _libctf_unused_, void *value,
- void *arg _libctf_unused_)
-{
- ctf_link_input_t *input = (ctf_link_input_t *) value;
- if (input->clin_arc)
- ctf_arc_close (input->clin_arc);
- input->clin_arc = NULL;
-}
-
-/* Link one input file's types into the output file. */
-static void
-ctf_link_one_input_archive (void *key, void *value, void *arg_)
-{
- const char *file_name = (const char *) key;
- ctf_link_input_t *input = (ctf_link_input_t *)value;
- ctf_link_in_member_cb_arg_t *arg = (ctf_link_in_member_cb_arg_t *) arg_;
- int err = 0;
-
- if (!input->clin_arc)
- {
- err = ctf_link_lazy_open (arg->out_fp, input);
- if (err == 0) /* Just no CTF. */
- return;
-
- if (err < 0)
- return; /* errno is set for us. */
- }
-
- arg->in_file_name = file_name;
- arg->done_parent = 0;
- if ((arg->in_fp_parent = ctf_dict_open (input->clin_arc,
- NULL, &err)) == NULL)
- if (err != ECTF_ARNNAME)
- {
- ctf_err_warn (arg->out_fp, 1, 0,
- _("cannot open main archive member in input file %s "
- "in the link: skipping: %s"), arg->in_file_name,
- ctf_errmsg (err));
- goto out;
- }
-
- if (ctf_link_one_input_archive_member (arg->in_fp_parent,
- _CTF_SECTION, arg) < 0)
- {
- ctf_dict_close (arg->in_fp_parent);
- goto out;
- }
- arg->done_parent = 1;
- if (ctf_archive_iter (input->clin_arc, ctf_link_one_input_archive_member,
- arg) < 0)
- ctf_err_warn (arg->out_fp, 0, 0, _("cannot traverse archive in input file "
- "%s: link cannot continue"),
- arg->in_file_name);
- else
- {
- /* The only error indication to the caller is the errno: so ensure that it
- is zero if there was no actual error from the caller. */
- ctf_set_errno (arg->out_fp, 0);
- }
- ctf_dict_close (arg->in_fp_parent);
-
- out:
- ctf_link_close_one_input_archive (key, value, NULL);
-}
-
typedef struct link_sort_inputs_cb_arg
{
int is_cu_mapped;
@@ -1130,21 +930,16 @@ ctf_link_deduplicating_variables (ctf_dict_t *fp, ctf_dict_t **inputs,
arg.cu_mapped = cu_mapped;
arg.out_fp = fp;
- arg.in_input_cu_file = 0;
for (i = 0; i < ninputs; i++)
{
arg.in_fp = inputs[i];
if (ctf_cuname (inputs[i]) != NULL)
- arg.in_file_name = ctf_cuname (inputs[i]);
+ arg.in_cuname = ctf_cuname (inputs[i]);
else
- arg.in_file_name = "unnamed-CU";
- arg.cu_name = arg.in_file_name;
+ arg.in_cuname = "unnamed-CU";
if (ctf_variable_iter (arg.in_fp, ctf_link_one_variable, &arg) < 0)
return ctf_set_errno (fp, ctf_errno (arg.in_fp));
-
- /* Outputs > 0 are per-CU. */
- arg.in_input_cu_file = 1;
}
return 0;
}
@@ -1237,8 +1032,7 @@ ctf_link_deduplicating_one_symtypetab (ctf_dict_t *fp, ctf_dict_t *input,
continue;
}
- if ((per_cu_out_fp = ctf_create_per_cu (fp, in_file_name,
- in_file_name)) == NULL)
+ if ((per_cu_out_fp = ctf_create_per_cu (fp, in_file_name)) == NULL)
return -1; /* errno is set for us. */
/* If the type was not found, check for it in the child too. */
@@ -1653,8 +1447,8 @@ ctf_link_deduplicating (ctf_dict_t *fp)
goto err;
}
-/* Merge types and variable sections in all files added to the link
- together. All the added files are closed. */
+/* Merge types and variable sections in all dicts added to the link together.
+ All the added dicts are closed. */
int
ctf_link (ctf_dict_t *fp, int flags)
{
@@ -1691,7 +1485,7 @@ ctf_link (ctf_dict_t *fp, int flags)
NULL)) == 0)
{
const char *to = (const char *) v;
- if (ctf_create_per_cu (fp, to, to) == NULL)
+ if (ctf_create_per_cu (fp, to) == NULL)
{
fp->ctf_flags &= ~LCTF_LINKING;
ctf_next_destroy (i);
@@ -1707,11 +1501,7 @@ ctf_link (ctf_dict_t *fp, int flags)
}
}
- if ((flags & CTF_LINK_NONDEDUP) || (getenv ("LD_NO_CTF_DEDUP")))
- ctf_dynhash_iter (fp->ctf_link_inputs, ctf_link_one_input_archive,
- &arg);
- else
- ctf_link_deduplicating (fp);
+ ctf_link_deduplicating (fp);
/* Discard the now-unnecessary mapping table data from all the outputs. */
if (fp->ctf_link_type_mapping)
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 05/10] libctf: remove reference to "unconflicted link mode".
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (3 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 04/10] libctf, include: remove the nondeduplicating CTF linker Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 06/10] libctf: add a deduplicator-specific type mapping table Nick Alcock
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
There is no such thing, and the comment makes no sense, and doesn't
match what the code is doing. We always want to put variables in the
same dicts as the types they relate to if at all possible.
libctf/ChangeLog
2021-02-23 Nick Alcock <nick.alcock@oracle.com>
* ctf-link.c (ctf_link_one_variable): Remove reference to
"unconflicted link mode".
---
libctf/ChangeLog | 5 +++++
libctf/ctf-link.c | 6 +++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 0f84eea9533..04955011f93 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-23 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-link.c (ctf_link_one_variable): Remove reference to
+ "unconflicted link mode".
+
2021-02-23 Nick Alcock <nick.alcock@oracle.com>
* ctf-link.c (ctf_create_per_cu): Drop FILENAME now that it is
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 5d813dbf8b3..d598b7848e8 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -499,9 +499,9 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
return 0;
}
- /* In unconflicted link mode, if this type is mapped to a type in the parent
- dict, we want to try to add to that first: if it reports a duplicate,
- or if the type is in a child already, add straight to the child. */
+ /* If this type is mapped to a type in the parent dict, we want to try to add
+ to that first: if it reports a duplicate, or if the type is in a child
+ already, add straight to the child. */
insert_fp = arg->out_fp;
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 06/10] libctf: add a deduplicator-specific type mapping table
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (4 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 05/10] libctf: remove reference to "unconflicted link mode" Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 07/10] libctf: minor error-handling fixes Nick Alcock
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
When CTF linking is done, the linker has to track the association
between types in the inputs and types in the outputs. The deduplicator
does this via the cd_output_emission_hashes, which maps from hashes of
types (valid in both the input and output) to the IDs of types in the
specific dict in which the cd_emission_hashes is held. However, the
nondeduplicating linker and ctf_add_type used a different mechanism, a
dedicated hashtab stored in the ctf_link_type_mapping, populated via
ctf_add_type_mapping and queried via the ctf_type_mapping function. To
allow the same functions to be used for variable and symbol population
in both the deduplicating and nondeduplicating linker, the deduplicator
carefully transferred all its input->output mappings into this hashtab
before returning.
This is *expensive*. The number of entries in this hashtab scales as the
number of input types, and unlike the hashing machinery the type mapping
machinery (the only other thing which scales that way) has not been much
optimized.
Now the nondeduplicating linker is gone, we can throw this out, move
the existing type mapping machinery to ctf-create.c and dedicate it to
ctf_add_type alone, and add a new function ctf_dedup_type_mapping which
uses the deduplicator's built-in knowledge of type mappings directly,
without requiring an expensive repopulation phase.
This speeds up a test link of nouveau.ko (a good worst-case candidate
with a lot of types in each of a lot of input files) from 9.11s to 7.15s
in my testing, a speedup of over 20%.
libctf/ChangeLog
2021-02-25 Nick Alcock <nick.alcock@oracle.com>
* ctf-impl.h (ctf_dict_t) <ctf_link_type_mapping>: No longer used
by the nondeduplicating linker.
(ctf_add_type_mapping): Removed, now static.
(ctf_type_mapping): Likewise.
(ctf_dedup_type_mapping): New.
(ctf_dedup_t) <cd_input_nums>: New.
* ctf-dedup.c (ctf_dedup_init): Populate it.
(ctf_dedup_fini): Free it again. Emphasise that this has to be
the last thing called.
(ctf_dedup): Populate it.
(ctf_dedup_populate_type_mapping): Removed.
(ctf_dedup_populate_type_mappings): Likewise.
(ctf_dedup_emit): No longer call it. No longer call
ctf_dedup_fini either.
(ctf_dedup_type_mapping): New.
* ctf-link.c (ctf_unnamed_cuname): New.
(ctf_create_per_cu): Arguments must be non-null now.
(ctf_in_member_cb_arg): Removed.
(ctf_link): No longer populate it. No longer discard the
mapping table.
(ctf_link_deduplicating_one_symtypetab): Use
ctf_dedup_type_mapping, not ctf_type_mapping. Use
ctf_unnamed_cuname.
(ctf_link_one_variable): Likewise. Pass in args individually: no
longer a ctf_variable_iter callback.
(empty_link_type_mapping): Removed.
(ctf_link_deduplicating_variables): Use ctf_variable_next, not
ctf_variable_iter. No longer pack arguments to
ctf_link_one_variable into a struct.
(ctf_link_deduplicating_per_cu): Call ctf_dedup_fini once
all link phases are done.
(ctf_link_deduplicating): Likewise.
(ctf_link_intern_extern_string): Improve comment.
(ctf_add_type_mapping): Migrate...
(ctf_type_mapping): ... these functions...
* ctf-create.c (ctf_add_type_mapping): ... here...
(ctf_type_mapping): ... and make static, for the sole use of
ctf_add_type.
---
libctf/ChangeLog | 41 ++++++
libctf/ctf-create.c | 95 +++++++++++++
libctf/ctf-dedup.c | 198 +++++++++++++--------------
libctf/ctf-impl.h | 17 +--
libctf/ctf-link.c | 321 ++++++++++++++------------------------------
5 files changed, 344 insertions(+), 328 deletions(-)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 04955011f93..a029113080f 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,44 @@
+2021-02-25 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-impl.h (ctf_dict_t) <ctf_link_type_mapping>: No longer used
+ by the nondeduplicating linker.
+ (ctf_add_type_mapping): Removed, now static.
+ (ctf_type_mapping): Likewise.
+ (ctf_dedup_type_mapping): New.
+ (ctf_dedup_t) <cd_input_nums>: New.
+ * ctf-dedup.c (ctf_dedup_init): Populate it.
+ (ctf_dedup_fini): Free it again. Emphasise that this has to be
+ the last thing called.
+ (ctf_dedup): Populate it.
+ (ctf_dedup_populate_type_mapping): Removed.
+ (ctf_dedup_populate_type_mappings): Likewise.
+ (ctf_dedup_emit): No longer call it. No longer call
+ ctf_dedup_fini either.
+ (ctf_dedup_type_mapping): New.
+ * ctf-link.c (ctf_unnamed_cuname): New.
+ (ctf_create_per_cu): Arguments must be non-null now.
+ (ctf_in_member_cb_arg): Removed.
+ (ctf_link): No longer populate it. No longer discard the
+ mapping table.
+ (ctf_link_deduplicating_one_symtypetab): Use
+ ctf_dedup_type_mapping, not ctf_type_mapping. Use
+ ctf_unnamed_cuname.
+ (ctf_link_one_variable): Likewise. Pass in args individually: no
+ longer a ctf_variable_iter callback.
+ (empty_link_type_mapping): Removed.
+ (ctf_link_deduplicating_variables): Use ctf_variable_next, not
+ ctf_variable_iter. No longer pack arguments to
+ ctf_link_one_variable into a struct.
+ (ctf_link_deduplicating_per_cu): Call ctf_dedup_fini once
+ all link phases are done.
+ (ctf_link_deduplicating): Likewise.
+ (ctf_link_intern_extern_string): Improve comment.
+ (ctf_add_type_mapping): Migrate...
+ (ctf_type_mapping): ... these functions...
+ * ctf-create.c (ctf_add_type_mapping): ... here...
+ (ctf_type_mapping): ... and make static, for the sole use of
+ ctf_add_type.
+
2021-02-23 Nick Alcock <nick.alcock@oracle.com>
* ctf-link.c (ctf_link_one_variable): Remove reference to
diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index c01ab7a10e2..d417922e7fd 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -2471,6 +2471,101 @@ membadd (const char *name, ctf_id_t type, unsigned long offset, void *arg)
return 0;
}
+/* Record the correspondence between a source and ctf_add_type()-added
+ destination type: both types are translated into parent type IDs if need be,
+ so they relate to the actual dictionary they are in. Outside controlled
+ circumstances (like linking) it is probably not useful to do more than
+ compare these pointers, since there is nothing stopping the user closing the
+ source dict whenever they want to.
+
+ Our OOM handling here is just to not do anything, because this is called deep
+ enough in the call stack that doing anything useful is painfully difficult:
+ the worst consequence if we do OOM is a bit of type duplication anyway. */
+
+static void
+ctf_add_type_mapping (ctf_dict_t *src_fp, ctf_id_t src_type,
+ ctf_dict_t *dst_fp, ctf_id_t dst_type)
+{
+ if (LCTF_TYPE_ISPARENT (src_fp, src_type) && src_fp->ctf_parent)
+ src_fp = src_fp->ctf_parent;
+
+ src_type = LCTF_TYPE_TO_INDEX(src_fp, src_type);
+
+ if (LCTF_TYPE_ISPARENT (dst_fp, dst_type) && dst_fp->ctf_parent)
+ dst_fp = dst_fp->ctf_parent;
+
+ dst_type = LCTF_TYPE_TO_INDEX(dst_fp, dst_type);
+
+ if (dst_fp->ctf_link_type_mapping == NULL)
+ {
+ ctf_hash_fun f = ctf_hash_type_key;
+ ctf_hash_eq_fun e = ctf_hash_eq_type_key;
+
+ if ((dst_fp->ctf_link_type_mapping = ctf_dynhash_create (f, e, free,
+ NULL)) == NULL)
+ return;
+ }
+
+ ctf_link_type_key_t *key;
+ key = calloc (1, sizeof (struct ctf_link_type_key));
+ if (!key)
+ return;
+
+ key->cltk_fp = src_fp;
+ key->cltk_idx = src_type;
+
+ /* No OOM checking needed, because if this doesn't work the worst we'll do is
+ add a few more duplicate types (which will probably run out of memory
+ anyway). */
+ ctf_dynhash_insert (dst_fp->ctf_link_type_mapping, key,
+ (void *) (uintptr_t) dst_type);
+}
+
+/* Look up a type mapping: return 0 if none. The DST_FP is modified to point to
+ the parent if need be. The ID returned is from the dst_fp's perspective. */
+static ctf_id_t
+ctf_type_mapping (ctf_dict_t *src_fp, ctf_id_t src_type, ctf_dict_t **dst_fp)
+{
+ ctf_link_type_key_t key;
+ ctf_dict_t *target_fp = *dst_fp;
+ ctf_id_t dst_type = 0;
+
+ if (LCTF_TYPE_ISPARENT (src_fp, src_type) && src_fp->ctf_parent)
+ src_fp = src_fp->ctf_parent;
+
+ src_type = LCTF_TYPE_TO_INDEX(src_fp, src_type);
+ key.cltk_fp = src_fp;
+ key.cltk_idx = src_type;
+
+ if (target_fp->ctf_link_type_mapping)
+ dst_type = (uintptr_t) ctf_dynhash_lookup (target_fp->ctf_link_type_mapping,
+ &key);
+
+ if (dst_type != 0)
+ {
+ dst_type = LCTF_INDEX_TO_TYPE (target_fp, dst_type,
+ target_fp->ctf_parent != NULL);
+ *dst_fp = target_fp;
+ return dst_type;
+ }
+
+ if (target_fp->ctf_parent)
+ target_fp = target_fp->ctf_parent;
+ else
+ return 0;
+
+ if (target_fp->ctf_link_type_mapping)
+ dst_type = (uintptr_t) ctf_dynhash_lookup (target_fp->ctf_link_type_mapping,
+ &key);
+
+ if (dst_type)
+ dst_type = LCTF_INDEX_TO_TYPE (target_fp, dst_type,
+ target_fp->ctf_parent != NULL);
+
+ *dst_fp = target_fp;
+ return dst_type;
+}
+
/* The ctf_add_type routine is used to copy a type from a source CTF dictionary
to a dynamic destination dictionary. This routine operates recursively by
following the source type's links and embedded member types. If the
diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index 001c2483a20..50da4ac5c11 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -1642,6 +1642,12 @@ ctf_dedup_init (ctf_dict_t *fp)
goto oom;
#endif
+ if ((d->cd_input_nums
+ = ctf_dynhash_create (ctf_hash_integer,
+ ctf_hash_eq_integer,
+ NULL, NULL)) == NULL)
+ goto oom;
+
if ((d->cd_emission_struct_members
= ctf_dynhash_create (ctf_hash_integer,
ctf_hash_eq_integer,
@@ -1661,6 +1667,8 @@ ctf_dedup_init (ctf_dict_t *fp)
return ctf_set_errno (fp, ENOMEM);
}
+/* No ctf_dedup calls are allowed after this call other than starting a new
+ deduplication via ctf_dedup (not even ctf_dedup_type_mapping lookups). */
void
ctf_dedup_fini (ctf_dict_t *fp, ctf_dict_t **outputs, uint32_t noutputs)
{
@@ -1682,6 +1690,7 @@ ctf_dedup_fini (ctf_dict_t *fp, ctf_dict_t **outputs, uint32_t noutputs)
#ifdef ENABLE_LIBCTF_HASH_DEBUGGING
ctf_dynhash_destroy (d->cd_output_mapping_guard);
#endif
+ ctf_dynhash_destroy (d->cd_input_nums);
ctf_dynhash_destroy (d->cd_emission_struct_members);
ctf_dynset_destroy (d->cd_conflicting_types);
@@ -1876,12 +1885,22 @@ ctf_dedup (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
size_t i;
ctf_next_t *it = NULL;
- for (i = 0; i < ninputs; i++)
- ctf_dprintf ("Input %i: %s\n", (int) i, ctf_link_input_name (inputs[i]));
-
if (ctf_dedup_init (output) < 0)
return -1; /* errno is set for us. */
+ for (i = 0; i < ninputs; i++)
+ {
+ ctf_dprintf ("Input %i: %s\n", (int) i, ctf_link_input_name (inputs[i]));
+ if (ctf_dynhash_insert (d->cd_input_nums, inputs[i],
+ (void *) (uintptr_t) i) < 0)
+ {
+ ctf_set_errno (output, errno);
+ ctf_err_warn (output, 0, errno, _("ctf_dedup: cannot initialize: %s\n"),
+ ctf_errmsg (errno));
+ goto err;
+ }
+ }
+
/* Some flags do not apply when CU-mapping: this is not a duplicated link,
because there is only one output and we really don't want to end up marking
all nonconflicting but appears-only-once types as conflicting (which in the
@@ -1937,6 +1956,10 @@ ctf_dedup (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
return -1; /* errno is set for us. */
}
return 0;
+
+ err:
+ ctf_dedup_fini (output, NULL, 0);
+ return -1;
}
static int
@@ -3003,100 +3026,6 @@ ctf_dedup_emit_struct_members (ctf_dict_t *output, ctf_dict_t **inputs,
return ctf_set_errno (output, err);
}
-/* Populate the type mapping used by the types in one FP (which must be an input
- dict containing a non-null cd_output resulting from a ctf_dedup_emit_type
- walk). */
-static int
-ctf_dedup_populate_type_mapping (ctf_dict_t *shared, ctf_dict_t *fp,
- ctf_dict_t **inputs)
-{
- ctf_dedup_t *d = &shared->ctf_dedup;
- ctf_dict_t *output = fp->ctf_dedup.cd_output;
- const void *k, *v;
- ctf_next_t *i = NULL;
- int err;
-
- /* The shared dict (the output) stores its types in the fp itself, not in a
- separate cd_output dict. */
- if (shared == fp)
- output = fp;
-
- /* There may be no types to emit at all, or all the types in this TU may be
- shared. */
- if (!output || !output->ctf_dedup.cd_output_emission_hashes)
- return 0;
-
- while ((err = ctf_dynhash_cnext (output->ctf_dedup.cd_output_emission_hashes,
- &i, &k, &v)) == 0)
- {
- const char *hval = (const char *) k;
- ctf_id_t id_out = (ctf_id_t) (uintptr_t) v;
- ctf_next_t *j = NULL;
- ctf_dynset_t *type_ids;
- const void *id;
-
- type_ids = ctf_dynhash_lookup (d->cd_output_mapping, hval);
- if (!ctf_assert (shared, type_ids))
- return -1;
-#ifdef ENABLE_LIBCTF_HASH_DEBUGGING
- ctf_dprintf ("Traversing emission hash: hval %s\n", hval);
-#endif
-
- while ((err = ctf_dynset_cnext (type_ids, &j, &id)) == 0)
- {
- ctf_dict_t *input = inputs[CTF_DEDUP_GID_TO_INPUT (id)];
- ctf_id_t id_in = CTF_DEDUP_GID_TO_TYPE (id);
-
-#ifdef ENABLE_LIBCTF_HASH_DEBUGGING
- ctf_dprintf ("Adding mapping from %i/%lx to %lx\n",
- CTF_DEDUP_GID_TO_INPUT (id), id_in, id_out);
-#endif
- ctf_add_type_mapping (input, id_in, output, id_out);
- }
- if (err != ECTF_NEXT_END)
- {
- ctf_next_destroy (i);
- goto err;
- }
- }
- if (err != ECTF_NEXT_END)
- goto err;
-
- return 0;
-
- err:
- ctf_err_warn (shared, 0, err, _("iteration error populating the type mapping"));
- return ctf_set_errno (shared, err);
-}
-
-/* Populate the type mapping machinery used by the rest of the linker,
- by ctf_add_type, etc. */
-static int
-ctf_dedup_populate_type_mappings (ctf_dict_t *output, ctf_dict_t **inputs,
- uint32_t ninputs)
-{
- size_t i;
-
- if (ctf_dedup_populate_type_mapping (output, output, inputs) < 0)
- {
- ctf_err_warn (output, 0, 0, _("cannot populate type mappings for shared "
- "CTF dict"));
- return -1; /* errno is set for us. */
- }
-
- for (i = 0; i < ninputs; i++)
- {
- if (ctf_dedup_populate_type_mapping (output, inputs[i], inputs) < 0)
- {
- ctf_err_warn (output, 0, ctf_errno (inputs[i]),
- _("cannot populate type mappings for per-CU CTF dict"));
- return ctf_set_errno (output, ctf_errno (inputs[i]));
- }
- }
-
- return 0;
-}
-
/* Emit deduplicated types into the outputs. The shared type repository is
OUTPUT, on which the ctf_dedup function must have already been called. The
PARENTS array contains the INPUTS index of the parent dict for every child
@@ -3127,9 +3056,6 @@ ctf_dedup_emit (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
if (ctf_dedup_emit_struct_members (output, inputs, ninputs, parents) < 0)
return NULL; /* errno is set for us. */
- if (ctf_dedup_populate_type_mappings (output, inputs, ninputs) < 0)
- return NULL; /* errno is set for us. */
-
for (i = 0; i < ninputs; i++)
{
if (inputs[i]->ctf_dedup.cd_output)
@@ -3163,6 +3089,76 @@ ctf_dedup_emit (ctf_dict_t *output, ctf_dict_t **inputs, uint32_t ninputs,
}
}
- ctf_dedup_fini (output, outputs, num_outputs);
return outputs;
}
+
+/* Determine what type SRC_FP / SRC_TYPE was emitted as in the FP, which
+ must be the shared dict or have it as a parent: return 0 if none. The SRC_FP
+ must be a past input to ctf_dedup. */
+
+ctf_id_t
+ctf_dedup_type_mapping (ctf_dict_t *fp, ctf_dict_t *src_fp, ctf_id_t src_type)
+{
+ ctf_dict_t *output = NULL;
+ ctf_dedup_t *d;
+ int input_num;
+ void *num_ptr;
+ void *type_ptr;
+ int found;
+ const char *hval;
+
+ /* It is an error (an internal error in the caller, in ctf-link.c) to call
+ this with an FP that is not a per-CU output or shared output dict, or with
+ a SRC_FP that was not passed to ctf_dedup as an input; it is an internal
+ error in ctf-dedup for the type passed not to have been hashed, though if
+ the src_fp is a child dict and the type is not a child type, it will have
+ been hashed under the GID corresponding to the parent. */
+
+ if (fp->ctf_dedup.cd_type_hashes != NULL)
+ output = fp;
+ else if (fp->ctf_parent && fp->ctf_parent->ctf_dedup.cd_type_hashes != NULL)
+ output = fp->ctf_parent;
+ else
+ {
+ ctf_set_errno (fp, ECTF_INTERNAL);
+ ctf_err_warn (fp, 0, ECTF_INTERNAL,
+ _("dict %p passed to ctf_dedup_type_mapping is not a "
+ "deduplicated output"), (void *) fp);
+ return CTF_ERR;
+ }
+
+ if (src_fp->ctf_parent && ctf_type_isparent (src_fp, src_type))
+ src_fp = src_fp->ctf_parent;
+
+ d = &output->ctf_dedup;
+
+ found = ctf_dynhash_lookup_kv (d->cd_input_nums, src_fp, NULL, &num_ptr);
+ if (!ctf_assert (output, found != 0))
+ return CTF_ERR; /* errno is set for us. */
+ input_num = (uintptr_t) num_ptr;
+
+ hval = ctf_dynhash_lookup (d->cd_type_hashes,
+ CTF_DEDUP_GID (output, input_num, src_type));
+
+ if (!ctf_assert (output, hval != NULL))
+ return CTF_ERR; /* errno is set for us. */
+
+ /* The emission hashes may be unset if this dict was created after
+ deduplication to house variables or other things that would conflict if
+ stored in the shared dict. */
+ if (fp->ctf_dedup.cd_output_emission_hashes)
+ if (ctf_dynhash_lookup_kv (fp->ctf_dedup.cd_output_emission_hashes, hval,
+ NULL, &type_ptr))
+ return (ctf_id_t) (uintptr_t) type_ptr;
+
+ if (fp->ctf_parent)
+ {
+ ctf_dict_t *pfp = fp->ctf_parent;
+ if (pfp->ctf_dedup.cd_output_emission_hashes)
+ if (ctf_dynhash_lookup_kv (pfp->ctf_dedup.cd_output_emission_hashes,
+ hval, NULL, &type_ptr))
+ return (ctf_id_t) (uintptr_t) type_ptr;
+ }
+
+ return 0;
+}
diff --git a/libctf/ctf-impl.h b/libctf/ctf-impl.h
index a6e1da58930..78a41ff4932 100644
--- a/libctf/ctf-impl.h
+++ b/libctf/ctf-impl.h
@@ -347,6 +347,11 @@ typedef struct ctf_dedup
/* A set (a hash) of hash values of conflicting types. */
ctf_dynset_t *cd_conflicting_types;
+ /* A hash mapping fp *'s of inputs to their input_nums. Used only by
+ functions outside the core ctf_dedup / ctf_dedup_emit machinery which do
+ not take an inputs array. */
+ ctf_dynhash_t *cd_input_nums;
+
/* Maps type hashes to ctf_id_t's in this dictionary. Populated only at
emission time, in the dictionary where emission is taking place. */
ctf_dynhash_t *cd_output_emission_hashes;
@@ -455,9 +460,8 @@ struct ctf_dict
ctf_dynhash_t *ctf_link_inputs; /* Inputs to this link. */
ctf_dynhash_t *ctf_link_outputs; /* Additional outputs from this link. */
- /* Map input types to output types: populated in each output dict.
- Key is a ctf_link_type_key_t: value is a type ID. Used by
- nondeduplicating links and ad-hoc ctf_add_type calls only. */
+ /* Map input types to output types for ctf_add_type. Key is a
+ ctf_link_type_key_t: value is a type ID. */
ctf_dynhash_t *ctf_link_type_mapping;
/* Map input CU names to output CTF dict names: populated in the top-level
@@ -703,11 +707,6 @@ extern ctf_id_t ctf_add_encoded (ctf_dict_t *, uint32_t, const char *,
extern ctf_id_t ctf_add_reftype (ctf_dict_t *, uint32_t, ctf_id_t,
uint32_t kind);
-extern void ctf_add_type_mapping (ctf_dict_t *src_fp, ctf_id_t src_type,
- ctf_dict_t *dst_fp, ctf_id_t dst_type);
-extern ctf_id_t ctf_type_mapping (ctf_dict_t *src_fp, ctf_id_t src_type,
- ctf_dict_t **dst_fp);
-
extern int ctf_dedup_atoms_init (ctf_dict_t *);
extern int ctf_dedup (ctf_dict_t *, ctf_dict_t **, uint32_t ninputs,
uint32_t *parents, int cu_mapped);
@@ -715,6 +714,8 @@ extern void ctf_dedup_fini (ctf_dict_t *, ctf_dict_t **, uint32_t);
extern ctf_dict_t **ctf_dedup_emit (ctf_dict_t *, ctf_dict_t **,
uint32_t ninputs, uint32_t *parents,
uint32_t *noutputs, int cu_mapped);
+extern ctf_id_t ctf_dedup_type_mapping (ctf_dict_t *fp, ctf_dict_t *src_fp,
+ ctf_id_t src_type);
extern void ctf_decl_init (ctf_decl_t *);
extern void ctf_decl_fini (ctf_decl_t *);
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index d598b7848e8..c0b0916f536 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -24,118 +24,18 @@
#pragma weak ctf_open
#endif
-/* Type tracking machinery. */
-
-/* Record the correspondence between a source and ctf_add_type()-added
- destination type: both types are translated into parent type IDs if need be,
- so they relate to the actual dictionary they are in. Outside controlled
- circumstances (like linking) it is probably not useful to do more than
- compare these pointers, since there is nothing stopping the user closing the
- source dict whenever they want to.
-
- Our OOM handling here is just to not do anything, because this is called deep
- enough in the call stack that doing anything useful is painfully difficult:
- the worst consequence if we do OOM is a bit of type duplication anyway. */
-
-void
-ctf_add_type_mapping (ctf_dict_t *src_fp, ctf_id_t src_type,
- ctf_dict_t *dst_fp, ctf_id_t dst_type)
-{
- if (LCTF_TYPE_ISPARENT (src_fp, src_type) && src_fp->ctf_parent)
- src_fp = src_fp->ctf_parent;
-
- src_type = LCTF_TYPE_TO_INDEX(src_fp, src_type);
-
- if (LCTF_TYPE_ISPARENT (dst_fp, dst_type) && dst_fp->ctf_parent)
- dst_fp = dst_fp->ctf_parent;
-
- dst_type = LCTF_TYPE_TO_INDEX(dst_fp, dst_type);
-
- if (dst_fp->ctf_link_type_mapping == NULL)
- {
- ctf_hash_fun f = ctf_hash_type_key;
- ctf_hash_eq_fun e = ctf_hash_eq_type_key;
-
- if ((dst_fp->ctf_link_type_mapping = ctf_dynhash_create (f, e, free,
- NULL)) == NULL)
- return;
- }
-
- ctf_link_type_key_t *key;
- key = calloc (1, sizeof (struct ctf_link_type_key));
- if (!key)
- return;
-
- key->cltk_fp = src_fp;
- key->cltk_idx = src_type;
-
- /* No OOM checking needed, because if this doesn't work the worst we'll do is
- add a few more duplicate types (which will probably run out of memory
- anyway). */
- ctf_dynhash_insert (dst_fp->ctf_link_type_mapping, key,
- (void *) (uintptr_t) dst_type);
-}
-
-/* Look up a type mapping: return 0 if none. The DST_FP is modified to point to
- the parent if need be. The ID returned is from the dst_fp's perspective. */
-ctf_id_t
-ctf_type_mapping (ctf_dict_t *src_fp, ctf_id_t src_type, ctf_dict_t **dst_fp)
-{
- ctf_link_type_key_t key;
- ctf_dict_t *target_fp = *dst_fp;
- ctf_id_t dst_type = 0;
-
- if (LCTF_TYPE_ISPARENT (src_fp, src_type) && src_fp->ctf_parent)
- src_fp = src_fp->ctf_parent;
-
- src_type = LCTF_TYPE_TO_INDEX(src_fp, src_type);
- key.cltk_fp = src_fp;
- key.cltk_idx = src_type;
-
- if (target_fp->ctf_link_type_mapping)
- dst_type = (uintptr_t) ctf_dynhash_lookup (target_fp->ctf_link_type_mapping,
- &key);
-
- if (dst_type != 0)
- {
- dst_type = LCTF_INDEX_TO_TYPE (target_fp, dst_type,
- target_fp->ctf_parent != NULL);
- *dst_fp = target_fp;
- return dst_type;
- }
-
- if (target_fp->ctf_parent)
- target_fp = target_fp->ctf_parent;
- else
- return 0;
-
- if (target_fp->ctf_link_type_mapping)
- dst_type = (uintptr_t) ctf_dynhash_lookup (target_fp->ctf_link_type_mapping,
- &key);
-
- if (dst_type)
- dst_type = LCTF_INDEX_TO_TYPE (target_fp, dst_type,
- target_fp->ctf_parent != NULL);
-
- *dst_fp = target_fp;
- return dst_type;
-}
-
-/* Linker machinery.
-
- CTF linking consists of adding CTF archives full of content to be merged into
+/* CTF linking consists of adding CTF archives full of content to be merged into
this one to the current file (which must be writable) by calling
- ctf_link_add_ctf(). Once this is done, a call to ctf_link() will merge the
- type tables together, generating new CTF files as needed, with this one as a
- parent, to contain types from the inputs which conflict.
- ctf_link_add_strtab() takes a callback which provides string/offset pairs to
- be added to the external symbol table and deduplicated from all CTF string
- tables in the output link; ctf_link_shuffle_syms() takes a callback which
- provides symtab entries in ascending order, and shuffles the function and
- data sections to match; and ctf_link_write() emits a CTF file (if there are
- no conflicts requiring per-compilation-unit sub-CTF files) or CTF archives
- (otherwise) and returns it, suitable for addition in the .ctf section of the
- output. */
+ ctf_link_add_ctf. Once this is done, a call to ctf_link will merge the type
+ tables together, generating new CTF files as needed, with this one as a
+ parent, to contain types from the inputs which conflict. ctf_link_add_strtab
+ takes a callback which provides string/offset pairs to be added to the
+ external symbol table and deduplicated from all CTF string tables in the
+ output link; ctf_link_shuffle_syms takes a callback which provides symtab
+ entries in ascending order, and shuffles the function and data sections to
+ match; and ctf_link_write emits a CTF file (if there are no conflicts
+ requiring per-compilation-unit sub-CTF files) or CTF archives (otherwise) and
+ returns it, suitable for addition in the .ctf section of the output. */
/* Return the name of the compilation unit this CTF dict or its parent applies
to, or a non-null string otherwise: prefer the parent. Used in debugging
@@ -151,6 +51,19 @@ ctf_link_input_name (ctf_dict_t *fp)
return "(unnamed)";
}
+/* Return the cuname of a dict, or the string "unnamed-CU" if none. */
+
+static const char *
+ctf_unnamed_cuname (ctf_dict_t *fp)
+{
+ const char *cuname = ctf_cuname (fp);
+
+ if (!cuname)
+ cuname = "unnamed-CU";
+
+ return cuname;
+}
+
/* The linker inputs look like this. clin_fp is used for short-circuited
CU-mapped links that can entirely avoid the first link phase in some
situations in favour of just passing on the contained ctf_dict_t: it is
@@ -279,6 +192,7 @@ ctf_link_add_ctf (ctf_dict_t *fp, ctf_archive_t *ctf, const char *name)
/* Return a per-CU output CTF dictionary suitable for the given CU, creating and
interning it if need be. */
+_libctf_nonnull_((1,2))
static ctf_dict_t *
ctf_create_per_cu (ctf_dict_t *fp, const char *cu_name)
{
@@ -429,21 +343,6 @@ ctf_link_set_memb_name_changer (ctf_dict_t *fp,
fp->ctf_link_memb_name_changer_arg = arg;
}
-typedef struct ctf_link_in_member_cb_arg
-{
- /* The shared output dictionary. */
- ctf_dict_t *out_fp;
-
- /* The cuname of the input file, and an fp to each dictionary in that file
- in turn. */
- const char *in_cuname;
- ctf_dict_t *in_fp;
-
- /* If true, this is the CU-mapped portion of a deduplicating link: no child
- dictionaries should be created. */
- int cu_mapped;
-} ctf_link_in_member_cb_arg_t;
-
/* Set a function which is used to filter out unwanted variables from the link. */
int
ctf_link_set_variable_filter (ctf_dict_t *fp, ctf_link_variable_filter_f *filter,
@@ -479,23 +378,22 @@ check_variable (const char *name, ctf_dict_t *fp, ctf_id_t type,
return 0; /* Already exists. */
}
-/* Link one variable in. */
+/* Link one variable named NAME of type TYPE found in IN_FP into FP. */
static int
-ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
+ctf_link_one_variable (ctf_dict_t *fp, ctf_dict_t *in_fp, const char *name,
+ ctf_id_t type, int cu_mapped)
{
- ctf_link_in_member_cb_arg_t *arg = (ctf_link_in_member_cb_arg_t *) arg_;
ctf_dict_t *per_cu_out_fp;
ctf_id_t dst_type = 0;
- ctf_dict_t *insert_fp;
ctf_dvdef_t *dvd;
/* See if this variable is filtered out. */
- if (arg->out_fp->ctf_link_variable_filter)
+ if (fp->ctf_link_variable_filter)
{
- void *farg = arg->out_fp->ctf_link_variable_filter_arg;
- if (arg->out_fp->ctf_link_variable_filter (arg->in_fp, name, type, farg))
+ void *farg = fp->ctf_link_variable_filter_arg;
+ if (fp->ctf_link_variable_filter (in_fp, name, type, farg))
return 0;
}
@@ -503,25 +401,25 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
to that first: if it reports a duplicate, or if the type is in a child
already, add straight to the child. */
- insert_fp = arg->out_fp;
+ if ((dst_type = ctf_dedup_type_mapping (fp, in_fp, type)) == CTF_ERR)
+ return -1; /* errno is set for us. */
- dst_type = ctf_type_mapping (arg->in_fp, type, &insert_fp);
if (dst_type != 0)
{
- if (insert_fp == arg->out_fp)
- {
- if (check_variable (name, insert_fp, dst_type, &dvd))
- {
- /* No variable here: we can add it. */
- if (ctf_add_variable (insert_fp, name, dst_type) < 0)
- return (ctf_set_errno (arg->out_fp, ctf_errno (insert_fp)));
- return 0;
- }
+ if (!ctf_assert (fp, ctf_type_isparent (fp, dst_type)))
+ return -1; /* errno is set for us. */
- /* Already present? Nothing to do. */
- if (dvd && dvd->dvd_type == dst_type)
- return 0;
+ if (check_variable (name, fp, dst_type, &dvd))
+ {
+ /* No variable here: we can add it. */
+ if (ctf_add_variable (fp, name, dst_type) < 0)
+ return -1; /* errno is set for us. */
+ return 0;
}
+
+ /* Already present? Nothing to do. */
+ if (dvd && dvd->dvd_type == dst_type)
+ return 0;
}
/* Can't add to the parent due to a name clash, or because it references a
@@ -529,29 +427,29 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
be. If we can't do that, skip it. Don't add to a child if we're doing a
CU-mapped link, since that has only one output. */
- if (arg->cu_mapped)
+ if (cu_mapped)
{
ctf_dprintf ("Variable %s in input file %s depends on a type %lx hidden "
- "due to conflicts: skipped.\n", name, arg->in_cuname,
- type);
+ "due to conflicts: skipped.\n", name,
+ ctf_unnamed_cuname (in_fp), type);
return 0;
}
- if ((per_cu_out_fp = ctf_create_per_cu (arg->out_fp, arg->in_cuname)) == NULL)
- return -1; /* Errno is set for us. */
+ if ((per_cu_out_fp = ctf_create_per_cu (fp, ctf_unnamed_cuname (in_fp))) == NULL)
+ return -1; /* errno is set for us. */
/* If the type was not found, check for it in the child too. */
if (dst_type == 0)
{
- insert_fp = per_cu_out_fp;
- dst_type = ctf_type_mapping (arg->in_fp, type, &insert_fp);
+ if ((dst_type = ctf_dedup_type_mapping (per_cu_out_fp,
+ in_fp, type)) == CTF_ERR)
+ return -1; /* errno is set for us. */
if (dst_type == 0)
{
- ctf_err_warn (arg->out_fp, 1, 0,
- _("type %lx for variable %s in input file %s "
- "not found: skipped"), type, name,
- arg->in_cuname);
+ ctf_err_warn (fp, 1, 0, _("type %lx for variable %s in input file %s "
+ "not found: skipped"), type, name,
+ ctf_unnamed_cuname (in_fp));
/* Do not terminate the link: just skip the variable. */
return 0;
}
@@ -559,21 +457,10 @@ ctf_link_one_variable (const char *name, ctf_id_t type, void *arg_)
if (check_variable (name, per_cu_out_fp, dst_type, &dvd))
if (ctf_add_variable (per_cu_out_fp, name, dst_type) < 0)
- return (ctf_set_errno (arg->out_fp, ctf_errno (per_cu_out_fp)));
+ return (ctf_set_errno (fp, ctf_errno (per_cu_out_fp)));
return 0;
}
-/* Dump the unnecessary link type mapping after one input file is processed. */
-static void
-empty_link_type_mapping (void *key _libctf_unused_, void *value,
- void *arg _libctf_unused_)
-{
- ctf_dict_t *fp = (ctf_dict_t *) value;
-
- if (fp->ctf_link_type_mapping)
- ctf_dynhash_empty (fp->ctf_link_type_mapping);
-}
-
/* Lazily open a CTF archive for linking, if not already open.
Returns the number of files contained within the opened archive (0 for none),
@@ -925,21 +812,24 @@ static int
ctf_link_deduplicating_variables (ctf_dict_t *fp, ctf_dict_t **inputs,
size_t ninputs, int cu_mapped)
{
- ctf_link_in_member_cb_arg_t arg;
size_t i;
- arg.cu_mapped = cu_mapped;
- arg.out_fp = fp;
-
for (i = 0; i < ninputs; i++)
{
- arg.in_fp = inputs[i];
- if (ctf_cuname (inputs[i]) != NULL)
- arg.in_cuname = ctf_cuname (inputs[i]);
- else
- arg.in_cuname = "unnamed-CU";
- if (ctf_variable_iter (arg.in_fp, ctf_link_one_variable, &arg) < 0)
- return ctf_set_errno (fp, ctf_errno (arg.in_fp));
+ ctf_next_t *it = NULL;
+ ctf_id_t type;
+ const char *name;
+
+ while ((type = ctf_variable_next (inputs[i], &it, &name)) != CTF_ERR)
+ {
+ if (ctf_link_one_variable (fp, inputs[i], name, type, cu_mapped) < 0)
+ {
+ ctf_next_destroy (it);
+ return -1; /* errno is set for us. */
+ }
+ }
+ if (ctf_errno (inputs[i]) != ECTF_NEXT_END)
+ return ctf_set_errno (fp, ctf_errno (inputs[i]));
}
return 0;
}
@@ -982,40 +872,35 @@ ctf_link_deduplicating_one_symtypetab (ctf_dict_t *fp, ctf_dict_t *input,
ctf_next_t *it = NULL;
const char *name;
ctf_id_t type;
- const char *in_file_name;
-
- if (ctf_cuname (input) != NULL)
- in_file_name = ctf_cuname (input);
- else
- in_file_name = "unnamed-CU";
while ((type = ctf_symbol_next (input, &it, &name, functions)) != CTF_ERR)
{
ctf_id_t dst_type;
ctf_dict_t *per_cu_out_fp;
- ctf_dict_t *insert_fp = fp;
int sym;
/* Look in the parent first. */
- dst_type = ctf_type_mapping (input, type, &insert_fp);
+ if ((dst_type = ctf_dedup_type_mapping (fp, input, type)) == CTF_ERR)
+ return -1; /* errno is set for us. */
+
if (dst_type != 0)
{
- if (insert_fp == fp)
- {
- sym = check_sym (fp, name, dst_type, functions);
+ if (!ctf_assert (fp, ctf_type_isparent (fp, dst_type)))
+ return -1; /* errno is set for us. */
- /* Already present: next symbol. */
- if (sym == 0)
- continue;
- /* Not present: add it. */
- else if (sym > 0)
- {
- if (ctf_add_funcobjt_sym (fp, functions,
- name, dst_type) < 0)
- return -1; /* errno is set for us. */
- continue;
- }
+ sym = check_sym (fp, name, dst_type, functions);
+
+ /* Already present: next symbol. */
+ if (sym == 0)
+ continue;
+ /* Not present: add it. */
+ else if (sym > 0)
+ {
+ if (ctf_add_funcobjt_sym (fp, functions,
+ name, dst_type) < 0)
+ return -1; /* errno is set for us. */
+ continue;
}
}
@@ -1028,24 +913,26 @@ ctf_link_deduplicating_one_symtypetab (ctf_dict_t *fp, ctf_dict_t *input,
{
ctf_dprintf ("Symbol %s in input file %s depends on a type %lx "
"hidden due to conflicts: skipped.\n", name,
- in_file_name, type);
+ ctf_unnamed_cuname (input), type);
continue;
}
- if ((per_cu_out_fp = ctf_create_per_cu (fp, in_file_name)) == NULL)
+ if ((per_cu_out_fp = ctf_create_per_cu (fp, ctf_unnamed_cuname (input))) == NULL)
return -1; /* errno is set for us. */
/* If the type was not found, check for it in the child too. */
if (dst_type == 0)
{
- insert_fp = per_cu_out_fp;
- dst_type = ctf_type_mapping (input, type, &insert_fp);
+ if ((dst_type = ctf_dedup_type_mapping (per_cu_out_fp,
+ input, type)) == CTF_ERR)
+ return -1; /* errno is set for us. */
if (dst_type == 0)
{
ctf_err_warn (fp, 1, 0,
_("type %lx for symbol %s in input file %s "
- "not found: skipped"), type, name, in_file_name);
+ "not found: skipped"), type, name,
+ ctf_unnamed_cuname (input));
continue;
}
}
@@ -1068,7 +955,7 @@ ctf_link_deduplicating_one_symtypetab (ctf_dict_t *fp, ctf_dict_t *input,
ctf_err_warn (fp, 0, ECTF_DUPLICATE,
_("symbol %s in input file %s found conflicting "
"even when trying in per-CU dict."), name,
- in_file_name);
+ ctf_unnamed_cuname (input));
return (ctf_set_errno (fp, ECTF_DUPLICATE));
}
}
@@ -1258,6 +1145,8 @@ ctf_link_deduplicating_per_cu (ctf_dict_t *fp)
goto err_inputs_outputs;
}
+ ctf_dedup_fini (out, outputs, noutputs);
+
/* For now, we omit symbol section linking for CU-mapped links, until it
is clear how to unify the symbol table across such links. (Perhaps we
should emit an unconditionally indexed symtab, like the compiler
@@ -1420,6 +1309,8 @@ ctf_link_deduplicating (ctf_dict_t *fp)
goto err_clean_outputs;
}
+ ctf_dedup_fini (fp, outputs, noutputs);
+
/* Now close all the inputs, including per-CU intermediates. */
if (ctf_link_deduplicating_close_inputs (fp, NULL, inputs, ninputs) < 0)
@@ -1452,12 +1343,9 @@ ctf_link_deduplicating (ctf_dict_t *fp)
int
ctf_link (ctf_dict_t *fp, int flags)
{
- ctf_link_in_member_cb_arg_t arg;
ctf_next_t *i = NULL;
int err;
- memset (&arg, 0, sizeof (struct ctf_link_in_member_cb_arg));
- arg.out_fp = fp;
fp->ctf_link_flags = flags;
if (fp->ctf_link_inputs == NULL)
@@ -1503,11 +1391,6 @@ ctf_link (ctf_dict_t *fp, int flags)
ctf_link_deduplicating (fp);
- /* Discard the now-unnecessary mapping table data from all the outputs. */
- if (fp->ctf_link_type_mapping)
- ctf_dynhash_empty (fp->ctf_link_type_mapping);
- ctf_dynhash_iter (fp->ctf_link_outputs, empty_link_type_mapping, NULL);
-
fp->ctf_flags &= ~LCTF_LINKING;
if ((ctf_errno (fp) != 0) && (ctf_errno (fp) != ECTF_NOCTFDATA))
return -1;
@@ -1537,8 +1420,8 @@ ctf_link_intern_extern_string (void *key _libctf_unused_, void *value,
/* Repeatedly call ADD_STRING to acquire strings from the external string table,
adding them to the atoms table for this CU and all subsidiary CUs.
- If ctf_link() is also called, it must be called first if you want the new CTF
- files ctf_link() can create to get their strings dedupped against the ELF
+ If ctf_link is also called, it must be called first if you want the new CTF
+ files ctf_link can create to get their strings dedupped against the ELF
strtab properly. */
int
ctf_link_add_strtab (ctf_dict_t *fp, ctf_link_strtab_string_f *add_string,
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 07/10] libctf: minor error-handling fixes
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (5 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 06/10] libctf: add a deduplicator-specific type mapping table Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 08/10] libctf: fix signed/unsigned comparison confusion Nick Alcock
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
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-02-25 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.
---
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 a029113080f..1f3176c5549 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,13 @@
+2021-02-25 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-02-25 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++)
{
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 08/10] libctf: fix signed/unsigned comparison confusion
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (6 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 07/10] libctf: minor error-handling fixes Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH 09/10] libctf: free ctf_dynsyms properly Nick Alcock
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
Comparing an encoding's cte_bits to a ctf_type_size needs a cast:
one is a uint32_t and the other is an ssize_t.
libctf/ChangeLog
2021-02-25 Nick Alcock <nick.alcock@oracle.com>
* ctf-dump.c (ctf_dump_format_type): Fix signed/unsigned confusion.
---
libctf/ChangeLog | 4 ++++
libctf/ctf-dump.c | 4 ++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 1f3176c5549..01286bcab54 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-25 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-dump.c (ctf_dump_format_type): Fix signed/unsigned confusion.
+
2021-02-25 Nick Alcock <nick.alcock@oracle.com>
* ctf-dedup.c (ctf_dedup): Pass on errors from ctf_dedup_hash_type.
diff --git a/libctf/ctf-dump.c b/libctf/ctf-dump.c
index 758d28d76d5..788355d9db1 100644
--- a/libctf/ctf-dump.c
+++ b/libctf/ctf-dump.c
@@ -144,7 +144,7 @@ ctf_dump_format_type (ctf_dict_t *fp, ctf_id_t id, int flag)
if (ctf_type_encoding (fp, id, &ep) == 0)
{
- if (ep.cte_bits != ctf_type_size (fp, id) * CHAR_BIT
+ if ((ssize_t) ep.cte_bits != ctf_type_size (fp, id) * CHAR_BIT
&& flag & CTF_FT_BITFIELD)
{
if (asprintf (&bit, ":%i", ep.cte_bits) < 0)
@@ -154,7 +154,7 @@ ctf_dump_format_type (ctf_dict_t *fp, ctf_id_t id, int flag)
bit = NULL;
}
- if (ep.cte_bits != ctf_type_size (fp, id) * CHAR_BIT
+ if ((ssize_t) ep.cte_bits != ctf_type_size (fp, id) * CHAR_BIT
|| ep.cte_offset != 0)
{
const char *slice = "";
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 09/10] libctf: free ctf_dynsyms properly
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (7 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 08/10] libctf: fix signed/unsigned comparison confusion Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 13:29 ` [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting Nick Alcock
2021-02-27 13:34 ` [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
In the "no symbols" case (commonplace for executables), we were freeing
the ctf_dynsyms using free(), instead of ctf_dynhash_destroy(), leaking
a little memory.
(This is harmless in the common case of ld usage, but libctf might be
used by persistent processes too.)
libctf/ChangeLog
2021-02-25 Nick Alcock <nick.alcock@oracle.com>
* ctf-link.c (ctf_link_shuffle_syms): Free ctf_dynsyms properly.
---
libctf/ChangeLog | 4 ++++
libctf/ctf-link.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index 01286bcab54..f30f79ec6ee 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-25 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-link.c (ctf_link_shuffle_syms): Free ctf_dynsyms properly.
+
2021-02-25 Nick Alcock <nick.alcock@oracle.com>
* ctf-dump.c (ctf_dump_format_type): Fix signed/unsigned confusion.
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 05733a0cb7d..882d4297e4f 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -1580,7 +1580,7 @@ ctf_link_shuffle_syms (ctf_dict_t *fp)
if (!ctf_dynhash_elements (fp->ctf_dynsyms))
{
ctf_dprintf ("No symbols: not a final link.\n");
- free (fp->ctf_dynsyms);
+ ctf_dynhash_destroy (fp->ctf_dynsyms);
fp->ctf_dynsyms = NULL;
return 0;
}
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (8 preceding siblings ...)
2021-02-27 13:29 ` [PATCH 09/10] libctf: free ctf_dynsyms properly Nick Alcock
@ 2021-02-27 13:29 ` Nick Alcock
2021-02-27 18:28 ` Nick Alcock
2021-03-01 15:17 ` Nick Clifton
2021-02-27 13:34 ` [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
10 siblings, 2 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:29 UTC (permalink / raw)
To: binutils
This is a tricky one. BFD, on the linker's behalf, reports symbols to
libctf via the ctf_new_symbol and ctf_new_dynsym callbacks, which
ultimately call ctf_link_add_linker_symbol. But while this happens
after strtab offsets are finalized, it happens before the .dynstr is
actually laid out, so we can't iterate over it at this stage and
it is not clear what the reported symbols are actually called. So
a second callback, examine_strtab, is called after the .dynstr is
finalized, which calls ctf_link_add_strtab and ultimately leads
to ldelf_ctf_strtab_iter_cb being called back repeatedly until the
offsets of every string in the .dynstr is passed to libctf.
libctf can then use this to get symbol names out of the input (which
usually stores symbol types in the form of a name -> type mapping at
this stage) and extract the types of those symbols, feeding them back
into their final form as a 1:1 association with the real symtab's
STT_OBJ and STT_FUNC symbols (with a few skipped, see
ctf_symtab_skippable).
This representation is compact, but has one problem: if libctf somehow
gets confused about the st_type of a symbol, it'll stick an entry into
the function symtypetab when it should put it into the object
symtypetab, or vice versa, and *every symbol from that one on* will have
the wrong CTF type because it's actually looking up the type for a
different symbol.
And we have just such a bug. ctf_link_add_strtab was not taking the
refcounts of strings into consideration, so even strings that had been
eliminated from the strtab by virtue of being in objects eliminated via
--as-needed etc were being reported. This is harmful because it can
lead to multiple strings with the same apparent offset, and if the last
duplicate to be reported relates to an eliminated symbol, we look up the
wrong symbol from the input and gets its type wrong: if it's unlucky and
the eliminated symbol is also of the wrong st_type, we will end up with
a corrupted symtypetab.
Thankfully the wrong-st_type case is already diagnosed by a
this-can-never-happen paranoid warning:
CTF warning: Symbol 61a added to CTF as a function but is of type 1
or the converse
* CTF warning: Symbol a3 added to CTF as a data object but is of type 2
so at least we can tell when the corruption has spread to more than one
symbol's type.
Skipping zero-refcounted strings is easy: teach _bfd_elf_strtab_str to
skip them, and ldelf_ctf_strtab_iter_cb to loop over skipped strings
until it falls off the end or finds one that isn't skipped.
bfd/ChangeLog
2021-02-26 Nick Alcock <nick.alcock@oracle.com>
* elf-strtab.c (_bfd_elf_strtab_str): Skip strings with zero refcount.
ld/ChangeLog
2021-02-26 Nick Alcock <nick.alcock@oracle.com>
* ldelfgen.c (ldelf_ctf_strtab_iter_cb): Skip zero-refcount strings.
libctf/ChangeLog
2021-02-26 Nick Alcock <nick.alcock@oracle.com>
* ctf-create.c (symtypetab_density): Report the symbol name as
well as index in the name != object error; note the likely
consequences.
* ctf-link.c (ctf_luink_shuffle_syms): Report the symbol index
as well as name.
---
bfd/ChangeLog | 4 ++++
bfd/elf-strtab.c | 2 ++
ld/ChangeLog | 4 ++++
ld/ldelfgen.c | 16 +++++++++++-----
libctf/ChangeLog | 8 ++++++++
libctf/ctf-create.c | 16 ++++++++++------
libctf/ctf-link.c | 3 ++-
7 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 2fef817d734..f1b4e8a2a46 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-26 Nick Alcock <nick.alcock@oracle.com>
+
+ * elf-strtab.c (_bfd_elf_strtab_str): Skip strings with zero refcount.
+
2021-02-25 Alan Modra <amodra@gmail.com>
PR 27441
diff --git a/bfd/elf-strtab.c b/bfd/elf-strtab.c
index 3e5e1622109..5a9aec55d60 100644
--- a/bfd/elf-strtab.c
+++ b/bfd/elf-strtab.c
@@ -302,6 +302,8 @@ _bfd_elf_strtab_str (struct elf_strtab_hash *tab, size_t idx,
return 0;
BFD_ASSERT (idx < tab->size);
BFD_ASSERT (tab->sec_size);
+ if (tab->array[idx]->refcount == 0)
+ return 0;
if (offset)
*offset = tab->array[idx]->u.index;
return tab->array[idx]->root.string;
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 6540407237c..5f5242b2a22 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-26 Nick Alcock <nick.alcock@oracle.com>
+
+ * ldelfgen.c (ldelf_ctf_strtab_iter_cb): Skip zero-refcount strings.
+
2021-02-26 Alan Modra <amodra@gmail.com>
PR 27441
diff --git a/ld/ldelfgen.c b/ld/ldelfgen.c
index df3dae0abe8..682cb23d469 100644
--- a/ld/ldelfgen.c
+++ b/ld/ldelfgen.c
@@ -375,13 +375,19 @@ ldelf_ctf_strtab_iter_cb (uint32_t *offset, void *arg_)
if (arg->next_i == 0)
arg->next_i = 1;
- if (arg->next_i >= _bfd_elf_strtab_len (arg->strtab))
+ /* Hunt through strings until we fall off the end or find one with
+ a nonzero refcount. */
+ do
{
- arg->next_i = 0;
- return NULL;
- }
+ if (arg->next_i >= _bfd_elf_strtab_len (arg->strtab))
+ {
+ arg->next_i = 0;
+ return NULL;
+ }
+
+ ret = _bfd_elf_strtab_str (arg->strtab, arg->next_i++, &off);
+ } while (ret == 0);
- ret = _bfd_elf_strtab_str (arg->strtab, arg->next_i++, &off);
*offset = off;
/* If we've overflowed, we cannot share any further strings: the CTF
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index f30f79ec6ee..a6cdf9f64fb 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,11 @@
+2021-02-26 Nick Alcock <nick.alcock@oracle.com>
+
+ * ctf-create.c (symtypetab_density): Report the symbol name as
+ well as index in the name != object error; note the likely
+ consequences.
+ * ctf-link.c (ctf_luink_shuffle_syms): Report the symbol index
+ as well as name.
+
2021-02-25 Nick Alcock <nick.alcock@oracle.com>
* ctf-link.c (ctf_link_shuffle_syms): Free ctf_dynsyms properly.
diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index d417922e7fd..3f2c5dacb03 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -318,18 +318,22 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
if ((flags & CTF_SYMTYPETAB_EMIT_FUNCTION)
&& sym->st_type != STT_FUNC)
{
- ctf_err_warn (fp, 1, 0, _("Symbol %x added to CTF as a function "
- "but is of type %x\n"),
- sym->st_symidx, sym->st_type);
+ ctf_err_warn (fp, 1, 0, _("symbol %s (%x) added to CTF as a "
+ "function but is of type %x. "
+ "The symbol type lookup tables "
+ "are probably corrupted"),
+ sym->st_name, sym->st_symidx, sym->st_type);
ctf_dynhash_remove (symhash, name);
continue;
}
else if (!(flags & CTF_SYMTYPETAB_EMIT_FUNCTION)
&& sym->st_type != STT_OBJECT)
{
- ctf_err_warn (fp, 1, 0, _("Symbol %x added to CTF as a data "
- "object but is of type %x\n"),
- sym->st_symidx, sym->st_type);
+ ctf_err_warn (fp, 1, 0, _("symbol %s (%x) added to CTF as a "
+ "data object but is of type %x. "
+ "The symbol type lookup tables "
+ "are probably corrupted"),
+ sym->st_name, sym->st_symidx, sym->st_type);
ctf_dynhash_remove (symhash, name);
continue;
}
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 882d4297e4f..5471fccd0f7 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -1552,7 +1552,8 @@ ctf_link_shuffle_syms (ctf_dict_t *fp)
for skippability here. */
if (!ctf_symtab_skippable (&did->cid_sym))
{
- ctf_dprintf ("symbol name from linker: %s\n", did->cid_sym.st_name);
+ ctf_dprintf ("symbol from linker: %s (%x)\n", did->cid_sym.st_name,
+ did->cid_sym.st_symidx);
if ((new_sym = malloc (sizeof (ctf_link_sym_t))) == NULL)
goto local_oom;
--
2.30.0.252.gc27e85e57d
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review)
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
` (9 preceding siblings ...)
2021-02-27 13:29 ` [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting Nick Alcock
@ 2021-02-27 13:34 ` Nick Alcock
10 siblings, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 13:34 UTC (permalink / raw)
To: binutils
On 27 Feb 2021, Nick Alcock via Binutils told this:
> libctf: fix ChangeLog date
Oops! This patch didn't appear in the set I posted because it was a
changelog fixup which was entirely erased by git format-patch. It's a
one-liner. I forgot to adjust changelog dates before pushing last time,
so I have to clean that up, like so:
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -24,7 +24,7 @@
* configure: Regenerate.
* Makefile.in: Regenerate.
-2021-02-17 Nick Alcock <nick.alcock@oracle.com>
+2021-02-20 Nick Alcock <nick.alcock@oracle.com>
* ctf-impl.h (ctf_dict_t) <ctf_symhash>: New.
<ctf_symhash_latest>: Likewise.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting
2021-02-27 13:29 ` [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting Nick Alcock
@ 2021-02-27 18:28 ` Nick Alcock
2021-03-01 15:17 ` Nick Clifton
1 sibling, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-02-27 18:28 UTC (permalink / raw)
To: Nick Alcock via Binutils
On 27 Feb 2021, Nick Alcock via Binutils outgrape:
> * ctf-link.c (ctf_luink_shuffle_syms): Report the symbol index
> as well as name.
I'll fix this spelling error: this is not a luinker.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting
2021-02-27 13:29 ` [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting Nick Alcock
2021-02-27 18:28 ` Nick Alcock
@ 2021-03-01 15:17 ` Nick Clifton
2021-03-02 14:26 ` Nick Alcock
2021-03-02 15:16 ` Nick Alcock
1 sibling, 2 replies; 16+ messages in thread
From: Nick Clifton @ 2021-03-01 15:17 UTC (permalink / raw)
To: Nick Alcock, binutils
Hi Nick,
> bfd/ChangeLog
> 2021-02-26 Nick Alcock <nick.alcock@oracle.com>
>
> * elf-strtab.c (_bfd_elf_strtab_str): Skip strings with zero refcount.
>
> ld/ChangeLog
> 2021-02-26 Nick Alcock <nick.alcock@oracle.com>
>
> * ldelfgen.c (ldelf_ctf_strtab_iter_cb): Skip zero-refcount strings.
This patch is approved, although I have a few minor formatting nits that you
might like to correct:
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 2fef817d734..f1b4e8a2a46 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-02-26 Nick Alcock <nick.alcock@oracle.com>
> +
> + * elf-strtab.c (_bfd_elf_strtab_str): Skip strings with zero refcount.
> +
Please can changelog entries not be submitted as context diffs.
They almost never apply cleanly...
> @@ -302,6 +302,8 @@ _bfd_elf_strtab_str (struct elf_strtab_hash *tab, size_t idx,
> return 0;
> BFD_ASSERT (idx < tab->size);
> BFD_ASSERT (tab->sec_size);
> + if (tab->array[idx]->refcount == 0)
> + return 0;
This should really be "return NULL;". (I know that there is a 'return 0;' a few
lines above, but that one is wrong too).
> @@ -375,13 +375,19 @@ ldelf_ctf_strtab_iter_cb (uint32_t *offset, void *arg_)
> if (arg->next_i == 0)
> arg->next_i = 1;
>
> - if (arg->next_i >= _bfd_elf_strtab_len (arg->strtab))
> + /* Hunt through strings until we fall off the end or find one with
> + a nonzero refcount. */
> + do
> {
> - arg->next_i = 0;
> - return NULL;
> - }
> + if (arg->next_i >= _bfd_elf_strtab_len (arg->strtab))
> + {
> + arg->next_i = 0;
> + return NULL;
> + }
> +
> + ret = _bfd_elf_strtab_str (arg->strtab, arg->next_i++, &off);
> + } while (ret == 0);
The while statement should be on its own line. IE:
}
while (ret == 0);
Cheers
Nick
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting
2021-03-01 15:17 ` Nick Clifton
@ 2021-03-02 14:26 ` Nick Alcock
2021-03-02 15:16 ` Nick Alcock
1 sibling, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-03-02 14:26 UTC (permalink / raw)
To: binutils
On 1 Mar 2021, Nick Clifton via Binutils spake thusly:
>> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
>> index 2fef817d734..f1b4e8a2a46 100644
>> --- a/bfd/ChangeLog
>> +++ b/bfd/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2021-02-26 Nick Alcock <nick.alcock@oracle.com>
>> +
>> + * elf-strtab.c (_bfd_elf_strtab_str): Skip strings with zero refcount.
>> +
>
> Please can changelog entries not be submitted as context diffs.
> They almost never apply cleanly...
Sorry! This is due to my absentmindedly using git format-patch rather
than the alias I normally use to chop them out when sending out patch
mails. (FWIW, the arcane git syntax needed to automatically elide
ChangeLogs is this line noise:
git format-patch "$@" -- ':!:./**/ChangeLog'
which explains why I normally use an alias.)
>> @@ -302,6 +302,8 @@ _bfd_elf_strtab_str (struct elf_strtab_hash *tab, size_t idx,
>> return 0;
>> BFD_ASSERT (idx < tab->size);
>> BFD_ASSERT (tab->sec_size);
>> + if (tab->array[idx]->refcount == 0)
>> + return 0;
>
> This should really be "return NULL;". (I know that there is a 'return 0;' a few
> lines above, but that one is wrong too).
I really should have spotted that. Fixed.
>> + ret = _bfd_elf_strtab_str (arg->strtab, arg->next_i++, &off);
>> + } while (ret == 0);
>
> The while statement should be on its own line. IE:
>
> }
> while (ret == 0);
... whoops, I had no idea GNU syntax decreed that (though it is
perfectly clear in standards.texi). Will fix. I'll fix up libctf to
follow this too (in a separate commit, coming soon, along with fixing up
some tabdamage etc) since it violates this all over the place.
Will push this batch later today.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting
2021-03-01 15:17 ` Nick Clifton
2021-03-02 14:26 ` Nick Alcock
@ 2021-03-02 15:16 ` Nick Alcock
1 sibling, 0 replies; 16+ messages in thread
From: Nick Alcock @ 2021-03-02 15:16 UTC (permalink / raw)
To: binutils
On 1 Mar 2021, Nick Clifton via Binutils verbalised:
>> + ret = _bfd_elf_strtab_str (arg->strtab, arg->next_i++, &off);
>> + } while (ret == 0);
>
> The while statement should be on its own line. IE:
>
> }
> while (ret == 0);
It's also checking the return value of the same function you just
commented on, so it too should be comparing with NULL. (Adjusted,
pushed, thanks for spotting this mindless cut-and-pasto before it spread
too far!)
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-03-02 15:16 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-27 13:29 [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) Nick Alcock
2021-02-27 13:29 ` [PATCH 01/10] libctf: ctf_archive_next should set the parent name consistently Nick Alcock
2021-02-27 13:29 ` [PATCH 02/10] libctf: reimplemement many _iter iterators in terms of _next Nick Alcock
2021-02-27 13:29 ` [PATCH 03/10] libctf: fix ChangeLog date Nick Alcock
2021-02-27 13:29 ` [PATCH 04/10] libctf, include: remove the nondeduplicating CTF linker Nick Alcock
2021-02-27 13:29 ` [PATCH 05/10] libctf: remove reference to "unconflicted link mode" Nick Alcock
2021-02-27 13:29 ` [PATCH 06/10] libctf: add a deduplicator-specific type mapping table Nick Alcock
2021-02-27 13:29 ` [PATCH 07/10] libctf: minor error-handling fixes Nick Alcock
2021-02-27 13:29 ` [PATCH 08/10] libctf: fix signed/unsigned comparison confusion Nick Alcock
2021-02-27 13:29 ` [PATCH 09/10] libctf: free ctf_dynsyms properly Nick Alcock
2021-02-27 13:29 ` [PATCH REVIEW 10/10] bfd, ld, libctf: skip zero-refcount strings in CTF string reporting Nick Alcock
2021-02-27 18:28 ` Nick Alcock
2021-03-01 15:17 ` Nick Clifton
2021-03-02 14:26 ` Nick Alcock
2021-03-02 15:16 ` Nick Alcock
2021-02-27 13:34 ` [PATCH 00/10] libctf: cleanups, speedups, and bugfixes (one needing review) 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).