From: Nick Alcock Date: Fri, 30 May 2025 14:26:26 +0000 (+0100) Subject: libctf: dedup: improve hiding of conflicting types in the same dict X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=002957be18e3b34e3e4863279335f58ae357cee5;p=thirdparty%2Fbinutils-gdb.git libctf: dedup: improve hiding of conflicting types in the same dict If types are conflicting, they are usually moved into separate child dicts -- but not always. If they are added to the same dict by the cu-mapping mechanism (as used e.g. for multi-TU kernel modules), we can easily end up adding multiple conflicting types with the same name to the same dict. The mechanism used for turning on the non-root-visible flag in order to do this had a kludge attached which always hid types with the same name, whether or not they were conflicting. This is unnecessary and can hide types that should not be hidden, as well as hiding bugs. Remove it, and replace it with two different approaches: - for everything but cu-mapped links (the in-memory first phase of a link with ctf_link_add_cu_mapping in force), check for duplicate names if types are conflicting, and mark them as hidden if the names are found. This will never happen in normal links (in an upcoming commit we will suppress doing even this much in such cases). - for cu-mapped links, the only case that merges multiple distinct target dicts into one, we apply a big hammer and simply hide everything! The non-root flag will be ignored in the next link phase anyway (which dedups the cu-mapped pieces against each other), and this way we can be sure that merging multiple types cannot incur name clashes at this stage. The result seems to work: the only annoyance is that when enums with conflicting enumerators are found in a single cu-mapped child (so, really multiple merged children), you may end up with every instance of that enum being hidden for reasons of conflictingness. I don't see a real way to avoid that. libctf/ PR libctf/33047 * ctf-dedup.c (ctf_dedup_emit_type): Only consider non conflicting types. Improve type hiding in the presence of clashing enumerators. Hide everything when doing a cu-mapped link: they will be unhidden by the next link pass if nonconflicting. --- diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c index e442f59ea4c..5a3c7bd6af2 100644 --- a/libctf/ctf-dedup.c +++ b/libctf/ctf-dedup.c @@ -2709,12 +2709,30 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs, name = ctf_strraw (real_input, tp->ctt_name); - /* Hide conflicting types, if we were asked to: also hide if a type with this - name already exists and is not a forward. */ - if (cu_mapped && is_conflicting) + /* Hide conflicting types, if this type is conflicting and a type with this + name already exists in the target and is not a forward. We don't want to + hide them if they are the only type of that name in a per-CU child, as is + usually the case, but in cu-mapped links and in the final phase after + cu-mapping distinct conflicting types from different CUs can end up in the + same cu-mapped target. Hide all but one in this case: it doesn't matter + which, all the conflicting types necessarily lost the popularity contest + anyway. + + cu_mapped links get absolutely *everything* marked non-root, named or not. + cu-mapped links, when we are merging multiple child CUs into one, are the + only point at which we can ever put conflicting and nonconflicting + instances of the same type into the same dict, and which one comes first is + arbitrary. Rather than having to figure out when we insert a type whether + another one is coming that might conflict with it without being so marked, + just mark everything as non-root: we'll disregard it in the next phase of + cu-mapped linking anyway. + + Note that enums also get their enumerands checked, below. */ + + if (cu_mapped) isroot = 0; - else if (name - && (maybe_dup = ctf_lookup_by_rawname (target, kind, name)) != 0) + else if (is_conflicting && name + && ((maybe_dup = ctf_lookup_by_rawname (target, kind, name)) != 0)) { if (ctf_type_kind (target, maybe_dup) != CTF_K_FORWARD) isroot = 0; @@ -2769,6 +2787,23 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs, { int val; errtype = _("enum"); + + /* Check enumerands for duplication: this is an extension of the isroot + check above. */ + + if (isroot) + { + const char *enumerand; + while ((enumerand = ctf_enum_next (input, type, &i, &val)) != NULL) + { + if (is_conflicting && name + && ctf_dynhash_lookup (target->ctf_names, enumerand) != NULL) + isroot = 0; + } + if (ctf_errno (input) != ECTF_NEXT_END) + goto err_input; + } + if ((new_type = ctf_add_enum (target, isroot, name)) == CTF_ERR) goto err_input; /* errno is set for us. */