[4/8] gdb/s390: Fill gen_return_address hook.

Message ID m3y49px8tf.fsf@oc1027705133.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez March 11, 2016, 4:45 p.m. UTC
  On Fri, Mar 11 2016, Pedro Alves wrote:

> On 03/11/2016 03:31 PM, Andreas Arnez wrote:
>> So I'm OK with the patch.  Please add a small comment stating that this
>> is a best-can-do approach that usually works near function entry and may
>> yield wrong results otherwise.
>
> I think that should be put in the manual, even.  Users will also trip on
> this, not just our tests.

Right, I thought about this as well.  How about this?

-- >8 --
Subject: [PATCH] Document possible unreliability of `$_ret'
  

Comments

Pedro Alves March 11, 2016, 5:02 p.m. UTC | #1
On 03/11/2016 04:45 PM, Andreas Arnez wrote:
> On Fri, Mar 11 2016, Pedro Alves wrote:
> 
>> On 03/11/2016 03:31 PM, Andreas Arnez wrote:
>>> So I'm OK with the patch.  Please add a small comment stating that this
>>> is a best-can-do approach that usually works near function entry and may
>>> yield wrong results otherwise.
>>
>> I think that should be put in the manual, even.  Users will also trip on
>> this, not just our tests.
> 
> Right, I thought about this as well.  How about this?
> 
> -- >8 --
> Subject: [PATCH] Document possible unreliability of `$_ret'
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4ec0ec1..a14fe19 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -12863,7 +12863,9 @@ Collect all local variables.
>   
>   @item $_ret
>   Collect the return address.  This is helpful if you want to see more
> -of a backtrace.
> +of a backtrace.  Note that the return address can not always be
> +determined reliably, and a wrong address may be collected instead.
> +The reliability is usually higher for tracepoints at function entry.

Hmm, this reads a bit as if the backtrace will be incorrect/bogus
later on, which is not true.

How about a merge of your suggestion with Marcin's previous reply,
and some extras on top:

@item $_ret
Collect the set of memory addresses and/or registers necessary to compute
the frame's return address.  This is helpful if you want to see 
more of a backtrace.

@emph{Note:} The necessary set can not always be reliability determined up
front, and the wrong address / registers may end up collected instead.
The reliability is usually higher for tracepoints at function entry.
When this happens, backtracing will stop because the return address
is found unavailable (unless another collect rule happened to match it).

Thanks,
Pedro Alves
  
Eli Zaretskii March 11, 2016, 6:06 p.m. UTC | #2
> From: Andreas Arnez <arnez@linux.vnet.ibm.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Cc: Marcin Kościelnicki <koriakin@0x04.net>,        gdb-patches@sourceware.org
> Date: Fri, 11 Mar 2016 17:45:16 +0100
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4ec0ec1..a14fe19 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -12863,7 +12863,9 @@ Collect all local variables.
>  
>  @item $_ret
>  Collect the return address.  This is helpful if you want to see more
> -of a backtrace.
> +of a backtrace.  Note that the return address can not always be
> +determined reliably, and a wrong address may be collected instead.
> +The reliability is usually higher for tracepoints at function entry.

LGTM, thanks.
  
Eli Zaretskii March 11, 2016, 6:16 p.m. UTC | #3
> Cc: Eli Zaretskii <eliz@gnu.org>,
>         Marcin Kościelnicki
>  <koriakin@0x04.net>,
>         gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 11 Mar 2016 17:02:19 +0000
> 
> >   @item $_ret
> >   Collect the return address.  This is helpful if you want to see more
> > -of a backtrace.
> > +of a backtrace.  Note that the return address can not always be
> > +determined reliably, and a wrong address may be collected instead.
> > +The reliability is usually higher for tracepoints at function entry.
> 
> Hmm, this reads a bit as if the backtrace will be incorrect/bogus
> later on, which is not true.
> 
> How about a merge of your suggestion with Marcin's previous reply,
> and some extras on top:
> 
> @item $_ret
> Collect the set of memory addresses and/or registers necessary to compute
> the frame's return address.  This is helpful if you want to see 
> more of a backtrace.
> 
> @emph{Note:} The necessary set can not always be reliability determined up
> front, and the wrong address / registers may end up collected instead.
> The reliability is usually higher for tracepoints at function entry.
> When this happens, backtracing will stop because the return address
> is found unavailable (unless another collect rule happened to match it).

Maybe it's me, but I don't see the significant difference between
these two versions.  (And there's a typo in the second one:
"reliability" should be "reliably".)

Thanks.
  
Pedro Alves March 11, 2016, 6:37 p.m. UTC | #4
On 03/11/2016 06:16 PM, Eli Zaretskii wrote:
>> Cc: Eli Zaretskii <eliz@gnu.org>,
>>          Marcin Kościelnicki
>>   <koriakin@0x04.net>,
>>          gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 11 Mar 2016 17:02:19 +0000
>>
>>>    @item $_ret
>>>    Collect the return address.  This is helpful if you want to see more
>>> -of a backtrace.
>>> +of a backtrace.  Note that the return address can not always be
>>> +determined reliably, and a wrong address may be collected instead.
>>> +The reliability is usually higher for tracepoints at function entry.
>>
>> Hmm, this reads a bit as if the backtrace will be incorrect/bogus
>> later on, which is not true.
>>
>> How about a merge of your suggestion with Marcin's previous reply,
>> and some extras on top:
>>
>> @item $_ret
>> Collect the set of memory addresses and/or registers necessary to compute
>> the frame's return address.  This is helpful if you want to see
>> more of a backtrace.
>>
>> @emph{Note:} The necessary set can not always be reliability determined up
>> front, and the wrong address / registers may end up collected instead.
>> The reliability is usually higher for tracepoints at function entry.
>> When this happens, backtracing will stop because the return address
>> is found unavailable (unless another collect rule happened to match it).
>
> Maybe it's me, but I don't see the significant difference between
> these two versions.

I think the original version can be misinterpreted as if the
backtrace will show the wrong caller when the wrong address
is collected.  This version clarifies that it won't.

> (And there's a typo in the second one:
> "reliability" should be "reliably".)

Whoops.

Thanks,
Pedro Alves
  
Eli Zaretskii March 11, 2016, 7:33 p.m. UTC | #5
> Cc: arnez@linux.vnet.ibm.com, koriakin@0x04.net, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 11 Mar 2016 18:37:22 +0000
> 
> On 03/11/2016 06:16 PM, Eli Zaretskii wrote:
> >> Cc: Eli Zaretskii <eliz@gnu.org>,
> >>          Marcin Kościelnicki
> >>   <koriakin@0x04.net>,
> >>          gdb-patches@sourceware.org
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Fri, 11 Mar 2016 17:02:19 +0000
> >>
> >>>    @item $_ret
> >>>    Collect the return address.  This is helpful if you want to see more
> >>> -of a backtrace.
> >>> +of a backtrace.  Note that the return address can not always be
> >>> +determined reliably, and a wrong address may be collected instead.
> >>> +The reliability is usually higher for tracepoints at function entry.
> >>
> >> Hmm, this reads a bit as if the backtrace will be incorrect/bogus
> >> later on, which is not true.
> >>
> >> How about a merge of your suggestion with Marcin's previous reply,
> >> and some extras on top:
> >>
> >> @item $_ret
> >> Collect the set of memory addresses and/or registers necessary to compute
> >> the frame's return address.  This is helpful if you want to see
> >> more of a backtrace.
> >>
> >> @emph{Note:} The necessary set can not always be reliability determined up
> >> front, and the wrong address / registers may end up collected instead.
> >> The reliability is usually higher for tracepoints at function entry.
> >> When this happens, backtracing will stop because the return address
> >> is found unavailable (unless another collect rule happened to match it).
> >
> > Maybe it's me, but I don't see the significant difference between
> > these two versions.
> 
> I think the original version can be misinterpreted as if the
> backtrace will show the wrong caller when the wrong address
> is collected.  This version clarifies that it won't.

My reading is the other way around: the original version only talks
about the return address, while the modified one talks about a set of
addresses.

Anyway, if you are happier with your proposal, I won't object.
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4ec0ec1..a14fe19 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -12863,7 +12863,9 @@  Collect all local variables.
 
 @item $_ret
 Collect the return address.  This is helpful if you want to see more
-of a backtrace.
+of a backtrace.  Note that the return address can not always be
+determined reliably, and a wrong address may be collected instead.
+The reliability is usually higher for tracepoints at function entry.
 
 @item $_probe_argc
 Collects the number of arguments from the static probe at which the