From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd41.google.com (mail-io1-xd41.google.com [IPv6:2607:f8b0:4864:20::d41]) by sourceware.org (Postfix) with ESMTPS id 8372A3894C1B for ; Fri, 28 Aug 2020 12:53:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8372A3894C1B Received: by mail-io1-xd41.google.com with SMTP id g14so1099213iom.0 for ; Fri, 28 Aug 2020 05:53:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=poueVFkPyLATe+2rlCyk+30ra3JsvxDgnyITFj7Fsak=; b=Kv8Iit4p7F8lLlFMcU4wnfq/ny4XXqyzEFfSuc2CJ3GIi6WV54wepmw2awcvUw86kx 7lk1dFLCQms2Tb4nkD8ND5BfyANIt1sTnXkpS2H88AexTIU/xSHGX87tZ5dY/o9E+6AZ BQBKfnYdT3Qq2qOI5BjhxSoImTajE1b80mpg9qeNqbeIWGVsyAdQC04L31CYh2Lcxicc zPtBP73JCr9TCprFX3zJk0fNdwHqJu5fgJmngiGZQ64mFfyI6ZUlM1Z7UIasj5LeVCEP URbb5dUm2f4zTfuNNLgI+rzNgi89gryTqrTcg0hzvcPAcjN3STOfGm5SY9Mx+Xx19xZ+ RHmw== X-Gm-Message-State: AOAM532PezapF/X8dQKLFOCG6XNR7bpyK/P0XH/TS9Hm8gg4tLs2M6XZ KW6eWdDs55GPHwecLvEkQ4R0dwTn1BnLLM9JwVQ= X-Google-Smtp-Source: ABdhPJxogVTDrT2bR1yOUx2f9gjPzrZy62C2gClFAp1IQd1z+/yqsEvQ4aaeGagorEmSedSHOPouXzs1ZMyUFxDJNT8= X-Received: by 2002:a5e:db42:: with SMTP id r2mr1193434iop.37.1598619225960; Fri, 28 Aug 2020 05:53:45 -0700 (PDT) MIME-Version: 1.0 References: <20200825172842.1212936-1-hjl.tools@gmail.com> <20200827135311.GD15695@bubble.grove.modra.org> <20200828015847.GG15695@bubble.grove.modra.org> In-Reply-To: <20200828015847.GG15695@bubble.grove.modra.org> From: "H.J. Lu" Date: Fri, 28 Aug 2020 05:53:10 -0700 Message-ID: Subject: Re: [PATCH] elf: Don't load archive element after dynamic definition To: Alan Modra Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Aug 2020 12:53:48 -0000 On Thu, Aug 27, 2020 at 6:59 PM Alan Modra wrote: > > On Thu, Aug 27, 2020 at 07:05:15AM -0700, H.J. Lu wrote: > > On Thu, Aug 27, 2020 at 6:53 AM Alan Modra wrote: > > > > > > On Tue, Aug 25, 2020 at 10:28:42AM -0700, H.J. Lu via Binutils wrote: > > > > Don't load the archive element after seeing a definition in a shared > > > > object. > > > > > > > > bfd/ > > > > > > > > PR ld/26530 > > > > * elflink.c (elf_link_add_object_symbols): Also preserve the > > > > dynamic_def bit for --as-needed. > > > > (elf_link_add_archive_symbols): Don't load the archive element > > > > after seeing a definition in a shared object. > > > > > > This seems over complicated to me. Why don't we just load the > > > as-needed shared library on the first pass? > > > > > > > In the first pass, when we are loading a shared object, do we have > > the IR symbol resolution? > > No, but why should that matter? Adding DT_NEEDED for an as-needed > library that turns out to not be needed after LTO is not a problem > IMO. Also I think it may be necessary to load shared libraries on the > first pass for LTO to work properly in corner cases. See the comment > in git commit a896df97b952. > > The condition we use to add --as-needed libraries is: > if (!add_needed > && matched > && definition > && ((dynsym > && h->ref_regular_nonweak) > || (h->ref_dynamic_nonweak > && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0 > && !on_needed_list (elf_dt_name (abfd), > htab->needed, NULL)))) > Note the test of dynsym, but earlier > /* Nor should we make plugin symbols dynamic. */ > if ((abfd->flags & BFD_PLUGIN) != 0) > dynsym = FALSE; > > So I'm thinking perhaps we should delete the above lines and instead > add the BFD_PLUGIN condition where we make symbols dynamic. > > if (dynsym && (abfd->flags & BFD_PLUGIN) == 0 && h->dynindx == -1) > { > if (! bfd_elf_link_record_dynamic_symbol (info, h)) > > I haven't tested the idea.. > I tested the patch enclosed here and it doesn't work. In the first pass, when we are loading a shared object, we can't assume that the shared object is needed simply because it satisfies a reference from an IR object without the IR symbol resolution. -- H.J. ---- diff --git a/bfd/elflink.c b/bfd/elflink.c index 5c085b14b7..3c559bafc5 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -5162,10 +5162,6 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info) && !bfd_link_relocatable (info)) dynsym = FALSE; - /* Nor should we make plugin symbols dynamic. */ - if ((abfd->flags & BFD_PLUGIN) != 0) - dynsym = FALSE; - if (definition) { h->target_internal = isym->st_target_internal; @@ -5192,7 +5188,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info) } } - if (dynsym && h->dynindx == -1) + if (dynsym + && (abfd->flags & BFD_PLUGIN) == 0 + && h->dynindx == -1) { if (! bfd_elf_link_record_dynamic_symbol (info, h)) goto error_free_vers;