From patchwork Sat May 28 00:56:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fangrui Song X-Patchwork-Id: 54474 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 288DF383919D for ; Sat, 28 May 2022 00:57:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 288DF383919D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1653699446; bh=tgmdZr+kSsdenvggFMd7WM34hW7Ic1zAMFXH1dK+Nrw=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=RvsLRTAkyQHyR49eSjCOSaB87yJsKMCf5lSF7Cw9EPONekt+w5uC1zhpT/7KSA9ZM mML2zxge+Qhh/wV3NK6FxbdsDiikVxM7h8MZGL7YU8QBYzYnMONJMF1pA9TV80VMgk 1UEg9IKOWxjvmWslfad2YRlNGbZBlmnT2V6T55qk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by sourceware.org (Postfix) with ESMTPS id D15A63857017 for ; Sat, 28 May 2022 00:57:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D15A63857017 Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-30974094b5cso18581057b3.20 for ; Fri, 27 May 2022 17:57:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=tgmdZr+kSsdenvggFMd7WM34hW7Ic1zAMFXH1dK+Nrw=; b=6k8U+TnJYcRZP90z+cefmgl3UWhFXUqauepb0/mBDECW58udKvIUXwiLRj7edt+mAo A+7QtN3Wm/Nx50N0k3z/d9TV3nwQZy0jEF+wubgIiLRrKNqLLvPtXSJ7z49jzplgJuzL tE+xFb+Vgxa2/NbKrOmQWbv6/fIlXat7rJFEq1obB8NMjC1tigQNDkaFJESGvglvJZGj JkYANsxTjdgaDKaqS2TI+IPOnTh5DovEwddyK/MDBkHK193kPUPRVL7KafGyljQQVjYM qlKsXv2FUgr/Zb2wu750uPSccR5PXdhKvBSjArb9KFjutpdnPylT+HB9+OcRDF81Pr6C xDrg== X-Gm-Message-State: AOAM530gTTdvB9YWHsGwapSHXHXt67tJPqMtVikxXyW6kI7PL047T/6K Y3GFKSv/drBQR8Zm8fiI4Akg21A3inycRvlYVqY63QXMdLBofMWR7nDTW9lpdDNWwTfWd7t9LBe fh8OdDE6WdiW9t3SLMAkPneLANIQpor2qHUO+GoUT1nrDFzwuIiuBaCQOetbVh5N6jGU9 X-Google-Smtp-Source: ABdhPJxKOEczC9JZS1ORbiRlTuA2f4ZzM7hCMdQ1RCI+7j/uu7EbFVJktiV1/jgTboq5Pkl526vFq670TtQT X-Received: from maskray1.svl.corp.google.com ([2620:15c:2ce:200:781f:286a:7724:6870]) (user=maskray job=sendgmr) by 2002:a25:9b87:0:b0:656:1629:7b1b with SMTP id v7-20020a259b87000000b0065616297b1bmr13587542ybo.523.1653699423200; Fri, 27 May 2022 17:57:03 -0700 (PDT) Date: Fri, 27 May 2022 17:56:58 -0700 Message-Id: <20220528005658.1088272-1-maskray@google.com> Mime-Version: 1.0 Subject: [PATCH v2] elf: Remove one-default-version check when searching an unversioned symbol To: libc-alpha@sourceware.org X-Spam-Status: No, score=-19.5 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Fangrui Song via Libc-alpha From: Fangrui Song Reply-To: Fangrui Song Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" When searching an unversioned symbol in an object with DT_VERDEF, we have two cases: * there is a VER_NDX_GLOBAL definition * there are a few version index>=2 definitions, with at most one being the default version `versioned_sym` may be set in the second case. Since the linker ensures at most one default version (otherwise ``multiple definition of `foo'``), it is of very little value for the loader to check this corner case. Delete `num_versions` to simplify code. While here, add some comments. --- Changes from v1 * Add comments * Clarify commit message --- elf/dl-lookup.c | 52 +++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c index a42f6d5390..0245f76799 100644 --- a/elf/dl-lookup.c +++ b/elf/dl-lookup.c @@ -66,8 +66,7 @@ check_match (const char *const undef_name, const Elf_Symndx symidx, const char *const strtab, const struct link_map *const map, - const ElfW(Sym) **const versioned_sym, - int *const num_versions) + const ElfW(Sym) **const versioned_sym) { unsigned int stt = ELFW(ST_TYPE) (sym->st_info); assert (ELF_RTYPE_CLASS_PLT == 1); @@ -92,6 +91,9 @@ check_match (const char *const undef_name, return NULL; const ElfW(Half) *verstab = map->l_versyms; + /* If the object is versioned, the linker ensures that either there is a + VER_NDX_GLOBAL definition or there are a few version index>=2 definitions, + with at most one being the default version. */ if (version != NULL) { if (__glibc_unlikely (verstab == NULL)) @@ -124,33 +126,22 @@ check_match (const char *const undef_name, } else { - /* No specific version is selected. There are two ways we - can got here: + /* Looking for an unversioned symbol. If the object is unversioned or + there is a VER_NDX_GLOBAL definition, return sym. Otherwise: - - a binary which does not include versioning information - is loaded + If DL_LOOKUP_RETURN_NEWEST is set, return the default version if + exists. - - dlsym() instead of dlvsym() is used to get a symbol which - might exist in more than one form - - If the library does not provide symbol version information - there is no problem at all: we simply use the symbol if it - is defined. - - These two lookups need to be handled differently if the - library defines versions. In the case of the old - unversioned application the oldest (default) version - should be used. In case of a dlsym() call the latest and - public interface should be returned. */ + If DL_LOOKUP_RETURN_NEWEST is unset, return the index 2 definition if + exists, otherwise return the default version if exists. When the + object became versioned, the original unversioned definition may take + index 2. */ if (verstab != NULL) { if ((verstab[symidx] & 0x7fff) >= ((flags & DL_LOOKUP_RETURN_NEWEST) ? 2 : 3)) { - /* Don't accept hidden symbols. */ - if ((verstab[symidx] & 0x8000) == 0 - && (*num_versions)++ == 0) - /* No version so far. */ + if ((verstab[symidx] & 0x8000) == 0) *versioned_sym = sym; return NULL; @@ -381,7 +372,6 @@ do_lookup_x (const char *undef_name, unsigned int new_hash, continue; Elf_Symndx symidx; - int num_versions = 0; const ElfW(Sym) *versioned_sym = NULL; /* The tables for this map. */ @@ -415,8 +405,7 @@ do_lookup_x (const char *undef_name, unsigned int new_hash, symidx = ELF_MACHINE_HASH_SYMIDX (map, hasharr); sym = check_match (undef_name, ref, version, flags, type_class, &symtab[symidx], symidx, - strtab, map, &versioned_sym, - &num_versions); + strtab, map, &versioned_sym); if (sym != NULL) goto found_it; } @@ -440,18 +429,17 @@ do_lookup_x (const char *undef_name, unsigned int new_hash, { sym = check_match (undef_name, ref, version, flags, type_class, &symtab[symidx], symidx, - strtab, map, &versioned_sym, - &num_versions); + strtab, map, &versioned_sym); if (sym != NULL) goto found_it; } } - /* If we have seen exactly one versioned symbol while we are - looking for an unversioned symbol and the version is not the - default version we still accept this symbol since there are - no possible ambiguities. */ - sym = num_versions == 1 ? versioned_sym : NULL; + /* When looking for an unversioned symbol, a default version is a fallback + when VER_NDX_GLOBAL is absent (and when DL_LOOKUP_RETURN_NEWEST is set, + index 2 is also absent). We don't repeat the check ensured by the + linker: a symbol name has at most one default version. */ + sym = versioned_sym; if (sym != NULL) {