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 34B9A3857C62 for ; Fri, 28 Aug 2020 14:51:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 34B9A3857C62 Received: by mail-io1-xd41.google.com with SMTP id g14so1510390iom.0 for ; Fri, 28 Aug 2020 07:51:28 -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=QORDyxpk0NZelegAnvobyyyIqi5/mjxVPovlZbJ5Dy4=; b=f45dLEj0SsD7+s+qR6/L0Ff8YEJJeb4Uw/wrsMcfMiwNRrxLDNpKdqb9bVvc0REYqh 6JjzHyjP7V8qfWrq+0/Bc1HoF5Hj9fih3E+t6HXrj+xTGINHEJ1D3cskZyoXfuYlYhZg oc3XTd/NxV/wUMWGLRMfgxBC61FgyGaOsZwmzzNVOeDnz2zC0tT1nf/YF5Qi+vQPSK2O UBFt85gEEX6NDGgRK5PQT3WTRujKFQOXk9VQUlFVzF7c1K928gFfuQUVpGZEyGLiZ3ie 3h+kTxfsvbunyU0Fy6aPNakb6DNWKZrAxDTMZgJ5LjTEzKlB2DRl7Eb/MX3SYYvU+nKw vGQQ== X-Gm-Message-State: AOAM532mhmEchCYtp+cymDqgvwrhc14HrJhWv2S4By8dW7+xb536hfjE jxxFZ7DuZYmQI4UXOPjrexJucnwCnN8EDVeo3vE= X-Google-Smtp-Source: ABdhPJzd/cahRdMZoqewXreuothWyLUL06BF1hbN9R+pBe6zmQupDw56l2xlb1b11MfHuC1KJusWvOrjmCKsQrzHBLA= X-Received: by 2002:a5e:db42:: with SMTP id r2mr1610947iop.37.1598626287675; Fri, 28 Aug 2020 07:51:27 -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> <20200828144914.GP15695@bubble.grove.modra.org> In-Reply-To: <20200828144914.GP15695@bubble.grove.modra.org> From: "H.J. Lu" Date: Fri, 28 Aug 2020 07:50:51 -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 14:51:30 -0000 On Fri, Aug 28, 2020 at 7:49 AM Alan Modra wrote: > > On Fri, Aug 28, 2020 at 05:53:10AM -0700, H.J. Lu wrote: > > 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. > > Please explain "doesn't work". PR ld/26530 test still failed. > > 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; > > -- > Alan Modra > Australia Development Lab, IBM -- H.J.