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

["Ulrich Weigand" <uweigand@de.ibm.com>]

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?

[ 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.  ]

Tested on powerpc64le-linux.

Bye,
Ulrich


gdb/ChangeLog:

	* corefile.c (struct captured_read_memory_integer_arguments): Remove.
	(do_captured_read_memory_integer): Remove.
	(safe_read_memory_integer): Use target_read_memory directly instead
	of catching errors in do_captured_read_memory_integer.

diff --git a/gdb/corefile.c b/gdb/corefile.c
index 1617392..a0bb2aa 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -290,40 +290,6 @@ read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     memory_error (status, memaddr);
 }
 
-/* Argument / return result struct for use with
-   do_captured_read_memory_integer().  MEMADDR and LEN are filled in
-   by gdb_read_memory_integer().  RESULT is the contents that were
-   successfully read from MEMADDR of length LEN.  */
-
-struct captured_read_memory_integer_arguments
-{
-  CORE_ADDR memaddr;
-  int len;
-  enum bfd_endian byte_order;
-  LONGEST result;
-};
-
-/* Helper function for gdb_read_memory_integer().  DATA must be a
-   pointer to a captured_read_memory_integer_arguments struct.
-   Return 1 if successful.  Note that the catch_errors() interface
-   will return 0 if an error occurred while reading memory.  This
-   choice of return code is so that we can distinguish between
-   success and failure.  */
-
-static int
-do_captured_read_memory_integer (void *data)
-{
-  struct captured_read_memory_integer_arguments *args
-    = (struct captured_read_memory_integer_arguments*) data;
-  CORE_ADDR memaddr = args->memaddr;
-  int len = args->len;
-  enum bfd_endian byte_order = args->byte_order;
-
-  args->result = read_memory_integer (memaddr, len, byte_order);
-
-  return 1;
-}
-
 /* Read memory at MEMADDR of length LEN and put the contents in
    RETURN_VALUE.  Return 0 if MEMADDR couldn't be read and non-zero
    if successful.  */
@@ -333,19 +299,13 @@ safe_read_memory_integer (CORE_ADDR memaddr, int len,
 			  enum bfd_endian byte_order,
 			  LONGEST *return_value)
 {
-  int status;
-  struct captured_read_memory_integer_arguments args;
-
-  args.memaddr = memaddr;
-  args.len = len;
-  args.byte_order = byte_order;
+  gdb_byte buf[sizeof (LONGEST)];
 
-  status = catch_errors (do_captured_read_memory_integer, &args,
-			 "", RETURN_MASK_ALL);
-  if (status)
-    *return_value = args.result;
+  if (target_read_memory (memaddr, buf, len))
+    return 0;
 
-  return status;
+  *return_value = extract_signed_integer (buf, len, byte_order);
+  return 1;
 }
 
 LONGEST


-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com