* RFA: report failure to restore selected frame, but continue
@ 2003-04-09 21:40 Jim Blandy
2003-04-09 22:14 ` David Carlton
2003-04-10 1:55 ` Andrew Cagney
0 siblings, 2 replies; 4+ messages in thread
From: Jim Blandy @ 2003-04-09 21:40 UTC (permalink / raw)
To: gdb-patches
2003-04-09 Jim Blandy <jimb@redhat.com>
* gdb.c++/derivation.exp, gdb.c++/overload.exp,
gdb.c++/userdef.exp: If GDB fails to restore the selected frame
after an inferior function call, report the failure, but allow the
test to continue.
Index: gdb/testsuite/gdb.c++/userdef.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/userdef.exp,v
retrieving revision 1.6
diff -c -r1.6 userdef.exp
*** gdb/testsuite/gdb.c++/userdef.exp 18 Feb 2002 19:07:29 -0000 1.6
--- gdb/testsuite/gdb.c++/userdef.exp 9 Apr 2003 21:33:04 -0000
***************
*** 66,71 ****
--- 66,128 ----
gdb_test "print one + two" "\\\$\[0-9\]* = {x = 6, y = 8}"
+ # Suppose an architecture uses the stack pointer as its frame base
+ # (that is, the 'base' member of its frame ID). (Ignore functions
+ # that use alloca for the moment.) This means that the youngest
+ # frame's base is the top of the stack. If that function calls a
+ # frameless function, then the caller and callee frames will have
+ # identical base addresses. frame_id_eq is supposed to use both the
+ # base and pc fields of the ID's to decide whether they're eq, but as
+ # of 4-2003 it doesn't, so code like frame_find_by_id can't
+ # distinguish between those two frames.
+ #
+ # This means that, on such architectures, if the marker function we're
+ # stopped at in this test is frameless, GDB doesn't properly restore
+ # the selected frame after an inferior function call:
+ # infrun.c:restore_selected_frame will select the youngest frame (the
+ # marker function), not its caller (main).
+ #
+ # Changing frame_id_eq isn't really a full solution, though. By
+ # definition, a frame base is constant over the lifetime of the frame.
+ # However, the stack pointer changes as a function's prologue code
+ # sets up the stack frame, making it unsuitable for use as a frame
+ # base. The address of the inner end of each frame --- furthest from
+ # the stack pointer --- would work better here; it remains unchanged
+ # through frame allocation, frame teardown, and any alloca'ing that
+ # might happen in between.
+ #
+ # Of course, strictly speaking, that just shifts the problem around:
+ # if there's a younger frame sitting on top of a frameless frame, then
+ # they'll have identical ID's. But frameless functions can't call
+ # other functions directly, at least using the standard ABI --- they
+ # have no place to save the return address. Only signal delivery
+ # frames and inferior calls can be there, and those have all sorts of
+ # other associated magic.
+ #
+ # Anyway, if GDB fails to restore the selected frame properly after
+ # the inferior function call above, all the subsequent tests will
+ # fail. We should detect report that failure, but let the marker call
+ # finish so that the rest of the tests can run undisturbed.
+
+ send_gdb "frame\n"
+ gdb_expect {
+ -re "#0 marker1.*$gdb_prompt $" {
+ setup_kfail "gdb/1155" s390-*-linux-gnu
+ fail "re-selected 'main' frame after inferior method call (gdb/1155)"
+ gdb_test "finish" ".*main.*at .*userdef.cc:27\[67\].*" \
+ "finish call to marker1"
+ }
+ -re "#1 ($hex in )?main.*$gdb_prompt $" {
+ pass "re-selected 'main' frame after inferior method call"
+ }
+ -re ".*$gdb_prompt $" {
+ fail "re-selected 'main' frame after inferior method call"
+ }
+ timeout {
+ fail "re-selected 'main' frame after inferior method call (timeout)"
+ }
+ }
+
gdb_test "print one - two" "\\\$\[0-9\]* = {x = -2, y = -2}"
gdb_test "print one * two" "\\\$\[0-9\]* = {x = 8, y = 15}"
Index: gdb/testsuite/gdb.c++/derivation.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/derivation.exp,v
retrieving revision 1.10
diff -c -r1.10 derivation.exp
*** gdb/testsuite/gdb.c++/derivation.exp 20 Jan 2002 19:22:13 -0000 1.10
--- gdb/testsuite/gdb.c++/derivation.exp 9 Apr 2003 21:33:04 -0000
***************
*** 300,305 ****
--- 300,326 ----
timeout { fail "(timeout) print value of g_instance.afoo()" }
}
+
+ # See explanation in userdef.exp for the test of the same name.
+ send_gdb "frame\n"
+ gdb_expect {
+ -re "#0 marker1.*$gdb_prompt $" {
+ setup_kfail "gdb/1155" s390-*-linux-gnu
+ fail "re-selected 'main' frame after inferior method call (gdb/1155)"
+ gdb_test "finish" ".*main.*at .*derivation.cc:21\[79\].*" \
+ "finish call to marker1"
+ }
+ -re "#1 ($hex in )?main.*$gdb_prompt $" {
+ pass "re-selected 'main' frame after inferior method call"
+ }
+ -re ".*$gdb_prompt $" {
+ fail "re-selected 'main' frame after inferior method call"
+ }
+ timeout {
+ fail "re-selected 'main' frame after inferior method call (timeout)"
+ }
+ }
+
send_gdb "print g_instance.bfoo()\n"
gdb_expect {
-re ".\[0-9\]* = 2.*$gdb_prompt $" {
Index: gdb/testsuite/gdb.c++/overload.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/overload.exp,v
retrieving revision 1.10
diff -c -r1.10 overload.exp
*** gdb/testsuite/gdb.c++/overload.exp 4 Feb 2003 21:19:27 -0000 1.10
--- gdb/testsuite/gdb.c++/overload.exp 9 Apr 2003 21:33:04 -0000
***************
*** 120,125 ****
--- 120,145 ----
}
+ # See explanation in userdef.exp for the test of the same name.
+ send_gdb "frame\n"
+ gdb_expect {
+ -re "#0 marker1.*$gdb_prompt $" {
+ setup_kfail "gdb/1155" s390-*-linux-gnu
+ fail "re-selected 'main' frame after inferior method call (gdb/1155)"
+ gdb_test "finish" ".*main.*at .*overload.cc:7\[78\].*" \
+ "finish call to marker1"
+ }
+ -re "#1 ($hex in )?main.*$gdb_prompt $" {
+ pass "re-selected 'main' frame after inferior method call"
+ }
+ -re ".*$gdb_prompt $" {
+ fail "re-selected 'main' frame after inferior method call"
+ }
+ timeout {
+ fail "re-selected 'main' frame after inferior method call (timeout)"
+ }
+ }
+
send_gdb "print foo_instance1.overloadargs(1, 2)\n"
gdb_expect {
-re ".\[0-9\]* = 2\r\n$gdb_prompt $" {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFA: report failure to restore selected frame, but continue
2003-04-09 21:40 RFA: report failure to restore selected frame, but continue Jim Blandy
@ 2003-04-09 22:14 ` David Carlton
2003-04-09 23:21 ` Jim Blandy
2003-04-10 1:55 ` Andrew Cagney
1 sibling, 1 reply; 4+ messages in thread
From: David Carlton @ 2003-04-09 22:14 UTC (permalink / raw)
To: Jim Blandy; +Cc: gdb-patches
On 09 Apr 2003 16:41:06 -0500, Jim Blandy <jimb@redhat.com> said:
[ message snipped ]
Basically fine, just a few comments. It might be nice if there were a
test for this somewhere appropriate in gdb.base, in which case the big
comment could go there: or is there a reason why this only manifests
itself in C++? Actually, I'm not convinced that all of the big
comment belongs in the test suite at all: why not put it some place in
the GDB source code and/or the bug database, with a reference to that
in the testsuite? The diagnosis of the problem should be in the place
where it people fixing the problem will be most likely to see it,
after all.
The use of a conditional setup_kfail followed by a fail is good:
normally, I don't like setup_kfail so much, but it's quite appropriate
here.
> + # of 4-2003 it doesn't, so code like frame_find_by_id can't
Personally, I'd say "April 2003" or "2003-04-09" or something instead
of "4-2003".
> + # fail. We should detect report that failure, but let the marker call
Typo.
> + send_gdb "frame\n"
> + gdb_expect {
> + -re "#0 marker1.*$gdb_prompt $" {
> + setup_kfail "gdb/1155" s390-*-linux-gnu
> + fail "re-selected 'main' frame after inferior method call (gdb/1155)"
> + gdb_test "finish" ".*main.*at .*userdef.cc:27\[67\].*" \
> + "finish call to marker1"
> + }
> + -re "#1 ($hex in )?main.*$gdb_prompt $" {
> + pass "re-selected 'main' frame after inferior method call"
> + }
> + -re ".*$gdb_prompt $" {
> + fail "re-selected 'main' frame after inferior method call"
> + }
> + timeout {
> + fail "re-selected 'main' frame after inferior method call (timeout)"
> + }
> + }
> +
We're using gdb_test_multiple now: replace the first two lines by:
gdb_test_multiple "frame" "re-selected 'main' frame after inferior method call" {
and delete the last two branches. (I'm pretty sure that's right, run
it to make sure.) You can see an example in gdb.c++/templates.exp.
Also, don't put (gdb/1155) in the fail message: the setup_kfail should
take care of that for you.
Ditto for the uses in the other two files, of course.
Other than that, it's fine: no need to resubmit it for approval after
making those changes unless you think it would be useful for some
reason.
David Carlton
carlton@math.stanford.edu
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFA: report failure to restore selected frame, but continue
2003-04-09 22:14 ` David Carlton
@ 2003-04-09 23:21 ` Jim Blandy
0 siblings, 0 replies; 4+ messages in thread
From: Jim Blandy @ 2003-04-09 23:21 UTC (permalink / raw)
To: David Carlton; +Cc: gdb-patches
David Carlton <carlton@math.stanford.edu> writes:
> Basically fine, just a few comments. It might be nice if there were a
> test for this somewhere appropriate in gdb.base, in which case the big
> comment could go there: or is there a reason why this only manifests
> itself in C++?
No, it's not specific to C++ at all; if the architecture makes a bad
choice of frame base, GDB may not preserve the selected frame across
inferior calls.
> Actually, I'm not convinced that all of the big
> comment belongs in the test suite at all: why not put it some place in
> the GDB source code and/or the bug database, with a reference to that
> in the testsuite? The diagnosis of the problem should be in the place
> where it people fixing the problem will be most likely to see it,
> after all.
My thought was, if you're working on a target that has this problem,
one of the earliest places you'll visit is the test script itself, to
see what the failing test is actually doing. The comments for 'struct
frame_id' could be expanded to point out the subtleties, but that is
less helpful for someone working from the failure: you only know where
to look in the sources after you've partially debugged the problem.
GDB PR 1155 has a decent enough explanation of it; I'll just put
references to that in the .exp files.
I've made the rest of the changes you suggest. Here's what I've
committed:
2003-04-09 Jim Blandy <jimb@redhat.com>
* gdb.c++/derivation.exp, gdb.c++/overload.exp,
gdb.c++/userdef.exp: If GDB fails to restore the selected frame
after an inferior function call, report the failure, but allow the
test to continue.
Index: gdb/testsuite/gdb.c++/derivation.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/derivation.exp,v
retrieving revision 1.10
diff -c -r1.10 derivation.exp
*** gdb/testsuite/gdb.c++/derivation.exp 20 Jan 2002 19:22:13 -0000 1.10
--- gdb/testsuite/gdb.c++/derivation.exp 9 Apr 2003 23:16:43 -0000
***************
*** 300,305 ****
--- 300,323 ----
timeout { fail "(timeout) print value of g_instance.afoo()" }
}
+
+ # If GDB fails to restore the selected frame properly after the
+ # inferior function call above (see GDB PR 1155 for an explanation of
+ # why this might happen), all the subsequent tests will fail. We
+ # should detect report that failure, but let the marker call finish so
+ # that the rest of the tests can run undisturbed.
+ gdb_test_multiple "frame" "re-selected 'main' frame after inferior call" {
+ -re "#0 marker1.*$gdb_prompt $" {
+ setup_kfail "gdb/1155" s390-*-linux-gnu
+ fail "re-selected 'main' frame after inferior call"
+ gdb_test "finish" ".*main.*at .*derivation.cc:21\[79\].*" \
+ "finish call to marker1"
+ }
+ -re "#1 ($hex in )?main.*$gdb_prompt $" {
+ pass "re-selected 'main' frame after inferior call"
+ }
+ }
+
send_gdb "print g_instance.bfoo()\n"
gdb_expect {
-re ".\[0-9\]* = 2.*$gdb_prompt $" {
Index: gdb/testsuite/gdb.c++/overload.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/overload.exp,v
retrieving revision 1.10
diff -c -r1.10 overload.exp
*** gdb/testsuite/gdb.c++/overload.exp 4 Feb 2003 21:19:27 -0000 1.10
--- gdb/testsuite/gdb.c++/overload.exp 9 Apr 2003 23:16:44 -0000
***************
*** 120,125 ****
--- 120,143 ----
}
+ # If GDB fails to restore the selected frame properly after the
+ # inferior function call above (see GDB PR 1155 for an explanation of
+ # why this might happen), all the subsequent tests will fail. We
+ # should detect and report that failure, but let the marker call
+ # finish so that the rest of the tests can run undisturbed.
+ gdb_test_multiple "frame" "re-selected 'main' frame after inferior call" {
+ -re "#0 marker1.*$gdb_prompt $" {
+ setup_kfail "gdb/1155" s390-*-linux-gnu
+ fail "re-selected 'main' frame after inferior call"
+ gdb_test "finish" ".*main.*at .*overload.cc:7\[78\].*" \
+ "finish call to marker1"
+ }
+ -re "#1 ($hex in )?main.*$gdb_prompt $" {
+ pass "re-selected 'main' frame after inferior call"
+ }
+ }
+
+
send_gdb "print foo_instance1.overloadargs(1, 2)\n"
gdb_expect {
-re ".\[0-9\]* = 2\r\n$gdb_prompt $" {
Index: gdb/testsuite/gdb.c++/userdef.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/userdef.exp,v
retrieving revision 1.6
diff -c -r1.6 userdef.exp
*** gdb/testsuite/gdb.c++/userdef.exp 18 Feb 2002 19:07:29 -0000 1.6
--- gdb/testsuite/gdb.c++/userdef.exp 9 Apr 2003 23:16:44 -0000
***************
*** 66,71 ****
--- 66,88 ----
gdb_test "print one + two" "\\\$\[0-9\]* = {x = 6, y = 8}"
+ # If GDB fails to restore the selected frame properly after the
+ # inferior function call above (see GDB PR 1155 for an explanation of
+ # why this might happen), all the subsequent tests will fail. We
+ # should detect report that failure, but let the marker call finish so
+ # that the rest of the tests can run undisturbed.
+ gdb_test_multiple "frame" "re-selected 'main' frame after inferior call" {
+ -re "#0 marker1.*$gdb_prompt $" {
+ setup_kfail "gdb/1155" s390-*-linux-gnu
+ fail "re-selected 'main' frame after inferior call"
+ gdb_test "finish" ".*main.*at .*userdef.cc:27\[67\].*" \
+ "finish call to marker1"
+ }
+ -re "#1 ($hex in )?main.*$gdb_prompt $" {
+ pass "re-selected 'main' frame after inferior call"
+ }
+ }
+
gdb_test "print one - two" "\\\$\[0-9\]* = {x = -2, y = -2}"
gdb_test "print one * two" "\\\$\[0-9\]* = {x = 8, y = 15}"
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFA: report failure to restore selected frame, but continue
2003-04-09 21:40 RFA: report failure to restore selected frame, but continue Jim Blandy
2003-04-09 22:14 ` David Carlton
@ 2003-04-10 1:55 ` Andrew Cagney
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Cagney @ 2003-04-10 1:55 UTC (permalink / raw)
To: Jim Blandy; +Cc: gdb-patches
Jim,
The opening line should state up front that ``An architecture
incorrectly using it's stack pointer as the frame ID's stack address
...''. That way, the specific bug identified by 1155, is made clear
from the start. At present it isn't mentioned until the third paragraph.
I should also note that frame_id_eq() is ~week from being `fixed'. You
may want to drop that part.
enjoy,
Andrew
> + # Suppose an architecture uses the stack pointer as its frame base
> + # (that is, the 'base' member of its frame ID). (Ignore functions
> + # that use alloca for the moment.) This means that the youngest
> + # frame's base is the top of the stack. If that function calls a
> + # frameless function, then the caller and callee frames will have
> + # identical base addresses. frame_id_eq is supposed to use both the
> + # base and pc fields of the ID's to decide whether they're eq, but as
> + # of 4-2003 it doesn't, so code like frame_find_by_id can't
> + # distinguish between those two frames.
> + #
> + # This means that, on such architectures, if the marker function we're
> + # stopped at in this test is frameless, GDB doesn't properly restore
> + # the selected frame after an inferior function call:
> + # infrun.c:restore_selected_frame will select the youngest frame (the
> + # marker function), not its caller (main).
> + #
> + # Changing frame_id_eq isn't really a full solution, though. By
> + # definition, a frame base is constant over the lifetime of the frame.
> + # However, the stack pointer changes as a function's prologue code
> + # sets up the stack frame, making it unsuitable for use as a frame
> + # base. The address of the inner end of each frame --- furthest from
> + # the stack pointer --- would work better here; it remains unchanged
> + # through frame allocation, frame teardown, and any alloca'ing that
> + # might happen in between.
> + #
> + # Of course, strictly speaking, that just shifts the problem around:
> + # if there's a younger frame sitting on top of a frameless frame, then
> + # they'll have identical ID's. But frameless functions can't call
> + # other functions directly, at least using the standard ABI --- they
> + # have no place to save the return address. Only signal delivery
> + # frames and inferior calls can be there, and those have all sorts of
> + # other associated magic.
> + #
> + # Anyway, if GDB fails to restore the selected frame properly after
> + # the inferior function call above, all the subsequent tests will
> + # fail. We should detect report that failure, but let the marker call
> + # finish so that the rest of the tests can run undisturbed.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2003-04-10 1:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-04-09 21:40 RFA: report failure to restore selected frame, but continue Jim Blandy
2003-04-09 22:14 ` David Carlton
2003-04-09 23:21 ` Jim Blandy
2003-04-10 1:55 ` Andrew Cagney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox