diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 222f78700dac..1fb363d3cf93 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2016-09-05 Pedro Alves + + PR backtrace/19927 + * frame.c (get_frame_id): Compute the frame id if not computed + yet. + (unwind_to_current_frame): Delete. + (get_current_frame): Use get_prev_frame_always_1 to get the + current frame and assert that that always succeeds. + (get_prev_frame_if_no_cycle): Skip cycle detection if returning + the current frame. + 2016-09-02 Tom Tromey PR gdb/11616: diff --git a/gdb/frame.c b/gdb/frame.c index c25ce4c41c3b..d3f30561dc06 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -511,7 +511,26 @@ get_frame_id (struct frame_info *fi) if (fi == NULL) return null_frame_id; - gdb_assert (fi->this_id.p); + if (!fi->this_id.p) + { + int stashed; + + /* If we haven't computed the frame id yet, then it must be that + this is the current frame. Compute it now, and stash the + result. The IDs of other frames are computed as soon as + they're created, in order to detect cycles. See + get_prev_frame_if_no_cycle. */ + gdb_assert (fi->level == 0); + + /* Compute. */ + compute_frame_id (fi); + + /* Since this is the first frame in the chain, this should + always succeed. */ + stashed = frame_stash_add (fi); + gdb_assert (stashed); + } + return fi->this_id.value; } @@ -1487,23 +1506,7 @@ frame_obstack_zalloc (unsigned long size) return data; } -/* Return the innermost (currently executing) stack frame. This is - split into two functions. The function unwind_to_current_frame() - is wrapped in catch exceptions so that, even when the unwind of the - sentinel frame fails, the function still returns a stack frame. */ - -static int -unwind_to_current_frame (struct ui_out *ui_out, void *args) -{ - struct frame_info *frame = get_prev_frame ((struct frame_info *) args); - - /* A sentinel frame can fail to unwind, e.g., because its PC value - lands in somewhere like start. */ - if (frame == NULL) - return 1; - current_frame = frame; - return 0; -} +static struct frame_info *get_prev_frame_always_1 (struct frame_info *this_frame); struct frame_info * get_current_frame (void) @@ -1525,16 +1528,28 @@ get_current_frame (void) if (current_frame == NULL) { + int stashed; struct frame_info *sentinel_frame = create_sentinel_frame (current_program_space, get_current_regcache ()); - if (catch_exceptions (current_uiout, unwind_to_current_frame, - sentinel_frame, RETURN_MASK_ERROR) != 0) - { - /* Oops! Fake a current frame? Is this useful? It has a PC - of zero, for instance. */ - current_frame = sentinel_frame; - } + + /* Set the current frame before computing the frame id, to avoid + recursion inside compute_frame_id, in case the frame's + unwinder decides to do a symbol lookup (which depends on the + selected frame's block). + + This call must always succeed. In particular, nothing inside + get_prev_frame_always_1 should try to unwind from the + sentinel frame, because that could fail/throw, and we always + want to leave with the current frame created and linked in -- + we should never end up with the sentinel frame as outermost + frame. */ + current_frame = get_prev_frame_always_1 (sentinel_frame); + gdb_assert (current_frame != NULL); + + /* No need to compute the frame id yet. That'll be done lazily + from inside get_frame_id instead. */ } + return current_frame; } @@ -1812,8 +1827,18 @@ get_prev_frame_if_no_cycle (struct frame_info *this_frame) struct cleanup *prev_frame_cleanup; prev_frame = get_prev_frame_raw (this_frame); - if (prev_frame == NULL) - return NULL; + + /* Don't compute the frame id of the current frame yet. Unwinding + the sentinel frame can fail (e.g., if the thread is gone and we + can't thus read its registers). If we let the cycle detection + code below try to compute a frame ID, then an error thrown from + within the frame ID computation would result in the sentinel + frame as outermost frame, which is bogus. Instead, we'll compute + the current frame's ID lazily in get_frame_id. Note that there's + no point in doing cycle detection when there's only one frame, so + nothing is lost here. */ + if (prev_frame->level == 0) + return prev_frame; /* The cleanup will remove the previous frame that get_prev_frame_raw linked onto THIS_FRAME. */ diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index ffb993cf56ee..14cbb5a3d490 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2016-09-05 Pedro Alves + + PR backtrace/19927 + * gdb.python/py-unwind-maint.exp: Adjust tests to not expect that + unwinders are immediately called as side effect of "source" or + "disable unwinder" commands. + * gdb.python/py-recurse-unwind.exp: Remove setup_kfail calls. + 2016-09-02 Yao Qi * gdb.base/return-nodebug.exp: Skip the test if skip_float_test diff --git a/gdb/testsuite/gdb.python/py-recurse-unwind.exp b/gdb/testsuite/gdb.python/py-recurse-unwind.exp index fe17d3fe6475..9629a974ba01 100644 --- a/gdb/testsuite/gdb.python/py-recurse-unwind.exp +++ b/gdb/testsuite/gdb.python/py-recurse-unwind.exp @@ -61,7 +61,6 @@ gdb_test_no_output "python TestUnwinder.reset_count()" # get hung up in potentially infinite recursion when invoking the # Python-based unwinder. -setup_kfail "gdb/19927" "*-*-*" gdb_test_sequence "bt" "backtrace" { "\\r\\n#0 .* ccc \\(arg=789\\) at " "\\r\\n#1 .* bbb \\(arg=456\\) at " @@ -71,5 +70,4 @@ gdb_test_sequence "bt" "backtrace" { # Test that the python-based unwinder / sniffer was actually called # during generation of the backtrace. -setup_kfail "gdb/19927" "*-*-*" gdb_test "python print(TestUnwinder.count > 0)" "True" diff --git a/gdb/testsuite/gdb.python/py-unwind-maint.exp b/gdb/testsuite/gdb.python/py-unwind-maint.exp index 3a98cb13f6ca..12530571ca0e 100644 --- a/gdb/testsuite/gdb.python/py-unwind-maint.exp +++ b/gdb/testsuite/gdb.python/py-unwind-maint.exp @@ -34,10 +34,13 @@ if ![runto_main ] then { return -1 } -gdb_test_sequence "source ${pyfile}" "import python scripts" { - "Python script imported" +gdb_test "source ${pyfile}" "Python script imported" \ + "import python scripts" + +gdb_test_sequence "frame" "All unwinders enabled" { "py_unwind_maint_ps_unwinder called" "global_unwinder called" + "#0 main" } gdb_test_sequence "info unwinder" "Show all unwinders" { @@ -56,7 +59,6 @@ gdb_test_sequence "continue" "Unwinders called" { gdb_test_sequence "disable unwinder global .*" "Unwinder disabled" { "1 unwinder disabled" - "py_unwind_maint_ps_unwinder called" } gdb_test_sequence "info unwinder" "Show with global unwinder disabled" {