[RFC] frame_id_inner check and -fsplit-stack

[RFC] frame_id_inner check and -fsplit-stack

[Joel Brobecker]

Ian Taylor just reported on the GDB IRC that the frame_id_inner
check in get_prev_frame_1 is making debugging difficult when
the program has been built with -fsplit-stack. This option
<<permits discontiguous stack segments. The stack segments are
just allocated using mmap, so there is no particular ordering>>.
As a result, if two frames are on different stack segments,
the ordering of the segments might be such that two valid frames
might be failing the frame_id_inner check.

Discussing with Ian, he indicated that it should be easy to produce
a section with magic name. This would allow us to determine that
the program uses split-stack and that the frame_id_inner may not
apply.

I can see the following options:

  1. do nothing (ahem);
  2. remove the check entirely; we currently apply it I think
     when the two frames are normal frames.  We already skip it
     when either frame is not normal.
  3. provide a user setting that allows the user to tell GDB
     that the program uses split stacks;
  4. skip the test when split stacks is detected.

I don't think we should really consider option (3) if we can indeed
easily detect split-stack. (1) seems a bit harsh. I am Ok with either
(2) or (4). It sounds like Ian is willing to make it easy for us to
detect split-stack, so I'd vote for (4).  Given that nearly all the code
we debug does not use split-stack, we can keep the frame_id_inner check
a while longer...

Thoughts?

-- 
Joel

Re: [RFC] frame_id_inner check and -fsplit-stack

[Mark Kettenis]

> Date: Tue, 29 Dec 2009 23:07:20 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Ian Taylor just reported on the GDB IRC that the frame_id_inner
> check in get_prev_frame_1 is making debugging difficult when
> the program has been built with -fsplit-stack. This option
> <<permits discontiguous stack segments. The stack segments are
> just allocated using mmap, so there is no particular ordering>>.
> As a result, if two frames are on different stack segments,
> the ordering of the segments might be such that two valid frames
> might be failing the frame_id_inner check.
> 
> Discussing with Ian, he indicated that it should be easy to produce
> a section with magic name. This would allow us to determine that
> the program uses split-stack and that the frame_id_inner may not
> apply.
> 
> I can see the following options:
> 
>   1. do nothing (ahem);
>   2. remove the check entirely; we currently apply it I think
>      when the two frames are normal frames.  We already skip it
>      when either frame is not normal.
>   3. provide a user setting that allows the user to tell GDB
>      that the program uses split stacks;
>   4. skip the test when split stacks is detected.
> 
> I don't think we should really consider option (3) if we can indeed
> easily detect split-stack. (1) seems a bit harsh. I am Ok with either
> (2) or (4). It sounds like Ian is willing to make it easy for us to
> detect split-stack, so I'd vote for (4).  Given that nearly all the code
> we debug does not use split-stack, we can keep the frame_id_inner check
> a while longer...
> 
> Thoughts?

Ugh, another small step towards completely undebuggable code.  Guess
somebody is trying to cram too much bloated overthreaded C++ code into
an onderpowered 32-bit device ;).

I think we shouldn't add a knob if we don't need to.  So I'd say we
defenitely should try (4).  My initial idea for implementing this
would be for the unwinder to mark the frames that "split" the stack
(i.e. make the not normal), and skip the check for those frames.  I
also think the information should be encoded in the debug information
instead of magic section names that could be lost during (re)linking.

Re: [RFC] frame_id_inner check and -fsplit-stack

[Ian Lance Taylor]

Mark Kettenis <mark.kettenis@xs4all.nl> writes:

>> Date: Tue, 29 Dec 2009 23:07:20 +0400
>> From: Joel Brobecker <brobecker@adacore.com>
>> 
>> Ian Taylor just reported on the GDB IRC that the frame_id_inner
>> check in get_prev_frame_1 is making debugging difficult when
>> the program has been built with -fsplit-stack. This option
>> <<permits discontiguous stack segments. The stack segments are
>> just allocated using mmap, so there is no particular ordering>>.
>> As a result, if two frames are on different stack segments,
>> the ordering of the segments might be such that two valid frames
>> might be failing the frame_id_inner check.
>> 
>> Discussing with Ian, he indicated that it should be easy to produce
>> a section with magic name. This would allow us to determine that
>> the program uses split-stack and that the frame_id_inner may not
>> apply.
>> 
>> I can see the following options:
>> 
>>   1. do nothing (ahem);
>>   2. remove the check entirely; we currently apply it I think
>>      when the two frames are normal frames.  We already skip it
>>      when either frame is not normal.
>>   3. provide a user setting that allows the user to tell GDB
>>      that the program uses split stacks;
>>   4. skip the test when split stacks is detected.
>> 
>> I don't think we should really consider option (3) if we can indeed
>> easily detect split-stack. (1) seems a bit harsh. I am Ok with either
>> (2) or (4). It sounds like Ian is willing to make it easy for us to
>> detect split-stack, so I'd vote for (4).  Given that nearly all the code
>> we debug does not use split-stack, we can keep the frame_id_inner check
>> a while longer...
>> 
>> Thoughts?
>
> Ugh, another small step towards completely undebuggable code.  Guess
> somebody is trying to cram too much bloated overthreaded C++ code into
> an onderpowered 32-bit device ;).

I actually wrote it for the Go programming language, where programs do
tend to be very threaded.


> I think we shouldn't add a knob if we don't need to.  So I'd say we
> defenitely should try (4).  My initial idea for implementing this
> would be for the unwinder to mark the frames that "split" the stack
> (i.e. make the not normal), and skip the check for those frames.  I
> also think the information should be encoded in the debug information
> instead of magic section names that could be lost during (re)linking.

Looking at DWARF, I see that there is a calling_convention enum which
can be added to the DW_TAG_subprogram for a function.  I don't know all
that much about DWARF; does that seem like the right sort of thing to
do?

That wouldn't help with the unwind info, though.  There I think we could
add a new augmentation code for a split-stack function.  The presence of
the augmentation would mean that the stack might be out of order when
unwinding through this function.  I think that would work because
unwinders which didn't recognize it would simply ignore it.  But again I
am far from being an expert in this area.

Ian

Re: [RFC] frame_id_inner check and -fsplit-stack

[Joel Brobecker]

> > I think we shouldn't add a knob if we don't need to.  So I'd say we
> > defenitely should try (4).  My initial idea for implementing this
> > would be for the unwinder to mark the frames that "split" the stack
> > (i.e. make the not normal), and skip the check for those frames.  I
> > also think the information should be encoded in the debug information
> > instead of magic section names that could be lost during (re)linking.
> 
> Looking at DWARF, I see that there is a calling_convention enum which
> can be added to the DW_TAG_subprogram for a function.  I don't know all
> that much about DWARF; does that seem like the right sort of thing to
> do?

As mentioned to Ian on IRC, the downside of this approach is that
the information would not be available unless the code was compiled
with debugging info. I think that this is a serious enough downside
that we should try to avoid it if possible.

> That wouldn't help with the unwind info, though.  There I think we could
> add a new augmentation code for a split-stack function.

I'm not a specialist of unwind info either.  But I think that this
approach, if implementable, would be more robust.

-- 
Joel

Re: [RFC] frame_id_inner check and -fsplit-stack

[Daniel Jacobowitz]

On Wed, Dec 30, 2009 at 12:06:09AM -0800, Ian Lance Taylor wrote:
> Looking at DWARF, I see that there is a calling_convention enum which
> can be added to the DW_TAG_subprogram for a function.  I don't know all
> that much about DWARF; does that seem like the right sort of thing to
> do?

No.  The split-stack-ness doesn't affect how the function is called;
the attribute's for cases like regparm.

> That wouldn't help with the unwind info, though.  There I think we could
> add a new augmentation code for a split-stack function.  The presence of
> the augmentation would mean that the stack might be out of order when
> unwinding through this function.  I think that would work because
> unwinders which didn't recognize it would simply ignore it.  But again I
> am far from being an expert in this area.

An augmentation ought to work for this.  Similarly, GDB could check
for the prologue sequence if there is no unwind info.

Didn't we have another report that the inner-than check was causing
trouble, just a few months ago?  I think it had to do with the Linux
kernel, but I can't find the details.  This check causes a lot of
trouble.

-- 
Daniel Jacobowitz
CodeSourcery