[patch] gdb_assert -> complaint for weird DWARF

[patch] gdb_assert -> complaint for weird DWARF

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Hi,

PR 16581:
	GDB crash on inherit_abstract_dies infinite recursion
	https://sourceware.org/bugzilla/show_bug.cgi?id=16581

fixed crash from an infinite recursion.  But in rare cases the new code can
now gdb_assert() due to weird DWARF file.

I do not yet fully understand why the DWARF is as it is but just GDB should
never crash due to invalid DWARF anyway.  The "invalid" DWARF I see only in
Fedora GCC build, not in FSF GCC build, more info at:
	https://bugzilla.redhat.com/show_bug.cgi?id=1069382
	http://people.redhat.com/jkratoch/gcc-debuginfo-4.8.2-7.fc20.x86_64-gnatbind.debug


Thanks,
Jan

[-- Attachment #2: complaint.patch --]
[-- Type: text/plain, Size: 723 bytes --]

gdb/
2014-02-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (process_die): Change gdb_assert to complaint.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3eaa0b1..71f5d34 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8029,7 +8029,13 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
   struct cleanup *in_process;
 
   /* We should only be processing those not already in process.  */
-  gdb_assert (!die->in_process);
+  if (die->in_process)
+    {
+      complaint (&symfile_complaints,
+		 _("DIE at 0x%x attempted to be processed twice"),
+		 die->offset.sect_off);
+      return;
+    }
 
   die->in_process = 1;
   in_process = make_cleanup (reset_die_in_process,die);

Re: [patch] gdb_assert -> complaint for weird DWARF

[Doug Evans]

On Mon, Feb 24, 2014 at 1:43 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> PR 16581:
>         GDB crash on inherit_abstract_dies infinite recursion
>         https://sourceware.org/bugzilla/show_bug.cgi?id=16581
>
> fixed crash from an infinite recursion.  But in rare cases the new code can
> now gdb_assert() due to weird DWARF file.
>
> I do not yet fully understand why the DWARF is as it is but just GDB should
> never crash due to invalid DWARF anyway.  The "invalid" DWARF I see only in
> Fedora GCC build, not in FSF GCC build, more info at:
>         https://bugzilla.redhat.com/show_bug.cgi?id=1069382
>         http://people.redhat.com/jkratoch/gcc-debuginfo-4.8.2-7.fc20.x86_64-gnatbind.debug
>
>
> Thanks,
> Jan
>
> gdb/
> 2014-02-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * dwarf2read.c (process_die): Change gdb_assert to complaint.

Obviously gdb shouldn't crash on invalid input, but IWBN if gdb could
have more robust guarantees internally.
[One could argue changing the assert to a complaint is papering over a bug.
Plus gdb's complaint system is off by default, out of sight makes for
out of mind thus such things tend to not get much attention.]

Can you dig into the details of why the assert is tripping?
I'm not comfortable with adding more complaints without at least
including documentation of details.

Re: [patch] gdb_assert -> complaint for weird DWARF

[Jan Kratochvil]

Hi Doug,

On Mon, 24 Feb 2014 22:55:49 +0100, Doug Evans wrote:
> Can you dig into the details of why the assert is tripping?
> I'm not comfortable with adding more complaints without at least
> including documentation of details.

I have filed the Fedora GCC bug:
	https://bugzilla.redhat.com/show_bug.cgi?id=1069382

This is all I can do now.  My build of FSF GCC does not upset GDB.

I agree the current situation is not great, IMO there is a gnat DWARF
generator bug.


Jan

Re: [patch] gdb_assert -> complaint for weird DWARF

[Doug Evans]

On Mon, Feb 24, 2014 at 2:04 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi Doug,
>
> On Mon, 24 Feb 2014 22:55:49 +0100, Doug Evans wrote:
>> Can you dig into the details of why the assert is tripping?
>> I'm not comfortable with adding more complaints without at least
>> including documentation of details.
>
> I have filed the Fedora GCC bug:
>         https://bugzilla.redhat.com/show_bug.cgi?id=1069382
>
> This is all I can do now.  My build of FSF GCC does not upset GDB.
>
> I agree the current situation is not great, IMO there is a gnat DWARF
> generator bug.

Can you send me the binaries for repro?
I see in the above bug report an abstract_origin which is what the
patch has specific code for to avoid tripping the assert.
We could probably generate a good testcase for gdb from that.

Another worry I have is that if my expectation that we shouldn't be
recursively calling process_die (even for bad debug info) is wrong,
then is there some obscure case where possible accidental re-reading
of a DIE is actually needed by the current code to get the right
answer (IOW is making this a complaint and returning also introducing
a bug? Less of a bug than crashing or infinite recursion of course,
but IWBN to invest some time to dig deeper given that we have a repro
at hand).

Re: [patch] gdb_assert -> complaint for weird DWARF

[Jan Kratochvil]

On Mon, 24 Feb 2014 23:59:50 +0100, Doug Evans wrote:
> Can you send me the binaries for repro?

It was sent in the previous mail:
	http://people.redhat.com/jkratoch/gcc-debuginfo-4.8.2-7.fc20.x86_64-gnatbind.debug


> We could probably generate a good testcase for gdb from that.

I was thinking about it but:
 * There is not much to test gdb_assert vs. complaint.
 * Currently I believe the generated DWARF is incorrect.  Detecting that such
   incorrect DWARF is identified as an incorrect one has limited sense IMO.

So far I find it just a GCC bug.


> Another worry I have is that if my expectation that we shouldn't be
> recursively calling process_die (even for bad debug info) is wrong,
> then is there some obscure case where possible accidental re-reading
> of a DIE is actually needed by the current code to get the right
> answer (IOW is making this a complaint and returning also introducing
> a bug? Less of a bug than crashing or infinite recursion of course,
> but IWBN to invest some time to dig deeper given that we have a repro
> at hand).

As I said so far I do not find the DWARF (in the gnatbind case) to be
meaningful.


Jan

Re: [patch] gdb_assert -> complaint for weird DWARF

[Jan Kratochvil]

On Mon, 24 Feb 2014 23:59:50 +0100, Doug Evans wrote:
> Can you send me the binaries for repro?

I have filed a reproducer with FSF GCC 4.8:
	Bug 60339 - gnat weird DW_AT_abstract_origin
	http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60339


Jan