* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
[not found] <8ce93c6f-5822-4e27-9e59-c6cbe158424d@email.android.com>
@ 2016-03-11 17:34 ` Pedro Alves
2016-03-11 17:43 ` Marcin Kościelnicki
0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2016-03-11 17:34 UTC (permalink / raw)
To: Marcin Kościelnicki; +Cc: gdb-patches, Andreas Arnez, Eli Zaretskii
On 03/11/2016 05:19 PM, Marcin KoÅcielnicki wrote:
>
> 11 mar 2016 6:02 PM Pedro Alves <palves@redhat.com> napisaÅ(a):
> >
> > 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).
>
> Note that this is arch-dependent: powerpc/s390/aarch64 will work at
> entry (since they dump LR), while x86 has the opposite problem: it uses
> the frame pointer and will never work at entry (or with
> -fomit-frame-pointer for that matter).
OK. I wouldn't want to go too deep into details here, I think just
pointing in the direction of experimenting with different tracepoint
addresses, if important, may be sufficient.
@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.
On some architectures the reliability is higher for tracepoints
at function entry, while on others it's the opposite.
When this happens, backtracing will stop because the return address
is found unavailable (unless another collect rule happened to match it).
WDYT?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 17:34 ` [PATCH 4/8] gdb/s390: Fill gen_return_address hook Pedro Alves
@ 2016-03-11 17:43 ` Marcin Kościelnicki
0 siblings, 0 replies; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-03-11 17:43 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Andreas Arnez, Eli Zaretskii
On 11/03/16 18:33, Pedro Alves wrote:
> On 03/11/2016 05:19 PM, Marcin KoÅcielnicki wrote:
>>
>> 11 mar 2016 6:02 PM Pedro Alves <palves@redhat.com> napisaÅ(a):
>> >
>> > 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).
>>
>> Note that this is arch-dependent: powerpc/s390/aarch64 will work at
>> entry (since they dump LR), while x86 has the opposite problem: it uses
>> the frame pointer and will never work at entry (or with
>> -fomit-frame-pointer for that matter).
>
> OK. I wouldn't want to go too deep into details here, I think just
> pointing in the direction of experimenting with different tracepoint
> addresses, if important, may be sufficient.
>
> @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.
> On some architectures the reliability is higher for tracepoints
> at function entry, while on others it's the opposite.
> When this happens, backtracing will stop because the return address
> is found unavailable (unless another collect rule happened to match it).
>
> WDYT?
>
> Thanks,
> Pedro Alves
>
Sounds OK to me.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 0/8] gdb/s390: Add regular and fast tracepoint support.
@ 2016-01-24 12:12 Marcin Kościelnicki
2016-01-24 12:12 ` [PATCH 4/8] gdb/s390: Fill gen_return_address hook Marcin Kościelnicki
0 siblings, 1 reply; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-01-24 12:12 UTC (permalink / raw)
To: gdb-patches
This patchset adds support for regular and fast tracepoints on
s390-ibm-linux-gnu and s390x-ibm-linux-gnu. It depends on the following
yet-unlanded patches:
- https://sourceware.org/ml/gdb-patches/2016-01/msg00597.html (for 31-bit
fast tracepoint support)
- https://sourceware.org/ml/gdb-patches/2016-01/msg00596.html (just
a testsuite fix)
- https://sourceware.org/ml/gdb-patches/2016-01/msg00510.html (fixes
setting a breakpoint and a tracepoint on the same location)
Patches 1-5 add working regular tracepoint support, patches 6-7 add
working fast tracepoint support on top of that, and patch 8 makes fast
tracepoints use compiled agent expressions.
Patches 1 and 7 are in target-independent code, the remaining ones
are in s390-specific code.
It has been tested on s390-ibm-linux-gnu and s390x-ibm-linux-gnu on
a z13 machine (without vector extensions). The parts handling vector
extensions are untested. It has also been regression-tested on x86_64.
There are 2 test failures still left in gdb.trace:
- gdb.trace/unavailable.exp: print derived_whole has 4 failures on 64-bit
only. The same failure happens on x86_64, so it's unlikely to be
s390-specific.
- gdb.trace/mi-tsv-changed.exp: create delete modify: tvariable $tvar3
modified is an intermittent failure. It seems to be a race of some
kind - in the failing runs, gdb shows the same messages, but in
different order. I strongly suspect this is a target-independent
issues that only happens on s390 due to timing factors.
There are also three issues affecting s390 tracepoints that don't show
up in the testsuite:
1. Target independent: tfile format doesn't contain target information
(tdesc). While this affects all platforms with multiple tdescs
(eg. x86_64 is unable to pull AVX registers from tfile for that
reason), it horribly breaks 31-bit s390 with high GPRs - it has
a completely different GPR layout from plain 31-bit s390, so the
collected registers will be garbled. I suppose the proper fix to
that would be to add tdesc information to tfile format. Unfortunately,
I don't see a way to extend it in a backwards-compatible way.
2. Target independent: 32-bit (or 31-bit for us) IPA cannot be used with
64-bit gdbserver, due to communication involving lots of structs with
pointer types. Fixing that would be quite involved, but possible
(I don't suppose we have to maintain compatibility between IPA/gdbserver
from different gdb versions?).
3. s390 specific: 31-bit gdbserver doesn't know about high GPRs, and
cannot collect them if they're in use. Seems fixable with average
effort. Unfortunately, fixing that will break tfile, unless (1)
is fixed first...
These three interact in bad ways, summarised below:
- 64-bit linux, 64-bit gdbserver, 64-bit target: works OK, but you won't be
able to see VX registers in tfile.
- 64-bit linux, 64-bit gdbserver, 31-bit target:
- no fast tracepoint support due to (2)
- tfile completely broken due to (1)
- 64-bit linux, 31-bit gdbserver, 31-bit target:
- works OK, but you won't be able to see VX registers or high GPRs at all,
due to (3)
- if (3) were fixed, tfile will be completely broken due to (1)
- 31-bit linux, 31-bit gdbserver, 31-bit target: works OK
In summary, there's no way at all to use fast tracepoints on 31-bit target
if what you want to collect involves high GPRs. While there's support for
it in the IPA, it's currently disabled, to match what gdbserver supports.
Fast tracepoint support assumes z900+ CPU. The g5/g6 CPUs only have jump
instuctions with +-64kiB of range, making them pretty much useless for
our purposes, so not much loss here.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-01-24 12:12 [PATCH 0/8] gdb/s390: Add regular and fast tracepoint support Marcin Kościelnicki
@ 2016-01-24 12:12 ` Marcin Kościelnicki
2016-02-07 14:02 ` Marcin Kościelnicki
0 siblings, 1 reply; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-01-24 12:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Marcin Kościelnicki
gdb/ChangeLog:
* s390-linux-tdep.c (s390_gen_return_address): New function.
(s390_gdbarch_init): Fill gen_return_address hook.
---
gdb/ChangeLog | 5 +++++
gdb/s390-linux-tdep.c | 11 +++++++++++
2 files changed, 16 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2cfd088..e0eb258 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2016-01-24 Marcin KoÅcielnicki <koriakin@0x04.net>
+ * s390-linux-tdep.c (s390_gen_return_address): New function.
+ (s390_gdbarch_init): Fill gen_return_address hook.
+
+2016-01-24 Marcin KoÅcielnicki <koriakin@0x04.net>
+
* s390-linux-tdep.c (s390_ax_pseudo_register_collect): New function.
(s390_ax_pseudo_register_push_stack): New function.
(s390_gdbarch_init): Fill ax_pseudo_register_collect and
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 00ff388..48801d2 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -617,6 +617,16 @@ s390_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
return 0;
}
+static void
+s390_gen_return_address (struct gdbarch *gdbarch,
+ struct agent_expr *ax, struct axs_value *value,
+ CORE_ADDR scope)
+{
+ value->type = register_type (gdbarch, S390_R14_REGNUM);
+ value->kind = axs_lvalue_register;
+ value->u.reg = S390_R14_REGNUM;
+}
+
/* A helper for s390_software_single_step, decides if an instruction
is a partial-execution instruction that needs to be executed until
@@ -7960,6 +7970,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
s390_ax_pseudo_register_collect);
set_gdbarch_ax_pseudo_register_push_stack
(gdbarch, s390_ax_pseudo_register_push_stack);
+ set_gdbarch_gen_return_address (gdbarch, s390_gen_return_address);
tdesc_use_registers (gdbarch, tdesc, tdesc_data);
set_gdbarch_register_name (gdbarch, s390_register_name);
--
2.7.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-01-24 12:12 ` [PATCH 4/8] gdb/s390: Fill gen_return_address hook Marcin Kościelnicki
@ 2016-02-07 14:02 ` Marcin Kościelnicki
2016-02-25 19:23 ` Marcin Kościelnicki
2016-03-11 11:20 ` Andreas Arnez
0 siblings, 2 replies; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-02-07 14:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Marcin Kościelnicki
gdb/ChangeLog:
* s390-linux-tdep.c (s390_gen_return_address): New function.
(s390_gdbarch_init): Fill gen_return_address hook.
---
Added missing comment.
gdb/ChangeLog | 5 +++++
gdb/s390-linux-tdep.c | 13 +++++++++++++
2 files changed, 18 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6260040..0cf8bfc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2016-02-07 Marcin KoÅcielnicki <koriakin@0x04.net>
+ * s390-linux-tdep.c (s390_gen_return_address): New function.
+ (s390_gdbarch_init): Fill gen_return_address hook.
+
+2016-02-07 Marcin KoÅcielnicki <koriakin@0x04.net>
+
* s390-linux-tdep.c (s390_ax_pseudo_register_collect): New function.
(s390_ax_pseudo_register_push_stack): New function.
(s390_gdbarch_init): Fill ax_pseudo_register_collect and
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 97bd564..0b91ed1 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -627,6 +627,18 @@ s390_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
return 0;
}
+/* The "gen_return_address" gdbarch method. */
+
+static void
+s390_gen_return_address (struct gdbarch *gdbarch,
+ struct agent_expr *ax, struct axs_value *value,
+ CORE_ADDR scope)
+{
+ value->type = register_type (gdbarch, S390_R14_REGNUM);
+ value->kind = axs_lvalue_register;
+ value->u.reg = S390_R14_REGNUM;
+}
+
/* A helper for s390_software_single_step, decides if an instruction
is a partial-execution instruction that needs to be executed until
@@ -7970,6 +7982,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
s390_ax_pseudo_register_collect);
set_gdbarch_ax_pseudo_register_push_stack
(gdbarch, s390_ax_pseudo_register_push_stack);
+ set_gdbarch_gen_return_address (gdbarch, s390_gen_return_address);
tdesc_use_registers (gdbarch, tdesc, tdesc_data);
set_gdbarch_register_name (gdbarch, s390_register_name);
--
2.7.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-02-07 14:02 ` Marcin Kościelnicki
@ 2016-02-25 19:23 ` Marcin Kościelnicki
2016-03-04 10:42 ` Marcin Kościelnicki
2016-03-11 11:20 ` Andreas Arnez
1 sibling, 1 reply; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-02-25 19:23 UTC (permalink / raw)
To: gdb-patches
Ping.
On 07/02/16 15:02, Marcin KoÅcielnicki wrote:
> gdb/ChangeLog:
>
> * s390-linux-tdep.c (s390_gen_return_address): New function.
> (s390_gdbarch_init): Fill gen_return_address hook.
> ---
> Added missing comment.
>
> gdb/ChangeLog | 5 +++++
> gdb/s390-linux-tdep.c | 13 +++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 6260040..0cf8bfc 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,10 @@
> 2016-02-07 Marcin KoÅcielnicki <koriakin@0x04.net>
>
> + * s390-linux-tdep.c (s390_gen_return_address): New function.
> + (s390_gdbarch_init): Fill gen_return_address hook.
> +
> +2016-02-07 Marcin KoÅcielnicki <koriakin@0x04.net>
> +
> * s390-linux-tdep.c (s390_ax_pseudo_register_collect): New function.
> (s390_ax_pseudo_register_push_stack): New function.
> (s390_gdbarch_init): Fill ax_pseudo_register_collect and
> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> index 97bd564..0b91ed1 100644
> --- a/gdb/s390-linux-tdep.c
> +++ b/gdb/s390-linux-tdep.c
> @@ -627,6 +627,18 @@ s390_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
> return 0;
> }
>
> +/* The "gen_return_address" gdbarch method. */
> +
> +static void
> +s390_gen_return_address (struct gdbarch *gdbarch,
> + struct agent_expr *ax, struct axs_value *value,
> + CORE_ADDR scope)
> +{
> + value->type = register_type (gdbarch, S390_R14_REGNUM);
> + value->kind = axs_lvalue_register;
> + value->u.reg = S390_R14_REGNUM;
> +}
> +
>
> /* A helper for s390_software_single_step, decides if an instruction
> is a partial-execution instruction that needs to be executed until
> @@ -7970,6 +7982,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> s390_ax_pseudo_register_collect);
> set_gdbarch_ax_pseudo_register_push_stack
> (gdbarch, s390_ax_pseudo_register_push_stack);
> + set_gdbarch_gen_return_address (gdbarch, s390_gen_return_address);
> tdesc_use_registers (gdbarch, tdesc, tdesc_data);
> set_gdbarch_register_name (gdbarch, s390_register_name);
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-02-25 19:23 ` Marcin Kościelnicki
@ 2016-03-04 10:42 ` Marcin Kościelnicki
0 siblings, 0 replies; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-03-04 10:42 UTC (permalink / raw)
To: gdb-patches
Ping.
On 25/02/16 20:23, Marcin KoÅcielnicki wrote:
> Ping.
>
> On 07/02/16 15:02, Marcin KoÅcielnicki wrote:
>> gdb/ChangeLog:
>>
>> * s390-linux-tdep.c (s390_gen_return_address): New function.
>> (s390_gdbarch_init): Fill gen_return_address hook.
>> ---
>> Added missing comment.
>>
>> gdb/ChangeLog | 5 +++++
>> gdb/s390-linux-tdep.c | 13 +++++++++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 6260040..0cf8bfc 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,5 +1,10 @@
>> 2016-02-07 Marcin KoÅcielnicki <koriakin@0x04.net>
>>
>> + * s390-linux-tdep.c (s390_gen_return_address): New function.
>> + (s390_gdbarch_init): Fill gen_return_address hook.
>> +
>> +2016-02-07 Marcin KoÅcielnicki <koriakin@0x04.net>
>> +
>> * s390-linux-tdep.c (s390_ax_pseudo_register_collect): New
>> function.
>> (s390_ax_pseudo_register_push_stack): New function.
>> (s390_gdbarch_init): Fill ax_pseudo_register_collect and
>> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
>> index 97bd564..0b91ed1 100644
>> --- a/gdb/s390-linux-tdep.c
>> +++ b/gdb/s390-linux-tdep.c
>> @@ -627,6 +627,18 @@ s390_ax_pseudo_register_push_stack (struct
>> gdbarch *gdbarch,
>> return 0;
>> }
>>
>> +/* The "gen_return_address" gdbarch method. */
>> +
>> +static void
>> +s390_gen_return_address (struct gdbarch *gdbarch,
>> + struct agent_expr *ax, struct axs_value *value,
>> + CORE_ADDR scope)
>> +{
>> + value->type = register_type (gdbarch, S390_R14_REGNUM);
>> + value->kind = axs_lvalue_register;
>> + value->u.reg = S390_R14_REGNUM;
>> +}
>> +
>>
>> /* A helper for s390_software_single_step, decides if an instruction
>> is a partial-execution instruction that needs to be executed until
>> @@ -7970,6 +7982,7 @@ s390_gdbarch_init (struct gdbarch_info info,
>> struct gdbarch_list *arches)
>> s390_ax_pseudo_register_collect);
>> set_gdbarch_ax_pseudo_register_push_stack
>> (gdbarch, s390_ax_pseudo_register_push_stack);
>> + set_gdbarch_gen_return_address (gdbarch, s390_gen_return_address);
>> tdesc_use_registers (gdbarch, tdesc, tdesc_data);
>> set_gdbarch_register_name (gdbarch, s390_register_name);
>>
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-02-07 14:02 ` Marcin Kościelnicki
2016-02-25 19:23 ` Marcin Kościelnicki
@ 2016-03-11 11:20 ` Andreas Arnez
2016-03-11 11:35 ` Marcin Kościelnicki
1 sibling, 1 reply; 23+ messages in thread
From: Andreas Arnez @ 2016-03-11 11:20 UTC (permalink / raw)
To: Marcin Kościelnicki; +Cc: gdb-patches
On Sun, Feb 07 2016, Marcin Kościelnicki wrote:
> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> index 97bd564..0b91ed1 100644
> --- a/gdb/s390-linux-tdep.c
> +++ b/gdb/s390-linux-tdep.c
> @@ -627,6 +627,18 @@ s390_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
> return 0;
> }
>
> +/* The "gen_return_address" gdbarch method. */
> +
> +static void
> +s390_gen_return_address (struct gdbarch *gdbarch,
> + struct agent_expr *ax, struct axs_value *value,
> + CORE_ADDR scope)
> +{
> + value->type = register_type (gdbarch, S390_R14_REGNUM);
> + value->kind = axs_lvalue_register;
> + value->u.reg = S390_R14_REGNUM;
> +}
Under which circumstances is this supposed to work? And how reliable
does it need to be? The ABI only guarantees that r14 holds the return
address at function entry. Anywhere else it likely doesn't.
--
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 11:20 ` Andreas Arnez
@ 2016-03-11 11:35 ` Marcin Kościelnicki
2016-03-11 12:18 ` Andreas Arnez
0 siblings, 1 reply; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-03-11 11:35 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 11/03/16 12:20, Andreas Arnez wrote:
> On Sun, Feb 07 2016, Marcin KoÅcielnicki wrote:
>
>> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
>> index 97bd564..0b91ed1 100644
>> --- a/gdb/s390-linux-tdep.c
>> +++ b/gdb/s390-linux-tdep.c
>> @@ -627,6 +627,18 @@ s390_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
>> return 0;
>> }
>>
>> +/* The "gen_return_address" gdbarch method. */
>> +
>> +static void
>> +s390_gen_return_address (struct gdbarch *gdbarch,
>> + struct agent_expr *ax, struct axs_value *value,
>> + CORE_ADDR scope)
>> +{
>> + value->type = register_type (gdbarch, S390_R14_REGNUM);
>> + value->kind = axs_lvalue_register;
>> + value->u.reg = S390_R14_REGNUM;
>> +}
>
> Under which circumstances is this supposed to work? And how reliable
> does it need to be? The ABI only guarantees that r14 holds the return
> address at function entry. Anywhere else it likely doesn't.
>
> --
> Andreas
>
Quoting gdbarch.sh:
# Generate bytecodes to collect the return address in a frame.
# Since the bytecodes run on the target, possibly with GDB not even
# connected, the full unwinding machinery is not available, and
# typically this function will issue bytecodes for one or more likely
# places that the return address may be found.
m:void:gen_return_address:struct agent_expr *ax, struct axs_value
*value, CORE_ADDR scope:ax, value, scope::default_gen_return_address::0
ie. it's supposed to collect some value that will likely help the
unwinder - if it collects the wrong thing, the unwinder (knowing the
right place for sure) will simply consider the previous frame PC to be
unavailable.
We could also try to collect 14*<wordsize>(%r11), hoping that's the save
slot for %r14, but the interface unfortunately doesn't support
collecting multiple values (no matter what the comment above says).
Unfortunately, this interface is just not very well-designed - both x86
and aarch64 just take a shot in the dark like this patch. A better way
would be to reuse the existing unwinders and remove this hook
altogether, or (for while-stepping, where we can't predict the PC)
actually allow multiple values and aim at a few likely locations. But
IMO that's not in scope for this patchset.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 11:35 ` Marcin Kościelnicki
@ 2016-03-11 12:18 ` Andreas Arnez
2016-03-11 12:26 ` Marcin Kościelnicki
0 siblings, 1 reply; 23+ messages in thread
From: Andreas Arnez @ 2016-03-11 12:18 UTC (permalink / raw)
To: Marcin Kościelnicki; +Cc: gdb-patches
On Fri, Mar 11 2016, Marcin Kościelnicki wrote:
> We could also try to collect 14*<wordsize>(%r11), hoping that's the
> save slot for %r14, but the interface unfortunately doesn't support
> collecting multiple values (no matter what the comment above says).
Nah, that doesn't help either, since most functions don't use r11 as a
frame pointer. There is just no way to locate the return address unless
we have call frame information or perform code analysis.
> Unfortunately, this interface is just not very well-designed - both
> x86 and aarch64 just take a shot in the dark like this patch. A
> better way would be to reuse the existing unwinders and remove this
> hook altogether, or (for while-stepping, where we can't predict the
> PC) actually allow multiple values and aim at a few likely locations.
> But IMO that's not in scope for this patchset.
The point I was trying to make is that r14 is fairly *unlikely* to
contain the return address, unless we're near function entry. If we
just called a function, then r14 contains an address within our own
function. Otherwise r14 can also contain something else entirely.
Is there a way to admit that we don't know the return address? What if
we always return garbage? E.g., maybe it's better to always return 0?
--
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 12:18 ` Andreas Arnez
@ 2016-03-11 12:26 ` Marcin Kościelnicki
2016-03-11 15:31 ` Andreas Arnez
0 siblings, 1 reply; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-03-11 12:26 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 11/03/16 13:18, Andreas Arnez wrote:
> On Fri, Mar 11 2016, Marcin KoÅcielnicki wrote:
>
>> We could also try to collect 14*<wordsize>(%r11), hoping that's the
>> save slot for %r14, but the interface unfortunately doesn't support
>> collecting multiple values (no matter what the comment above says).
>
> Nah, that doesn't help either, since most functions don't use r11 as a
> frame pointer. There is just no way to locate the return address unless
> we have call frame information or perform code analysis.
>
>> Unfortunately, this interface is just not very well-designed - both
>> x86 and aarch64 just take a shot in the dark like this patch. A
>> better way would be to reuse the existing unwinders and remove this
>> hook altogether, or (for while-stepping, where we can't predict the
>> PC) actually allow multiple values and aim at a few likely locations.
>> But IMO that's not in scope for this patchset.
>
> The point I was trying to make is that r14 is fairly *unlikely* to
> contain the return address, unless we're near function entry. If we
> just called a function, then r14 contains an address within our own
> function. Otherwise r14 can also contain something else entirely.
Well, it works for leaf functions... not much, but not totally useless
either.
>
> Is there a way to admit that we don't know the return address? What if
> we always return garbage? E.g., maybe it's better to always return 0?
>
We can always error() in there (and KFAIL the testcase in gdb.trace that
exercises it). However, returning garbage here doesn't result in
garbage backtrace - this only collects data, if the unwinder actually
doing the work later determines it should look for the return address on
the stack, it'll just ignore our collected $r14 and consider the return
address unavailable (unless another collect rule happened to match it).
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 12:26 ` Marcin Kościelnicki
@ 2016-03-11 15:31 ` Andreas Arnez
2016-03-11 15:44 ` Pedro Alves
2016-03-13 9:53 ` Marcin Kościelnicki
0 siblings, 2 replies; 23+ messages in thread
From: Andreas Arnez @ 2016-03-11 15:31 UTC (permalink / raw)
To: Marcin Kościelnicki; +Cc: gdb-patches
On Fri, Mar 11 2016, Marcin Kościelnicki wrote:
> We can always error() in there (and KFAIL the testcase in gdb.trace
> that exercises it). However, returning garbage here doesn't result in
> garbage backtrace - this only collects data, if the unwinder actually
> doing the work later determines it should look for the return address
> on the stack, it'll just ignore our collected $r14 and consider the
> return address unavailable (unless another collect rule happened to
> match it).
Well, from that test case it appears that `$_ret' is generally not
expected to work very reliably. Since r14 usually does work near
function entry, this may be sufficient for now.
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.
--
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 15:31 ` Andreas Arnez
@ 2016-03-11 15:44 ` Pedro Alves
2016-03-11 16:45 ` Andreas Arnez
2016-03-13 9:53 ` Marcin Kościelnicki
1 sibling, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2016-03-11 15:44 UTC (permalink / raw)
To: Andreas Arnez, Marcin Kościelnicki; +Cc: gdb-patches
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.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 15:44 ` Pedro Alves
@ 2016-03-11 16:45 ` Andreas Arnez
2016-03-11 17:02 ` Pedro Alves
2016-03-11 18:07 ` Eli Zaretskii
0 siblings, 2 replies; 23+ messages in thread
From: Andreas Arnez @ 2016-03-11 16:45 UTC (permalink / raw)
To: Pedro Alves; +Cc: Eli Zaretskii, Marcin Kościelnicki, gdb-patches
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.
@item $_probe_argc
Collects the number of arguments from the static probe at which the
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 16:45 ` Andreas Arnez
@ 2016-03-11 17:02 ` Pedro Alves
2016-03-11 18:17 ` Eli Zaretskii
2016-03-11 18:07 ` Eli Zaretskii
1 sibling, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2016-03-11 17:02 UTC (permalink / raw)
To: Andreas Arnez; +Cc: Eli Zaretskii, Marcin Kościelnicki, gdb-patches
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 17:02 ` Pedro Alves
@ 2016-03-11 18:17 ` Eli Zaretskii
2016-03-11 18:37 ` Pedro Alves
0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-03-11 18:17 UTC (permalink / raw)
To: Pedro Alves; +Cc: arnez, koriakin, gdb-patches
> 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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 18:17 ` Eli Zaretskii
@ 2016-03-11 18:37 ` Pedro Alves
2016-03-11 19:34 ` Eli Zaretskii
0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2016-03-11 18:37 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: arnez, koriakin, gdb-patches
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 18:37 ` Pedro Alves
@ 2016-03-11 19:34 ` Eli Zaretskii
2016-03-15 11:11 ` Pedro Alves
0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-03-11 19:34 UTC (permalink / raw)
To: Pedro Alves; +Cc: arnez, koriakin, gdb-patches
> 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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 19:34 ` Eli Zaretskii
@ 2016-03-15 11:11 ` Pedro Alves
2016-03-15 11:23 ` Andreas Arnez
0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2016-03-15 11:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: arnez, koriakin, gdb-patches
On 03/11/2016 07:33 PM, Eli Zaretskii wrote:
>> 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.
I see what you mean. I reverted part of the change, and made
the note say the simpler "location" instead now.
> Anyway, if you are happier with your proposal, I won't object.
Below's what I pushed.
Thanks!
From 45fa2529db961adff41c52c3a560808cb135beb2 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 15 Mar 2016 11:08:52 +0000
Subject: [PATCH] Document possible unreliability of '$_ret'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
gdb/doc/ChangeLog:
2016-03-15 Pedro Alves <palves@redhat.com>
Andreas Arnez <arnez@linux.vnet.ibm.com>
Marcin KoÅcielnicki <koriakin@0x04.net>
* gdb.texinfo (Tracepoint Actions): Document possible
unreliability of '$_ret'.
---
gdb/doc/ChangeLog | 7 +++++++
gdb/doc/gdb.texinfo | 7 +++++++
2 files changed, 14 insertions(+)
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 3d49085..0606d9d 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,10 @@
+2016-03-15 Pedro Alves <palves@redhat.com>
+ Andreas Arnez <arnez@linux.vnet.ibm.com>
+ Marcin KoÅcielnicki <koriakin@0x04.net>
+
+ * gdb.texinfo (Tracepoint Actions): Document possible
+ unreliability of '$_ret'.
+
2016-03-11 Andrew Burgess <andrew.burgess@embecosm.com>
* gdb.texinfo (Symbols): Document new 'maint info line-table'
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bf7df35..5f88335 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -12878,6 +12878,13 @@ Collect all local variables.
Collect the return address. This is helpful if you want to see more
of a backtrace.
+@emph{Note:} The return address location can not always be reliability
+determined up front, and the wrong address / registers may end up
+collected instead. On some architectures the reliability is higher
+for tracepoints at function entry, while on others it's the opposite.
+When this happens, backtracing will stop because the return address is
+found unavailable (unless another collect rule happened to match it).
+
@item $_probe_argc
Collects the number of arguments from the static probe at which the
tracepoint is located.
--
2.5.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-15 11:11 ` Pedro Alves
@ 2016-03-15 11:23 ` Andreas Arnez
2016-03-15 11:30 ` Pedro Alves
0 siblings, 1 reply; 23+ messages in thread
From: Andreas Arnez @ 2016-03-15 11:23 UTC (permalink / raw)
To: Pedro Alves; +Cc: Eli Zaretskii, koriakin, gdb-patches
On Tue, Mar 15 2016, Pedro Alves wrote:
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index bf7df35..5f88335 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -12878,6 +12878,13 @@ Collect all local variables.
> Collect the return address. This is helpful if you want to see more
> of a backtrace.
>
> +@emph{Note:} The return address location can not always be reliability
^^^^^^^^^^^
Should be "reliably" ;-)
> +determined up front, and the wrong address / registers may end up
> +collected instead. On some architectures the reliability is higher
> +for tracepoints at function entry, while on others it's the opposite.
> +When this happens, backtracing will stop because the return address is
> +found unavailable (unless another collect rule happened to match it).
> +
> @item $_probe_argc
> Collects the number of arguments from the static probe at which the
> tracepoint is located.
--
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-15 11:23 ` Andreas Arnez
@ 2016-03-15 11:30 ` Pedro Alves
0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2016-03-15 11:30 UTC (permalink / raw)
To: Andreas Arnez; +Cc: Eli Zaretskii, koriakin, gdb-patches
On 03/15/2016 11:23 AM, Andreas Arnez wrote:
> On Tue, Mar 15 2016, Pedro Alves wrote:
>
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index bf7df35..5f88335 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -12878,6 +12878,13 @@ Collect all local variables.
>> Collect the return address. This is helpful if you want to see more
>> of a backtrace.
>>
>> +@emph{Note:} The return address location can not always be reliability
> ^^^^^^^^^^^
> Should be "reliably" ;-)
Bah. I knew I'd forget something... Thanks for double checking.
Fixed with the patch below.
From 2a60e18f8fe3bf9512671e02b39acacb484bb8c6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 15 Mar 2016 11:29:03 +0000
Subject: [PATCH] Fix typo in previous gdb/doc/ commit
Should be s/reliability/reliably/.
gdb/doc/ChangeLog:
2016-03-15 Pedro Alves <palves@redhat.com>
* gdb.texinfo (Tracepoint Actions): Fix typo.
---
gdb/doc/ChangeLog | 4 ++++
gdb/doc/gdb.texinfo | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 0606d9d..a48d1c4 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,4 +1,8 @@
2016-03-15 Pedro Alves <palves@redhat.com>
+
+ * gdb.texinfo (Tracepoint Actions): Fix typo.
+
+2016-03-15 Pedro Alves <palves@redhat.com>
Andreas Arnez <arnez@linux.vnet.ibm.com>
Marcin KoÅcielnicki <koriakin@0x04.net>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5f88335..71657b0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -12878,7 +12878,7 @@ Collect all local variables.
Collect the return address. This is helpful if you want to see more
of a backtrace.
-@emph{Note:} The return address location can not always be reliability
+@emph{Note:} The return address location can not always be reliably
determined up front, and the wrong address / registers may end up
collected instead. On some architectures the reliability is higher
for tracepoints at function entry, while on others it's the opposite.
--
2.5.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 16:45 ` Andreas Arnez
2016-03-11 17:02 ` Pedro Alves
@ 2016-03-11 18:07 ` Eli Zaretskii
1 sibling, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2016-03-11 18:07 UTC (permalink / raw)
To: Andreas Arnez; +Cc: palves, koriakin, gdb-patches
> 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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-11 15:31 ` Andreas Arnez
2016-03-11 15:44 ` Pedro Alves
@ 2016-03-13 9:53 ` Marcin Kościelnicki
2016-03-14 10:07 ` Andreas Arnez
1 sibling, 1 reply; 23+ messages in thread
From: Marcin Kościelnicki @ 2016-03-13 9:53 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 11/03/16 16:31, Andreas Arnez wrote:
> On Fri, Mar 11 2016, Marcin KoÅcielnicki wrote:
>
>> We can always error() in there (and KFAIL the testcase in gdb.trace
>> that exercises it). However, returning garbage here doesn't result in
>> garbage backtrace - this only collects data, if the unwinder actually
>> doing the work later determines it should look for the return address
>> on the stack, it'll just ignore our collected $r14 and consider the
>> return address unavailable (unless another collect rule happened to
>> match it).
>
> Well, from that test case it appears that `$_ret' is generally not
> expected to work very reliably. Since r14 usually does work near
> function entry, this may be sufficient for now.
>
> 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.
>
Thanks, pushed with this comment:
/* The "gen_return_address" gdbarch method. Since this is supposed to be
just a best-effort method, and we don't really have the means to run
the full unwinder here, just collect the link register. */
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/8] gdb/s390: Fill gen_return_address hook.
2016-03-13 9:53 ` Marcin Kościelnicki
@ 2016-03-14 10:07 ` Andreas Arnez
0 siblings, 0 replies; 23+ messages in thread
From: Andreas Arnez @ 2016-03-14 10:07 UTC (permalink / raw)
To: Marcin Kościelnicki; +Cc: gdb-patches
On Sun, Mar 13 2016, Marcin Kościelnicki wrote:
> On 11/03/16 16:31, Andreas Arnez wrote:
>> On Fri, Mar 11 2016, Marcin Kościelnicki wrote:
>>
>>> We can always error() in there (and KFAIL the testcase in gdb.trace
>>> that exercises it). However, returning garbage here doesn't result in
>>> garbage backtrace - this only collects data, if the unwinder actually
>>> doing the work later determines it should look for the return address
>>> on the stack, it'll just ignore our collected $r14 and consider the
>>> return address unavailable (unless another collect rule happened to
>>> match it).
>>
>> Well, from that test case it appears that `$_ret' is generally not
>> expected to work very reliably. Since r14 usually does work near
>> function entry, this may be sufficient for now.
>>
>> 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.
>>
>
> Thanks, pushed with this comment:
>
> /* The "gen_return_address" gdbarch method. Since this is supposed to be
> just a best-effort method, and we don't really have the means to run
> the full unwinder here, just collect the link register. */
Very good, thanks.
--
Andreas Arnez
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-03-15 11:30 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <8ce93c6f-5822-4e27-9e59-c6cbe158424d@email.android.com>
2016-03-11 17:34 ` [PATCH 4/8] gdb/s390: Fill gen_return_address hook Pedro Alves
2016-03-11 17:43 ` Marcin Kościelnicki
2016-01-24 12:12 [PATCH 0/8] gdb/s390: Add regular and fast tracepoint support Marcin Kościelnicki
2016-01-24 12:12 ` [PATCH 4/8] gdb/s390: Fill gen_return_address hook Marcin Kościelnicki
2016-02-07 14:02 ` Marcin Kościelnicki
2016-02-25 19:23 ` Marcin Kościelnicki
2016-03-04 10:42 ` Marcin Kościelnicki
2016-03-11 11:20 ` Andreas Arnez
2016-03-11 11:35 ` Marcin Kościelnicki
2016-03-11 12:18 ` Andreas Arnez
2016-03-11 12:26 ` Marcin Kościelnicki
2016-03-11 15:31 ` Andreas Arnez
2016-03-11 15:44 ` Pedro Alves
2016-03-11 16:45 ` Andreas Arnez
2016-03-11 17:02 ` Pedro Alves
2016-03-11 18:17 ` Eli Zaretskii
2016-03-11 18:37 ` Pedro Alves
2016-03-11 19:34 ` Eli Zaretskii
2016-03-15 11:11 ` Pedro Alves
2016-03-15 11:23 ` Andreas Arnez
2016-03-15 11:30 ` Pedro Alves
2016-03-11 18:07 ` Eli Zaretskii
2016-03-13 9:53 ` Marcin Kościelnicki
2016-03-14 10:07 ` Andreas Arnez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).