From patchwork Mon Jul 7 17:22:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: 'Timothy Arceri' via Patchwork Forward X-Patchwork-Id: 33169 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ob0-f198.google.com (mail-ob0-f198.google.com [209.85.214.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id C672A20C88 for ; Mon, 7 Jul 2014 17:22:40 +0000 (UTC) Received: by mail-ob0-f198.google.com with SMTP id uy5sf27761389obc.1 for ; Mon, 07 Jul 2014 10:22:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:reply-to :content-type; bh=Bz9j2MhBYHeJEpHFvqKJj7xdayOHp9nN5B6gwyv/m2I=; b=c5H9fvCjuYD9wtdtzrtb7MQTDd5xXEHcNRqwSFGwAtPoexGMO54Uj/nhLyb0IuSGvk GAlJye6e1R0ze8zi8hHSRpLV6xsUIFI2/5yY5ca6Ffc/0HB40FE38mtT5fniEkHZEkJ/ T+ehgAtiV/ll2Rr+j9d0sZ6OunMSFiE+Y5tIVDcEw67EWqLDdMqA/mcMh4LApjDcb1n3 Nz3ajk7h/QFPiSZGmz5E9iY459/SwoD0JctxLTAD+kNBmHTYttyHOQXLxtUj9uoYibns nXYtYO7hyQ5N4p7nP1Gya8KpYhAs6ERfSbQOoScXdbff54GkV8hm1DGhW4j+pKXA3l1C JRrg== X-Gm-Message-State: ALoCoQmiZJuxxXCNEngMUcQggAEknqX1wQqIdVY/HLd82uUnTcm03PHCCPoBMpPoGMZ9Keao8RMB X-Received: by 10.43.92.195 with SMTP id br3mr15238172icc.1.1404753760146; Mon, 07 Jul 2014 10:22:40 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.88.85 with SMTP id s79ls1814929qgd.77.gmail; Mon, 07 Jul 2014 10:22:40 -0700 (PDT) X-Received: by 10.220.179.197 with SMTP id br5mr65860vcb.80.1404753760052; Mon, 07 Jul 2014 10:22:40 -0700 (PDT) Received: from mail-vc0-x234.google.com (mail-vc0-x234.google.com [2607:f8b0:400c:c03::234]) by mx.google.com with ESMTPS id ak16si10050907vdc.93.2014.07.07.10.22.40 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 07 Jul 2014 10:22:40 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2607:f8b0:400c:c03::234 as permitted sender) client-ip=2607:f8b0:400c:c03::234; Received: by mail-vc0-f180.google.com with SMTP id im17so4221917vcb.39 for ; Mon, 07 Jul 2014 10:22:40 -0700 (PDT) X-Received: by 10.58.188.199 with SMTP id gc7mr28876359vec.4.1404753759965; Mon, 07 Jul 2014 10:22:39 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp687679vcb; Mon, 7 Jul 2014 10:22:39 -0700 (PDT) X-Received: by 10.224.13.4 with SMTP id z4mr47601414qaz.51.1404753759308; Mon, 07 Jul 2014 10:22:39 -0700 (PDT) Received: from mail-qg0-x229.google.com (mail-qg0-x229.google.com [2607:f8b0:400d:c04::229]) by mx.google.com with ESMTPS id 9si52246564qal.1.2014.07.07.10.22.39 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 07 Jul 2014 10:22:39 -0700 (PDT) Received-SPF: pass (google.com: domain of ccoutant@google.com designates 2607:f8b0:400d:c04::229 as permitted sender) client-ip=2607:f8b0:400d:c04::229; Received: by mail-qg0-f41.google.com with SMTP id i50so4108438qgf.0 for ; Mon, 07 Jul 2014 10:22:39 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.224.22.12 with SMTP id l12mr51578619qab.88.1404753759118; Mon, 07 Jul 2014 10:22:39 -0700 (PDT) Received: by 10.140.85.47 with HTTP; Mon, 7 Jul 2014 10:22:38 -0700 (PDT) In-Reply-To: References: <52E8E59A.4040005@linaro.org> Date: Mon, 7 Jul 2014 10:22:38 -0700 Message-ID: Subject: Re: [Bug15639][ARM] gold and -flto always fails with an internal error on arm-linux-gnueabi* From: "'Cary Coutant' via Patchwork Forward" To: =?UTF-8?B?RG91ZyBLd2FuICjpl5zmjK/lvrcp?= Cc: Kugan , binutils , "patches@linaro.org" X-Original-Sender: ccoutant@google.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2607:f8b0:400c:c03::234 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@google.com; dmarc=pass (p=REJECT dis=NONE) header.from=google.com Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , X-Original-From: Cary Coutant Reply-To: Cary Coutant My apologies -- I must have missed this patch (and your ping). My first reaction is that we probably shouldn't be calling the Target version of do_read_symbols from do_layout_deferred_sections. This same problem could occur on other targets that have an override of do_read_symbols. I'd suggest instead factoring out the body of Sized_relobj_file::do_read_symbols() into a non-virtual, protected, method Sized_relobj_file::base_read_symbols(), and have Sized_relobj_file::do_read_symbols() call it. Then call that instead of read_symbols() from do_layout_deferred_sections(). The target-specific overrides of do_read_symbols() should also call the new method directly instead of calling the base class' do_read_symbols(). Could you give the attached patch a try and see if it fixes your problem? -cary On Fri, Jul 4, 2014 at 2:47 PM, Doug Kwan (關振德) wrote: > Looks good to me. I don't have approval power. Cary, could you approval > this? > > -Doug > > > On Wed, Jan 29, 2014 at 3:27 AM, Kugan > wrote: >> >> Hi, >> >> In Sized_relobj_file> big_endian>::do_layout_deferred_sections(Layout* layout), while reading >> .eh_frame, Arm_relobj::do_read_symbols(Read_symbols_data* >> sd) is called second time around and this triggering the assert. >> >> This patch removes the assert and skips reading if this section is >> already read. >> >> Is this OK? >> >> Thanks, >> Kugan >> >> >> gold/ >> +2014-01-29 Kugan Vivekanandarajah >> + >> + * arm.cc (Arm_relobj::do_read_symbols): Skip reading >> + .ARM.attributes section if already read. >> + > > commit bc0b896929c1cc8735f391b82940f5e46f38dcb9 Author: Cary Coutant Date: Mon Jul 7 10:14:45 2014 -0700 Fix internal error with LTO on ARM. This prevents the target-specific do_read_symbols methods from being called twice when do_layout_deferred_sections needs to layout an .eh_frame section. gold/ * dynobj.h (Sized_dynobj::base_read_symbols): New method. * dynobj.cc (Sized_dynobj::do_read_symbols): Move body to... (Sized_dynobj::base_read_symbols): ...new method. * object.h (Sized_relobj_file::base_read_symbols): New method. * object.cc (Sized_relobj_file::do_read_symbols): Move body to... (Sized_relobj_file::base_read_symbols): ...new method. * arm.cc (Arm_relobj::do_read_symbols): Call base_read_symbols. * mips.cc: (Mips_relobj::do_read_symbols): Likewise. * powerpc.cc (Powerpc_dynobj::do_read_symbols): Likewise. diff --git a/gold/arm.cc b/gold/arm.cc index 607f9d6..6c472bb 100644 --- a/gold/arm.cc +++ b/gold/arm.cc @@ -6683,7 +6683,7 @@ void Arm_relobj::do_read_symbols(Read_symbols_data* sd) { // Call parent class to read symbol information. - Sized_relobj_file<32, big_endian>::do_read_symbols(sd); + this->base_read_symbols(sd); // If this input file is a binary file, it has no processor // specific flags and attributes section. @@ -6974,7 +6974,7 @@ void Arm_dynobj::do_read_symbols(Read_symbols_data* sd) { // Call parent class to read symbol information. - Sized_dynobj<32, big_endian>::do_read_symbols(sd); + this->base_read_symbols(sd); // Read processor-specific flags in ELF file header. const unsigned char* pehdr = this->get_view(elfcpp::file_header_offset, diff --git a/gold/dynobj.cc b/gold/dynobj.cc index 2a1b9a3..baf8489 100644 --- a/gold/dynobj.cc +++ b/gold/dynobj.cc @@ -336,6 +336,17 @@ template void Sized_dynobj::do_read_symbols(Read_symbols_data* sd) { + this->base_read_symbols(sd); +} + +// Read the symbols and sections from a dynamic object. We read the +// dynamic symbols, not the normal symbols. This is common code for +// all target-specific overrides of do_read_symbols(). + +template +void +Sized_dynobj::base_read_symbols(Read_symbols_data* sd) +{ this->read_section_data(&this->elf_file_, sd); const unsigned char* const pshdrs = sd->section_headers->data(); diff --git a/gold/dynobj.h b/gold/dynobj.h index b8d4b90..03b8053 100644 --- a/gold/dynobj.h +++ b/gold/dynobj.h @@ -270,6 +270,12 @@ class Sized_dynobj : public Dynobj do_get_global_symbols() const { return this->symbols_; } + protected: + // Read the symbols. This is common code for all target-specific + // overrides of do_read_symbols(). + void + base_read_symbols(Read_symbols_data*); + private: // For convenience. typedef Sized_dynobj This; diff --git a/gold/mips.cc b/gold/mips.cc index 50d0227..450883e 100644 --- a/gold/mips.cc +++ b/gold/mips.cc @@ -5857,7 +5857,7 @@ void Mips_relobj::do_read_symbols(Read_symbols_data* sd) { // Call parent class to read symbol information. - Sized_relobj_file::do_read_symbols(sd); + this->base_read_symbols(sd); // Read processor-specific flags in ELF file header. const unsigned char* pehdr = this->get_view(elfcpp::file_header_offset, diff --git a/gold/object.cc b/gold/object.cc index c894c13..1811cda 100644 --- a/gold/object.cc +++ b/gold/object.cc @@ -755,6 +755,16 @@ template void Sized_relobj_file::do_read_symbols(Read_symbols_data* sd) { + this->base_read_symbols(sd); +} + +// Read the sections and symbols from an object file. This is common +// code for all target-specific overrides of do_read_symbols(). + +template +void +Sized_relobj_file::base_read_symbols(Read_symbols_data* sd) +{ this->read_section_data(&this->elf_file_, sd); const unsigned char* const pshdrs = sd->section_headers->data(); @@ -1848,7 +1858,7 @@ Sized_relobj_file::do_layout_deferred_sections(Layout* layout) // Reading the symbols again here may be slow. Read_symbols_data sd; - this->read_symbols(&sd); + this->base_read_symbols(&sd); this->layout_eh_frame_section(layout, sd.symbols->data(), sd.symbols_size, diff --git a/gold/object.h b/gold/object.h index 38b06f0..92cdbdd 100644 --- a/gold/object.h +++ b/gold/object.h @@ -2214,6 +2214,11 @@ class Sized_relobj_file : public Sized_relobj void do_read_symbols(Read_symbols_data*); + // Read the symbols. This is common code for all target-specific + // overrides of do_read_symbols. + void + base_read_symbols(Read_symbols_data*); + // Return the value of a local symbol. uint64_t do_local_symbol_value(unsigned int symndx, uint64_t addend) const diff --git a/gold/powerpc.cc b/gold/powerpc.cc index 96432ed..0a9ab7d 100644 --- a/gold/powerpc.cc +++ b/gold/powerpc.cc @@ -1839,7 +1839,7 @@ template void Powerpc_relobj::do_read_symbols(Read_symbols_data* sd) { - Sized_relobj_file::do_read_symbols(sd); + this->base_read_symbols(sd); if (size == 64) { const int shdr_size = elfcpp::Elf_sizes::shdr_size; @@ -1896,14 +1896,14 @@ Powerpc_dynobj::set_abiversion(int ver) } } -// Call Sized_dynobj::do_read_symbols to read the symbols then +// Call Sized_dynobj::base_read_symbols to read the symbols then // read .opd from a dynamic object, filling in opd_ent_ vector, template void Powerpc_dynobj::do_read_symbols(Read_symbols_data* sd) { - Sized_dynobj::do_read_symbols(sd); + this->base_read_symbols(sd); if (size == 64) { const int shdr_size = elfcpp::Elf_sizes::shdr_size;