From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Buettner To: Michael Snyder , Jim Blandy Cc: gdb-patches@sources.redhat.com Subject: [PATCH RFA] breakpoint.c: More check_duplicates() changes. Date: Sat, 12 May 2001 01:01:00 -0000 Message-id: <1010512080125.ZM29521@ocotillo.lan> X-SW-Source: 2001-05/msg00267.html I decided to poke about in breakpoint.c a bit more to see if there were any other breakpoint types that needed to be handled specially in check_duplicates(). Recall that until recently, check_duplicates() contained some code at the beginning of the function which looked like this: if (address == 0) /* Watchpoints are uninteresting */ return; check_duplicates() was recently changed to allow zero-valued addresses to be checked for. The above code was changed to now look like this: /* Watchpoints are uninteresting. */ if (bpt->type == bp_watchpoint || bpt->type == bp_hardware_watchpoint || bpt->type == bp_read_watchpoint || bpt->type == bp_access_watchpoint) return; The bad thing about comments is that they often lie about what the code is doing. The above comment isn't telling a blatant lie, but it's also not giving us all of the information that we need in order to correctly transform the code. It turns out that there are several other breakpoint types which were using zero-valued addresses to cause an early return from check_duplicates(). They are: bp_catch_exec bp_longjmp_resume bp_catch_fork bp_catch_vfork Examination of the code which creates a breakpoint of each of these types will reveal that the ``sal'' struct will be zeroed out. (Note too that all of the watchpoint types will also have a zeroed ``sal''.) This means that the original code was also returning early for each of these additional four types in addition to the watchpoint types. The patch below creates a new function called duplicate_okay() and uses this function to effect the early return. I.e, the above code has again been rewritten as follows: if (duplicate_okay (bpt)) return; I removed the half-truth telling comment too. I think the above statement is reasonably self documenting. It turns out that duplicate_okay() is also needed later in the function too for the duplicate removal phase. To understand why it's needed, suppose one or more watchpoints already exist. As noted earlier, these watchpoints will have zero valued addresses. (Their addresses are zero simply because the address field is meaningless, yet has to have some value.) Now suppose that the user adds a breakpoint for address zero. If we don't exclude those breakpoints from consideration which have zero-valued addresses simply because the address field is meaningless, check_duplicates() will end up marking those breakpoints as duplicates of the newly added breakpoint at zero. Okay to commit? * breakpoint.c (duplicate_okay): New function. (check_duplicates): Use duplicate_okay() to determine which types of breakpoints may have duplicates. Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.37 diff -u -p -r1.37 breakpoint.c --- breakpoint.c 2001/05/12 04:08:23 1.37 +++ breakpoint.c 2001/05/12 07:26:50 @@ -3735,6 +3735,27 @@ set_default_breakpoint (int valid, CORE_ default_breakpoint_line = line; } +/* Return true for all breakpoint types which are permitted to have + addresses which may be a duplicate of some other breakpoint. E.g, + watchpoints will always have zero valued addresses and we don't + want check_duplicates() to mark a watchpoint as a duplicate of an + actual breakpoint at address zero. */ + +static int +duplicate_okay (struct breakpoint *bpt) +{ + enum bptype type = bpt->type; + + return type == bp_watchpoint + || type == bp_hardware_watchpoint + || type == bp_read_watchpoint + || type == bp_access_watchpoint + || type == bp_catch_exec + || type == bp_longjmp_resume + || type == bp_catch_fork + || type == bp_catch_vfork; +} + /* Rescan breakpoints at the same address and section as BPT, marking the first one as "first" and any others as "duplicates". This is so that the bpt instruction is only inserted once. @@ -3750,11 +3771,7 @@ check_duplicates (struct breakpoint *bpt CORE_ADDR address = bpt->address; asection *section = bpt->section; - /* Watchpoints are uninteresting. */ - if (bpt->type == bp_watchpoint - || bpt->type == bp_hardware_watchpoint - || bpt->type == bp_read_watchpoint - || bpt->type == bp_access_watchpoint) + if (duplicate_okay (bpt)) return; ALL_BREAKPOINTS (b) @@ -3762,7 +3779,8 @@ check_duplicates (struct breakpoint *bpt && b->enable != shlib_disabled && b->enable != call_disabled && b->address == address - && (overlay_debugging == 0 || b->section == section)) + && (overlay_debugging == 0 || b->section == section) + && !duplicate_okay (b)) { /* Have we found a permanent breakpoint? */ if (b->enable == permanent) @@ -3800,7 +3818,8 @@ check_duplicates (struct breakpoint *bpt && b->enable != shlib_disabled && b->enable != call_disabled && b->address == address - && (overlay_debugging == 0 || b->section == section)) + && (overlay_debugging == 0 || b->section == section) + && !duplicate_okay (b)) b->duplicate = 1; } }