Remove previous frame if an error occurs when computing frame id during unwind.

https://sourceware.org/ml/gdb-patches/2014-05/msg00712.html

If an error is thrown during computing a frame id then the frame is left
in existence but without a valid frame id, this will trigger internal
errors if/when the frame is later visited (for example in a backtrace).

This patch catches errors raised while computing the frame id, and
arranges for the new frame, the one without a frame id, to be removed
from the linked list of frames.

gdb/ChangeLog:

	* frame.c (remove_prev_frame): New function.
	(get_prev_frame_if_no_cycle): Create / discard cleanup using
	remove_prev_frame.

gdb/testsuite/ChangeLog:

	* gdb.arch/amd64-invalid-stack-middle.S: New file.
	* gdb.arch/amd64-invalid-stack-middle.c: New file.
	* gdb.arch/amd64-invalid-stack-middle.exp: New file.
	* gdb.arch/amd64-invalid-stack-top.c: New file.
	* gdb.arch/amd64-invalid-stack-top.exp: New file.
This commit is contained in:
Andrew Burgess 2014-04-02 17:02:51 +01:00
parent f6fb832249
commit 938f0e2f67
8 changed files with 1843 additions and 14 deletions

View File

@ -1,3 +1,9 @@
2014-05-30 Andrew Burgess <aburgess@broadcom.com>
* frame.c (remove_prev_frame): New function.
(get_prev_frame_if_no_cycle): Create / discard cleanup using
remove_prev_frame.
2014-05-29 Pedro Alves <palves@redhat.com>
* infrun.c (resume): Rename local 'hw_step' to 'entry_step'

View File

@ -1738,6 +1738,22 @@ frame_register_unwind_location (struct frame_info *this_frame, int regnum,
}
}
/* Called during frame unwinding to remove a previous frame pointer from a
frame passed in ARG. */
static void
remove_prev_frame (void *arg)
{
struct frame_info *this_frame, *prev_frame;
this_frame = (struct frame_info *) arg;
prev_frame = this_frame->prev;
gdb_assert (prev_frame != NULL);
prev_frame->next = NULL;
this_frame->prev = NULL;
}
/* Get the previous raw frame, and check that it is not identical to
same other frame frame already in the chain. If it is, there is
most likely a stack cycle, so we discard it, and mark THIS_FRAME as
@ -1750,28 +1766,36 @@ static struct frame_info *
get_prev_frame_if_no_cycle (struct frame_info *this_frame)
{
struct frame_info *prev_frame;
struct cleanup *prev_frame_cleanup;
prev_frame = get_prev_frame_raw (this_frame);
if (prev_frame == NULL)
return NULL;
compute_frame_id (prev_frame);
if (frame_stash_add (prev_frame))
return prev_frame;
/* The cleanup will remove the previous frame that get_prev_frame_raw
linked onto THIS_FRAME. */
prev_frame_cleanup = make_cleanup (remove_prev_frame, this_frame);
/* Another frame with the same id was already in the stash. We just
detected a cycle. */
if (frame_debug)
compute_frame_id (prev_frame);
if (!frame_stash_add (prev_frame))
{
fprintf_unfiltered (gdb_stdlog, "-> ");
fprint_frame (gdb_stdlog, NULL);
fprintf_unfiltered (gdb_stdlog, " // this frame has same ID }\n");
/* Another frame with the same id was already in the stash. We just
detected a cycle. */
if (frame_debug)
{
fprintf_unfiltered (gdb_stdlog, "-> ");
fprint_frame (gdb_stdlog, NULL);
fprintf_unfiltered (gdb_stdlog, " // this frame has same ID }\n");
}
this_frame->stop_reason = UNWIND_SAME_ID;
/* Unlink. */
prev_frame->next = NULL;
this_frame->prev = NULL;
prev_frame = NULL;
}
this_frame->stop_reason = UNWIND_SAME_ID;
/* Unlink. */
prev_frame->next = NULL;
this_frame->prev = NULL;
return NULL;
discard_cleanups (prev_frame_cleanup);
return prev_frame;
}
/* Return a "struct frame_info" corresponding to the frame that called

View File

@ -1,3 +1,11 @@
2014-05-30 Andrew Burgess <aburgess@broadcom.com>
* gdb.arch/amd64-invalid-stack-middle.S: New file.
* gdb.arch/amd64-invalid-stack-middle.c: New file.
* gdb.arch/amd64-invalid-stack-middle.exp: New file.
* gdb.arch/amd64-invalid-stack-top.c: New file.
* gdb.arch/amd64-invalid-stack-top.exp: New file.
2014-05-30 Pedro Alves <palves@redhat.com>
PR breakpoints/17000

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,89 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2014 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include <sys/mman.h>
#include <unistd.h>
#include <assert.h>
void
breakpt (void)
{
/* Nothing. */
}
void
func5 (void)
{
breakpt ();
}
void
func4 (void)
{
func5 ();
}
void
func3 (void)
{
func4 ();
}
void
func2 (void *ptr)
{
func3 ();
}
void
func1 (void *ptr)
{
func2 (ptr);
}
/* Finds and returns an invalid pointer, mmaps in a page, grabs a pointer
to it then unmaps the page again. This is almost certainly "undefined"
behaviour, but should be good enough for this small test program. */
static void *
make_invalid_ptr (void)
{
int page_size, ans;
void *ptr;
page_size = getpagesize ();
ptr = mmap (0, page_size, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS,
-1, 0);
assert (ptr != MAP_FAILED);
ans = munmap (ptr, page_size);
assert (ans == 0);
return ptr;
}
int
main (void)
{
void *invalid_ptr;
invalid_ptr = make_invalid_ptr ();
func1 (invalid_ptr);
return 0;
}

View File

@ -0,0 +1,108 @@
# Copyright (C) 2014 Free Software Foundation, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# In this test we're looking at how gdb handles backtraces and
# investigating the stack depth when confronted with an "invalid" stack,
# that is a stack where the first few frames are normal, and then there's a
# frame where the stack in unreadable.
#
# One interesting bug that has been observed is that gdb will sometime
# exhibit different behaviour the first time a stack command is run
# compared to the second (and later) times a command is run. This is
# because the first time a command is run gdb actually tries to figure out
# the answer, while the second (and later) times gdb relies on the answer
# cached from the first time. As a result in this test each command is
# run twice, and we restart gdb before testing each different command to
# ensure that nothing is being cached.
set opts {}
standard_testfile .S
if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
verbose "Skipping ${testfile}."
return
}
if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} $opts] } {
return -1
}
if ![runto breakpt] {
return -1
}
gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nCannot access memory at address 0x\[0-9a-f\]+" \
"first backtrace, with error message"
send_gdb "bt\n"
gdb_expect {
-re "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nCannot access memory at address 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
# Currently gdb will not display the error message associated with
# the truncated backtrace after the first backtrace has been
# completed. Ideally, we would do this. If this case is ever hit
# then we have started to display the backtrace in all cases and
# the xpass should becomd a pass, and the previous pass case below
# should be removed, or changed to a fail.
xpass "second backtrace, with error message"
}
-re "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\n$gdb_prompt $" {
pass "second backtrace, without error message"
}
timeout {
fail "second backtrace (timeout)"
}
}
clean_restart ${binfile}
if ![runto breakpt] {
return -1
}
set test_name "check mi -stack-info-depth command, first time"
send_gdb "interpreter-exec mi \"-stack-info-depth\"\n"
gdb_expect {
-re "\\^done,depth=\"4\"\r\n$gdb_prompt $" {
pass $test_name
}
-re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
xfail $test_name
}
}
gdb_test "interpreter-exec mi \"-stack-info-depth\"" \
"\\^done,depth=\"4\"" \
"check mi -stack-info-depth command, second time"
clean_restart ${binfile}
if ![runto breakpt] {
return -1
}
set test_name "check mi -stack-list-frames command, first time"
send_gdb "interpreter-exec mi \"-stack-list-frames\"\n"
gdb_expect {
-re "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"breakpt\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"func5\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"func4\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"3\",addr=\"$hex\",func=\"func3\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\}\\\]\r\n$gdb_prompt $" {
pass $test_name
}
-re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
xfail $test_name
}
}
gdb_test "interpreter-exec mi \"-stack-list-frames\"" \
"\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"breakpt\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"func5\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"func4\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"3\",addr=\"$hex\",func=\"func3\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\}\\\]" \
"check mi -stack-list-frames command, second time"

View File

@ -0,0 +1,73 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2014 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include <sys/mman.h>
#include <unistd.h>
#include <assert.h>
void *global_invalid_ptr = NULL;
void
func2 (void)
{
/* Replace the current stack pointer and frame pointer with the invalid
pointer. */
asm ("mov %0, %%rsp\n\tmov %0, %%rbp" : : "r" (global_invalid_ptr));
/* Create a label for a breakpoint. */
asm (".global breakpt\nbreakpt:");
}
void
func1 (void *ptr)
{
global_invalid_ptr = ptr;
func2 ();
}
/* Finds and returns an invalid pointer, mmaps in a page, grabs a pointer
to it then unmaps the page again. This is almost certainly "undefined"
behaviour, but should be good enough for this small test program. */
static void *
make_invalid_ptr (void)
{
int page_size, ans;
void *ptr;
page_size = getpagesize ();
ptr = mmap (0, page_size, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS,
-1, 0);
assert (ptr != MAP_FAILED);
ans = munmap (ptr, page_size);
assert (ans == 0);
return ptr;
}
int
main (void)
{
void *invalid_ptr;
invalid_ptr = make_invalid_ptr ();
func1 (invalid_ptr);
return 0;
}

View File

@ -0,0 +1,111 @@
# Copyright (C) 2014 Free Software Foundation, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# In this test we're looking at how gdb handles backtraces and
# investigating the stack depth when confronted with an "invalid" stack,
# that is a stack where the first few frames are normal, and then there's a
# frame where the stack in unreadable.
#
# One interesting bug that has been observed is that gdb will sometime
# exhibit different behaviour the first time a stack command is run
# compared to the second (and later) times a command is run. This is
# because the first time a command is run gdb actually tries to figure out
# the answer, while the second (and later) times gdb relies on the answer
# cached from the first time. As a result in this test each command is
# run twice, and we restart gdb before testing each different command to
# ensure that nothing is being cached.
set opts {}
standard_testfile .c
if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
verbose "Skipping ${testfile}."
return
}
if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} $opts] } {
return -1
}
if ![runto breakpt] {
return -1
}
# Use 'bt no-filters' here as the python filters will raise their own
# error during initialisation, the no-filters case is simpler.
gdb_test "bt no-filters" "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\nCannot access memory at address 0x\[0-9a-f\]+" \
"first backtrace, with error message"
send_gdb "bt no-filters\n"
gdb_expect {
-re "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\nCannot access memory at address 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
# Currently gdb will not display the error message associated with
# the truncated backtrace after the first backtrace has been
# completed. Ideally, we would do this. If this case is ever hit
# then we have started to display the backtrace in all cases and
# the xpass should becomd a pass, and the previous pass case below
# should be removed, or changed to a fail.
xpass "second backtrace, with error message"
}
-re "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\n$gdb_prompt $" {
pass "second backtrace, without error message"
}
timeout {
fail "second backtrace (timeout)"
}
}
clean_restart ${binfile}
if ![runto breakpt] {
return -1
}
set test_name "check mi -stack-info-depth command, first time"
send_gdb "interpreter-exec mi \"-stack-info-depth\"\n"
gdb_expect {
-re "\\^done,depth=\"1\"\r\n$gdb_prompt $" {
pass $test_name
}
-re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
xfail $test_name
}
}
gdb_test "interpreter-exec mi \"-stack-info-depth\"" \
"\\^done,depth=\"1\"" \
"check mi -stack-info-depth command, second time"
clean_restart ${binfile}
if ![runto breakpt] {
return -1
}
set test_name "check mi -stack-list-frames command, first time"
send_gdb "interpreter-exec mi \"-stack-list-frames\"\n"
gdb_expect {
-re "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"func2\"\}\\\]\r\n$gdb_prompt $" {
pass $test_name
}
-re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
xfail $test_name
}
}
gdb_test "interpreter-exec mi \"-stack-list-frames\"" \
"\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"func2\"\}\\\]" \
"check mi -stack-list-frames command, second time"