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

[Joel Brobecker <brobecker at adacore dot com>]

> 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