From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9752 invoked by alias); 21 Jan 2011 20:19:50 -0000 Received: (qmail 9621 invoked by uid 22791); 21 Jan 2011 20:19:48 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 21 Jan 2011 20:19:33 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id p0LKJTlJ019658 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 21 Jan 2011 15:19:29 -0500 Received: from mesquite.lan (ovpn-113-42.phx2.redhat.com [10.3.113.42]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p0LKJTcS025269 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 21 Jan 2011 15:19:29 -0500 Date: Fri, 21 Jan 2011 20:42:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Cc: Doug Evans Subject: Re: [RFC] Fix variable lifetime related problem in gdb.base/store.exp Message-ID: <20110121131928.20f083b4@mesquite.lan> In-Reply-To: References: <20110119162636.092db33a@mesquite.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-01/txt/msg00447.txt.bz2 On Wed, 19 Jan 2011 23:17:35 -0800 Doug Evans 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_ functions are too simple to adequately test this behavior on all architectures. On some architectures (and for some types), code generated for add_ 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 : addiu sp,sp,-16 0xa0100400 : sd ra,8(sp) 0xa0100404 : sd s8,0(sp) 0xa0100408 : move s8,sp 0xa010040c : move v1,a0 0xa0100410 : move v0,a1 0xa0100414 : move a0,v1 0xa0100418 : move a1,v0 0xa010041c : jal 0xa01014a0 <__addsf3> 0xa0100420 : nop 0xa0100424 : move sp,s8 0xa0100428 : ld ra,8(sp) 0xa010042c : ld s8,0(sp) 0xa0100430 : addiu sp,sp,16 0xa0100434 : jr ra 0xa0100438 : 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 : addiu sp,sp,-8 0xa0100438 : sd s8,0(sp) 0xa010043c : move s8,sp 0xa0100440 : mov.s $f1,$f12 0xa0100444 : mov.s $f0,$f13 0xa0100448 : add.s $f0,$f1,$f0 0xa010044c : move sp,s8 0xa0100450 : ld s8,0(sp) 0xa0100454 : addiu sp,sp,8 0xa0100458 : jr ra 0xa010045c : 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