From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10971 invoked by alias); 20 Dec 2013 18:19:31 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 10959 invoked by uid 89); 20 Dec 2013 18:19:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Dec 2013 18:19:29 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBKIJNCW006161 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 20 Dec 2013 13:19:23 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rBKIJLDY011458; Fri, 20 Dec 2013 13:19:22 -0500 Message-ID: <52B48A28.2000402@redhat.com> Date: Fri, 20 Dec 2013 18:19:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Pierre Muller CC: "'Joel Brobecker'" , gdb-patches@sourceware.org Subject: Re: [RFA-v2] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section References: <004801cef65e$cb82d1b0$62887510$@muller@ics-cnrs.unistra.fr> <20131211170204.GD3227@adacore.com> <52ab7ec0.c8da420a.12c6.ffffb3f4SMTPIN_ADDED_BROKEN@mx.google.com> In-Reply-To: <52ab7ec0.c8da420a.12c6.ffffb3f4SMTPIN_ADDED_BROKEN@mx.google.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-12/txt/msg00851.txt.bz2 On 12/13/2013 09:39 PM, Pierre Muller wrote: > >> I just re-read the code, and I really think it would be better if >> someone who actually understands the general framework could comment. >> The problem seems, as you stated, relatively well understood, but >> I am not sure how we are expected to fix it. >> >>> 2013-11-27 Pierre Muller >>> >>> PR 16201 >>> coff-pe-read.c (read_pe_exported_syms): Set sect_index_text, Missing '*' >>> sect_index_data and sect_index_bss of objfile struct, even if >>> there is no canonical '.text', '.data' or '.bss' named >> section. >> >> My only comment is that the patch could gain from some additional >> comments explaining _why_ you're forcing the sect_index field >> ("event if already set before"), and what you are trying to achieve. > > Here is a new version in which I try to explain > more clearly that if we find the canonical > '.text', '.data' or '.bss' section names, > we should use these sections to set sect_index_XXX. > Otherwise, we use the first section that is used later with > for which we set ms_type to mst_XXX to also set sect_index_XXX. > This ensure that sect_index_XXX is always set if > any exported symbol is in inserted using > prim_rcord_minimal_symbol with ms_type parameter set to mst_XXX > > I hope this clarifies the patch . > So in the DLL in question, there was no .data section, but there was another section with IMAGE_SCN_CNT_INITIALIZED_DATA set. What was this section? From the PR: $ objdump -h icudt49.dll icudt49.dll: file format pei-i386 Sections: Idx Name Size VMA LMA File off Algn 0 .rdata 0111f4fa 10001000 10001000 00000400 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 1 .rsrc 00000458 11121000 11121000 0111fa00 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA >From the PR, we see the dll exported a icudt49_dat symbol: ... #1 0x0054ae16 in prim_record_minimal_symbol (name=name@entry=0x8019db78 "icudt49!icudt49_dat", address=address@entry=1585713152, ms_type=mst_data, objfile=objfile@entry=0x8027a9c8) ... So the fix is this part: else if (characteristics & IMAGE_SCN_CNT_INITIALIZED_DATA) - section_data[otherix].ms_type = mst_data; + { + section_data[otherix].ms_type = mst_data; + if (objfile->sect_index_data == -1) + objfile->sect_index_data = otherix; + } It's not clear to me that forcing sect_index_... when the canonical section is found is better than using the first / lowest section that looks like code/data/bss. I'd suggest just taking the first found. IOW, do: if (objfile->sect_index_data == -1) objfile->sect_index_data = otherix; in the other branch too. But, hmmm, don't we know the symbol's section? Wouldn't it be even better to make add_pe_exported_sym call prim_record_minimal_symbol_and_info directly, rather than prim_record_minimal_symbol ? -- Pedro Alves