From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 854 invoked by alias); 7 Jan 2014 23:40:23 -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 842 invoked by uid 89); 7 Jan 2014 23:40:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,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.216) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Jan 2014 23:40:20 +0000 Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antispam (Postfix) with ESMTP id 7479E14082E; Wed, 8 Jan 2014 00:40:17 +0100 (CET) Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antivirus (Postfix) with ESMTP id 63E7A14072C; Wed, 8 Jan 2014 00:40:17 +0100 (CET) Received: from md14.u-strasbg.fr (md14.u-strasbg.fr [130.79.200.249]) by mr6.u-strasbg.fr (Postfix) with ESMTP id 20138140832; Wed, 8 Jan 2014 00:40:14 +0100 (CET) Received: from ms16.u-strasbg.fr (ms16.u-strasbg.fr [130.79.204.116]) by md14.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id s07NeEkR025595 ; Wed, 8 Jan 2014 00:40:14 +0100 Received: from E6510Muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (Authenticated sender: mullerp) by ms16.u-strasbg.fr (Postfix) with ESMTPSA id 47C341FD91; Wed, 8 Jan 2014 00:40:10 +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> <52b76e14.8886420a.29e6.ffffddb2SMTPIN_ADDED_BROKEN@mx.google.com> <52CAF71D.3050008@redhat.com> <52cbe1c5.67ed440a.67e6.7288SMTPIN_ADDED_BROKEN@mx.google.com> <52CC6A80.1000906@redhat.com> In-Reply-To: <52CC6A80.1000906@redhat.com> Subject: [COMMIT-v4] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section Date: Tue, 07 Jan 2014 23:40:00 -0000 Message-ID: <002e01cf0c01$ce262260$6a726720$@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: 2014-01/txt/msg00168.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: mardi 7 janvier 2014 21:59 > =C0=A0: Pierre Muller > Cc=A0: 'Joel Brobecker'; gdb-patches@sourceware.org > Objet=A0: Re: [RFA-v3] Fix PR 16201: internal error on a cygwin program > linked against a DLL with no .data section >=20 > On 01/07/2014 11:15 AM, Pierre Muller wrote: > >> On 12/22/2013 10:55 PM, Pierre Muller wrote: > >>> @@ -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); > >> > >> Can't coff have sections with duplicate names? > > I did not find anything in the PE COFF description > > that explicitly said that each section should have a unique name > > but I always assumed that the assembler/linker would > > always group all sections with the same name. >=20 > Usually, but it's also not usually mandatory. We're reading > a linked PE file, so I'm really not sure. In any case, > relying on section names usually indicates something is being > done wrong (and GDB is full of that, unfortunately)... Given that > bfd itself creates sections from the PE's sections, I'd guess > the indexes should match, maybe with some offset. >=20 > Anyway, I don't want to invest time to try this out myself. Fine > with me to leave it looking up by name for now, if you'd like. OK, thanks. =20 > > > >> If so, > >> then it'd be better to match the section some other way, > >> I guess by address? > > > > I would not know how to do this... >=20 > You'd just walk over the sections, and compare addresses. > Look for "bfd->sections" in symfile.c for example. But > anyway, it might be that duplicate sections would be > overlapping, so that wouldn't be the ideal match. Yes, but there is all this rva stuff that is also going on on the same time... I am pretty sure that duplicate section names are not possible in a linker generated Windows PE file. =20 > > > >>> + 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; > >> > >> Do you still need this part? > > This is still an improvement as it sets > > these sect_index_XXX fields that might be needed > > elsewhere in the code. >=20 > It's the "might" part that I don't like. If you don't need > it, I'd rather remove it, as it might be hiding some other > similar problem elsewhere. It's not clear to me overriding > is the best choice. And if those aren't set, won't > init_objfile_sect_indices / symfile_find_segment_sections > end up setting them anyway? OK, I removed that part completely, as the problem reported is fixed without. =20 > > @@ -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. */ >=20 > Which index? bfd or PE ? That should be clear in the comment, > at least. Changed comment to BFD section number =20 > > @@ -455,17 +462,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; >=20 > (See? It looks quite odd to me to need to handle the case > of bfd not creating section listed in the PE header. I'd > assume bfd reads the same section list when creating > it's own list of sections ?) I also suppose that both arrays are the same, but I also did not want to take the risk of dereferencing a NULL pointer... =20 > Otherwise looks fine to me. Thanks for the approval, =20 Pierre For the record, this is what I committed: 2014-01-08 Pierre Muller Fix PR16201. * coff-pe-read.c (struct read_pe_section_data): Add index field. (add_pe_exported_sym): Use SECTION_DATA->INDEX for call to prim_record_mininal_symbol_and_info. (add_pe_forwarded_sym): Use known section number of forwarded symbol in call to prim_record_minimal_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 749c109..0fcd15f 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; /* BFD section number. */ char *section_name; /* Recorded section name. */ }; =20 @@ -93,7 +94,7 @@ read_pe_section_index (const char *section_name) } } =20 -/* Get the index of the named section in our own full arrayi. +/* Get the index of the named section in our own full array. text, data and bss in that order. Return PE_SECTION_INDEX_INVALID if passed an unrecognised section name. */ =20 @@ -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); =20 - 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); =20 /* 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); @@ -209,6 +212,7 @@ add_pe_forwarded_sym (const char *sym_name, const char *forward_dll_name, int forward_func_name_len =3D strlen (forward_func_name); int forward_len =3D forward_dll_name_len + forward_func_name_len + 2; char *forward_qualified_name =3D alloca (forward_len); + short section; =20 xsnprintf (forward_qualified_name, forward_len, "%s!%s", forward_dll_name, forward_func_name); @@ -242,6 +246,7 @@ add_pe_forwarded_sym (const char *sym_name, const char *forward_dll_name, =20 vma =3D SYMBOL_VALUE_ADDRESS (msymbol.minsym); msymtype =3D MSYMBOL_TYPE (msymbol.minsym); + section =3D SYMBOL_SECTION (msymbol.minsym); =20 /* Generate a (hopefully unique) qualified name using the first part of the dll name, e.g. KERNEL32!AddAtomA. This matches the style @@ -254,10 +259,12 @@ add_pe_forwarded_sym (const char *sym_name, const char *forward_dll_name, =20 qualified_name =3D xstrprintf ("%s!%s", dll_name, bare_name); =20 - prim_record_minimal_symbol (qualified_name, vma, msymtype, objfile); + prim_record_minimal_symbol_and_info (qualified_name, vma, msymtype, + section, objfile); =20 /* Enter the plain name as well, which might not be unique. */ - prim_record_minimal_symbol (bare_name, vma, msymtype, objfile); + prim_record_minimal_symbol_and_info (bare_name, vma, msymtype, + section, objfile); xfree (qualified_name); xfree (bare_name); =20 @@ -455,17 +462,25 @@ 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; =20 bfd_seek (dll, (file_ptr) secptr1 + 0, SEEK_SET); bfd_bread (sec_name, (bfd_size_type) SCNNMLEN, dll); sec_name[SCNNMLEN] =3D '\0'; =20 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; =20 if (sectix !=3D PE_SECTION_INDEX_INVALID) { section_data[sectix].rva_start =3D vaddr; section_data[sectix].rva_end =3D vaddr + vsize; + section_data[sectix].index =3D bfd_section_index; } else { @@ -479,6 +494,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) =20