From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54345 invoked by alias); 28 Apr 2015 18:16:52 -0000 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 Received: (qmail 54333 invoked by uid 89); 28 Apr 2015 18:16:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-vn0-f51.google.com Received: from mail-vn0-f51.google.com (HELO mail-vn0-f51.google.com) (209.85.216.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 28 Apr 2015 18:16:50 +0000 Received: by vnbg7 with SMTP id g7so410773vnb.10 for ; Tue, 28 Apr 2015 11:16:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=nYSm8foXtpVdgA3clgUfh1sVDMLWKXc9koogKeCivbA=; b=OSb9kTHhElu5JYF50U/Gqx1zxktIlraPXydXEzka0pw76fUq1VWcXyA/CSjveuga1K A9zFMNp9wXvsl+s1+mq6JKX8LPa92Xm0qx6daoRVMU331tCvg5Y6nXvkS4RztBEMW/lE Scl4/l4+Hpm4SzXHKxRBWLO3pWgfYbfGqazeUdm9EiDi3pwSpAkCRreQS9N8j9tpQOu2 h7XohmQWr4p7ZC48MvWcrvM1jXjdgzpMmmW5TapjospHNvM+xsboovtzHzQULKbWiK7f 8D4hGkKq7S1kXrB0m3lueGUW5hJ1rjcCRwu9Bg5wPul6at/LKeSNbuo41cM7gkibb4h0 OBuA== X-Gm-Message-State: ALoCoQmk05eOzpCbW7xhC/znlWUv7bp8XipEulyW4tKuOP1KI8OIZAoHfeKFxQKeobB4ZJEQffN+ MIME-Version: 1.0 X-Received: by 10.52.240.45 with SMTP id vx13mr38094973vdc.22.1430245007669; Tue, 28 Apr 2015 11:16:47 -0700 (PDT) Received: by 10.52.122.52 with HTTP; Tue, 28 Apr 2015 11:16:47 -0700 (PDT) In-Reply-To: References: Date: Tue, 28 Apr 2015 18:16:00 -0000 Message-ID: Subject: Re: [patch] Ignore non relobj files in gc From: Sriraman Tallam To: Cary Coutant Cc: =?UTF-8?Q?Rafael_Esp=C3=ADndola?= , Binutils Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00434.txt.bz2 On Mon, Apr 27, 2015 at 1:17 PM, Cary Coutant wrote: >> If a relocation points to a dynamic object we can ignore it, since we >> cannot gc an already existing .so file. >> >> The attached patch produces an identical chromium binary, but does so >> a bit faster (see the attached perf logs). > > I'd think the one-line patch below accomplishes the same result with > much less disruption: > > @@ -340,7 +341,7 @@ gc_process_relocs( > src_obj)); > } > > - if (gsym->source() != Symbol::FROM_OBJECT) > + if (dst_obj == NULL || dst_obj->is_dynamic()) > continue; > if (!is_ordinary) > continue; > >> Using Relobj also helps me in my patch for gcing parts of SHF_MERGE sections. > > If that's the case, let's move those changes to a separate patch. > Nevertheless, I think it would be cleaner to change Section_id to use > a Relobj* directly. That has some cascading effects, but doesn't > affect quite as much target-independent code, and we really never need > a Section_id that refers to a non-relocatable object. The attached > patch does that. > > Sri, would you mind taking a look to make sure I haven't done anything > in the attached patch that might break --icf? Here's the only place > where we might have created a Section_id with a pointer to a Dynobj, > but I don't think that makes sense: > > @@ -324,8 +326,11 @@ gc_process_relocs( > if (is_icf_tracked) > { > Address symvalue = dst_off - addend; > - if (is_ordinary && gsym->source() == Symbol::FROM_OBJECT) > - (*secvec).push_back(Section_id(dst_obj, dst_indx)); > + if (is_ordinary > + && gsym->source() == Symbol::FROM_OBJECT > + && !dst_obj->is_dynamic()) > + (*secvec).push_back(Section_id(static_cast(dst_obj), > + dst_indx)); > else > (*secvec).push_back(Section_id(NULL, 0)); > (*symvec).push_back(gsym); You have two sections that are mostly identical except a call instruction that is calling two different functions in two different shared objects, you are not tracking that anymore. We are tracking the symbol name in the following line by pushing gsym onto symvec, so this seems fine. If there is ever a case where a symbol corresponding to a relocation to a dynamic object has a NULL name, we could be in trouble but I cannot think of anything like that. Thanks Sri > > Perhaps it does make sense here to track a relocation pointing into a > dynamic object, but in this case, the static_cast might still be OK, > since I think at this point, we're only using the object pointer as an > opaque ID for the object. > > -cary