This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] step over permanent breakpoint
- From: Aleksandar Ristovski <aristovski at qnx dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Tue, 19 Aug 2008 11:37:59 -0400
- Subject: Re: [patch] step over permanent breakpoint
- References: <g7cbe2$k68$1@ger.gmane.org> <20080818110709.GD16894@adacore.com>
Joel Brobecker wrote:
Overall, this looks reasonable. I have a few comments, though. See below.
BTW: I just approved a patch for HP/UX that also deals with permanent
breakpoints. But fortunately, they don't seem to collide at all. See:
http://www.sourceware.org/ml/gdb-patches/2008-08/msg00466.html.
I addressed your suggestions; additionally, I moved new code from
create_breakpoint into a new function (bp_loc_permanent). I think that
makes code clear without additional comments.
Thanks,
Aleksandar Ristovski
QNX Software Systems
2008-08-19 Aleksandar Ristovski <aristovski@qnx.com>
* breakpoint.c (bp_loc_permanent): New function.
(breakpoint_init_inferior): Mark as not inserted only
non-permanent breakpoints.
(create_breakpoint): Check if the location points to a permanent
breakpoint.
(update_breakpoint_locations): Make sure new locations of permanent
breakpoints are properly initialized.
* i386-tdep.c (i386_skip_permanent_breakpoint): New function.
(i386_gdbarch_init): Set gdbarch_skip_permanent_breakpoint.
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.343
diff -u -p -r1.343 breakpoint.c
--- gdb/breakpoint.c 17 Aug 2008 16:58:17 -0000 1.343
+++ gdb/breakpoint.c 19 Aug 2008 15:21:47 -0000
@@ -195,6 +195,8 @@ static int is_hardware_watchpoint (struc
static void insert_breakpoint_locations (void);
+static int bp_loc_permanent (struct bp_location *loc);
+
static const char *
bpdisp_text (enum bpdisp disp)
{
@@ -1743,7 +1745,8 @@ breakpoint_init_inferior (enum inf_conte
struct bp_location *bpt;
ALL_BP_LOCATIONS (bpt)
- bpt->inserted = 0;
+ if (bpt->owner->enable_state != bp_permanent)
+ bpt->inserted = 0;
ALL_BREAKPOINTS_SAFE (b, temp)
{
@@ -3081,7 +3084,8 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
/* We will stop here */
if (b->disposition == disp_disable)
{
- b->enable_state = bp_disabled;
+ if (b->enable_state != bp_permanent)
+ b->enable_state = bp_disabled;
update_global_location_list (0);
}
if (b->silent)
@@ -5120,6 +5124,9 @@ create_breakpoint (struct symtabs_and_li
loc = add_location_to_breakpoint (b, type, &sal);
}
+ if (bp_loc_permanent (loc))
+ make_breakpoint_permanent (b);
+
if (b->cond_string)
{
char *arg = b->cond_string;
@@ -7374,6 +7381,10 @@ update_breakpoint_locations (struct brea
b->line_number = sals.sals[i].line;
}
+ /* Update locationos of permanent breakpoints. */
+ if (b->enable_state == bp_permanent)
+ make_breakpoint_permanent (b);
+
/* If possible, carry over 'disable' status from existing breakpoints. */
{
struct bp_location *e = existing_locations;
@@ -8137,6 +8148,30 @@ single_step_breakpoint_inserted_here_p (
return 0;
}
+/* Determine if the location is pointing to a permanent breakpoint. */
+
+static int
+bp_loc_permanent (struct bp_location *loc)
+{
+ const int READ_SUCCESS = 0;
+ int len;
+ CORE_ADDR addr;
+ const gdb_byte *brk;
+ gdb_byte target_mem[32];
+
+ gdb_assert (loc != NULL);
+
+ addr = loc->address;
+ brk = gdbarch_breakpoint_from_pc (current_gdbarch, &addr, &len);
+
+ if (target_read_memory (loc->address, target_mem, len) == READ_SUCCESS
+ && memcmp (target_mem, brk, len) == 0)
+ return 1;
+
+ return 0;
+}
+
+
/* This help string is used for the break, hbreak, tbreak and thbreak commands.
It is defined as a macro to prevent duplication.
Index: gdb/i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.262
diff -u -p -r1.262 i386-tdep.c
--- gdb/i386-tdep.c 9 Aug 2008 16:27:39 -0000 1.262
+++ gdb/i386-tdep.c 19 Aug 2008 15:21:47 -0000
@@ -2624,6 +2624,17 @@ i386_fetch_pointer_argument (struct fram
return read_memory_unsigned_integer (sp + (4 * (argi + 1)), 4);
}
+static void
+i386_skip_permanent_breakpoint (struct regcache *regcache)
+{
+ CORE_ADDR current_pc = regcache_read_pc (regcache);
+ /* On i386, breakpoint is exactly 1 byte long, so we just
+ adjust the PC in the regcache. */
+ current_pc += 1;
+ regcache_write_pc (regcache, current_pc);
+}
+
+
static struct gdbarch *
i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -2812,6 +2823,9 @@ i386_gdbarch_init (struct gdbarch_info i
if (tdep->mm0_regnum == 0)
tdep->mm0_regnum = gdbarch_num_regs (gdbarch);
+ set_gdbarch_skip_permanent_breakpoint (gdbarch,
+ i386_skip_permanent_breakpoint);
+
return gdbarch;
}
2008-08-19 Aleksandar Ristovski <aristovski@qnx.com>
* gdb.arch/i386-bp_permanent.exp: New test.
# Copyright (C) 2008 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/>.
# This file is part of the gdb testsuite.
if $tracelevel {
strace $tracelevel
}
# Test stepping over permanent breakpoints on i386.
if ![istarget "i?86-*-*"] then {
verbose "Skipping skip over permanent breakpoint on i386 tests."
return
}
set testfile "i386-prologue"
set srcfile ${testfile}.c
set binfile ${objdir}/${subdir}/${testfile}
# some targets have leading underscores on assembly symbols.
# TODO: detect this automatically
set additional_flags ""
if [istarget "i?86-*-cygwin*"] then {
set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
}
# Don't use "debug", so that we don't have line information for the assembly
# fragments.
if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list $additional_flags]] != "" } {
untested i386-prologue.exp
return -1
}
gdb_exit
gdb_start
gdb_reinitialize_dir $srcdir/$subdir
gdb_load ${binfile}
#
# Run to `main' where we begin our tests.
#
if ![runto_main] then {
return -1
}
set pattern_matched 0
set function standard
set retcode [gdb_test_multiple "disassemble $function" "Disassemble function '$function'" {
-re ".*($hex) <$function\\+0>.*($hex) <$function\\+4>.*($hex) <$function\\+5>.*($hex) <$function\\+6>.*" {
set function_start $expect_out(1,string);
set address $expect_out(2,string);
set address1 $expect_out(3,string);
set address2 $expect_out(4,string);
}
}]
if {$retcode != $pattern_matched} {
fail "Disassemble failed, skipping entire test."
return -1
}
gdb_breakpoint "*$function_start"
gdb_breakpoint "*$address"
gdb_test "continue" "Breakpoint .*, $function_start in $function.*" \
"Stop at the '$function' start breakpoint (fetching esp)."
# We want to fetch esp at the start of '$function' function to make sure
# skip_permanent_breakpoint implementation really skips only the perm.
# breakpoint. If, for whatever reason, 'leave' instruction doesn't get
# executed, esp will not have this value.
set start_esp 0
gdb_test_multiple "print \$esp\n" "Fetch esp value." {
-re ".1.*($hex).*" {
set start_esp $expect_out(1,string);
}
}
gdb_test "continue" "Breakpoint .*, $address in $function.*" \
"Stop at permanent breakpoint."
gdb_test "stepi" "$address1|$address2 in $function.*" \
"Single stepping past permanent breakpoint."
gdb_test "print \$esp" ".*$start_esp.*" \
"ESP value does not match - step_permanent_breakpoint wrong."