[nios2] Fix bug in nios2 prologue analysis.

Message ID bbe0ebc4-9924-1dd5-3976-42f41039a5d7@codesourcery.com
State New, archived
Headers

Commit Message

Sandra Loosemore Aug. 15, 2019, 8:41 p.m. UTC
  This patch fixes a bug in the nios2 prologue analyzer that was causing 
failures in a couple of test cases.  The bug has been there forever, but 
this code isn't terribly well exercised by the testsuite since the nios2 
backend normally uses the dwarf2 unwinder.

I realize the nios2 backend has a more general problem with using int or 
unsigned int to represent target word-sized values when it really ought 
to be using int32_t or uint32_t.  I think that is a different issue than 
the signedness bug, though.  So, OK to check this in?

-Sandra
  

Comments

Sandra Loosemore Aug. 22, 2019, 8:12 p.m. UTC | #1
On 8/15/19 2:41 PM, Sandra Loosemore wrote:
> This patch fixes a bug in the nios2 prologue analyzer that was causing 
> failures in a couple of test cases.  The bug has been there forever, but 
> this code isn't terribly well exercised by the testsuite since the nios2 
> backend normally uses the dwarf2 unwinder.

Ping!

-Sandra
  
Kevin Buettner Aug. 23, 2019, 4:29 a.m. UTC | #2
On Thu, 15 Aug 2019 14:41:43 -0600
Sandra Loosemore <sandra@codesourcery.com> wrote:

> This patch fixes a bug in the nios2 prologue analyzer that was causing 
> failures in a couple of test cases.  The bug has been there forever, but 
> this code isn't terribly well exercised by the testsuite since the nios2 
> backend normally uses the dwarf2 unwinder.
> 
> I realize the nios2 backend has a more general problem with using int or 
> unsigned int to represent target word-sized values when it really ought 
> to be using int32_t or uint32_t.  I think that is a different issue than 
> the signedness bug, though.  So, OK to check this in?

Okay.

Kevin
  

Patch

commit 40a7589d1e2a0a8fe6a1891db7fcef156261937b
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Thu Aug 15 13:12:00 2019 -0700

    Fix bug in nios2 prologue analysis.
    
    The nios2 prologue analyzer was mistakenly using an unsigned int field
    to represent a 32-bit signed value.  This caused problems with an
    incorrect conversion being applied to negative values when they were
    automatically promoted for addition to a 64-bit CORE_ADDR value.
    
    This patch fixes test failures in gdb.base/large-frame.exp and
    gdb.dwarf2/dw2-ref-missing-frame.exp.  Normally the nios2 backend
    prefers to use the dwarf2 unwinder so the prologue analyzer is only
    invoked if there is no dwarf2 information.
    
    2019-08-15  Sandra Loosemore  <sandra@codesourcery.com>
    
    	gdb/
    	* nios2-tdep.c (struct reg_value): Improve comments.  Make
    	the offset field signed.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b6953b0..9775018 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-08-15  Sandra Loosemore  <sandra@codesourcery.com>
+
+	* nios2-tdep.c (struct reg_value): Improve comments.  Make
+	the offset field signed.
+
 2019-08-15  Tom de Vries  <tdevries@suse.de>
 
 	* linux-nat-trad.c: Include gdbarch.h.
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 2a21c33..8d685d4 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -58,13 +58,15 @@  static int nios2_debug = 0;
    analysis; see the reg_value and reg_saved tables in
    struct nios2_unwind_cache, respectively.  */
 
-/* struct reg_value is used to record that a register has the same value
-   as reg at the given offset from the start of a function.  */
+/* struct reg_value is used to record that a register has reg's initial
+   value at the start of a function plus the given constant offset.
+   If reg == 0, then the value is just the offset.
+   If reg < 0, then the value is unknown.  */
 
 struct reg_value
 {
   int reg;
-  unsigned int offset;
+  int offset;
 };
 
 /* struct reg_saved is used to record that a register value has been saved at