| Message ID | 20240123215229.2385404-1-thiago.bauermann@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gdb/arm: Fix epilogue frame id | expand |
On 1/23/24 21:52, Thiago Jung Bauermann wrote: > arm_epilogue_frame_this_id has a comment saying that it fall backs to using > the current PC if the function start address can't be identified, but it > actually uses only the PC to make the frame id. > > This patch makes the code match the comment. Another hint that it's what > is intended is that arm_prologue_this_id, a function almost identical to > it, does that. > > The problem was found by code inspection. It fixes the following testsuite > failures: > > FAIL: gdb.base/unwind-on-each-insn.exp: foo: instruction 9: check frame-id matches > FAIL: gdb.reverse/solib-reverse.exp: reverse-next third shr1 > FAIL: gdb.reverse/solib-reverse.exp: reverse-next second shr1 > FAIL: gdb.reverse/solib-reverse.exp: reverse-next first shr1 > FAIL: gdb.reverse/solib-reverse.exp: reverse-next generic > FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function one > FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function one > FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function two > FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function two > > Tested on arm-linux-gnueabi-hf. > --- > gdb/arm-tdep.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > index f1aa730579bc..0d0431e0d1cd 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -3252,7 +3252,7 @@ arm_epilogue_frame_this_id (frame_info_ptr this_frame, > > arm_gdbarch_tdep *tdep > = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame)); > - *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), pc); > + *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), func); > } > > /* Implementation of function hook 'prev_register' in Thanks. This is OK. Approved-By: Luis Machado <luis.machado@arm.com>
Hello Luis, Luis Machado <luis.machado@arm.com> writes: > On 1/23/24 21:52, Thiago Jung Bauermann wrote: >> arm_epilogue_frame_this_id has a comment saying that it fall backs to using >> the current PC if the function start address can't be identified, but it >> actually uses only the PC to make the frame id. >> >> This patch makes the code match the comment. Another hint that it's what >> is intended is that arm_prologue_this_id, a function almost identical to >> it, does that. >> >> The problem was found by code inspection. It fixes the following testsuite >> failures: >> >> FAIL: gdb.base/unwind-on-each-insn.exp: foo: instruction 9: check frame-id matches >> FAIL: gdb.reverse/solib-reverse.exp: reverse-next third shr1 >> FAIL: gdb.reverse/solib-reverse.exp: reverse-next second shr1 >> FAIL: gdb.reverse/solib-reverse.exp: reverse-next first shr1 >> FAIL: gdb.reverse/solib-reverse.exp: reverse-next generic >> FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function one >> FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function one >> FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function two >> FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function two >> >> Tested on arm-linux-gnueabi-hf. >> --- >> gdb/arm-tdep.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c >> index f1aa730579bc..0d0431e0d1cd 100644 >> --- a/gdb/arm-tdep.c >> +++ b/gdb/arm-tdep.c >> @@ -3252,7 +3252,7 @@ arm_epilogue_frame_this_id (frame_info_ptr this_frame, >> >> arm_gdbarch_tdep *tdep >> = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame)); >> - *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), pc); >> + *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), func); >> } >> >> /* Implementation of function hook 'prev_register' in > > Thanks. This is OK. Thanks for the quick review. Pushed as commit 44acb01769b0. > Approved-By: Luis Machado <luis.machado@arm.com> Sorry, I just noticed that I forgot to add your tag to the commit message.
On 1/24/24 14:48, Thiago Jung Bauermann wrote: > > Sorry, I just noticed that I forgot to add your tag to the commit > message. > Don't worry. Not a big deal.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index f1aa730579bc..0d0431e0d1cd 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -3252,7 +3252,7 @@ arm_epilogue_frame_this_id (frame_info_ptr this_frame, arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame)); - *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), pc); + *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), func); } /* Implementation of function hook 'prev_register' in