From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13025 invoked by alias); 6 Jul 2011 00:51:32 -0000 Received: (qmail 13017 invoked by uid 22791); 6 Jul 2011 00:51:31 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-gy0-f169.google.com (HELO mail-gy0-f169.google.com) (209.85.160.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 06 Jul 2011 00:51:08 +0000 Received: by gyg13 with SMTP id 13so3030666gyg.0 for ; Tue, 05 Jul 2011 17:51:08 -0700 (PDT) Received: by 10.236.155.133 with SMTP id j5mr9074702yhk.488.1309913468024; Tue, 05 Jul 2011 17:51:08 -0700 (PDT) Received: from bubble.grove.modra.org ([115.187.252.19]) by mx.google.com with ESMTPS id 1sm4883179yhs.53.2011.07.05.17.51.05 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 05 Jul 2011 17:51:07 -0700 (PDT) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 1AF45175A005; Wed, 6 Jul 2011 10:21:01 +0930 (CST) Date: Wed, 06 Jul 2011 03:27:00 -0000 From: Alan Modra To: "H.J. Lu" Cc: binutils@sourceware.org Subject: Re: RFC: PATCH: ld/12942: Plugin not handling correctly resolution of COMDATs. Message-ID: <20110706005101.GF26365@bubble.grove.modra.org> Mail-Followup-To: "H.J. Lu" , binutils@sourceware.org References: <20110704225328.GA16831@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110704225328.GA16831@intel.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2011-07/txt/msg00068.txt.bz2 On Mon, Jul 04, 2011 at 03:53:28PM -0700, H.J. Lu wrote: > Hi, > > This adds SEC_LTO_COMDAT and GNU_LTO_COMAT_SECTION_NAME. They are > used to support LTO COMDAT symbols. Any comments? This is a hard patch to review, almost as much work as writing it in the first place, because you don't explain why you need to do what you do. Also, with a slightly out of date gcc I couldn't reproduce the problem on powerpc or powerpc64, so is there something special about x86_64 or did gcc recently start using comdat groups for lto? > case SEC_LINK_DUPLICATES_DISCARD: > + if (info->loading_lto_outputs > + && (l_owner->flags & BFD_PLUGIN) != 0) It took me a while to convince myself this was correct, so I'd like to see this test commented. Perhaps /* If we found an LTO IR match for this comdat group on the first pass, replace it with the LTO output on the second pass. We can't simply choose real object files over IR because the first pass may contain a mix of LTO and normal objects and we must keep the first match, be it IR or real. */ > + { > + /* Replace the plugin dummy with the LTO output. */ > + l->linked = *linked; > + return; > + } > break; > + l_owner = l_sec->owner; > + l_sec = l->linked.u.sec; Ordering problem above. Did this compile? Please explain why the following change is necessary. I'd like to know why you need to have the correct symbol on the first pass. > + /* A symbol with comdat key in IR dummy BFD may override > + the comdat symbol in a real BFD. */ > + if (link_info.loading_lto_outputs > + || (h->u.def.section->flags & SEC_LTO_COMDAT) == 0) > + { > + h->type = bfd_link_hash_undefweak; > + h->u.undef.abfd = h->u.def.section->owner; > + } > + else > + h->non_ir_ref = TRUE; -- Alan Modra Australia Development Lab, IBM