This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Fix variable lifetime related problem in gdb.base/store.exp
On Wed, 19 Jan 2011 23:17:35 -0800
Doug Evans <dje@google.com> wrote:
> The worry of course is that by changing the test to use r instead of l
> are we breaking the intent of the test, at least on *some* target.
> [since r has to survive the call gcc may be less inclined to put it in
> a register]
This is a valid concern. I have no ready answer.
> OTOH, does up_set really provide any more coverage, as far as the
> intent of the test (setting values that are in registers), than
> check_set. I don't know.
I think, perhaps, that the idea might have been to see if registers
up a frame, and now potentially saved on the stack, can be modified.
If this was the idea, the problem with its implementation is that
the various add_<type> functions are too simple to adequately test
this behavior on all architectures. On some architectures (and for
some types), code generated for add_<type> will be cause one or more
registers to be saved on the stack. On others it won't. Also, even
when registers are saved on the stack, we have no idea whether the
registers of interest (i.e. those assigned to `l' and/or `r') have
been saved.
Even different multilibs using the same architecture may show
different behavior. Here's an example...
The disassembly for add_float() as generated for the default
vr4300-sim multilib is as follows:
0xa01003fc <add_float>: addiu sp,sp,-16
0xa0100400 <add_float+4>: sd ra,8(sp)
0xa0100404 <add_float+8>: sd s8,0(sp)
0xa0100408 <add_float+12>: move s8,sp
0xa010040c <add_float+16>: move v1,a0
0xa0100410 <add_float+20>: move v0,a1
0xa0100414 <add_float+24>: move a0,v1
0xa0100418 <add_float+28>: move a1,v0
0xa010041c <add_float+32>: jal 0xa01014a0 <__addsf3>
0xa0100420 <add_float+36>: nop
0xa0100424 <add_float+40>: move sp,s8
0xa0100428 <add_float+44>: ld ra,8(sp)
0xa010042c <add_float+48>: ld s8,0(sp)
0xa0100430 <add_float+52>: addiu sp,sp,16
0xa0100434 <add_float+56>: jr ra
0xa0100438 <add_float+60>: nop
Note that it calls the helper function __addsf3 and therefore cannot
be a leaf function. The ra register is saved on the stack and when
the test case modifes 'l' up a frame in wack_float(), it is actually
modifying the stack location at which l/ra has been saved.
When I look at the locations of l and r in wack_float(), I see:
(gdb) info address r
Symbol "r" is a variable in $s0.
(gdb) info address l
Symbol "l" is a variable in $ra.
So, in this instance, changing the test to modify 'r' instead of 'l'
does change what is being tested since s0 has not been saved on the
stack, but ra has. The original test was modifying memory. My patch
causes a register to be changed.
But, on the other hand, if I look at the disassembly for add_float
as generated for vr4300-sim/-march=vr5500, I see:
0xa0100434 <add_float>: addiu sp,sp,-8
0xa0100438 <add_float+4>: sd s8,0(sp)
0xa010043c <add_float+8>: move s8,sp
0xa0100440 <add_float+12>: mov.s $f1,$f12
0xa0100444 <add_float+16>: mov.s $f0,$f13
0xa0100448 <add_float+20>: add.s $f0,$f1,$f0
0xa010044c <add_float+24>: move sp,s8
0xa0100450 <add_float+28>: ld s8,0(sp)
0xa0100454 <add_float+32>: addiu sp,sp,8
0xa0100458 <add_float+36>: jr ra
0xa010045c <add_float+40>: nop
In this instance, the ra register is not saved on the stack; having
the test case modify `r' instead of `l' is similar in that a register
is being modified in both cases. (I should note that I don't see
the same failures for this multilib since r and l are allocated to
floating point registers.)
> Since the high order bit here is, AIUI, to prevent the SIGBUS, an
> alternative is to restore l afterwards.
> I don't know if that's a better way to go.
That seemed like a reasonable alternative to me, so I gave it a try.
But, sadly, it doesn't work. Changing local variable `l' changes the
ra register which in turn changes GDB's notion of the frame that it's
in:
Breakpoint 1, add_float (u=-1, v=-2)
at testsuite/gdb.base/store.c:47
47 return u + v;
(gdb) up
#1 0xa010072c in wack_float (u=-1, v=-2)
at testsuite/gdb.base/store.c:110
110 l = add_float (l, r);
(gdb) info frame
Stack level 1, frame at 0x807fffb0:
pc = 0xa010072c in wack_float
(testsuite/gdb.base/store.c:110); saved pc 0xa010111c
called by frame at 0x807fffc0, caller of frame at 0x807fff98
source language c.
Arglist at 0x807fffb0, args: u=-1, v=-2
Locals at 0x807fffb0, Previous frame's sp is 0x807fffb0
Saved registers:
s0 at 0x807fff98, s8 at 0x807fffa0, ra at 0x807fffa8, pc at 0x807fffa8
(gdb) set variable l = 4
(gdb) info frame
Stack level 0, frame at 0x807fff98:
pc = 0xa0100414 in add_float
(testsuite/gdb.base/store.c:47); saved pc 0x40800000
called by frame at 0x807fff98
source language c.
Arglist at 0x807fff98, args: u=-1, v=-2
Locals at 0x807fff98, Previous frame's sp is 0x807fff98
Saved registers:
s8 at 0x807fff88, ra at 0x807fff90, pc at 0x807fff90
That means that GDB can no longer find local variable l in order to
restore it. Even printing it fails:
(gdb) print l
No symbol "l" in current context.
Even saving the entire register state and restoring it afterwards won't
always work. As shown earlier, `l' can be allocated to ra and then
saved on the stack. In that case, it's that stack location that would
need to be restored. But the act of changing `l' (and therefore ra)
prevents us from being able to correctly restore it later on.
I haven't been able to think of a better fix than the patch I've
already posted.
Kevin