Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid

[Pedro Alves <palves@redhat.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