From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23098 invoked by alias); 22 Dec 2013 22:56:17 -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 23089 invoked by uid 89); 22 Dec 2013 22:56:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL,BAYES_00,MSGID_MULTIPLE_AT autolearn=no version=3.3.2 X-HELO: mailhost.u-strasbg.fr Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.222.218) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 22 Dec 2013 22:56:15 +0000 Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antispam (Postfix) with ESMTP id 6305C220624; Sun, 22 Dec 2013 23:56:12 +0100 (CET) Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antivirus (Postfix) with ESMTP id 524E122062D; Sun, 22 Dec 2013 23:56:12 +0100 (CET) Received: from md15.u-strasbg.fr (md15.u-strasbg.fr [130.79.200.204]) by mr8.u-strasbg.fr (Postfix) with ESMTP id E144B220624; Sun, 22 Dec 2013 23:56:05 +0100 (CET) Received: from ms15.u-strasbg.fr (ms15.u-strasbg.fr [130.79.204.115]) by md15.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id rBMMu4hN006629 ; Sun, 22 Dec 2013 23:56:04 +0100 (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from E6510Muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (Authenticated sender: mullerp) by ms15.u-strasbg.fr (Postfix) with ESMTPSA id A583B1FD8A; Sun, 22 Dec 2013 23:56:01 +0100 (CET) From: "Pierre Muller" To: "'Pedro Alves'" Cc: "'Joel Brobecker'" , References: <004801cef65e$cb82d1b0$62887510$@muller@ics-cnrs.unistra.fr> <20131211170204.GD3227@adacore.com> <52ab7ec0.c8da420a.12c6.ffffb3f4SMTPIN_ADDED_BROKEN@mx.google.com> <52B48A28.2000402@redhat.com> In-Reply-To: <52B48A28.2000402@redhat.com> Subject: RE: [RFA-v2] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section Date: Sun, 22 Dec 2013 22:56:00 -0000 Message-ID: <000301ceff68$fd342de0$f79c89a0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-12/txt/msg00865.txt.bz2 > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Pedro Alves > Envoy=E9=A0: vendredi 20 d=E9cembre 2013 19:19 > =C0=A0: Pierre Muller > Cc=A0: 'Joel Brobecker'; gdb-patches@sourceware.org > Objet=A0: Re: [RFA-v2] Fix PR 16201: internal error on a cygwin program > linked against a DLL with no .data section >=20 > On 12/13/2013 09:39 PM, Pierre Muller wrote: > > >=20 > >> 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, >=20 > Missing '*' Whoops,=20 > >>> 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 . > > >=20 > 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: >=20 > $ objdump -h icudt49.dll >=20 > icudt49.dll: file format pei-i386 >=20 > 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 >=20 > From the PR, we see the dll exported a icudt49_dat symbol: >=20 > ... > #1 0x0054ae16 in prim_record_minimal_symbol > (name=3Dname@entry=3D0x8019db78 "icudt49!icudt49_dat", > address=3Daddress@entry=3D1585713152, ms_type=3Dmst_data, > objfile=3Dobjfile@entry=3D0x8027a9c8) > ... >=20 > So the fix is this part: >=20 > else if (characteristics & IMAGE_SCN_CNT_INITIALIZED_DATA) > - section_data[otherix].ms_type =3D mst_data; > + { > + section_data[otherix].ms_type =3D mst_data; > + if (objfile->sect_index_data =3D=3D -1) > + objfile->sect_index_data =3D otherix; > + } >=20 >=20 > 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: >=20 > if (objfile->sect_index_data =3D=3D -1) > objfile->sect_index_data =3D otherix; >=20 > in the other branch too. >=20 > 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 ? Of course, I didn't dig down to the level where I should have seen this... Here is an updated patch. As I was not sure that the bfd index order was the same as the PE index order, I played it safe. Comments? Pierre Muller 2013-12-22 Pierre Muller Fix PR16201. * coff-pe-read.c (section_data): Add index field. (add_pe_exported_sym): Use SECTION_DATA->INDEX for call to prim_record_mininal_symbol_and_info. (read_pe_exported_syms): Set index field of section_data. diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c index 91ee3f6..e04e9ff 100644 --- a/gdb/coff-pe-read.c +++ b/gdb/coff-pe-read.c @@ -53,6 +53,7 @@ struct read_pe_section_data unsigned long rva_end; /* End offset within the pe. */ enum minimal_symbol_type ms_type; /* Type to assign symbols in section. */ + unsigned int index; /* Section number. */ char *section_name; /* Recorded section name. */ }; @@ -175,11 +176,13 @@ add_pe_exported_sym (const char *sym_name, " for entry \"%s\" in dll \"%s\"\n"), section_data->section_name, sym_name, dll_name); - prim_record_minimal_symbol (qualified_name, vma, - section_data->ms_type, objfile); + prim_record_minimal_symbol_and_info (qualified_name, vma, + section_data->ms_type, + section_data->index, objfile); /* Enter the plain name as well, which might not be unique. */ - prim_record_minimal_symbol (bare_name, vma, section_data->ms_type, objfile); + prim_record_minimal_symbol_and_info (bare_name, vma, section_data->ms_type, + section_data->index, objfile); if (debug_coff_pe_read > 1) fprintf_unfiltered (gdb_stdlog, _("Adding exported symbol \"%s\"" " in dll \"%s\"\n"), sym_name, dll_name); @@ -455,17 +458,34 @@ read_pe_exported_syms (struct objfile *objfile) unsigned long characteristics =3D pe_get32 (dll, secptr1 + 36); char sec_name[SCNNMLEN + 1]; int sectix; + unsigned int bfd_section_index; + asection *section; bfd_seek (dll, (file_ptr) secptr1 + 0, SEEK_SET); bfd_bread (sec_name, (bfd_size_type) SCNNMLEN, dll); sec_name[SCNNMLEN] =3D '\0'; sectix =3D read_pe_section_index (sec_name); + section =3D bfd_get_section_by_name (dll, sec_name); + if (section) + bfd_section_index =3D section->index; + else + bfd_section_index =3D -1; if (sectix !=3D PE_SECTION_INDEX_INVALID) { section_data[sectix].rva_start =3D vaddr; section_data[sectix].rva_end =3D vaddr + vsize; + /* For .text, .data and .bss section + set corresponding sect_index_XXX, + even if it was already set before. */ + if (sectix =3D=3D PE_SECTION_INDEX_TEXT) + objfile->sect_index_text =3D sectix; + if (sectix =3D=3D PE_SECTION_INDEX_DATA) + objfile->sect_index_data =3D sectix; + if (sectix =3D=3D PE_SECTION_INDEX_BSS) + objfile->sect_index_bss =3D sectix; + section_data[sectix].index =3D bfd_section_index; } else { @@ -479,6 +499,7 @@ read_pe_exported_syms (struct objfile *objfile) section_data[otherix].rva_start =3D vaddr; section_data[otherix].rva_end =3D vaddr + vsize; section_data[otherix].vma_offset =3D 0; + section_data[otherix].index =3D bfd_section_index; if (characteristics & IMAGE_SCN_CNT_CODE) section_data[otherix].ms_type =3D mst_text; else if (characteristics & IMAGE_SCN_CNT_INITIALIZED_DATA)