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

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

[Ulrich Weigand]

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

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

[Pedro Alves]

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

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

[Ulrich Weigand]

Pedro Alves wrote:
> On 09/17/2014 01:41 PM, Ulrich Weigand wrote:
> > 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.

OK, I've committed that patch now.

Bye,
Ulrich

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

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

[Pedro Alves]

On 09/12/2014 02:50 PM, Joel Brobecker wrote:
>> We still need to use gdb_run_cmd to cover remote testing,
>> so that'd be:
>>
>>  gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"
>>
>> ISTR that gdb_test doesn't allow empty command, but I may well
>> be mistaken.  And if it doesn't, maybe it should.
> 
> This is me pretending that I had noticed that the command was empty
> and knowing that this was still OK :-). But once you mentioned it,
> I knew I had already done something like that. See gdb.ada/bp_reset.exp:
> 
>     gdb_run_cmd
>     gdb_test "" "Breakpoint $decimal, foo\\.nested_sub \\(\\).*"
> 
> Doing a quick grep, we have a number of occurences where we use
> an empty command when calling gdb_test.  And looking at gdb_test's
> implementation, it just passes the first argument to gdb_test_multiple,
> so it should indeed be equivalent.  (phew, that was close! ;-)).

:-)

I'm writing a test that converts all gdb_run_cmd -> gdb_expect
cases to avoid this from spreading further.

Thanks,
Pedro Alves

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

[Joel Brobecker]

> We still need to use gdb_run_cmd to cover remote testing,
> so that'd be:
> 
>  gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"
> 
> ISTR that gdb_test doesn't allow empty command, but I may well
> be mistaken.  And if it doesn't, maybe it should.

This is me pretending that I had noticed that the command was empty
and knowing that this was still OK :-). But once you mentioned it,
I knew I had already done something like that. See gdb.ada/bp_reset.exp:

    gdb_run_cmd
    gdb_test "" "Breakpoint $decimal, foo\\.nested_sub \\(\\).*"

Doing a quick grep, we have a number of occurences where we use
an empty command when calling gdb_test.  And looking at gdb_test's
implementation, it just passes the first argument to gdb_test_multiple,
so it should indeed be equivalent.  (phew, that was close! ;-)).

-- 
Joel

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

[Pedro Alves]

On 09/12/2014 02:00 PM, Joel Brobecker wrote:
>> set test "run until SIGSEGV"
>> gdb_test_multiple "" $test {
>>     -re "Program received signal SIGSEGV.*$gdb_prompt $" {
>> 	pass $test
>>     }
>> }
> 
> Taking this one step further, wouldn't a simpler gdb_test also work
> in this case?

Yeah, good point.

We still need to use gdb_run_cmd to cover remote testing,
so that'd be:

 gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"

ISTR that gdb_test doesn't allow empty command, but I may well
be mistaken.  And if it doesn't, maybe it should.

Thanks,
Pedro Alves

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

[Joel Brobecker]

> set test "run until SIGSEGV"
> gdb_test_multiple "" $test {
>     -re "Program received signal SIGSEGV.*$gdb_prompt $" {
> 	pass $test
>     }
> }

Taking this one step further, wouldn't a simpler gdb_test also work
in this case?

-- 
Joel

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

[Edjunior Barbosa Machado]

Thank you all for the review. I've just pushed the following patch with the
suggested additional fixes.

  https://sourceware.org/ml/gdb-cvs/2014-09/msg00055.html

Thanks and regards,
--
Edjunior

gdb/ChangeLog
2014-09-12  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
	    Ulrich Weigand  <uweigand@de.ibm.com>

	PR tdep/17379
	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
	instead of read_memory_unsigned_integer.

gdb/testcase/ChangeLog
2014-09-12  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	PR tdep/17379
	* gdb.arch/powerpc-stackless.S: New file.
	* gdb.arch/powerpc-stackless.exp: New file.

---
 gdb/rs6000-tdep.c                            |   11 +++++--
 gdb/testsuite/gdb.arch/powerpc-stackless.S   |   24 +++++++++++++++
 gdb/testsuite/gdb.arch/powerpc-stackless.exp |   42 ++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.S
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.exp

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 730afe7..dabf448 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
     }
 
   if (!fdata.frameless)
-    /* Frameless really means stackless.  */
-    cache->base
-      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
+    {
+      /* Frameless really means stackless.  */
+      LONGEST backchain;
+
+      if (safe_read_memory_integer (cache->base, wordsize,
+				    byte_order, &backchain))
+        cache->base = (CORE_ADDR) backchain;
+    }
 
   trad_frame_set_value (cache->saved_regs,
 			gdbarch_sp_regnum (gdbarch), cache->base);
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.S b/gdb/testsuite/gdb.arch/powerpc-stackless.S
new file mode 100644
index 0000000..bbf92bb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.S
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <ppc-asm.h>
+
+FUNC_START(main)
+        li      sp,0
+        mtlr    sp
+        blr
+FUNC_END(main)
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
new file mode 100644
index 0000000..420bcbc
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
@@ -0,0 +1,42 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Testcase for PR tdep/17379.
+
+if {![istarget "powerpc*-*-*"]} then {
+    verbose "Skipping powerpc-stackless.exp"
+    return
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    untested powerpc-stackless.exp
+    return -1
+}
+
+# Run until SIGSEGV.
+gdb_run_cmd
+
+set test "run until SIGSEGV"
+gdb_test_multiple "" $test {
+    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
+  pass $test
+    }
+}
+
+# Ensure that 'info registers' works properly and does not generate
+# an internal-error.
+gdb_test "info registers" "r0.*" "info registers"
-- 
1.7.9.5

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

[Pedro Alves]

On 09/12/2014 03:46 AM, Edjunior Barbosa Machado wrote:
> +# Run until SIGSEGV.
> +gdb_run_cmd
> +
> +gdb_expect {
> +    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
> +	pass "run until SIGSEGV"
> +    }
> +    -re ".*$gdb_prompt $" {
> +	fail "run until SIGSEGV"
> +    }
> +    timeout {
> +	fail "run until SIGSEGV (timeout)"
> +    }
> +}

gdb_expect should only be used when gdb_test or gdb_test_multiple
really can't be used.  Please write instead:

gdb_run_cmd

set test "run until SIGSEGV"
gdb_test_multiple "" $test {
    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
	pass $test
    }
}

gdb_test_multiple will already issue a FAIL if it sees the prompt
or gets a timeout, and in addition will catch other problems,
like internal errors.

Thanks,
Pedro Alves

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

[Ulrich Weigand]

Sergio Durigan Junior wrote:
> On Thursday, September 11 2014, Edjunior Barbosa Machado wrote:
> 
> > Thanks Sergio for your feedback. I'm resending the patch with an additional
> > testcase as you suggested.
> 
> Nice, thanks :-).
> 
> > diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
> > new file mode 100644
> > index 0000000..f4b2a90
> [...]
> > +standard_testfile powerpc-stackless.S
> 
> Just another really minor nit, no need to resubmit the patch because of
> this.  You can write:
> 
>   standard_testfile .S
> 
> Other than that, it is perfect.
> 
> Thanks a lot!

Thanks for the testcase (and the review)!

Edjunior, the patch is OK to check in with the change Sergio suggested.

Bye,
Ulrich

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

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

[Sergio Durigan Junior]

On Thursday, September 11 2014, Edjunior Barbosa Machado wrote:

> Thanks Sergio for your feedback. I'm resending the patch with an additional
> testcase as you suggested.

Nice, thanks :-).

> diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
> new file mode 100644
> index 0000000..f4b2a90
[...]
> +standard_testfile powerpc-stackless.S

Just another really minor nit, no need to resubmit the patch because of
this.  You can write:

  standard_testfile .S

Other than that, it is perfect.

Thanks a lot!

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

[Edjunior Barbosa Machado]

Thanks Sergio for your feedback. I'm resending the patch with an additional
testcase as you suggested.

--
Edjunior

gdb/ChangeLog
2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
	    Ulrich Weigand  <uweigand@de.ibm.com>

	PR tdep/17379
	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
	instead of read_memory_unsigned_integer.

gdb/testcase/ChangeLog
2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	PR tdep/17379
	* gdb.arch/powerpc-stackless.S: New file.
	* gdb.arch/powerpc-stackless.exp: New file.

---
 gdb/rs6000-tdep.c                            |   11 ++++--
 gdb/testsuite/gdb.arch/powerpc-stackless.S   |   24 +++++++++++++
 gdb/testsuite/gdb.arch/powerpc-stackless.exp |   48 ++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.S
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.exp

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 730afe7..dabf448 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
     }
 
   if (!fdata.frameless)
-    /* Frameless really means stackless.  */
-    cache->base
-      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
+    {
+      /* Frameless really means stackless.  */
+      LONGEST backchain;
+
+      if (safe_read_memory_integer (cache->base, wordsize,
+				    byte_order, &backchain))
+        cache->base = (CORE_ADDR) backchain;
+    }
 
   trad_frame_set_value (cache->saved_regs,
 			gdbarch_sp_regnum (gdbarch), cache->base);
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.S b/gdb/testsuite/gdb.arch/powerpc-stackless.S
new file mode 100644
index 0000000..bbf92bb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.S
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <ppc-asm.h>
+
+FUNC_START(main)
+        li      sp,0
+        mtlr    sp
+        blr
+FUNC_END(main)
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
new file mode 100644
index 0000000..f4b2a90
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
@@ -0,0 +1,48 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Testcase for PR tdep/17379.
+
+if {![istarget "powerpc*-*-*"]} then {
+    verbose "Skipping powerpc-stackless.exp"
+    return
+}
+
+standard_testfile powerpc-stackless.S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    untested powerpc-stackless.exp
+    return -1
+}
+
+# Run until SIGSEGV.
+gdb_run_cmd
+
+gdb_expect {
+    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
+	pass "run until SIGSEGV"
+    }
+    -re ".*$gdb_prompt $" {
+	fail "run until SIGSEGV"
+    }
+    timeout {
+	fail "run until SIGSEGV (timeout)"
+    }
+}
+
+# Ensure that 'info registers' works properly and does not generate
+# an internal-error.
+gdb_test "info registers" "r0.*" "info registers"
-- 
1.7.9.5

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

[Sergio Durigan Junior]

On Thursday, September 11 2014, Edjunior Barbosa Machado wrote:

> The problem is that rs6000_frame_cache attempts to read the stack backchain via
> read_memory_unsigned_integer, which throws an exception if the stack pointer is
> invalid.  With this path, it calls safe_read_memory_integer instead, which
> doesn't throw an exception and allows for safe handling of that situation.
> Regression tested on ppc64{,le}.  Ok?

Heya!

Thanks for the patch.  Not having reviewed the code deeply to understand
if it's the best approach, I would just like to point that a testcase
for this would be awesome.  As it turns out, you actually already have a
testcase almost written in the bug description :-).

Again, thanks for addressing those issues!

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

[Edjunior Barbosa Machado]

Hi,

this patch intends to fix PR tdep/17379:
  https://sourceware.org/bugzilla/show_bug.cgi?id=17379

The problem is that rs6000_frame_cache attempts to read the stack backchain via
read_memory_unsigned_integer, which throws an exception if the stack pointer is
invalid.  With this path, it calls safe_read_memory_integer instead, which
doesn't throw an exception and allows for safe handling of that situation.
Regression tested on ppc64{,le}.  Ok?

Thanks and regards,
--
Edjunior

gdb/
2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
	    Ulrich Weigand  <uweigand@de.ibm.com>

	PR tdep/17379
	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
	instead of read_memory_unsigned_integer.

---
 gdb/rs6000-tdep.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 730afe7..dabf448 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
     }
 
   if (!fdata.frameless)
-    /* Frameless really means stackless.  */
-    cache->base
-      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
+    {
+      /* Frameless really means stackless.  */
+      LONGEST backchain;
+
+      if (safe_read_memory_integer (cache->base, wordsize,
+				    byte_order, &backchain))
+        cache->base = (CORE_ADDR) backchain;
+    }
 
   trad_frame_set_value (cache->saved_regs,
 			gdbarch_sp_regnum (gdbarch), cache->base);
-- 
1.7.9.5