From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by sourceware.org (Postfix) with ESMTPS id 090ED3857C62 for ; Fri, 28 Aug 2020 14:49:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 090ED3857C62 Received: by mail-pf1-x443.google.com with SMTP id g207so786916pfb.1 for ; Fri, 28 Aug 2020 07:49:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=+RNGr2tNGsssmssEUuYN413uV1cEL/UvGKNGgOAUm0I=; b=EBu3weSSh7VbACgCvzK1GW09uk+kgLKGoxpWVu3+3Sl5Lor6oj16hli75KjqswbzyD Fsgb1kmD7txIh0Bh0r2RdMl+cB0Mnslx0BoTbh1G0zMFeyV3mHHXy5QLthTz7UirR+0a SyhyqZw6Ob4dPOvY/PodyhsmgmhjuYZt9CTa+aJ+1gQTPDEaFtYdOhSeQiDE/+ydrA8v uN1qIc0p9rEgieQ4AiPQukdWHMf2filjHOMe0GqwbsYsguBMy9icnwITtN2sfosk54Lw LXk2OPkPVv1wnWj7163LIThF8wSmogM6ZbEtNmZ7IIkrrZARiBe6rO8taeUlCWagqVtC XMUQ== X-Gm-Message-State: AOAM5317CXlMdFE4D6YEuAxBH8qAj/rKEEc+xWYzUtXrEtpkW0saOsIu 3dKabSc+VyC/zHQK3u5LQYU= X-Google-Smtp-Source: ABdhPJw6jE8/65d3Cv4FSd3H5fL9AhhwfLqQQ1GssG5qr+LLsAE4AceP/joffxH7nlanzZFhgCxPAg== X-Received: by 2002:a05:6a00:1356:: with SMTP id k22mr1587337pfu.76.1598626159202; Fri, 28 Aug 2020 07:49:19 -0700 (PDT) Received: from bubble.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id e17sm1882240pfm.60.2020.08.28.07.49.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Aug 2020 07:49:18 -0700 (PDT) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id DB9A0815E6; Sat, 29 Aug 2020 00:19:14 +0930 (ACST) Date: Sat, 29 Aug 2020 00:19:14 +0930 From: Alan Modra To: "H.J. Lu" Cc: Binutils Subject: Re: [PATCH] elf: Don't load archive element after dynamic definition Message-ID: <20200828144914.GP15695@bubble.grove.modra.org> References: <20200825172842.1212936-1-hjl.tools@gmail.com> <20200827135311.GD15695@bubble.grove.modra.org> <20200828015847.GG15695@bubble.grove.modra.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-Spam-Status: No, score=-12.5 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:49:21 -0000 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". > 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