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

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

[Pierre Muller]

Other than adding this issue to the list to be handled before 7.7 release,
see:
https://sourceware.org/gdb/wiki/GDB_7.7_Release

and an answer from Joel stating that he would prefer 
that someone else with more knowledge about objfile struct
would review my RFA...
https://sourceware.org/ml/gdb-patches/2013-12/msg00036.html

We got no other reaction yet.

Should I press Joel to review it nonetheless
or does someone else volunteer to review this patch?

Pierre
 

>>>>>>>>>>>>>>>>>>>> Original message

  See discussion in 
https://sourceware.org/bugzilla/show_bug.cgi?id=16201

  The patch below seems to fix the issues as it avoids calling
prim_record_minimal_symbol
with ms_type argument being equal to mst_XXX (where XXX can be text, data or
bss)
without having set sect_index_XXX field of the corresponding objfile.

  Is this OK?

Pierre Muller


2013-11-27  Pierre Muller  <muller@sourceware.org>

        PR 16201
        coff-pe-read.c (read_pe_exported_syms): Set sect_index_text,
        sect_index_data and sect_index_bss of objfile struct, even if
        there is no canonical '.text', '.data' or '.bss' named section.

diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
index 91ee3f6..954c457 100644
--- a/gdb/coff-pe-read.c
+++ b/gdb/coff-pe-read.c
@@ -466,6 +466,13 @@ read_pe_exported_syms (struct objfile *objfile)
        {
          section_data[sectix].rva_start = vaddr;
          section_data[sectix].rva_end = vaddr + vsize;
+         /* Force sect_index, 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;
        }
       else
        {
@@ -480,11 +487,23 @@ read_pe_exported_syms (struct objfile *objfile)
          section_data[otherix].rva_end = vaddr + vsize;
          section_data[otherix].vma_offset = 0;
          if (characteristics & IMAGE_SCN_CNT_CODE)
-           section_data[otherix].ms_type = mst_text;
+           {
+             section_data[otherix].ms_type = mst_text;
+             if (objfile->sect_index_text == -1)
+               objfile->sect_index_text = otherix;
+           }
          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;
+           }
          else if (characteristics & IMAGE_SCN_CNT_UNINITIALIZED_DATA)
-           section_data[otherix].ms_type = mst_bss;
+           {
+             section_data[otherix].ms_type = mst_bss;
+             if (objfile->sect_index_bss == -1)
+               objfile->sect_index_bss = otherix;
+           }
          else
            section_data[otherix].ms_type = mst_unknown;
          otherix++;

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

[Joel Brobecker]

> and an answer from Joel stating that he would prefer 
> that someone else with more knowledge about objfile struct
> would review my RFA...
> https://sourceware.org/ml/gdb-patches/2013-12/msg00036.html
> 
> We got no other reaction yet.
> 
> Should I press Joel to review it nonetheless
> or does someone else volunteer to review this patch?

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  <muller@sourceware.org>
> 
>         PR 16201
>         coff-pe-read.c (read_pe_exported_syms): Set sect_index_text,
>         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.

> 
> diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
> index 91ee3f6..954c457 100644
> --- a/gdb/coff-pe-read.c
> +++ b/gdb/coff-pe-read.c
> @@ -466,6 +466,13 @@ read_pe_exported_syms (struct objfile *objfile)
>         {
>           section_data[sectix].rva_start = vaddr;
>           section_data[sectix].rva_end = vaddr + vsize;
> +         /* Force sect_index, 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;
>         }
>        else
>         {
> @@ -480,11 +487,23 @@ read_pe_exported_syms (struct objfile *objfile)
>           section_data[otherix].rva_end = vaddr + vsize;
>           section_data[otherix].vma_offset = 0;
>           if (characteristics & IMAGE_SCN_CNT_CODE)
> -           section_data[otherix].ms_type = mst_text;
> +           {
> +             section_data[otherix].ms_type = mst_text;
> +             if (objfile->sect_index_text == -1)
> +               objfile->sect_index_text = otherix;
> +           }
>           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;
> +           }
>           else if (characteristics & IMAGE_SCN_CNT_UNINITIALIZED_DATA)
> -           section_data[otherix].ms_type = mst_bss;
> +           {
> +             section_data[otherix].ms_type = mst_bss;
> +             if (objfile->sect_index_bss == -1)
> +               objfile->sect_index_bss = otherix;
> +           }
>           else
>             section_data[otherix].ms_type = mst_unknown;
>           otherix++;

-- 
Joel

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

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : mercredi 11 décembre 2013 18:02
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [PING RFA] Fix PR 16201: internal error on a cygwin program
> linked against a DLL with no .data section
> 
> > and an answer from Joel stating that he would prefer
> > that someone else with more knowledge about objfile struct
> > would review my RFA...
> > https://sourceware.org/ml/gdb-patches/2013-12/msg00036.html
> >
> > We got no other reaction yet.
> >
> > Should I press Joel to review it nonetheless
> > or does someone else volunteer to review this patch?
> 
> 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  <muller@sourceware.org>
> >
> >         PR 16201
> >         coff-pe-read.c (read_pe_exported_syms): Set sect_index_text,
> >         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 .

  
Pierre




ChangeLog entry:

2013-12-13  Pierre Muller  <muller@sourceware.org>

        Fix PR16201.
        coff-pe-read.c (read_pe_exported_syms): Set OBJFILE->sect_index_XXX
        for XXX text, data or bss to any section that sets ms_type
        field of section_data to mst_XXX, with preference
        to canonical names '.text', '.data' and '.bss'.

 
diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
index 91ee3f6..ec9aed5 100644
--- a/gdb/coff-pe-read.c
+++ b/gdb/coff-pe-read.c
@@ -466,6 +466,15 @@ read_pe_exported_syms (struct objfile *objfile)
        {
          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;
        }
       else
        {
@@ -480,11 +489,23 @@ read_pe_exported_syms (struct objfile *objfile)
          section_data[otherix].rva_end = vaddr + vsize;
          section_data[otherix].vma_offset = 0;
          if (characteristics & IMAGE_SCN_CNT_CODE)
-           section_data[otherix].ms_type = mst_text;
+           {
+             section_data[otherix].ms_type = mst_text;
+             if (objfile->sect_index_text == -1)
+               objfile->sect_index_text = otherix;
+           }
          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;
+           }
          else if (characteristics & IMAGE_SCN_CNT_UNINITIALIZED_DATA)
-           section_data[otherix].ms_type = mst_bss;
+           {
+             section_data[otherix].ms_type = mst_bss;
+             if (objfile->sect_index_bss == -1)
+               objfile->sect_index_bss = otherix;
+           }
          else
            section_data[otherix].ms_type = mst_unknown;
          otherix++;

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

[Joel Brobecker]

> 2013-12-13  Pierre Muller  <muller@sourceware.org>
> 
>         Fix PR16201.
>         coff-pe-read.c (read_pe_exported_syms): Set OBJFILE->sect_index_XXX
>         for XXX text, data or bss to any section that sets ms_type
>         field of section_data to mst_XXX, with preference
>         to canonical names '.text', '.data' and '.bss'.

It's been a month since you posted the patch, and no one but me really
commented on it. Since we need this before branching for 7.7, and
since I do not see anything obviously wrong with it, I will give it
the OK. Let's wait until Friday to commit, in order to give anyone
else a last chance to comment, but otherwise, my only request is that
you expand the comment you added to explain __why__ you set the various
sect_index_XXX fields, especially since you do it even when already set.
Only nit-picking request is also that it would be nice if the comment
was formatted so that line lengths would be closer to 70 - while most
people tend to write lines that are too long, you seem to be to the
opposite :).

Thank you!

> diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
> index 91ee3f6..ec9aed5 100644
> --- a/gdb/coff-pe-read.c
> +++ b/gdb/coff-pe-read.c
> @@ -466,6 +466,15 @@ read_pe_exported_syms (struct objfile *objfile)
>         {
>           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;
>         }
>        else
>         {
> @@ -480,11 +489,23 @@ read_pe_exported_syms (struct objfile *objfile)
>           section_data[otherix].rva_end = vaddr + vsize;
>           section_data[otherix].vma_offset = 0;
>           if (characteristics & IMAGE_SCN_CNT_CODE)
> -           section_data[otherix].ms_type = mst_text;
> +           {
> +             section_data[otherix].ms_type = mst_text;
> +             if (objfile->sect_index_text == -1)
> +               objfile->sect_index_text = otherix;
> +           }
>           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;
> +           }
>           else if (characteristics & IMAGE_SCN_CNT_UNINITIALIZED_DATA)
> -           section_data[otherix].ms_type = mst_bss;
> +           {
> +             section_data[otherix].ms_type = mst_bss;
> +             if (objfile->sect_index_bss == -1)
> +               objfile->sect_index_bss = otherix;
> +           }
>           else
>             section_data[otherix].ms_type = mst_unknown;
>           otherix++;

-- 
Joel

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

[Pedro Alves]

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  <muller@sourceware.org>
>>>
>>>         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

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

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : vendredi 20 décembre 2013 19:19
> À : 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
> 
> 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  <muller@sourceware.org>
> >>>
> >>>         PR 16201
> >>>         coff-pe-read.c (read_pe_exported_syms): Set
> sect_index_text,
> 
> Missing '*'
Whoops, 
> >>>         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 ?
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  <muller@sourceware.org>

        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 = 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 +499,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)

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

[asmwarrior]

Hi, Pierre.
I have a suggestion here. 
I save your email in .eml format, and see no "TAB"s in your patch, all are white spaces.
So git apply command get failed.
I need to manually convert many 8 spaces to one tab, that was not a simple task I see.
Hope you will fix them in the future.

Thanks.

Yuanhui Zhang

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

[Pedro Alves]

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?  If so,
then it'd be better to match the section some other way,
I guess by address?

> +      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?

>         }

-- 
Pedro Alves

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

[Pierre Muller]

> -----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)

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

[Pedro Alves]

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 = 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.

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.

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.

> 
>> 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...

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.

>  
>>> +      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.

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?

> @@ -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.  */

Which index?  bfd or PE ?  That should be clear in the comment,
at least.

> @@ -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;

(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 ?)

Otherwise looks fine to me.

-- 
Pedro Alves

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

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : mardi 7 janvier 2014 21:59
> À : Pierre Muller
> Cc : 'Joel Brobecker'; gdb-patches@sourceware.org
> Objet : Re: [RFA-v3] Fix PR 16201: internal error on a cygwin program
> linked against a DLL with no .data section
> 
> 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 = 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.
> 
> 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.
> 
> 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.
 
> >
> >> 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...
> 
> 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.
 
> >
> >>> +      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.
> 
> 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.
 
> > @@ -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.  */
> 
> Which index?  bfd or PE ?  That should be clear in the comment,
> at least.
  Changed comment to BFD section number
 
> > @@ -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;
> 
> (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...
 
> Otherwise looks fine to me.

  Thanks for the approval,
 
Pierre

For the record, this is what I committed:

2014-01-08  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..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.  */
 };
 
@@ -93,7 +94,7 @@ read_pe_section_index (const char *section_name)
     }
 }
 
-/* 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.  */
 
@@ -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,25 @@ 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;
+	  section_data[sectix].index = bfd_section_index;
 	}
       else
 	{
@@ -479,6 +494,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)