[5/8] Detect a frameless frame by comparing the FP register to -1.

Message ID 1465678115-58170-6-git-send-email-jhb@FreeBSD.org
State New, archived
Headers

Commit Message

John Baldwin June 11, 2016, 8:48 p.m. UTC
  Since CORE_ADDR is unsigned, the saved FP register is always greater than
or equal to zero.  Comparing to -1 results in a direct comparison with the
value used to initialize the saved FP register to an uninitialized state.
---
 gdb/ChangeLog   | 5 +++++
 gdb/sh64-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
  

Comments

Yao Qi June 13, 2016, 11:22 a.m. UTC | #1
On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
>
> -  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
> +  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
>      cache->uses_fp = 1;

I suspect it should be " != -1".  saved_regs[MEDIA_FP_REGNUM] is initialized
to -1, so if it is not the initialized value (-1), FP should be set in
the prologue, and mark the flag uses_fp.
  
John Baldwin June 14, 2016, 6:46 p.m. UTC | #2
On Monday, June 13, 2016 12:22:41 PM Yao Qi wrote:
> On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
> >
> > -  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
> > +  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
> >      cache->uses_fp = 1;
> 
> I suspect it should be " != -1".  saved_regs[MEDIA_FP_REGNUM] is initialized
> to -1, so if it is not the initialized value (-1), FP should be set in
> the prologue, and mark the flag uses_fp.

Yes, you are correct.  Alternatively, we could change this code to explicitly
set 'cache->uses_fp' in the cases earlier in this function where it sets
MEDIA_FP_REGNUM to a value.  sh-tdep.c follows this model, but it only needs
to set uses_fp in a few places.
  
Yao Qi June 15, 2016, 8:44 a.m. UTC | #3
On Tue, Jun 14, 2016 at 7:46 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Monday, June 13, 2016 12:22:41 PM Yao Qi wrote:
>> On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
>> >
>> > -  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
>> > +  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
>> >      cache->uses_fp = 1;
>>
>> I suspect it should be " != -1".  saved_regs[MEDIA_FP_REGNUM] is initialized
>> to -1, so if it is not the initialized value (-1), FP should be set in
>> the prologue, and mark the flag uses_fp.
>
> Yes, you are correct.  Alternatively, we could change this code to explicitly
> set 'cache->uses_fp' in the cases earlier in this function where it sets
> MEDIA_FP_REGNUM to a value.  sh-tdep.c follows this model, but it only needs
> to set uses_fp in a few places.
>

That is fine by me.
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 73f42e2..243b717 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@ 
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
 
+	* sh64-tdep.c (sh64_analyze_prologue): Compare FP register against
+	-1 to detect missing FP.
+
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
 	* score-tdep.c (score7_malloc_and_get_memblock): Remove check for
 	negative size.
 
diff --git a/gdb/sh64-tdep.c b/gdb/sh64-tdep.c
index e6b1e27..f5a6504 100644
--- a/gdb/sh64-tdep.c
+++ b/gdb/sh64-tdep.c
@@ -971,7 +971,7 @@  sh64_analyze_prologue (struct gdbarch *gdbarch,
 	}
     }
 
-  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
+  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
     cache->uses_fp = 1;
 }