From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31686 invoked by alias); 27 Oct 2008 18:21:21 -0000 Received: (qmail 31676 invoked by uid 22791); 27 Oct 2008 18:21:19 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 27 Oct 2008 18:20:44 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id BE8582A9695 for ; Mon, 27 Oct 2008 14:20:41 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id C6K9S5qzss2T for ; Mon, 27 Oct 2008 14:20:41 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 109232A9692 for ; Mon, 27 Oct 2008 14:20:41 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 0F688E7ACD; Mon, 27 Oct 2008 11:20:39 -0700 (PDT) Date: Mon, 27 Oct 2008 19:30:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: [commit] Reimplement "catch exec" using bp_catchpoint Message-ID: <20081027182038.GC3955@adacore.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="tKW2IUtsqtDRztdT" Content-Disposition: inline User-Agent: Mutt/1.4.2.2i Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00666.txt.bz2 --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 1985 Hello, This is just code cleanup to remove the need for the bp_catch_exec enum bptype kind, and simplify a little the breakpoint code by using the breakpoint_ops structure. There is one little change of output: exec catchpoints are now displayed as follow: > 5 catchpoint keep y exec > 5 catchpoint keep y exec, program "/[...]/execd-prog" rather than: > 5 catch exec keep y > 5 catch exec keep y program "/[...]/execd-prog" This is identical to what we did for fork/vfork catchpoints. gdb/ 2008-10-27 Joel Brobecker * breakpoint.h (enum bptype): Delete bp_catch_exec. * breakpoint.c (insert_catchpoint): Remove handling for bp_catch_exec breakpoint kinds. (insert_bp_location, update_breakpoints_after_exec, remove_breakpoint) (ep_is_catchpoint, print_it_typical, bpstat_check_location), (bpstat_check_location, bpstat_what, print_one_breakpoint_location) (print_one_breakpoint_location, user_settable_breakpoint) (breakpoint_address_is_meaningful, adjust_breakpoint_address) (allocate_bp_location, mention, breakpoint_re_set_one) (disable_command, enable_command): Likewise. (create_exec_event_catchpoint): Delete. (insert_catch_exec, remove_catch_exec, breakpoint_hit_catch_exec) (print_it_catch_exec, print_one_catch_exec, print_mention_catch_exec): New functions. (catch_exec_breakpoint_ops): New static global. (catch_exec_command_1): Use create_catchpoint instead of create_exec_event_catchpoint to create the exec catchpoint. gdb/testsuite/ 2008-10-27 Joel Brobecker gdb.base/foll-exec.exp: Update the expected output of a couple of "info breakpoints" tests. Tested on x86-linux. No regression. I'll commit in a couple of days unless there are some corrections to be made. -- Joel --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="catch-exec.diff" Content-length: 12756 diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -119,14 +119,6 @@ enum bptype /* These breakpoints are used to implement the "catch unload" command on platforms whose dynamic linkers support such functionality. */ bp_catch_unload, - - /* These are not really breakpoints, but are catchpoints that - implement the "catch fork", "catch vfork" and "catch exec" commands - on platforms whose kernel support such functionality. (I.e., - kernels which can raise an event when a fork or exec occurs, as - opposed to the debugger setting breakpoints on functions named - "fork" or "exec".) */ - bp_catch_exec, }; /* States of enablement of breakpoint. */ diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -791,19 +791,10 @@ insert_catchpoint (struct ui_out *uo, vo struct breakpoint *b = (struct breakpoint *) args; int val = -1; - switch (b->type) - { - case bp_catchpoint: - gdb_assert (b->ops != NULL && b->ops->insert != NULL); - b->ops->insert (b); - break; - case bp_catch_exec: - target_insert_exec_catchpoint (PIDGET (inferior_ptid)); - break; - default: - internal_error (__FILE__, __LINE__, _("unknown breakpoint type")); - break; - } + gdb_assert (b->type == bp_catchpoint); + gdb_assert (b->ops != NULL && b->ops->insert != NULL); + + b->ops->insert (b); } static int @@ -1238,8 +1229,7 @@ Note: automatically using hardware break bpt->inserted = (val != -1); } - else if (bpt->owner->type == bp_catchpoint - || bpt->owner->type == bp_catch_exec) + else if (bpt->owner->type == bp_catchpoint) { struct gdb_exception e = catch_exception (uiout, insert_catchpoint, bpt->owner, RETURN_MASK_ERROR); @@ -1501,13 +1491,6 @@ update_breakpoints_after_exec (void) the catchpoints need to something, we will need to add a new method, and call this method from here. */ continue; - } - - /* Don't delete an exec catchpoint, because else the inferior - won't stop when it ought! */ - if (b->type == bp_catch_exec) - { - continue; } /* bp_finish is a special case. The only way we ought to be able @@ -1691,24 +1674,6 @@ remove_breakpoint (struct bp_location *b return val; b->inserted = (is == mark_inserted); } - else if (b->owner->type == bp_catch_exec - && breakpoint_enabled (b->owner) - && !b->duplicate) - { - val = -1; - switch (b->owner->type) - { - case bp_catch_exec: - val = target_remove_exec_catchpoint (PIDGET (inferior_ptid)); - break; - default: - warning (_("Internal error, %s line %d."), __FILE__, __LINE__); - break; - } - if (val) - return val; - b->inserted = (is == mark_inserted); - } return 0; } @@ -1967,8 +1932,7 @@ ep_is_catchpoint (struct breakpoint *ep) { return (ep->type == bp_catchpoint) || (ep->type == bp_catch_load) - || (ep->type == bp_catch_unload) - || (ep->type == bp_catch_exec); + || (ep->type == bp_catch_unload); /* ??rehrauer: Add more kinds here, as are implemented... */ } @@ -2359,14 +2323,6 @@ print_it_typical (bpstat bs) printf_filtered (_("\nCatchpoint %d (unloaded %s), "), b->number, b->triggered_dll_pathname); - return PRINT_SRC_AND_LOC; - break; - - case bp_catch_exec: - annotate_catchpoint (b->number); - printf_filtered (_("\nCatchpoint %d (exec'd %s), "), - b->number, - b->exec_pathname); return PRINT_SRC_AND_LOC; break; @@ -2788,8 +2744,7 @@ bpstat_check_location (const struct bp_l && b->type != bp_read_watchpoint && b->type != bp_access_watchpoint && b->type != bp_hardware_breakpoint - && b->type != bp_catchpoint - && b->type != bp_catch_exec) /* a non-watchpoint bp */ + && b->type != bp_catchpoint) /* a non-watchpoint bp */ { if (bl->address != bp_addr) /* address doesn't match */ return 0; @@ -2856,10 +2811,6 @@ bpstat_check_location (const struct bp_l return 0; } - if ((b->type == bp_catch_exec) - && !inferior_has_execd (inferior_ptid, &b->exec_pathname)) - return 0; - return 1; } @@ -3385,7 +3336,6 @@ bpstat_what (bpstat bs) bs_class = no_effect; break; case bp_catchpoint: - case bp_catch_exec: if (bs->stop) { if (bs->print) @@ -3565,8 +3515,7 @@ print_one_breakpoint_location (struct br {bp_overlay_event, "overlay events"}, {bp_catchpoint, "catchpoint"}, {bp_catch_load, "catch load"}, - {bp_catch_unload, "catch unload"}, - {bp_catch_exec, "catch exec"} + {bp_catch_unload, "catch unload"} }; static char bpenables[] = "nynny"; @@ -3696,21 +3645,6 @@ print_one_breakpoint_location (struct br { ui_out_text (uiout, "library \""); ui_out_field_string (uiout, "what", b->dll_pathname); - ui_out_text (uiout, "\" "); - } - break; - - case bp_catch_exec: - /* Field 4, the address, is omitted (which makes the columns - not line up too nicely with the headers, but the effect - is relatively readable). */ - if (addressprint) - ui_out_field_skip (uiout, "addr"); - annotate_field (5); - if (b->exec_pathname != NULL) - { - ui_out_text (uiout, "program \""); - ui_out_field_string (uiout, "what", b->exec_pathname); ui_out_text (uiout, "\" "); } break; @@ -3914,7 +3848,6 @@ user_settable_breakpoint (const struct b || b->type == bp_catchpoint || b->type == bp_catch_load || b->type == bp_catch_unload - || b->type == bp_catch_exec || b->type == bp_hardware_breakpoint || b->type == bp_watchpoint || b->type == bp_read_watchpoint @@ -4121,8 +4054,7 @@ set_default_breakpoint (int valid, CORE_ bp_hardware_watchpoint bp_read_watchpoint bp_access_watchpoint - bp_catchpoint - bp_catch_exec */ + bp_catchpoint */ static int breakpoint_address_is_meaningful (struct breakpoint *bpt) @@ -4133,8 +4065,7 @@ breakpoint_address_is_meaningful (struct && type != bp_hardware_watchpoint && type != bp_read_watchpoint && type != bp_access_watchpoint - && type != bp_catchpoint - && type != bp_catch_exec); + && type != bp_catchpoint); } /* Rescan breakpoints at the same address and section as BPT, @@ -4249,8 +4180,7 @@ adjust_breakpoint_address (CORE_ADDR bpa || bptype == bp_hardware_watchpoint || bptype == bp_read_watchpoint || bptype == bp_access_watchpoint - || bptype == bp_catchpoint - || bptype == bp_catch_exec) + || bptype == bp_catchpoint) { /* Watchpoints and the various bp_catch_* eventpoints should not have their addresses modified. */ @@ -4317,7 +4247,6 @@ allocate_bp_location (struct breakpoint break; case bp_watchpoint: case bp_catchpoint: - case bp_catch_exec: loc->loc_type = bp_loc_other; break; default: @@ -4923,31 +4852,68 @@ create_fork_vfork_event_catchpoint (int b->forked_inferior_pid = null_ptid; } -static void -create_exec_event_catchpoint (int tempflag, char *cond_string) -{ - struct symtab_and_line sal; - struct breakpoint *b; - int thread = -1; /* All threads. */ - - init_sal (&sal); - sal.pc = 0; - sal.symtab = NULL; - sal.line = 0; - - b = set_raw_breakpoint (sal, bp_catch_exec); - set_breakpoint_count (breakpoint_count + 1); - b->number = breakpoint_count; - b->cond_string = (cond_string == NULL) ? - NULL : savestring (cond_string, strlen (cond_string)); - b->thread = thread; - b->addr_string = NULL; - b->enable_state = bp_enabled; - b->disposition = tempflag ? disp_del : disp_donttouch; - update_global_location_list (1); - - mention (b); -} +/* Exec catchpoints. */ + +static void +insert_catch_exec (struct breakpoint *b) +{ + target_insert_exec_catchpoint (PIDGET (inferior_ptid)); +} + +static int +remove_catch_exec (struct breakpoint *b) +{ + return target_remove_exec_catchpoint (PIDGET (inferior_ptid)); +} + +static int +breakpoint_hit_catch_exec (struct breakpoint *b) +{ + return inferior_has_execd (inferior_ptid, &b->exec_pathname); +} + +static enum print_stop_action +print_it_catch_exec (struct breakpoint *b) +{ + annotate_catchpoint (b->number); + printf_filtered (_("\nCatchpoint %d (exec'd %s), "), b->number, + b->exec_pathname); + return PRINT_SRC_AND_LOC; +} + +static void +print_one_catch_exec (struct breakpoint *b, CORE_ADDR *last_addr) +{ + /* Field 4, the address, is omitted (which makes the columns + not line up too nicely with the headers, but the effect + is relatively readable). */ + if (addressprint) + ui_out_field_skip (uiout, "addr"); + annotate_field (5); + ui_out_text (uiout, "exec"); + if (b->exec_pathname != NULL) + { + ui_out_text (uiout, ", program \""); + ui_out_field_string (uiout, "what", b->exec_pathname); + ui_out_text (uiout, "\" "); + } +} + +static void +print_mention_catch_exec (struct breakpoint *b) +{ + printf_filtered (_("Catchpoint %d (exec)"), b->number); +} + +static struct breakpoint_ops catch_exec_breakpoint_ops = +{ + insert_catch_exec, + remove_catch_exec, + breakpoint_hit_catch_exec, + print_it_catch_exec, + print_one_catch_exec, + print_mention_catch_exec +}; static int hw_breakpoint_used_count (void) @@ -5155,10 +5121,6 @@ mention (struct breakpoint *b) (b->type == bp_catch_load) ? "load" : "unload", (b->dll_pathname != NULL) ? b->dll_pathname : ""); - break; - case bp_catch_exec: - printf_filtered (_("Catchpoint %d (exec)"), - b->number); break; case bp_until: @@ -6614,7 +6576,7 @@ catch_exec_command_1 (char *arg, int fro /* If this target supports it, create an exec catchpoint and enable reporting of such events. */ - create_exec_event_catchpoint (tempflag, cond_string); + create_catchpoint (tempflag, cond_string, &catch_exec_breakpoint_ops); } static void @@ -7751,7 +7713,6 @@ breakpoint_re_set_one (void *bint) that requests them is unaffected by e.g., new libraries being loaded. */ case bp_catchpoint: - case bp_catch_exec: break; default: @@ -8020,7 +7981,6 @@ disable_command (char *args, int from_tt case bp_catchpoint: case bp_catch_load: case bp_catch_unload: - case bp_catch_exec: case bp_hardware_breakpoint: case bp_watchpoint: case bp_hardware_watchpoint: @@ -8153,7 +8113,6 @@ enable_command (char *args, int from_tty case bp_catchpoint: case bp_catch_load: case bp_catch_unload: - case bp_catch_exec: case bp_hardware_breakpoint: case bp_watchpoint: case bp_hardware_watchpoint: diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp --- a/gdb/testsuite/gdb.base/foll-exec.exp +++ b/gdb/testsuite/gdb.base/foll-exec.exp @@ -217,14 +217,11 @@ proc do_exec_tests {} { # Verify that the catchpoint is mentioned in an "info breakpoints", # and further that the catchpoint mentions no program name. # - send_gdb "info breakpoints\n" - gdb_expect { - -re ".*catch exec.*keep y.*$gdb_prompt $"\ - {pass "info shows catchpoint without exec pathname"} - -re ".*catch exec.*program \"\".*$gdb_prompt $"\ - {fail "info shows catchpoint without exec pathname"} - -re "$gdb_prompt $" {fail "info shows catchpoint without exec pathname"} - timeout {fail "(timeout) info shows catchpoint without exec pathname"} + set msg "info shows catchpoint without exec pathname" + gdb_test_multiple "info breakpoints" $msg { + -re ".*catchpoint.*keep y.*exec\[\n\r\]+$gdb_prompt $" { + pass $msg + } } # DTS CLLbs16760 @@ -248,12 +245,11 @@ proc do_exec_tests {} { # and further that the catchpoint managed to capture the exec'd # program's name. # - send_gdb "info breakpoints\n" - gdb_expect { - -re ".*catch exec .*program \".*${testfile2}\".*$gdb_prompt $"\ - {pass "info shows catchpoint exec pathname"} - -re "$gdb_prompt $" {fail "info shows catchpoint exec pathname"} - timeout {fail "(timeout) info shows catchpoint exec pathname"} + set msg "info shows catchpoint exec pathname" + gdb_test_multiple "info breakpoints" $msg { + -re ".*catchpoint.*keep y.*exec, program \".*${testfile2}\".*$gdb_prompt $" { + pass $msg + } } # Verify that we can continue from the catchpoint, and land in the --tKW2IUtsqtDRztdT--