[RFC] Fix MIPS frame prologue scan problem

[RFC] Fix MIPS frame prologue scan problem

[Pierre Muller]

  I am trying to extend the Free Pascal compiler to support
MIPS architecture.

  From what I read so far, register $s8 (register number 30) can be used as
a frame register,
but when I set $s8 to the value of the stack pointer ($sp, register number
29)
I get all my locals and parameter of functions wrong.

  I traced it down to the fact that GDB seems to use a
'virtual' frame pointer register called $fp,
but which is miscalculated in my case.

  In GCC generated code, $s8 register gets the same value as
$sp register, so that this problem does not show up in that case,
but for me, if I have a prologue that reserves 80 bytes, 
I will typically get 

  # Reserve 80 bytes for locals and area for called function parameters
  addi $sp,$sp,-80
  # Save $ra and $s8 registers, there could be others...
  sw    $ra,44($sp)
  sw   $s8,40($sp)
  # Set $s8 to function entry value of $sp
  addi $s8,$sp,80 

  Analysis of first instruction leads to setting of
frame_offset to 80.

  The problem is that when the last instruction
is analyzed by mips32_scan_prologue,
it switches the frame_reg from $sp to $s8,
but does not modify frame_offset value.
  This leads to a frame pointer $fp
being computed as $s8 + frame_offset
which is equal to $sp + 2*frame_offset.
  Thus all my locals are wrong :(

  Substraction of the constant in the last addi instruction (low_word)
to frame_offset seems to cure my problem.

 
 I tried to run a testsuite comparison and
I got a bunch of regression, but I have no idea if those
regression are relevant or a problem of stability of results...
See below if you understand those failures.


Comments are most welcome,


Pierre Muller


2012-06-11  Pierre Muller  <muller@ics.u-strasbg.fr>

        * mips-tdep.c (mpis32_scan_prologue): Fix value of frame_offset
        for ADDIU $s8,$sp,FrameSize.


Index: src/gdb/mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.556
diff -u -p -r1.556 mips-tdep.c
--- src/gdb/mips-tdep.c 6 Jun 2012 21:34:12 -0000       1.556
+++ src/gdb/mips-tdep.c 10 Jun 2012 23:43:44 -0000
@@ -3226,6 +3226,7 @@ restart:
                (this_frame, gdbarch_num_regs (gdbarch) + 30);

              alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
+             frame_offset -= low_word;
              if (alloca_adjust > 0)
                {
                   /* FP > SP + frame_size.  This may be because of

Regressions with my patch:

muller@gcc42:~/auto-test-gdb/state/patched/2012_06_13_10_34_41$ cat report
Calling update for "HEAD"
cleanup called
Calling git clean -d -x -f
Calling git reset --hard
cleanup called
Calling git clean -d -x -f
Calling git reset --hard
apply_patch called
cleanup called
Calling git clean -d -x -f
Calling git reset --hard
(cat /home/muller/auto-test-gdb/state/testing/patched/report
with your patch there are 23 regressions.
list of regressions with your patch:
gdb.sum gdb.base/checkpoint.exp: break2 10 one
gdb.sum gdb.base/checkpoint.exp: break2 2 one
gdb.sum gdb.base/checkpoint.exp: break2 3 one
gdb.sum gdb.base/checkpoint.exp: break2 4 one
gdb.sum gdb.base/checkpoint.exp: break2 5 one
gdb.sum gdb.base/checkpoint.exp: break2 6 one
gdb.sum gdb.base/checkpoint.exp: break2 7 one
gdb.sum gdb.base/checkpoint.exp: break2 8 one
gdb.sum gdb.base/checkpoint.exp: break2 9 one
gdb.sum gdb.base/checkpoint.exp: break4 one
gdb.sum gdb.base/checkpoint.exp: delete copy1
gdb.sum gdb.base/checkpoint.exp: outfile still open 1
gdb.sum gdb.base/checkpoint.exp: outfile still open 10
gdb.sum gdb.base/checkpoint.exp: outfile still open 2
gdb.sum gdb.base/checkpoint.exp: outfile still open 3
gdb.sum gdb.base/checkpoint.exp: outfile still open 4
gdb.sum gdb.base/checkpoint.exp: outfile still open 5
gdb.sum gdb.base/checkpoint.exp: outfile still open 6
gdb.sum gdb.base/checkpoint.exp: outfile still open 7
gdb.sum gdb.base/checkpoint.exp: outfile still open 8
gdb.sum gdb.base/checkpoint.exp: outfile still open 9
gdb.sum gdb.base/checkpoint.exp: restart 0 one
gdb.sum gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
after
 the second fork
tac)
(cat /home/muller/auto-test-gdb/state/testing/patched/gdb.sum.diff
1c1
< Test Run By muller on Wed Jun 13 12:04:38 2012
---
> Test Run By muller on Wed Jun 13 10:52:26 2012
2759,2760c2759,2760
< FAIL: gdb.base/checkpoint.exp: breakpoint 1 6 one (timeout)
< FAIL: gdb.base/checkpoint.exp: step in 6 two
---
> PASS: gdb.base/checkpoint.exp: breakpoint 1 6 one
> FAIL: gdb.base/checkpoint.exp: step in 6 two (timeout)
2767,2772c2767,2775
< PASS: gdb.base/checkpoint.exp: restart 0 one
< PASS: gdb.base/checkpoint.exp: break4 one
< PASS: gdb.base/checkpoint.exp: delete copy1
< PASS: gdb.base/checkpoint.exp: restart 1 three
< PASS: gdb.base/checkpoint.exp: break2 1 one
< PASS: gdb.base/checkpoint.exp: outfile still open 1
---
> FAIL: gdb.base/checkpoint.exp: restart 0 one
> FAIL: gdb.base/checkpoint.exp: setting breakpoint at 58
> FAIL: gdb.base/checkpoint.exp: break4 one
> FAIL: gdb.base/checkpoint.exp: delete copy1
> ERROR: breakpoints not deleted
> UNRESOLVED: gdb.base/checkpoint.exp: setting breakpoint at 53 (timeout)
> FAIL: gdb.base/checkpoint.exp: restart 1 three (got interactive prompt)
> FAIL: gdb.base/checkpoint.exp: break2 1 one (the program exited)
> FAIL: gdb.base/checkpoint.exp: outfile still open 1
2774,2775c2777,2778
< PASS: gdb.base/checkpoint.exp: break2 2 one
< PASS: gdb.base/checkpoint.exp: outfile still open 2
---
> FAIL: gdb.base/checkpoint.exp: break2 2 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 2
2777,2778c2780,2781
< PASS: gdb.base/checkpoint.exp: break2 3 one
< PASS: gdb.base/checkpoint.exp: outfile still open 3
---
> FAIL: gdb.base/checkpoint.exp: break2 3 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 3
2780,2781c2783,2784
< PASS: gdb.base/checkpoint.exp: break2 4 one
< PASS: gdb.base/checkpoint.exp: outfile still open 4
---
> FAIL: gdb.base/checkpoint.exp: break2 4 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 4
2783,2784c2786,2787
< PASS: gdb.base/checkpoint.exp: break2 5 one
< PASS: gdb.base/checkpoint.exp: outfile still open 5
---
> FAIL: gdb.base/checkpoint.exp: break2 5 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 5
2786,2787c2789,2790
< PASS: gdb.base/checkpoint.exp: break2 6 one
< PASS: gdb.base/checkpoint.exp: outfile still open 6
---
> FAIL: gdb.base/checkpoint.exp: break2 6 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 6
2789,2790c2792,2793
< PASS: gdb.base/checkpoint.exp: break2 7 one
< PASS: gdb.base/checkpoint.exp: outfile still open 7
---
> FAIL: gdb.base/checkpoint.exp: break2 7 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 7
2792,2793c2795,2796
< PASS: gdb.base/checkpoint.exp: break2 8 one
< PASS: gdb.base/checkpoint.exp: outfile still open 8
---
> FAIL: gdb.base/checkpoint.exp: break2 8 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 8
2795,2796c2798,2799
< PASS: gdb.base/checkpoint.exp: break2 9 one
< PASS: gdb.base/checkpoint.exp: outfile still open 9
---
> FAIL: gdb.base/checkpoint.exp: break2 9 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 9
2798,2799c2801,2802
< PASS: gdb.base/checkpoint.exp: break2 10 one
< PASS: gdb.base/checkpoint.exp: outfile still open 10
---
> FAIL: gdb.base/checkpoint.exp: break2 10 one
> FAIL: gdb.base/checkpoint.exp: outfile still open 10
5022,5023c5025,5026
< gdb compile failed, /tmp/ccv7IQuN.s: Assembler messages:
< /tmp/ccv7IQuN.s:7: Error: unrecognized symbol type "gnu_indirect_function"
---
> gdb compile failed, /tmp/cc3V0Vhb.s: Assembler messages:
> /tmp/cc3V0Vhb.s:7: Error: unrecognized symbol type "gnu_indirect_function"
8861,8867c8864,8870
< FAIL: gdb.base/sigaltstack.exp: finish from catch LEAF (the program
exited)
< FAIL: gdb.base/sigaltstack.exp: finish to throw INNER (the program is no
longe
r running)
< FAIL: gdb.base/sigaltstack.exp: finish to catch INNER (the program is no
longe
r running)
< FAIL: gdb.base/sigaltstack.exp: finish from catch INNER (the program is no
lon
ger running)
< FAIL: gdb.base/sigaltstack.exp: finish to OUTER (the program is no longer
runn
ing)
< FAIL: gdb.base/sigaltstack.exp: finish to catch MAIN (the program is no
longer
 running)
< FAIL: gdb.base/sigaltstack.exp: finish to MAIN (the program is no longer
runni
ng)
---
> PASS: gdb.base/sigaltstack.exp: finish from catch LEAF
> PASS: gdb.base/sigaltstack.exp: finish to throw INNER
> PASS: gdb.base/sigaltstack.exp: finish to catch INNER
> PASS: gdb.base/sigaltstack.exp: finish from catch INNER
> PASS: gdb.base/sigaltstack.exp: finish to OUTER
> PASS: gdb.base/sigaltstack.exp: finish to catch MAIN
> PASS: gdb.base/sigaltstack.exp: finish to MAIN
15571c15574
< FAIL: gdb.java/jprint.exp: unambiguous static call
---
> FAIL: gdb.java/jprint.exp: unambiguous static call (the program exited)
19245,19249c19248,19252
< FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: hardware
breakpo
ints work (timeout)
< FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoints
work

< FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
after
 the first fork
< FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoint
after
 the first fork (timeout)
< PASS: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
after
 the second fork
---
> PASS: gdb.threads/watchpoint-fork.exp: child: singlethreaded: hardware
breakpo
ints work
> PASS: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoints
work

> FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
after
 the first fork (timeout)
> FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoint
after
 the first fork
> FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
after
 the second fork
19265c19268
< FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: watchpoint A
afte
r the second fork (timeout)
---
> FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: watchpoint A
afte
r the second fork
19267c19270
< FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: finish
---
> FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: finish
(timeout)
19564,19565c19567,19568
< # of expected passes          17184
< # of unexpected failures      1033
---
> # of expected passes          17169
> # of unexpected failures      1049
19570c19573
< # of unresolved testcases     32
---
> # of unresolved testcases     33
tac)
FAILs with patched version in failed
FAILs with pristine version in pristine-failed
The files used for the validation of your patch are stored in
/home/muller/auto-
test-gdb/state/patched/2012_06_13_10_34_41 on the tester machine.

PING [RFC] Fix MIPS frame prologue scan problem

[Pierre Muller]

I got no reaction to this RFC...

Maybe Maciej or Daniel?

Pierre Muller


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : mercredi 13 juin 2012 14:20
> À : 'GDB Patches'
> Objet : [RFC] Fix MIPS frame prologue scan problem
> 
>   I am trying to extend the Free Pascal compiler to support
> MIPS architecture.
> 
>   From what I read so far, register $s8 (register number 30) can be used
as
> a frame register,
> but when I set $s8 to the value of the stack pointer ($sp, register number
> 29)
> I get all my locals and parameter of functions wrong.
> 
>   I traced it down to the fact that GDB seems to use a
> 'virtual' frame pointer register called $fp,
> but which is miscalculated in my case.
> 
>   In GCC generated code, $s8 register gets the same value as
> $sp register, so that this problem does not show up in that case,
> but for me, if I have a prologue that reserves 80 bytes,
> I will typically get
> 
>   # Reserve 80 bytes for locals and area for called function parameters
>   addi $sp,$sp,-80
>   # Save $ra and $s8 registers, there could be others...
>   sw    $ra,44($sp)
>   sw   $s8,40($sp)
>   # Set $s8 to function entry value of $sp
>   addi $s8,$sp,80
> 
>   Analysis of first instruction leads to setting of
> frame_offset to 80.
> 
>   The problem is that when the last instruction
> is analyzed by mips32_scan_prologue,
> it switches the frame_reg from $sp to $s8,
> but does not modify frame_offset value.
>   This leads to a frame pointer $fp
> being computed as $s8 + frame_offset
> which is equal to $sp + 2*frame_offset.
>   Thus all my locals are wrong :(
> 
>   Substraction of the constant in the last addi instruction (low_word)
> to frame_offset seems to cure my problem.
> 
> 
>  I tried to run a testsuite comparison and
> I got a bunch of regression, but I have no idea if those
> regression are relevant or a problem of stability of results...
> See below if you understand those failures.
> 
> 
> Comments are most welcome,
> 
> 
> Pierre Muller
> 
> 
> 2012-06-11  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
>         * mips-tdep.c (mpis32_scan_prologue): Fix value of frame_offset
>         for ADDIU $s8,$sp,FrameSize.
> 
> 
> Index: src/gdb/mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.556
> diff -u -p -r1.556 mips-tdep.c
> --- src/gdb/mips-tdep.c 6 Jun 2012 21:34:12 -0000       1.556
> +++ src/gdb/mips-tdep.c 10 Jun 2012 23:43:44 -0000
> @@ -3226,6 +3226,7 @@ restart:
>                 (this_frame, gdbarch_num_regs (gdbarch) + 30);
> 
>               alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
> +             frame_offset -= low_word;
>               if (alloca_adjust > 0)
>                 {
>                    /* FP > SP + frame_size.  This may be because of
> 
> Regressions with my patch:
> 
> muller@gcc42:~/auto-test-gdb/state/patched/2012_06_13_10_34_41$ cat report
> Calling update for "HEAD"
> cleanup called
> Calling git clean -d -x -f
> Calling git reset --hard
> cleanup called
> Calling git clean -d -x -f
> Calling git reset --hard
> apply_patch called
> cleanup called
> Calling git clean -d -x -f
> Calling git reset --hard
> (cat /home/muller/auto-test-gdb/state/testing/patched/report
> with your patch there are 23 regressions.
> list of regressions with your patch:
> gdb.sum gdb.base/checkpoint.exp: break2 10 one
> gdb.sum gdb.base/checkpoint.exp: break2 2 one
> gdb.sum gdb.base/checkpoint.exp: break2 3 one
> gdb.sum gdb.base/checkpoint.exp: break2 4 one
> gdb.sum gdb.base/checkpoint.exp: break2 5 one
> gdb.sum gdb.base/checkpoint.exp: break2 6 one
> gdb.sum gdb.base/checkpoint.exp: break2 7 one
> gdb.sum gdb.base/checkpoint.exp: break2 8 one
> gdb.sum gdb.base/checkpoint.exp: break2 9 one
> gdb.sum gdb.base/checkpoint.exp: break4 one
> gdb.sum gdb.base/checkpoint.exp: delete copy1
> gdb.sum gdb.base/checkpoint.exp: outfile still open 1
> gdb.sum gdb.base/checkpoint.exp: outfile still open 10
> gdb.sum gdb.base/checkpoint.exp: outfile still open 2
> gdb.sum gdb.base/checkpoint.exp: outfile still open 3
> gdb.sum gdb.base/checkpoint.exp: outfile still open 4
> gdb.sum gdb.base/checkpoint.exp: outfile still open 5
> gdb.sum gdb.base/checkpoint.exp: outfile still open 6
> gdb.sum gdb.base/checkpoint.exp: outfile still open 7
> gdb.sum gdb.base/checkpoint.exp: outfile still open 8
> gdb.sum gdb.base/checkpoint.exp: outfile still open 9
> gdb.sum gdb.base/checkpoint.exp: restart 0 one
> gdb.sum gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
> after
>  the second fork
> tac)
> (cat /home/muller/auto-test-gdb/state/testing/patched/gdb.sum.diff
> 1c1
> < Test Run By muller on Wed Jun 13 12:04:38 2012
> ---
> > Test Run By muller on Wed Jun 13 10:52:26 2012
> 2759,2760c2759,2760
> < FAIL: gdb.base/checkpoint.exp: breakpoint 1 6 one (timeout)
> < FAIL: gdb.base/checkpoint.exp: step in 6 two
> ---
> > PASS: gdb.base/checkpoint.exp: breakpoint 1 6 one
> > FAIL: gdb.base/checkpoint.exp: step in 6 two (timeout)
> 2767,2772c2767,2775
> < PASS: gdb.base/checkpoint.exp: restart 0 one
> < PASS: gdb.base/checkpoint.exp: break4 one
> < PASS: gdb.base/checkpoint.exp: delete copy1
> < PASS: gdb.base/checkpoint.exp: restart 1 three
> < PASS: gdb.base/checkpoint.exp: break2 1 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 1
> ---
> > FAIL: gdb.base/checkpoint.exp: restart 0 one
> > FAIL: gdb.base/checkpoint.exp: setting breakpoint at 58
> > FAIL: gdb.base/checkpoint.exp: break4 one
> > FAIL: gdb.base/checkpoint.exp: delete copy1
> > ERROR: breakpoints not deleted
> > UNRESOLVED: gdb.base/checkpoint.exp: setting breakpoint at 53 (timeout)
> > FAIL: gdb.base/checkpoint.exp: restart 1 three (got interactive prompt)
> > FAIL: gdb.base/checkpoint.exp: break2 1 one (the program exited)
> > FAIL: gdb.base/checkpoint.exp: outfile still open 1
> 2774,2775c2777,2778
> < PASS: gdb.base/checkpoint.exp: break2 2 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 2
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 2 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 2
> 2777,2778c2780,2781
> < PASS: gdb.base/checkpoint.exp: break2 3 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 3
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 3 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 3
> 2780,2781c2783,2784
> < PASS: gdb.base/checkpoint.exp: break2 4 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 4
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 4 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 4
> 2783,2784c2786,2787
> < PASS: gdb.base/checkpoint.exp: break2 5 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 5
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 5 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 5
> 2786,2787c2789,2790
> < PASS: gdb.base/checkpoint.exp: break2 6 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 6
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 6 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 6
> 2789,2790c2792,2793
> < PASS: gdb.base/checkpoint.exp: break2 7 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 7
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 7 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 7
> 2792,2793c2795,2796
> < PASS: gdb.base/checkpoint.exp: break2 8 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 8
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 8 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 8
> 2795,2796c2798,2799
> < PASS: gdb.base/checkpoint.exp: break2 9 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 9
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 9 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 9
> 2798,2799c2801,2802
> < PASS: gdb.base/checkpoint.exp: break2 10 one
> < PASS: gdb.base/checkpoint.exp: outfile still open 10
> ---
> > FAIL: gdb.base/checkpoint.exp: break2 10 one
> > FAIL: gdb.base/checkpoint.exp: outfile still open 10
> 5022,5023c5025,5026
> < gdb compile failed, /tmp/ccv7IQuN.s: Assembler messages:
> < /tmp/ccv7IQuN.s:7: Error: unrecognized symbol type
"gnu_indirect_function"
> ---
> > gdb compile failed, /tmp/cc3V0Vhb.s: Assembler messages:
> > /tmp/cc3V0Vhb.s:7: Error: unrecognized symbol type
"gnu_indirect_function"
> 8861,8867c8864,8870
> < FAIL: gdb.base/sigaltstack.exp: finish from catch LEAF (the program
> exited)
> < FAIL: gdb.base/sigaltstack.exp: finish to throw INNER (the program is no
> longe
> r running)
> < FAIL: gdb.base/sigaltstack.exp: finish to catch INNER (the program is no
> longe
> r running)
> < FAIL: gdb.base/sigaltstack.exp: finish from catch INNER (the program is
no
> lon
> ger running)
> < FAIL: gdb.base/sigaltstack.exp: finish to OUTER (the program is no
longer
> runn
> ing)
> < FAIL: gdb.base/sigaltstack.exp: finish to catch MAIN (the program is no
> longer
>  running)
> < FAIL: gdb.base/sigaltstack.exp: finish to MAIN (the program is no longer
> runni
> ng)
> ---
> > PASS: gdb.base/sigaltstack.exp: finish from catch LEAF
> > PASS: gdb.base/sigaltstack.exp: finish to throw INNER
> > PASS: gdb.base/sigaltstack.exp: finish to catch INNER
> > PASS: gdb.base/sigaltstack.exp: finish from catch INNER
> > PASS: gdb.base/sigaltstack.exp: finish to OUTER
> > PASS: gdb.base/sigaltstack.exp: finish to catch MAIN
> > PASS: gdb.base/sigaltstack.exp: finish to MAIN
> 15571c15574
> < FAIL: gdb.java/jprint.exp: unambiguous static call
> ---
> > FAIL: gdb.java/jprint.exp: unambiguous static call (the program exited)
> 19245,19249c19248,19252
> < FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: hardware
> breakpo
> ints work (timeout)
> < FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded:
watchpoints
> work
> 
> < FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
> after
>  the first fork
> < FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoint
> after
>  the first fork (timeout)
> < PASS: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
> after
>  the second fork
> ---
> > PASS: gdb.threads/watchpoint-fork.exp: child: singlethreaded: hardware
> breakpo
> ints work
> > PASS: gdb.threads/watchpoint-fork.exp: child: singlethreaded:
watchpoints
> work
> 
> > FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
> after
>  the first fork (timeout)
> > FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoint
> after
>  the first fork
> > FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint
> after
>  the second fork
> 19265c19268
> < FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: watchpoint
A
> afte
> r the second fork (timeout)
> ---
> > FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: watchpoint
A
> afte
> r the second fork
> 19267c19270
> < FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: finish
> ---
> > FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: finish
> (timeout)
> 19564,19565c19567,19568
> < # of expected passes          17184
> < # of unexpected failures      1033
> ---
> > # of expected passes          17169
> > # of unexpected failures      1049
> 19570c19573
> < # of unresolved testcases     32
> ---
> > # of unresolved testcases     33
> tac)
> FAILs with patched version in failed
> FAILs with pristine version in pristine-failed
> The files used for the validation of your patch are stored in
> /home/muller/auto-
> test-gdb/state/patched/2012_06_13_10_34_41 on the tester machine.

Re: [RFC] Fix MIPS frame prologue scan problem

[Maciej W. Rozycki]

Hi Pierre,

 Sorry about the delay, I've been swamped with stuff recently.

On Wed, 13 Jun 2012, Pierre Muller wrote:

>   I am trying to extend the Free Pascal compiler to support
> MIPS architecture.
> 
>   From what I read so far, register $s8 (register number 30) can be used as
> a frame register,
> but when I set $s8 to the value of the stack pointer ($sp, register number
> 29)
> I get all my locals and parameter of functions wrong.
> 
>   I traced it down to the fact that GDB seems to use a
> 'virtual' frame pointer register called $fp,
> but which is miscalculated in my case.
> 
>   In GCC generated code, $s8 register gets the same value as
> $sp register, so that this problem does not show up in that case,
> but for me, if I have a prologue that reserves 80 bytes, 
> I will typically get 
> 
>   # Reserve 80 bytes for locals and area for called function parameters
>   addi $sp,$sp,-80
>   # Save $ra and $s8 registers, there could be others...
>   sw    $ra,44($sp)
>   sw   $s8,40($sp)
>   # Set $s8 to function entry value of $sp
>   addi $s8,$sp,80 
> 
>   Analysis of first instruction leads to setting of
> frame_offset to 80.
> 
>   The problem is that when the last instruction
> is analyzed by mips32_scan_prologue,
> it switches the frame_reg from $sp to $s8,
> but does not modify frame_offset value.
>   This leads to a frame pointer $fp
> being computed as $s8 + frame_offset
> which is equal to $sp + 2*frame_offset.
>   Thus all my locals are wrong :(
> 
>   Substraction of the constant in the last addi instruction (low_word)
> to frame_offset seems to cure my problem.

 Well, to put it short, you're not supposed to do that if you want to 
follow the MIPS ABI.  The MIPS processor has no hardware stack and the 
software implementation of the stack has been made such that there is 
generally no need to arrange for a hard frame pointer (in a register 
separate from the stack pointer), except where dynamic stack allocation 
is used (alloca in C terms).

 Therefore the right place to look for how the hard frame pointer has been 
specified is the "Dynamic Allocation of Stack Space" section in Chapter 3 
"Machine Interface" of the MIPS psABI document:

"When a function requires dynamically allocated stack space it manifests a 
frame pointer on entry to the function.  The frame pointer is kept in a 
callee-saved register so that it is not changed across subsequent function 
calls.  Dynamic stack allocation requires the following steps.

 1. On function entry, the function adjusts the stack pointer by the size
    of the static stack frame.  The frame pointer is then set to this 
    initial sp value and is used for referencing the static elements 
    within the stack frame, performing the normal function of the stack 
    pointer."

 So in fact both GCC and GDB are correct, you're not supposed to add a 
constant to the stack pointer when calculating the value of the frame 
pointer -- it is supposed to hold the value of the stack pointer *after* 
the frame has been allocated (in other words any frame offsets are 
non-negative).  You need to adjust your code generated (BTW, note that the 
convention assumed by the ABI is to use non-trapping arithmetic; I'm 
assuming that you deliberately want to trap on overflows to detect the 
stack pointer crossing the user/kernel segment boundary, right?).

 NB I suggest that you get real debug information generated as well; it 
can be stabs if DWARF-2 is too difficult to start with.  The heuristic 
unwinder is really the last-chance attempt made by GDB to find its way 
around, can only be relied on when applied to conservative code and is 
best avoided if possible.

 I hope this helps, good luck with your port!

  Maciej

RE: [RFC] Fix MIPS frame prologue scan problem

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Maciej W. Rozycki
> Envoyé : vendredi 22 juin 2012 01:12
> À : Pierre Muller
> Cc : 'GDB Patches'
> Objet : Re: [RFC] Fix MIPS frame prologue scan problem
> 
> Hi Pierre,
> 
>  Sorry about the delay, I've been swamped with stuff recently.
 Thanks for your reply.
 
> On Wed, 13 Jun 2012, Pierre Muller wrote:
> 
> >   I am trying to extend the Free Pascal compiler to support
> > MIPS architecture.
> >
> >   From what I read so far, register $s8 (register number 30) can be used
> as
> > a frame register,
> > but when I set $s8 to the value of the stack pointer ($sp, register
number
> > 29)
> > I get all my locals and parameter of functions wrong.
> >
> >   I traced it down to the fact that GDB seems to use a
> > 'virtual' frame pointer register called $fp,
> > but which is miscalculated in my case.
> >
> >   In GCC generated code, $s8 register gets the same value as
> > $sp register, so that this problem does not show up in that case,
> > but for me, if I have a prologue that reserves 80 bytes,
> > I will typically get
> >
> >   # Reserve 80 bytes for locals and area for called function parameters
> >   addi $sp,$sp,-80
> >   # Save $ra and $s8 registers, there could be others...
> >   sw    $ra,44($sp)
> >   sw   $s8,40($sp)
> >   # Set $s8 to function entry value of $sp
> >   addi $s8,$sp,80
> >
> >   Analysis of first instruction leads to setting of
> > frame_offset to 80.
> >
> >   The problem is that when the last instruction
> > is analyzed by mips32_scan_prologue,
> > it switches the frame_reg from $sp to $s8,
> > but does not modify frame_offset value.
> >   This leads to a frame pointer $fp
> > being computed as $s8 + frame_offset
> > which is equal to $sp + 2*frame_offset.
> >   Thus all my locals are wrong :(
> >
> >   Substraction of the constant in the last addi instruction (low_word)
> > to frame_offset seems to cure my problem.
> 
>  Well, to put it short, you're not supposed to do that if you want to
> follow the MIPS ABI.  The MIPS processor has no hardware stack and the
> software implementation of the stack has been made such that there is
> generally no need to arrange for a hard frame pointer (in a register
> separate from the stack pointer), except where dynamic stack allocation
> is used (alloca in C terms).

  I tried to read several MIPS documents,
and the message was not that clear to me...
 
>  Therefore the right place to look for how the hard frame pointer has been
> specified is the "Dynamic Allocation of Stack Space" section in Chapter 3
> "Machine Interface" of the MIPS psABI document:
> 
> "When a function requires dynamically allocated stack space it manifests a
> frame pointer on entry to the function.  The frame pointer is kept in a
> callee-saved register so that it is not changed across subsequent function
> calls.  Dynamic stack allocation requires the following steps.
> 
>  1. On function entry, the function adjusts the stack pointer by the size
>     of the static stack frame.  The frame pointer is then set to this
>     initial sp value and is used for referencing the static elements
>     within the stack frame, performing the normal function of the stack
>     pointer."
> 
>  So in fact both GCC and GDB are correct, you're not supposed to add a
> constant to the stack pointer when calculating the value of the frame
> pointer -- it is supposed to hold the value of the stack pointer *after*
> the frame has been allocated (in other words any frame offsets are
> non-negative).

  Our current problem is that we don't yet knoow the
stacksize that we need for the function while we generate
its code, so that using a frame pointer at previous value of stack pointer
makes this
really easier for now.

>  You need to adjust your code generated (BTW, note that the
> convention assumed by the ABI is to use non-trapping arithmetic; I'm
Is this the difference between
  ADDI and ADDIU?
I thought it was only a signed/unsigned difference,
Do that mean that you never generate any exception if you use the U version?
I am really new to MIPS assembly...

> assuming that you deliberately want to trap on overflows to detect the
> stack pointer crossing the user/kernel segment boundary, right?).
  Not really as explained above ...
 
>  NB I suggest that you get real debug information generated as well; it
> can be stabs if DWARF-2 is too difficult to start with.  The heuristic
> unwinder is really the last-chance attempt made by GDB to find its way
> around, can only be relied on when applied to conservative code and is
> best avoided if possible.

  But my problem is really that
GDB found my I do generate stabs debugging information,
and give parameters and locals 
offsets relative to frame pointer.

But in mips32_scan_prologue,
the first
  ADDI $s8,$sp,LocalSize
instruction,
  interpreted it in mips32_scan_prologue function
but ended up with a wrong position of my
non-ABI standard frame pointer
because it changed frame pointer register from sp to s8 register,
but kept frame_offset value as set by the
  SUBI $sp, $sp, LocalSize 
instruction
  analyzed before.

  Thus GDB wrongly ends up with a
frame pointer located a
  value of $s8 register (as from ADDI instruction analysis)
+ LocalSize (from SUBI instruction)

  This means that of
  $sp is say at address addr
  $s8  is at addr +LocalSize
and the virtual frame pointer 
  $fp at  $s8 + LocalSize = addr + 2 * LocalSize


  This means that it would be better to remove
analysis of the ADDI $s8, $sp, LocalSize
than to leave the current behavior.

  I think that we should either use my proposed patch,
or completely remove the analysis of this ADDI $s8, $sp, LocalSize...
  
 
>  I hope this helps, good luck with your port!
> 
>   Maciej

  Thanks,

Pierre

PING [RFC] Fix MIPS frame prologue scan problem

[Pierre Muller]

    Hi Marciej,

  I didn't get any answer on the question I raised 
on the bottom of my answer to your comments.

  I just summarize here:
1) It's true that my pot might not be ABI complaint, but I saw
several other assembly code that use the same approach (it might
be really old assembler)
2) The current code in mips-tdep.c should either 
handle the 
  add $s8, $sp, LOCALSIZE
in a way similar to what I propose or
otherwise at least complain about non-ABI code, no?

Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : vendredi 22 juin 2012 08:53
> À : 'Maciej W. Rozycki'
> Cc : 'GDB Patches'
> Objet : RE: [RFC] Fix MIPS frame prologue scan problem
> 
> 
> 
> > -----Message d'origine-----
> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] De la part de Maciej W. Rozycki
> > Envoyé : vendredi 22 juin 2012 01:12
> > À : Pierre Muller
> > Cc : 'GDB Patches'
> > Objet : Re: [RFC] Fix MIPS frame prologue scan problem
> >
> > Hi Pierre,
> >
> >  Sorry about the delay, I've been swamped with stuff recently.
>  Thanks for your reply.
> 
> > On Wed, 13 Jun 2012, Pierre Muller wrote:
> >
> > >   I am trying to extend the Free Pascal compiler to support
> > > MIPS architecture.
> > >
> > >   From what I read so far, register $s8 (register number 30) can be
used
> > as
> > > a frame register,
> > > but when I set $s8 to the value of the stack pointer ($sp, register
> number
> > > 29)
> > > I get all my locals and parameter of functions wrong.
> > >
> > >   I traced it down to the fact that GDB seems to use a
> > > 'virtual' frame pointer register called $fp,
> > > but which is miscalculated in my case.
> > >
> > >   In GCC generated code, $s8 register gets the same value as
> > > $sp register, so that this problem does not show up in that case,
> > > but for me, if I have a prologue that reserves 80 bytes,
> > > I will typically get
> > >
> > >   # Reserve 80 bytes for locals and area for called function
parameters
> > >   addi $sp,$sp,-80
> > >   # Save $ra and $s8 registers, there could be others...
> > >   sw    $ra,44($sp)
> > >   sw   $s8,40($sp)
> > >   # Set $s8 to function entry value of $sp
> > >   addi $s8,$sp,80
> > >
> > >   Analysis of first instruction leads to setting of
> > > frame_offset to 80.
> > >
> > >   The problem is that when the last instruction
> > > is analyzed by mips32_scan_prologue,
> > > it switches the frame_reg from $sp to $s8,
> > > but does not modify frame_offset value.
> > >   This leads to a frame pointer $fp
> > > being computed as $s8 + frame_offset
> > > which is equal to $sp + 2*frame_offset.
> > >   Thus all my locals are wrong :(
> > >
> > >   Substraction of the constant in the last addi instruction (low_word)
> > > to frame_offset seems to cure my problem.
> >
> >  Well, to put it short, you're not supposed to do that if you want to
> > follow the MIPS ABI.  The MIPS processor has no hardware stack and the
> > software implementation of the stack has been made such that there is
> > generally no need to arrange for a hard frame pointer (in a register
> > separate from the stack pointer), except where dynamic stack allocation
> > is used (alloca in C terms).
> 
>   I tried to read several MIPS documents,
> and the message was not that clear to me...
> 
> >  Therefore the right place to look for how the hard frame pointer has
been
> > specified is the "Dynamic Allocation of Stack Space" section in Chapter
3
> > "Machine Interface" of the MIPS psABI document:
> >
> > "When a function requires dynamically allocated stack space it manifests
a
> > frame pointer on entry to the function.  The frame pointer is kept in a
> > callee-saved register so that it is not changed across subsequent
function
> > calls.  Dynamic stack allocation requires the following steps.
> >
> >  1. On function entry, the function adjusts the stack pointer by the
size
> >     of the static stack frame.  The frame pointer is then set to this
> >     initial sp value and is used for referencing the static elements
> >     within the stack frame, performing the normal function of the stack
> >     pointer."
> >
> >  So in fact both GCC and GDB are correct, you're not supposed to add a
> > constant to the stack pointer when calculating the value of the frame
> > pointer -- it is supposed to hold the value of the stack pointer *after*
> > the frame has been allocated (in other words any frame offsets are
> > non-negative).
> 
>   Our current problem is that we don't yet knoow the
> stacksize that we need for the function while we generate
> its code, so that using a frame pointer at previous value of stack pointer
> makes this
> really easier for now.
> 
> >  You need to adjust your code generated (BTW, note that the
> > convention assumed by the ABI is to use non-trapping arithmetic; I'm
> Is this the difference between
>   ADDI and ADDIU?
> I thought it was only a signed/unsigned difference,
> Do that mean that you never generate any exception if you use the U
version?
> I am really new to MIPS assembly...
> 
> > assuming that you deliberately want to trap on overflows to detect the
> > stack pointer crossing the user/kernel segment boundary, right?).
>   Not really as explained above ...
> 
> >  NB I suggest that you get real debug information generated as well; it
> > can be stabs if DWARF-2 is too difficult to start with.  The heuristic
> > unwinder is really the last-chance attempt made by GDB to find its way
> > around, can only be relied on when applied to conservative code and is
> > best avoided if possible.
> 
>   But my problem is really that
> GDB found my I do generate stabs debugging information,
> and give parameters and locals
> offsets relative to frame pointer.
> 
> But in mips32_scan_prologue,
> the first
>   ADDI $s8,$sp,LocalSize
> instruction,
>   interpreted it in mips32_scan_prologue function
> but ended up with a wrong position of my
> non-ABI standard frame pointer
> because it changed frame pointer register from sp to s8 register,
> but kept frame_offset value as set by the
>   SUBI $sp, $sp, LocalSize
> instruction
>   analyzed before.
> 
>   Thus GDB wrongly ends up with a
> frame pointer located a
>   value of $s8 register (as from ADDI instruction analysis)
> + LocalSize (from SUBI instruction)
> 
>   This means that of
>   $sp is say at address addr
>   $s8  is at addr +LocalSize
> and the virtual frame pointer
>   $fp at  $s8 + LocalSize = addr + 2 * LocalSize
> 
> 
>   This means that it would be better to remove
> analysis of the ADDI $s8, $sp, LocalSize
> than to leave the current behavior.
> 
>   I think that we should either use my proposed patch,
> or completely remove the analysis of this ADDI $s8, $sp, LocalSize...
> 
> 
> >  I hope this helps, good luck with your port!
> >
> >   Maciej
> 
>   Thanks,
> 
> Pierre

Re: PING [RFC] Fix MIPS frame prologue scan problem

[Maciej W. Rozycki]

Pierre,

>   I didn't get any answer on the question I raised 
> on the bottom of my answer to your comments.
> 
>   I just summarize here:
> 1) It's true that my pot might not be ABI complaint, but I saw
> several other assembly code that use the same approach (it might
> be really old assembler)
> 2) The current code in mips-tdep.c should either 
> handle the 
>   add $s8, $sp, LOCALSIZE
> in a way similar to what I propose or
> otherwise at least complain about non-ABI code, no?

 Thanks for the reminder and apologies to have lost your case.  See 
further comments below.

> > > >   I am trying to extend the Free Pascal compiler to support
> > > > MIPS architecture.
> > > >
> > > >   From what I read so far, register $s8 (register number 30) can be
> used
> > > as
> > > > a frame register,
> > > > but when I set $s8 to the value of the stack pointer ($sp, register
> > number
> > > > 29)
> > > > I get all my locals and parameter of functions wrong.
> > > >
> > > >   I traced it down to the fact that GDB seems to use a
> > > > 'virtual' frame pointer register called $fp,
> > > > but which is miscalculated in my case.
> > > >
> > > >   In GCC generated code, $s8 register gets the same value as
> > > > $sp register, so that this problem does not show up in that case,
> > > > but for me, if I have a prologue that reserves 80 bytes,
> > > > I will typically get
> > > >
> > > >   # Reserve 80 bytes for locals and area for called function
> parameters
> > > >   addi $sp,$sp,-80
> > > >   # Save $ra and $s8 registers, there could be others...
> > > >   sw    $ra,44($sp)
> > > >   sw   $s8,40($sp)
> > > >   # Set $s8 to function entry value of $sp
> > > >   addi $s8,$sp,80
> > > >
> > > >   Analysis of first instruction leads to setting of
> > > > frame_offset to 80.
> > > >
> > > >   The problem is that when the last instruction
> > > > is analyzed by mips32_scan_prologue,
> > > > it switches the frame_reg from $sp to $s8,
> > > > but does not modify frame_offset value.
> > > >   This leads to a frame pointer $fp
> > > > being computed as $s8 + frame_offset
> > > > which is equal to $sp + 2*frame_offset.
> > > >   Thus all my locals are wrong :(
> > > >
> > > >   Substraction of the constant in the last addi instruction (low_word)
> > > > to frame_offset seems to cure my problem.
> > >
> > >  Well, to put it short, you're not supposed to do that if you want to
> > > follow the MIPS ABI.  The MIPS processor has no hardware stack and the
> > > software implementation of the stack has been made such that there is
> > > generally no need to arrange for a hard frame pointer (in a register
> > > separate from the stack pointer), except where dynamic stack allocation
> > > is used (alloca in C terms).
> > 
> >   I tried to read several MIPS documents,
> > and the message was not that clear to me...

 I agree documentation appears ambiguous, for example SGI's "MIPSpro 
Assembly Language Programmer's Guide" (doc #007-2418-004) refers to the 
"virtual frame pointer ($fp)" as the incoming value of $sp.  However I 
still think the MIPS psABI document referred below is the normative 
standard.

> > >  Therefore the right place to look for how the hard frame pointer has
> been
> > > specified is the "Dynamic Allocation of Stack Space" section in Chapter
> 3
> > > "Machine Interface" of the MIPS psABI document:
> > >
> > > "When a function requires dynamically allocated stack space it manifests
> a
> > > frame pointer on entry to the function.  The frame pointer is kept in a
> > > callee-saved register so that it is not changed across subsequent
> function
> > > calls.  Dynamic stack allocation requires the following steps.
> > >
> > >  1. On function entry, the function adjusts the stack pointer by the
> size
> > >     of the static stack frame.  The frame pointer is then set to this
> > >     initial sp value and is used for referencing the static elements
> > >     within the stack frame, performing the normal function of the stack
> > >     pointer."
> > >
> > >  So in fact both GCC and GDB are correct, you're not supposed to add a
> > > constant to the stack pointer when calculating the value of the frame
> > > pointer -- it is supposed to hold the value of the stack pointer *after*
> > > the frame has been allocated (in other words any frame offsets are
> > > non-negative).
> > 
> >   Our current problem is that we don't yet knoow the
> > stacksize that we need for the function while we generate
> > its code, so that using a frame pointer at previous value of stack pointer
> > makes this
> > really easier for now.

 NB the use of the frame pointer at the incoming value of $sp does not 
seem to help you as the local stack variables are meant to be allocated 
first (toward the bottom of the stack), followed by the incoming register 
save area and finally the argument build area.  See Figure 3-21 in the 
MIPS psABI document.  And you need to know the offset from the frame 
pointer register to save the incoming registers that happens first in the 
function prologue.

 However this simple program:

$ cat alloca.c
#include <alloca.h>

extern int bar(void *, int *);

int foo(unsigned int i)
{
	int v;

	return bar(alloca(i), &v);
}
$

is compiled to this by GCC (I've stripped some less relevant bits for 
brevity):

	.text
	.align	2
	.globl	foo
	.ent	foo
	.type	foo, @function
foo:
	.frame	$fp,40,$31		# vars= 8, regs= 2/0, args= 16, gp= 8
	.mask	0xc0000000,-4
	.fmask	0x00000000,0
	.set	noreorder
	.set	nomacro
	addiu	$4,$4,14
	srl	$2,$4,3
	addiu	$sp,$sp,-40
	sll	$2,$2,3
	sw	$31,36($sp)
	sw	$fp,32($sp)
	move	$fp,$sp
	subu	$sp,$sp,$2
	addiu	$4,$sp,16
	jal	bar
	addiu	$5,$fp,24

	move	$sp,$fp
	lw	$31,36($sp)
	lw	$fp,32($sp)
	j	$31
	addiu	$sp,$sp,40

	.set	macro
	.set	reorder
	.end	foo
	.size	foo, .-foo

so this does not really follow the standard referred above, as v (at $fp + 
24) is clearly allocated farther from the bottom of the stack than the 
incoming register save area (from $fp + 32 on).

 And then the stack layout used in code above is hardwired in the 
implementation of the MIPS16 SAVE instruction, so I think this means 
you're essentially free to pick your choice.

> > >  You need to adjust your code generated (BTW, note that the
> > > convention assumed by the ABI is to use non-trapping arithmetic; I'm
> > Is this the difference between
> >   ADDI and ADDIU?
> > I thought it was only a signed/unsigned difference,
> > Do that mean that you never generate any exception if you use the U
> version?

 Correct, the use of signed/unsigned reference in the context of additive 
operations (ADDI/ADD/SUB and likewise the doubleword variants) is a bit of 
a misnomer.  The only difference between each pair of signed/unsigned 
operations is that the former takes an overflow exception on a signed 
overflow and the latter does not.  The result computed is otherwise the 
same for both operations in each pair.

> > I am really new to MIPS assembly...

 Well, I suggest a MIPS assembly book first then, getting a grasp on the 
dialect purely from technical specs may be quite a challenge.  Especially 
for such a long-lived architecure with its all twists and quirks 
accumulated over the years.  Dominic Sweetman's "See MIPS Run" (ISBN: 
0-12-088421-6) is one I can recommend, and it covers much more than lone 
MIPS assembly programming.

 Although for some reason the hard frame pointer usage consideration in 
Section 11.2.9 there also builds on the approach outlined in the SGI 
document referred above.  I don't know why, I have never seen a piece of 
code doing anything like this and there is no hard example of such code in 
either text either, although there are many assembly pieces using $sp only 
quoted.

> > > assuming that you deliberately want to trap on overflows to detect the
> > > stack pointer crossing the user/kernel segment boundary, right?).
> >   Not really as explained above ...

 Ack.

> > >  NB I suggest that you get real debug information generated as well; it
> > > can be stabs if DWARF-2 is too difficult to start with.  The heuristic
> > > unwinder is really the last-chance attempt made by GDB to find its way
> > > around, can only be relied on when applied to conservative code and is
> > > best avoided if possible.
> > 
> >   But my problem is really that
> > GDB found my I do generate stabs debugging information,
> > and give parameters and locals
> > offsets relative to frame pointer.

 I'm not sure what you mean -- do you want to say that with stabs debug 
information GDB still uses the heuristic unwinders to examine function 
prologues?  Hmm, I have been to short into MIPS debug internals to have 
ever had a need to use stabs, for me DWARF-2 support has been there since 
forever.  But I realise it may be easier for you to start with this 
simpler debugging format and it's a bit unfortunate that it's not capable 
enough to get away without the heuristic unwinders, as they are overall 
quite fragile.

> > But in mips32_scan_prologue,
> > the first
> >   ADDI $s8,$sp,LocalSize
> > instruction,
> >   interpreted it in mips32_scan_prologue function
> > but ended up with a wrong position of my
> > non-ABI standard frame pointer
> > because it changed frame pointer register from sp to s8 register,
> > but kept frame_offset value as set by the
> >   SUBI $sp, $sp, LocalSize
> > instruction
> >   analyzed before.
> > 
> >   Thus GDB wrongly ends up with a
> > frame pointer located a
> >   value of $s8 register (as from ADDI instruction analysis)
> > + LocalSize (from SUBI instruction)
> > 
> >   This means that of
> >   $sp is say at address addr
> >   $s8  is at addr +LocalSize
> > and the virtual frame pointer
> >   $fp at  $s8 + LocalSize = addr + 2 * LocalSize
> > 
> > 
> >   This means that it would be better to remove
> > analysis of the ADDI $s8, $sp, LocalSize
> > than to leave the current behavior.
> > 
> >   I think that we should either use my proposed patch,
> > or completely remove the analysis of this ADDI $s8, $sp, LocalSize...

 Given the ambiguities noted above and following the principle of being 
liberal as to what to accept I agree your proposal makes sense.  I am 
fairly sure though that in the presence of a hard frame pointer ($fp) it 
is that value we should rely on as it's there for a reason.

 I'll get back to you shortly, sorry for the delay and thanks for 
persistence.

  Maciej

[committed] Fix MIPS frame prologue scan problem

[Maciej W. Rozycki]

On Mon, 26 Nov 2012, Maciej W. Rozycki wrote:

> > > But in mips32_scan_prologue,
> > > the first
> > >   ADDI $s8,$sp,LocalSize
> > > instruction,
> > >   interpreted it in mips32_scan_prologue function
> > > but ended up with a wrong position of my
> > > non-ABI standard frame pointer
> > > because it changed frame pointer register from sp to s8 register,
> > > but kept frame_offset value as set by the
> > >   SUBI $sp, $sp, LocalSize
> > > instruction
> > >   analyzed before.
> > > 
> > >   Thus GDB wrongly ends up with a
> > > frame pointer located a
> > >   value of $s8 register (as from ADDI instruction analysis)
> > > + LocalSize (from SUBI instruction)
> > > 
> > >   This means that of
> > >   $sp is say at address addr
> > >   $s8  is at addr +LocalSize
> > > and the virtual frame pointer
> > >   $fp at  $s8 + LocalSize = addr + 2 * LocalSize
> > > 
> > > 
> > >   This means that it would be better to remove
> > > analysis of the ADDI $s8, $sp, LocalSize
> > > than to leave the current behavior.
> > > 
> > >   I think that we should either use my proposed patch,
> > > or completely remove the analysis of this ADDI $s8, $sp, LocalSize...
> 
>  Given the ambiguities noted above and following the principle of being 
> liberal as to what to accept I agree your proposal makes sense.  I am 
> fairly sure though that in the presence of a hard frame pointer ($fp) it 
> is that value we should rely on as it's there for a reason.

 Thanks for your patience.  I have now looked through it in depth and your 
observation is actually a regression introduced sometime between 4.16 and 
4.17, probably when the function was rewritten when MIPS16 support was 
added sometime in 1997 and the original heuristic_proc_desc code forked 
into mips32_heuristic_proc_desc and mips16_heuristic_proc_desc, which is 
how the respective functions were called back then.

 Before that regression frame_offset was unconditionally forced to zero 
whenever $fp was used to hold the virtual frame pointer -- the only use 
case supported for $fp back then.  Original code read:

    if (has_frame_reg) {
	PROC_FRAME_REG(&temp_proc_desc) = 30;
	PROC_FRAME_OFFSET(&temp_proc_desc) = 0;
    }
    else {
	PROC_FRAME_REG(&temp_proc_desc) = SP_REGNUM;
	PROC_FRAME_OFFSET(&temp_proc_desc) = frame_size;
    }

After the problematic change the PROC_FRAME_OFFSET(&temp_proc_desc) = 0 
assignment was lost.

 I have therefore applied the change below; no change is needed for MIPS16 
or microMIPS analysers as they use different code -- with frame_adjust 
holding the extra frame adjustment in the corresponding case (it may make 
sense to make all the three pieces more uniform, but that's another 
matter).

 This is not a code path that is ever used with modern GCC or it would 
have likely been spotted much earlier than 15 years after the regression 
and it is therefore not really covered by the test suite.  I have run it 
regardless, for the MIPS/Linux target and a couple multilibs, just for the 
peace of mind.

 Again, thanks for your patience and apologies to take so long.

  Maciej

2013-02-24  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/
	* mips-tdep.c (mips32_scan_prologue): Reset frame_offset to zero
	if $fp is used as the virtual frame pointer.

Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c	2013-02-24 00:29:58.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c	2013-02-24 01:34:10.834052078 +0000
@@ -3316,6 +3316,7 @@ mips32_scan_prologue (struct gdbarch *gd
 	      frame_reg = 30;
 	      frame_addr = get_frame_register_signed
 		(this_frame, gdbarch_num_regs (gdbarch) + 30);
+	      frame_offset = 0;
 
 	      alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
 	      if (alloca_adjust > 0)

RE: [committed] Fix MIPS frame prologue scan problem

[Pierre Muller]

Thank you so much for fixing this issue!

I can confirm that after your patch, I can use 
unmodified GDB CVS tree to generate a GDB executable
that is able to correctly handle the prologue generated by the
free pascal compiler which uses

Dump of assembler code for function TOPTION__INTERPRET_OPTION:
   0x005ef624 <+0>:     addiu   sp,sp,-1928
   0x005ef628 <+4>:     sw      ra,212(sp)
   0x005ef62c <+8>:     sw      s8,208(sp)
   0x005ef630 <+12>:    addiu   s8,sp,1928

  The value of $fp is equal to $s8
after your patch, which makes it undistinguishable
from the patch I proposed...


I did not find any gdb_7_6-branch yet, which probably means
that this will be in 7.6 release, no?

  Let me end by thanking you again  


Pierre Muller
as Free Pascal core developer


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Maciej W. Rozycki
> Envoyé : dimanche 24 février 2013 13:55
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : [committed] Fix MIPS frame prologue scan problem
> 
> On Mon, 26 Nov 2012, Maciej W. Rozycki wrote:
> 
> > > > But in mips32_scan_prologue,
> > > > the first
> > > >   ADDI $s8,$sp,LocalSize
> > > > instruction,
> > > >   interpreted it in mips32_scan_prologue function
> > > > but ended up with a wrong position of my
> > > > non-ABI standard frame pointer
> > > > because it changed frame pointer register from sp to s8 register,
> > > > but kept frame_offset value as set by the
> > > >   SUBI $sp, $sp, LocalSize
> > > > instruction
> > > >   analyzed before.
> > > >
> > > >   Thus GDB wrongly ends up with a
> > > > frame pointer located a
> > > >   value of $s8 register (as from ADDI instruction analysis)
> > > > + LocalSize (from SUBI instruction)
> > > >
> > > >   This means that of
> > > >   $sp is say at address addr
> > > >   $s8  is at addr +LocalSize
> > > > and the virtual frame pointer
> > > >   $fp at  $s8 + LocalSize = addr + 2 * LocalSize
> > > >
> > > >
> > > >   This means that it would be better to remove
> > > > analysis of the ADDI $s8, $sp, LocalSize
> > > > than to leave the current behavior.
> > > >
> > > >   I think that we should either use my proposed patch,
> > > > or completely remove the analysis of this ADDI $s8, $sp,
LocalSize...
> >
> >  Given the ambiguities noted above and following the principle of being
> > liberal as to what to accept I agree your proposal makes sense.  I am
> > fairly sure though that in the presence of a hard frame pointer ($fp) it
> > is that value we should rely on as it's there for a reason.
> 
>  Thanks for your patience.  I have now looked through it in depth and your
> observation is actually a regression introduced sometime between 4.16 and
> 4.17, probably when the function was rewritten when MIPS16 support was
> added sometime in 1997 and the original heuristic_proc_desc code forked
> into mips32_heuristic_proc_desc and mips16_heuristic_proc_desc, which is
> how the respective functions were called back then.
> 
>  Before that regression frame_offset was unconditionally forced to zero
> whenever $fp was used to hold the virtual frame pointer -- the only use
> case supported for $fp back then.  Original code read:
> 
>     if (has_frame_reg) {
> 	PROC_FRAME_REG(&temp_proc_desc) = 30;
> 	PROC_FRAME_OFFSET(&temp_proc_desc) = 0;
>     }
>     else {
> 	PROC_FRAME_REG(&temp_proc_desc) = SP_REGNUM;
> 	PROC_FRAME_OFFSET(&temp_proc_desc) = frame_size;
>     }
> 
> After the problematic change the PROC_FRAME_OFFSET(&temp_proc_desc) = 0
> assignment was lost.
> 
>  I have therefore applied the change below; no change is needed for MIPS16
> or microMIPS analysers as they use different code -- with frame_adjust
> holding the extra frame adjustment in the corresponding case (it may make
> sense to make all the three pieces more uniform, but that's another
> matter).
> 
>  This is not a code path that is ever used with modern GCC or it would
> have likely been spotted much earlier than 15 years after the regression
> and it is therefore not really covered by the test suite.  I have run it
> regardless, for the MIPS/Linux target and a couple multilibs, just for the
> peace of mind.
> 
>  Again, thanks for your patience and apologies to take so long.
> 
>   Maciej
> 
> 2013-02-24  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	gdb/
> 	* mips-tdep.c (mips32_scan_prologue): Reset frame_offset to zero
> 	if $fp is used as the virtual frame pointer.
> 
> Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c	2013-02-24
00:29:58.000000000
> +0000
> +++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c	2013-02-24
01:34:10.834052078
> +0000
> @@ -3316,6 +3316,7 @@ mips32_scan_prologue (struct gdbarch *gd
>  	      frame_reg = 30;
>  	      frame_addr = get_frame_register_signed
>  		(this_frame, gdbarch_num_regs (gdbarch) + 30);
> +	      frame_offset = 0;
> 
>  	      alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
>  	      if (alloca_adjust > 0)

RE: [committed] Fix MIPS frame prologue scan problem

[Maciej W. Rozycki]

On Sun, 24 Feb 2013, Pierre Muller wrote:

> I did not find any gdb_7_6-branch yet, which probably means
> that this will be in 7.6 release, no?

 Last Monday Joel wrote he would not be able to branch 7.6 off before 
today.  I had this in mind while working on this issue as I promised you 
to get it there.

  Maciej

Re: [committed] Fix MIPS frame prologue scan problem

[Pedro Alves]

On 02/24/2013 09:51 PM, Pierre Muller wrote:

> I did not find any gdb_7_6-branch yet, which probably means
> that this will be in 7.6 release, no?

Right, 7.6 has not branched yet.

-- 
Pedro Alves