* [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section
@ 2013-11-28 1:23 Pierre Muller
2013-12-02 3:30 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Muller @ 2013-11-28 1:23 UTC (permalink / raw)
To: gdb-patches
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++;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section
2013-11-28 1:23 [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section Pierre Muller
@ 2013-12-02 3:30 ` Joel Brobecker
2013-12-02 10:44 ` Pierre Muller
0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2013-12-02 3:30 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> 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.
I am unsure about this patch, at the moment. But is it a regression,
and if yes, should be listed as a must-have for 7.7? If yes again,
let's have it added to the release wiki page:
https://sourceware.org/gdb/wiki/GDB_7.7_Release
Thank you!
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section
2013-12-02 3:30 ` Joel Brobecker
@ 2013-12-02 10:44 ` Pierre Muller
2013-12-02 11:37 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Muller @ 2013-12-02 10:44 UTC (permalink / raw)
To: 'Joel Brobecker'; +Cc: gdb-patches
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : lundi 2 décembre 2013 04:31
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix PR 16201: internal error on a cygwin program
> linked against a DLL with no .data section
>
> > 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.
>
> I am unsure about this patch, at the moment. But is it a regression,
> and if yes, should be listed as a must-have for 7.7? If yes again,
> let's have it added to the release wiki page:
> https://sourceware.org/gdb/wiki/GDB_7.7_Release
I can now confirm (after testing)
that, for mingw32 gdb executable, this bug is present in
gdb-7.6 release but not in gdb-7.5.
The internal error appears due to a change inside coff-pe-read.c source,
which is used both by cygwin and mingw executables,
so that I expect that cygwin GDB exhibit that same pattern.
This bug appears because of my patch dated 2012-12-13,
(line 481 in current git blame)
https://sourceware.org/ml/gdb-cvs/2012-12/msg00075.html
which extends the number of sections analyzed past the
three default sections .text .data and .bss.
In that patch, I extend the size of section_data array,
which triggers the parsing of .rdata section which was ignored
before.
In versions prior to 7.6, the exported symbols that
where not inside those default sections were not added
to the minimal symbol list.
With my change, they got added, but the problem arises then that
they are added using ms_text or ms_data or ms_bss field,
which triggers the internal error if the corresponding sect_index_XXX
is not set.
> Thank you!
I hope that I convinced you that this is a regression,
and that it should thus be included in 7.7!
Pierre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section
2013-12-02 10:44 ` Pierre Muller
@ 2013-12-02 11:37 ` Joel Brobecker
2013-12-02 14:04 ` Pierre Muller
0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2013-12-02 11:37 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> > Thank you!
> I hope that I convinced you that this is a regression,
> and that it should thus be included in 7.7!
Thanks for the analysis. I didn't really need convincing, just
an answer ;-). Can you please add an entry in the release wiki
page, then? Without it, you run the risk of us releasing without
the patch.
https://sourceware.org/gdb/wiki/GDB_7.7_Release
Thank you,
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section
2013-12-02 11:37 ` Joel Brobecker
@ 2013-12-02 14:04 ` Pierre Muller
2013-12-02 14:10 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Muller @ 2013-12-02 14:04 UTC (permalink / raw)
To: 'Joel Brobecker'; +Cc: gdb-patches
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : lundi 2 décembre 2013 12:37
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix PR 16201: internal error on a cygwin program
> linked against a DLL with no .data section
>
> > > Thank you!
> > I hope that I convinced you that this is a regression,
> > and that it should thus be included in 7.7!
>
> Thanks for the analysis. I didn't really need convincing, just
> an answer ;-). Can you please add an entry in the release wiki
> page, then? Without it, you run the risk of us releasing without
> the patch.
>
> https://sourceware.org/gdb/wiki/GDB_7.7_Release
Done...
But could you elaborate onto why you are unsure about the patch?
What is bothering you?
Pierre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section
2013-12-02 14:04 ` Pierre Muller
@ 2013-12-02 14:10 ` Joel Brobecker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Brobecker @ 2013-12-02 14:10 UTC (permalink / raw)
To: Pierre Muller; +Cc: gdb-patches
> But could you elaborate onto why you are unsure about the patch?
> What is bothering you?
Sure. I may have expressed myself poorly. The patch wasn't obvious,
and I didn't have time to look into it more closely. So if I were
the one reviewing it, I'd need a small chunk of free time to really
understand it well, and double-check its possible unwanted side-effects.
Maybe someone else, who is more familiar with objfiles et al, might
find it easier to review.
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-02 14:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-28 1:23 [RFA] Fix PR 16201: internal error on a cygwin program linked against a DLL with no .data section Pierre Muller
2013-12-02 3:30 ` Joel Brobecker
2013-12-02 10:44 ` Pierre Muller
2013-12-02 11:37 ` Joel Brobecker
2013-12-02 14:04 ` Pierre Muller
2013-12-02 14:10 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox