This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
- From: Pedro Alves <palves at redhat dot com>
- To: Ulrich Weigand <uweigand at de dot ibm dot com>
- Cc: Edjunior Barbosa Machado <emachado at linux dot vnet dot ibm dot com>, gdb-patches at sourceware dot org
- Date: Wed, 17 Sep 2014 14:02:48 +0100
- Subject: Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
- Authentication-results: sourceware.org; auth=none
- References: <201409171241 dot s8HCfHqs007521 at d06av02 dot portsmouth dot uk dot ibm dot com>
On 09/17/2014 01:41 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
>
>> See https://sourceware.org/bugzilla/show_bug.cgi?id=17384 .
>>
>> When safe_read_memory_integer call fails, GDB prints a
>> surprising/confusing error message, more so in case the unwinder
>> is triggered for some reason other than the "bt" command, like
>> with "step"/"next". I take you're now seeing the same errors
>> with this patch.
>>
>> IMO, printing the error is not something a low-level helper function
>> like safe_read_memory_integer should be doing, as GDB uses it when
>> probing with heuristics because it can't sure its guesses make sense
>> (whether there's a frame at all, etc.) safe_frame_unwind_memory, which is
>> used in rs6000_in_function_epilogue_p doesn't print the error either.
>
> Agreed, it doesn't make sense for safe_read_memory_integer to ever
> print an error. In fact, it doesn't make sense for it to start
> using a routine that raises exceptions and then attempt to catch it.
> The following patch simplifies the whole logic by just using
> target_read_memory directly. Does this look reasonable?
Definitely reasonable. Looks great to me. Thanks for doing this.
>
> [ B.t.w. the naming of safe_frame_unwind_memory is a bit weird. This
> should either be "safe_read_memory" in corefile.c, or else something
> like safe_get_frame_memory in analogy to get_frame_memory. ]
Agreed. It seems like that and get_frame_memory were added
in order to make sure frame code consistently used
target_read_memory_nobpt to mask out breakpoints:
https://sourceware.org/ml/gdb-patches/2004-04/msg00067.html
Seems like all that wrapping is unnecessary nowadays, as we have to
go out of way to bypass breakpoint masking.
Thanks,
Pedro Alves