From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Tromey To: Insight List Subject: Patch: saving breakpoints Date: Fri, 11 May 2001 18:47:00 -0000 Message-id: <877kzn72bx.fsf@creche.redhat.com> X-SW-Source: 2001-q2/msg00237.html I've wanted Insight to save breakpoint information in sessions for a long time now. Tonight I blew off my other volunteer hacking (writing java.awt or working on the automake 1.5 release) to do it. First a note about saving breakpoints. I believe that it makes the most sense to save the breakpoint as the user typed it. If I type `b function_name', and then go off and hack, when I come back I expect the breakpoint to be on function_name -- not some random source location which corresponds only to where `function_name' used to be. For breakpoints I set in Insight I expect the location to be the file/line number at which I set them in the GUI. So, this is what I implemented. I know this area is a bit controversial -- Fernando and I have disagreed about it before. I still think this makes sense though. I think we all agree that, since sources can change in any way, there is no perfect approach to saving breakpoints, only heuristics with varying degrees of accuracy. Anyway, I implemented what I think is most in line with user expectation. I'm willing to discuss it though. Second, this patch is not perfect. However, the flaws are unlikely to be noticeable by the casual user, and furthermore they are fixable without requiring a major revamp of the breakpoint-saving machinery. That is, the fixes to the problems I know of can be done incrementally, and I think this patch unambiguously represents forward motion (with one possible exception, see the last bullet). Some problems: * We don't handle the breakpoint language. This info isn't returned by gdb_get_breakpoint_info, and by the time I realized we might want it I didn't feel like going back and adding it. Plus at this point I think it would be more appropriate to change gdb_get_breakpoint_info to return something other than a huge list. * We don't handle the input radix when setting the breakpoint condition. To me it seems like there is some real confusion here. It seems to me that the condition and the breakpoint commands should each hold their own language and input radix state, neither of which is done right now. This change is large enough that I didn't even consider it. * (I just noticed this and it is only tangentially related: we should save signal-handling info too. I don't use this feature much so it is no suprise it slipped my mind.) * I added new uses of gdb_cmd. I know this is wrong. Unfortunately fixing this means writing a a new command in gdbtk-bp.c, since the existing bp commands are still insufficient. This is a real problem. If we want to require no new gdb_cmd, that is fine, but it will probably be some time before I can work on this again. In that case I'll just save it in my patch repository until the next time. The deeper problem here is that as the new Tcl commands are written, they are done in an ad hoc way. This means all kinds of new functionality require new C code (or a change in the C code and corresponding massive sed-like changes on the Tcl source -- as in this patch). To me it seems like every time we do this we're increasing the maintenance problem and making the C code harder to follow. Some automated wrapper solution would be nice. Yeah, I realize this is huge. Is this ok to commit? 2001-05-11 Tom Tromey * library/session.tcl (session_save): Save breakpoints. (SESSION_serialize_bps): New proc. (SESSION_recreate_bps): New proc. (session_load): Recreate breakpoints. * library/util.tcl (bp_exists): Expect user specification in breakpoint info. * library/srctextwin.itb (SrcTextWin::showBPBalloon): Expect user specification in breakpoint info. * library/gdbevent.itb (BreakpointEvent::_init): Initialize _user_specification. (BreakpointEvent::get): Handle user_specification. * library/gdbevent.ith (BreakpointEvent): Added _user_specification field. * library/bpwin.itb (BpWin::bp_store): Expect user specification and use it when saving. (BpWin::bp_type): Expect user specification. * generic/gdbtk-bp.c (BREAKPOINT_IS_WATCHPOINT): New macro. (gdb_get_breakpoint_info): Added `user specification' to result. Tom Index: generic/gdbtk-bp.c =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-bp.c,v retrieving revision 1.5 diff -u -r1.5 gdbtk-bp.c --- gdbtk-bp.c 2001/05/11 20:01:57 1.5 +++ gdbtk-bp.c 2001/05/12 01:27:59 @@ -61,6 +61,13 @@ || (bp)->type == bp_read_watchpoint \ || (bp)->type == bp_access_watchpoint) +/* Is this breakpoint a watchpoint? */ +#define BREAKPOINT_IS_WATCHPOINT(bp) \ +((bp)->type == bp_watchpoint \ + || (bp)->type == bp_hardware_watchpoint \ + || (bp)->type == bp_read_watchpoint \ + || (bp)->type == bp_access_watchpoint) + /* * These are routines we need from breakpoint.c. * at some point make these static in breakpoint.c and move GUI code there @@ -279,7 +286,7 @@ * Tcl Result: * A list with {file, function, line_number, address, type, enabled?, * disposition, ignore_count, {list_of_commands}, - * condition, thread, hit_count} + * condition, thread, hit_count user_specification} */ static int gdb_get_breakpoint_info (ClientData clientData, Tcl_Interp *interp, int objc, @@ -355,6 +362,11 @@ Tcl_NewIntObj (b->thread)); Tcl_ListObjAppendElement (NULL, result_ptr->obj_ptr, Tcl_NewIntObj (b->hit_count)); + + Tcl_ListObjAppendElement (NULL, result_ptr->obj_ptr, + Tcl_NewStringObj (BREAKPOINT_IS_WATCHPOINT (b) + ? b->exp_string + : b->addr_string, -1)); return TCL_OK; } Index: library/bpwin.itb =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/library/bpwin.itb,v retrieving revision 1.5 diff -u -r1.5 bpwin.itb --- bpwin.itb 2001/04/19 22:51:02 1.5 +++ bpwin.itb 2001/05/12 01:28:00 @@ -298,18 +298,21 @@ foreach breakpoint [gdb_get_breakpoint_list] { # This is an lassign foreach {file function line_no address type \ - enable_p disp ignore cmds thread hit_count} \ + enable_p disp ignore cmds thread hit_count user_spec} \ [gdb_get_breakpoint_info $breakpoint] { break } - if {$file != ""} { + if {$user_spec != ""} { + set bp_specifier $user_spec + } elseif {$file != ""} { set filename [file tail $file] set bp_specifier $filename:$line_no } else { set bp_specifier *$address } + # FIXME: doesn't handle watchpoints. if {[string compare $disp "delete"] == 0} { puts $outH "tbreak $bp_specifier" } else { @@ -542,7 +545,7 @@ #debug "bp_type $i $bpnum" set bpinfo [gdb_get_breakpoint_info $bpnum] lassign $bpinfo file func line pc type enabled disposition \ - ignore_count commands cond thread hit_count + ignore_count commands cond thread hit_count user_spec bp_select $i switch $disposition { delete { Index: library/gdbevent.itb =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/library/gdbevent.itb,v retrieving revision 1.2 diff -u -r1.2 gdbevent.itb --- gdbevent.itb 2001/04/20 17:20:02 1.2 +++ gdbevent.itb 2001/05/12 01:28:00 @@ -31,8 +31,9 @@ condition { return $_condition } thread { return $_thread } hit_count { return $_hit_count } + user_specification { return $_user_specification } - default { error "unknown event data \"$what\": should be: action|number|file|function|line|address|type|enabled|disposition|ignore_count|commands|condition|thread|hit_count" } + default { error "unknown event data \"$what\": should be: action|number|file|function|line|address|type|enabled|disposition|ignore_count|commands|condition|thread|hit_count|user_specification" } } } @@ -53,6 +54,7 @@ set _condition {} set _thread {} set _hit_count {} + set _user_specification {} } else { lassign $bpinfo \ _file \ @@ -66,7 +68,8 @@ _commands \ _condition \ _thread \ - _hit_count + _hit_count \ + _user_specification } } Index: library/gdbevent.ith =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/library/gdbevent.ith,v retrieving revision 1.2 diff -u -r1.2 gdbevent.ith --- gdbevent.ith 2001/04/20 17:20:02 1.2 +++ gdbevent.ith 2001/05/12 01:28:00 @@ -40,6 +40,8 @@ # condition .... BP condition # thread ....... thread in which BP is set (or -1 for all threads) # hit_count .... number of times BP has been hit +# user_specification +# .. text the user initially used to set this breakpoint class BreakpointEvent { inherit GDBEvent @@ -71,6 +73,7 @@ private variable _condition {} private variable _thread {} private variable _hit_count {} + private variable _user_specification {} private method _init {} } Index: library/session.tcl =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/library/session.tcl,v retrieving revision 1.6 diff -u -r1.6 session.tcl --- session.tcl 2001/04/18 17:44:00 1.6 +++ session.tcl 2001/05/12 01:28:01 @@ -11,6 +11,91 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. +# An internal function used when saving sessions. Returns a string +# that can be used to recreate all pertinent breakpoint state. +proc SESSION_serialize_bps {} { + set result {} + + foreach bp_num [gdb_get_breakpoint_list] { + lassign [gdb_get_breakpoint_info $bp_num] file function line_number \ + address type enabled disposition ignore_count command_list \ + condition thread hit_count user_specification + + switch -glob -- $type { + "breakpoint" - + "hw breakpoint" { + if {$disposition == "delete"} { + set cmd tbreak + } else { + set cmd break + } + + append cmd " " + if {$user_specification != ""} { + append cmd "$user_specification" + } elseif {$file != ""} { + # BpWin::bp_store uses file tail here, but I think that is + # wrong. + append cmd "$file:$line_number" + } else { + append cmd "*$address" + } + } + "watchpoint" - + "hw watchpoint" { + set cmd watch + if {$user_specification != ""} { + append cmd " $user_specification" + } else { + # There's nothing sensible to do. + continue + } + } + + "catch*" { + # FIXME: Don't know what to do. + continue + } + + default { + # Can't serialize anything other than those listed above. + continue + } + } + + lappend result [list $cmd $enabled $condition $command_list] + } + + return $result +} + +# An internal function used when loading sessions. It takes a +# breakpoint string and recreates all the breakpoints. +proc SESSION_recreate_bps {specs} { + foreach spec $specs { + lassign $spec create enabled condition commands + + # Create the breakpoint + gdb_cmd $create + + # Below we use `\$bpnum'. This means we don't have to figure out + # the number of the breakpoint when doing further manipulations. + + if {! $enabled} { + gdb_cmd "disable \$bpnum" + } + + if {$condition != ""} { + gdb_cmd "cond \$bpnum $condition" + } + + if {[llength $commands]} { + lappend commands end + gdb_cmd "commands \$bpnum\n[join $commands \n]" + } + } +} + # # This procedure decides what makes up a gdb `session'. Roughly a # session is whatever the user found useful when debugging a certain @@ -39,6 +124,9 @@ set values(pwd) $gdb_current_directory set values(target) $gdb_target_name + # Breakpoints. + set values(breakpoints) [SESSION_serialize_bps] + # Recompute list of recent sessions. Trim to no more than 5 sessions. set recent [concat [list $name] \ [lremove [pref getd gdb/recent-projects] $name]] @@ -86,6 +174,10 @@ gdb_clear_file set_exe_name $values(executable) set_exe + } + + if {[info exists values(breakpoints)]} { + SESSION_recreate_bps $values(breakpoints) } if {[info exists values(target)]} { Index: library/srctextwin.itb =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/library/srctextwin.itb,v retrieving revision 1.24 diff -u -r1.24 srctextwin.itb --- srctextwin.itb 2001/04/20 18:47:33 1.24 +++ srctextwin.itb 2001/05/12 01:28:03 @@ -2312,7 +2312,7 @@ foreach b $bps { set bpinfo [gdb_get_breakpoint_info $b] lassign $bpinfo file func linenum addr type enabled disposition \ - ignore_count commands cond thread hit_count + ignore_count commands cond thread hit_count user_specification if {$thread == "-1"} {set thread "all"} set file [lindex [file split $file] end] if {$enabled} { Index: library/util.tcl =================================================================== RCS file: /cvs/src/src/gdb/gdbtk/library/util.tcl,v retrieving revision 1.6 diff -u -r1.6 util.tcl --- util.tcl 2001/05/03 18:13:21 1.6 +++ util.tcl 2001/05/12 01:28:04 @@ -175,7 +175,7 @@ foreach bpnum $bps { set bpinfo [gdb_get_breakpoint_info $bpnum] lassign $bpinfo file func line pc type enabled disposition \ - ignore_count commands cond thread hit_count + ignore_count commands cond thread hit_count user_specification if {$filename == $file && $function == $func && $addr == $pc} { return $bpnum }