From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30919 invoked by alias); 20 Jan 2011 07:17:47 -0000 Received: (qmail 30883 invoked by uid 22791); 20 Jan 2011 07:17:45 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 20 Jan 2011 07:17:39 +0000 Received: from wpaz17.hot.corp.google.com (wpaz17.hot.corp.google.com [172.24.198.81]) by smtp-out.google.com with ESMTP id p0K7HbXO032528 for ; Wed, 19 Jan 2011 23:17:37 -0800 Received: from qyk10 (qyk10.prod.google.com [10.241.83.138]) by wpaz17.hot.corp.google.com with ESMTP id p0K7Halg010242 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NOT) for ; Wed, 19 Jan 2011 23:17:36 -0800 Received: by qyk10 with SMTP id 10so280001qyk.7 for ; Wed, 19 Jan 2011 23:17:36 -0800 (PST) MIME-Version: 1.0 Received: by 10.229.213.146 with SMTP id gw18mr1445451qcb.211.1295507856121; Wed, 19 Jan 2011 23:17:36 -0800 (PST) Received: by 10.220.4.70 with HTTP; Wed, 19 Jan 2011 23:17:35 -0800 (PST) In-Reply-To: <20110119162636.092db33a@mesquite.lan> References: <20110119162636.092db33a@mesquite.lan> Date: Thu, 20 Jan 2011 12:05:00 -0000 Message-ID: Subject: Re: [RFC] Fix variable lifetime related problem in gdb.base/store.exp From: Doug Evans To: Kevin Buettner Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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/msg00426.txt.bz2 On Wed, Jan 19, 2011 at 3:26 PM, Kevin Buettner wrote: > I'm seeing some interesting failures in gdb.base/store.exp for > mips64vrel-elf with --target_board set to vr4300-sim. =A0(The exact > target_board setting isn't all that critical; vr4300-sim is but one > example where things go awry.) > > Here is relevant log file output showing the problem: > > tbreak add_float > Temporary breakpoint 15 at 0xa0100414: file /ironwood1/sourceware-clean/m= ips64vrel-elf/../src/gdb/testsuite/gdb.base/store.c, line 47. > (gdb) PASS: gdb.base/store.exp: tbreak add_float > continue > Continuing. > > Temporary breakpoint 15, add_float (u=3D-1, v=3D-2) at /ironwood1/sourcew= are-clean/mips64vrel-elf/../src/gdb/testsuite/gdb.base/store.c:47 > 47 =A0 =A0 =A0 =A0return u + v; > (gdb) PASS: gdb.base/store.exp: continue to add_float > up > #1 =A00xa010072c in wack_float (u=3D-1, v=3D-2) at /ironwood1/sourceware-= clean/mips64vrel-elf/../src/gdb/testsuite/gdb.base/store.c:110 > 110 =A0 =A0 =A0 l =3D add_float (l, r); > (gdb) PASS: gdb.base/store.exp: upvar float l; up > print l > $48 =3D -1.21996474e-19 > (gdb) FAIL: gdb.base/store.exp: upvar float l; print old l, expecting -1 > print r > $49 =3D -2 > (gdb) PASS: gdb.base/store.exp: upvar float l; print old r, expecting -2 > set variable l =3D 4 > (gdb) PASS: gdb.base/store.exp: upvar float l; set l to 4 > print l > No symbol "l" in current context. > (gdb) FAIL: gdb.base/store.exp: upvar float l; print new l, expecting 4 > tbreak add_double > Temporary breakpoint 16 at 0xa0100454: file /ironwood1/sourceware-clean/m= ips64vrel-elf/../src/gdb/testsuite/gdb.base/store.c, line 53. > (gdb) PASS: gdb.base/store.exp: tbreak add_double > continue > Continuing. > mips-core: 4 byte read to unmapped address 0x40800000 at 0x40800000 > > Program received signal SIGBUS, Bus error. > 0x40800000 in ?? () > (gdb) FAIL: gdb.base/store.exp: continue to add_double > > These failures occur due to `l' in wack_float() being assigned (for > this particular MIPS toolchain) to the `ra' (return address) register. > > The function wack_float() is defined as follows: > > float > wack_float (register float u, register float v) > { > =A0register float l =3D u, r =3D v; > =A0l =3D add_float (l, r); > =A0return l + r; > } > > The test case places a breakpoint in add_float(), runs to that > breakpoint, and then goes up a frame to examine and modify certain > variables. > > The lifetime of `l' with value obtained from `u' ends with invocation > of add_float(). =A0(More precisely, it ends as soon as l's value is > copied to the argument register for the call to add_float().) =A0The > result of the add_float() call is placed in `l' when the call is > finished. > > Thus it's perfectly acceptable (though a bit perverse) to put l's value > in the ra register. =A0(However, given that this is occurring at -O0, it = is > also very annoying that the user is unable to inspect l's pre-call > value.) > > In any case, due to the fact that the lifetime of l-with-the-value-of-u > ends once the call to add_float() is set up, the compiler is then free > to use ra for other purposes which, of course, during a call, is used > to temporarily hold the return address. > > This is why the test fails to print the old value of 'l'; the register > to which it was assigned is now being used for another purpose, i.e, > holding the return address. > > And, moreover, it explains the BUS error after the assignment: > add_float() is a leaf function and, therefore, does not need to save > ra on the stack. =A0So it simply uses it as is not expecting it to be > changed upon return. > > That BUS error is reponsible for many cascade failures later on. > > My fix, below, is to assign (via GDB's "set variable" command) to `r' > instead of `l' since the lifetime of r's value set at the beginning of > the function extends beyond the call to add_float(). =A0The patch also > causes 'r', instead of 'l', to be examined immediately after the > assignment. > > It won't fix the initial failure with `l' apparently having the wrong > value. =A0I think this test ought to be preserved to remind us - should > it fail - that although GCC's generated code might be correct, it is > not as useful as it should be for debugging purposes. > > This fix does prevent the BUS error and the resulting cascade > failures. =A0Stopping the cascade failures is important because we can > then determine if there are any real failures amidst the cascade. > > Comments? > > =A0 =A0 =A0 =A0* gdb.base/store.exp (proc up_set): Set, and check, the va= riable > =A0 =A0 =A0 =A0`r' rather than `l' due to the fact that the lifetime for = the > =A0 =A0 =A0 =A0pre-call value for `l' need not extend beyond the call of > =A0 =A0 =A0 =A0add_. > > Index: testsuite/gdb.base/store.exp > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/testsuite/gdb.base/store.exp,v > retrieving revision 1.17 > diff -u -p -r1.17 store.exp > --- testsuite/gdb.base/store.exp =A0 =A0 =A0 =A01 Jan 2011 15:33:43 -0000= =A0 =A0 =A0 1.17 > +++ testsuite/gdb.base/store.exp =A0 =A0 =A0 =A019 Jan 2011 22:22:38 -0000 > @@ -95,10 +95,10 @@ proc up_set { t l r new } { > =A0 =A0 =A0 =A0"${prefix}; print old l, expecting ${l}" > =A0 =A0 gdb_test "print r" " =3D ${r}" \ > =A0 =A0 =A0 =A0"${prefix}; print old r, expecting ${r}" > - =A0 =A0gdb_test_no_output "set variable l =3D 4" \ > - =A0 =A0 =A0 "${prefix}; set l to 4" > - =A0 =A0gdb_test "print l" " =3D ${new}" \ > - =A0 =A0 =A0 "${prefix}; print new l, expecting ${new}" > + =A0 =A0gdb_test_no_output "set variable r =3D 4" \ > + =A0 =A0 =A0 "${prefix}; set r to 4" > + =A0 =A0gdb_test "print r" " =3D ${new}" \ > + =A0 =A0 =A0 "${prefix}; print new r, expecting ${new}" > =A0} > > =A0up_set "charest" "-1 .*" "-2 .*" "4 ..004." > Nasty. 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] 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. 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.