From nobody Sun Oct 24 21:29:53 2021 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 93187181E0C6; Sun, 24 Oct 2021 21:29:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Hcrp93kvNz4yZQ; Sun, 24 Oct 2021 21:29:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5C3761CD07; Sun, 24 Oct 2021 21:29:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 19OLTrlQ081681; Sun, 24 Oct 2021 21:29:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 19OLTrmw081680; Sun, 24 Oct 2021 21:29:53 GMT (envelope-from git) Date: Sun, 24 Oct 2021 21:29:53 GMT Message-Id: <202110242129.19OLTrmw081680@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 39545ce06ca8 - stable/12 - libctf: Improve check for duplicate SOU definitions in ctf_add_type() List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 39545ce06ca8088aecc68b92c028b78bcae888a2 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/12 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=39545ce06ca8088aecc68b92c028b78bcae888a2 commit 39545ce06ca8088aecc68b92c028b78bcae888a2 Author: Mark Johnston AuthorDate: 2021-10-04 16:28:22 +0000 Commit: Mark Johnston CommitDate: 2021-10-24 21:29:36 +0000 libctf: Improve check for duplicate SOU definitions in ctf_add_type() When copying a struct or union from one CTF container to another, ctf_add_type() checks whether it matches an existing type in the destination container. It does so by looking for a type with the same name and kind as the new type, and if one exists, it iterates over all members of the source type and checks whether a member with matching name and offset exists in the matched destination type. This can produce false positives, for example because member types are not compared, but this is not expected to arise in practice. If the match fails, ctf_add_type() returns an error. The procedure used for member comparison breaks down in the face of anonymous struct and union members. ctf_member_iter() visits each member in the source definition and looks up the corresponding member in the desination definition by name using ctf_member_info(), but this function will descend into anonymous members and thus fail to match. Fix the problem by introducing a custom comparison routine which does not assume member names are unique. This should also be faster for types with many members; in the previous scheme, membcmp() would perform a linear scan of the desination type's members to perform a lookup by name. The new routine steps through the members of both types in a single loop. PR: 258763 Sponsored by: The FreeBSD Foundation (cherry picked from commit 105fd928b0b5b35ab529e5f6914788dc49582901) --- cddl/contrib/opensolaris/common/ctf/ctf_create.c | 100 +++++++++++++++++------ 1 file changed, 73 insertions(+), 27 deletions(-) diff --git a/cddl/contrib/opensolaris/common/ctf/ctf_create.c b/cddl/contrib/opensolaris/common/ctf/ctf_create.c index a2ca81960f73..bf9b3b26b200 100644 --- a/cddl/contrib/opensolaris/common/ctf/ctf_create.c +++ b/cddl/contrib/opensolaris/common/ctf/ctf_create.c @@ -1196,17 +1196,6 @@ enumadd(const char *name, int value, void *arg) name, value) == CTF_ERR); } -/*ARGSUSED*/ -static int -membcmp(const char *name, ctf_id_t type, ulong_t offset, void *arg) -{ - ctf_bundle_t *ctb = arg; - ctf_membinfo_t ctm; - - return (ctf_member_info(ctb->ctb_file, ctb->ctb_type, - name, &ctm) == CTF_ERR || ctm.ctm_offset != offset); -} - static int membadd(const char *name, ctf_id_t type, ulong_t offset, void *arg) { @@ -1240,6 +1229,71 @@ membadd(const char *name, ctf_id_t type, ulong_t offset, void *arg) return (0); } +static long +soucmp(ctf_file_t *src_fp, ctf_id_t src_type, ctf_file_t *dst_fp, + ctf_id_t dst_type) +{ + const struct ctf_type *src_tp, *dst_tp; + const char *src_name, *dst_name; + ssize_t src_sz, dst_sz, src_inc, dst_inc; + uint_t kind, n; + + if ((src_type = ctf_type_resolve(src_fp, src_type)) == CTF_ERR) + return (CTF_ERR); + if ((dst_type = ctf_type_resolve(dst_fp, dst_type)) == CTF_ERR) + return (CTF_ERR); + + if ((src_tp = ctf_lookup_by_id(&src_fp, src_type)) == NULL) + return (CTF_ERR); + if ((dst_tp = ctf_lookup_by_id(&dst_fp, dst_type)) == NULL) + return (CTF_ERR); + + if ((kind = LCTF_INFO_KIND(src_fp, src_tp->ctt_info)) != + LCTF_INFO_KIND(dst_fp, dst_tp->ctt_info)) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + if (kind != CTF_K_STRUCT && kind != CTF_K_UNION) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + if ((n = LCTF_INFO_VLEN(src_fp, src_tp->ctt_info)) != + LCTF_INFO_VLEN(dst_fp, dst_tp->ctt_info)) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + + (void) ctf_get_ctt_size(src_fp, src_tp, &src_sz, &src_inc); + (void) ctf_get_ctt_size(dst_fp, dst_tp, &dst_sz, &dst_inc); + if (src_sz != dst_sz || src_inc != dst_inc) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + + if (src_sz < CTF_LSTRUCT_THRESH) { + const ctf_member_t *src_mp, *dst_mp; + + src_mp = (const ctf_member_t *)((uintptr_t)src_tp + src_inc); + dst_mp = (const ctf_member_t *)((uintptr_t)dst_tp + dst_inc); + for (; n != 0; n--, src_mp++, dst_mp++) { + if (src_mp->ctm_offset != dst_mp->ctm_offset) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + src_name = ctf_strptr(src_fp, src_mp->ctm_name); + dst_name = ctf_strptr(dst_fp, dst_mp->ctm_name); + if (strcmp(src_name, dst_name) != 0) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + } + } else { + const ctf_lmember_t *src_mp, *dst_mp; + + src_mp = (const ctf_lmember_t *)((uintptr_t)src_tp + src_inc); + dst_mp = (const ctf_lmember_t *)((uintptr_t)dst_tp + dst_inc); + for (; n != 0; n--, src_mp++, dst_mp++) { + if (src_mp->ctlm_offsethi != dst_mp->ctlm_offsethi || + src_mp->ctlm_offsetlo != dst_mp->ctlm_offsetlo) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + src_name = ctf_strptr(src_fp, src_mp->ctlm_name); + dst_name = ctf_strptr(dst_fp, dst_mp->ctlm_name); + if (strcmp(src_name, dst_name) != 0) + return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); + } + } + + return (0); +} + /* * The ctf_add_type routine is used to copy a type from a source CTF container * to a dynamic destination container. This routine operates recursively by @@ -1439,23 +1493,15 @@ ctf_add_type(ctf_file_t *dst_fp, ctf_file_t *src_fp, ctf_id_t src_type) ctf_dmdef_t *dmd; int errs = 0; - /* - * Technically to match a struct or union we need to check both - * ways (src members vs. dst, dst members vs. src) but we make - * this more optimal by only checking src vs. dst and comparing - * the total size of the structure (which we must do anyway) - * which covers the possibility of dst members not in src. - * This optimization can be defeated for unions, but is so - * pathological as to render it irrelevant for our purposes. - */ if (dst_type != CTF_ERR && dst_kind != CTF_K_FORWARD) { - if (ctf_type_size(src_fp, src_type) != - ctf_type_size(dst_fp, dst_type)) - return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); - - if (ctf_member_iter(src_fp, src_type, membcmp, &dst)) - return (ctf_set_errno(dst_fp, ECTF_CONFLICT)); - + /* + * Compare the sizes and fields of the two types. + * The field comparisons only check the names and + * offsets, so this is not perfect but is good enough + * for scenarios that we care about. + */ + if (soucmp(src_fp, src_type, dst_fp, dst_type) != 0) + return (CTF_ERR); /* errno is set for us */ break; }