[RFA-v3] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : lundi 6 janvier 2014 19:34
> À : Pierre Muller
> Cc : 'Joel Brobecker'; gdb-patches@sourceware.org
> Objet : Re: [RFA-v2] Fix PR 16201: internal error on a cygwin program
> linked against a DLL with no .data section
> 
> Hi Pierre,
> 
> Sorry for the slow reply.  Been away on vacation.
> 
> On 12/22/2013 10:55 PM, Pierre Muller wrote:
> > @@ -455,17 +458,34 @@ read_pe_exported_syms (struct objfile *objfile)
> >        unsigned long characteristics = 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] = '\0';
> >
> >        sectix = read_pe_section_index (sec_name);
> > +      section = 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.
  Is there some object merge utility that could 
group objects without merging?

> 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...
 
> > +      if (section)
> > +       bfd_section_index = section->index;
> > +      else
> > +       bfd_section_index = -1;
> >
> >        if (sectix != PE_SECTION_INDEX_INVALID)
> >         {
> >           section_data[sectix].rva_start = vaddr;
> >           section_data[sectix].rva_end = vaddr + vsize;
> > +         /* For .text, .data and .bss section
> > +             set corresponding sect_index_XXX,
> > +             even if it was already set before.  */
> > +         if (sectix == PE_SECTION_INDEX_TEXT)
> > +           objfile->sect_index_text = sectix;
> > +         if (sectix == PE_SECTION_INDEX_DATA)
> > +           objfile->sect_index_data = sectix;
> > +         if (sectix == PE_SECTION_INDEX_BSS)
> > +           objfile->sect_index_bss = sectix;
> > +         section_data[sectix].index = 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.
  Remember that the bug comes from the fact that those fields
were not set. However, I agree that the exact source
of the bug should be removed even without this part.

  I would prefer to leave it inside this patch, as
it still reduces a potential problem elsewhere.
  Here is an update, which also handles the forwarded
symbol by retrieving the section index of the minimal symbol.

  

Pierre Muller

2014-01-07  Pierre Muller  <muller@sourceware.org>

	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..01d6c69 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);
@@ -209,6 +212,7 @@ add_pe_forwarded_sym (const char *sym_name, const char
*forward_dll_name,
   int forward_func_name_len = strlen (forward_func_name);
   int forward_len = forward_dll_name_len + forward_func_name_len + 2;
   char *forward_qualified_name = alloca (forward_len);
+  short section;
 
   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,
 
   vma = SYMBOL_VALUE_ADDRESS (msymbol.minsym);
   msymtype = MSYMBOL_TYPE (msymbol.minsym);
+  section = SYMBOL_SECTION (msymbol.minsym);
 
   /* 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,
 
   qualified_name = xstrprintf ("%s!%s", dll_name, bare_name);
 
-  prim_record_minimal_symbol (qualified_name, vma, msymtype, objfile);
+  prim_record_minimal_symbol_and_info (qualified_name, vma, msymtype,
+				       section, objfile);
 
   /* 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);
 
@@ -455,17 +462,34 @@ read_pe_exported_syms (struct objfile *objfile)
       unsigned long characteristics = 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] = '\0';
 
       sectix = read_pe_section_index (sec_name);
+      section = bfd_get_section_by_name (dll, sec_name);
+      if (section)
+	bfd_section_index = section->index;
+      else
+	bfd_section_index = -1;
 
       if (sectix != PE_SECTION_INDEX_INVALID)
 	{
 	  section_data[sectix].rva_start = vaddr;
 	  section_data[sectix].rva_end = vaddr + vsize;
+	  /* For .text, .data and .bss section
+             set corresponding sect_index_XXX,
+             even if it was already set before.  */
+	  if (sectix == PE_SECTION_INDEX_TEXT)
+	    objfile->sect_index_text = sectix;
+	  if (sectix == PE_SECTION_INDEX_DATA)
+	    objfile->sect_index_data = sectix;
+	  if (sectix == PE_SECTION_INDEX_BSS)
+	    objfile->sect_index_bss = sectix;
+	  section_data[sectix].index = bfd_section_index;
 	}
       else
 	{
@@ -479,6 +503,7 @@ read_pe_exported_syms (struct objfile *objfile)
 	  section_data[otherix].rva_start = vaddr;
 	  section_data[otherix].rva_end = vaddr + vsize;
 	  section_data[otherix].vma_offset = 0;
+	  section_data[otherix].index = bfd_section_index;
 	  if (characteristics & IMAGE_SCN_CNT_CODE)
 	    section_data[otherix].ms_type = mst_text;
 	  else if (characteristics & IMAGE_SCN_CNT_INITIALIZED_DATA)