From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28443 invoked by alias); 9 Nov 2011 18:24:19 -0000 Received: (qmail 28415 invoked by uid 22791); 9 Nov 2011 18:24:16 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 09 Nov 2011 18:24:01 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pA9INuoi023840 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 9 Nov 2011 13:23:56 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pA9INtRo015179; Wed, 9 Nov 2011 13:23:55 -0500 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id pA9INsXV019418; Wed, 9 Nov 2011 13:23:54 -0500 From: Tom Tromey To: Pedro Alves Cc: gdb-patches@sourceware.org, Ulrich Weigand Subject: Re: RFC: don't set the pspace on ordinary breakpoints References: <201111022021.03286.pedro@codesourcery.com> <201111031601.51221.pedro@codesourcery.com> Date: Wed, 09 Nov 2011 18:24:00 -0000 In-Reply-To: (Tom Tromey's message of "Tue, 08 Nov 2011 13:23:13 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.90 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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: 2011-11/txt/msg00246.txt.bz2 >>>>> "Tom" == Tom Tromey writes: Tom> I *think* the should_be_inserted change is all that was really needed, Tom> after re-reading all the messages in this thread. Let me know what you Tom> think. Here is a refresh of this patch. This one is relative to CVS HEAD. I didn't attempt to test it in isolation. Tom 2011-10-28 Tom Tromey * elfread.c (elf_gnu_ifunc_resolver_return_stop): Allow breakpoint's pspace to be NULL. * breakpoint.h (enum enable_state) : Remove. (struct breakpoint) : Update comment. * breakpoint.c (should_be_inserted): Explicitly check if program space is executing startup. (describe_other_breakpoints): Update. (init_raw_breakpoint): Conditionally set breakpoint's pspace. (disable_breakpoints_before_startup): Change executing_startup earlier. Use location's pspace. (enable_breakpoints_after_startup): Likewise. (init_breakpoint_sal): Don't set breakpoint's pspace. Don't use bp_startup_disabled. (create_breakpoint): Conditionally set breakpoint's pspace. Don't use bp_startup_disabled. (update_global_location_list): Use should_be_inserted. (bkpt_re_set): Update. (prepare_re_set_context): Conditionally switch program space. >From 0315321273c5dad6ec98744439c72e1e00729be7 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Tue, 8 Nov 2011 13:50:12 -0700 Subject: [PATCH 1/2] remove bp_startup_disabled --- gdb/ChangeLog | 21 ++++++++++++++ gdb/breakpoint.c | 77 ++++++++++++++++++++++------------------------------- gdb/breakpoint.h | 12 ++------ gdb/elfread.c | 2 +- 4 files changed, 57 insertions(+), 55 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 8c98bef..8ab09ba 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1568,6 +1568,9 @@ should_be_inserted (struct bp_location *bl) if (!bl->enabled || bl->shlib_disabled || bl->duplicate) return 0; + if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup) + return 0; + /* This is set for example, when we're attached to the parent of a vfork, and have detached from the child. The child is running free, and we expect it to do an exec or exit, at which point the @@ -5313,8 +5316,7 @@ describe_other_breakpoints (struct gdbarch *gdbarch, printf_filtered (" (thread %d)", b->thread); printf_filtered ("%s%s ", ((b->enable_state == bp_disabled - || b->enable_state == bp_call_disabled - || b->enable_state == bp_startup_disabled) + || b->enable_state == bp_call_disabled) ? " (disabled)" : b->enable_state == bp_permanent ? " (permanent)" @@ -5790,9 +5792,11 @@ init_raw_breakpoint (struct breakpoint *b, struct gdbarch *gdbarch, b->loc->address = adjusted_address; b->loc->pspace = sal.pspace; - /* Store the program space that was used to set the breakpoint, for - breakpoint resetting. */ - b->pspace = sal.pspace; + /* Store the program space that was used to set the breakpoint, + except for ordinary breakpoints, which are independent of the + program space. */ + if (bptype != bp_breakpoint && bptype != bp_hardware_breakpoint) + b->pspace = sal.pspace; if (sal.symtab == NULL) b->source_file = NULL; @@ -6933,48 +6937,48 @@ enable_watchpoints_after_interactive_call_stop (void) void disable_breakpoints_before_startup (void) { - struct breakpoint *b; + struct bp_location *loc, **locp_tmp; int found = 0; - ALL_BREAKPOINTS (b) + current_program_space->executing_startup = 1; + + ALL_BP_LOCATIONS (loc, locp_tmp) { - if (b->pspace != current_program_space) + if (loc->pspace != current_program_space) continue; - if ((b->type == bp_breakpoint - || b->type == bp_hardware_breakpoint) - && breakpoint_enabled (b)) + if ((loc->owner->type == bp_breakpoint + || loc->owner->type == bp_hardware_breakpoint) + && breakpoint_enabled (loc->owner)) { - b->enable_state = bp_startup_disabled; found = 1; + break; } } if (found) update_global_location_list (0); - - current_program_space->executing_startup = 1; } void enable_breakpoints_after_startup (void) { - struct breakpoint *b; + struct bp_location *loc, **locp_tmp; int found = 0; current_program_space->executing_startup = 0; - ALL_BREAKPOINTS (b) + ALL_BP_LOCATIONS (loc, locp_tmp) { - if (b->pspace != current_program_space) + if (loc->pspace != current_program_space) continue; - if ((b->type == bp_breakpoint - || b->type == bp_hardware_breakpoint) - && b->enable_state == bp_startup_disabled) + if ((loc->owner->type == bp_breakpoint + || loc->owner->type == bp_hardware_breakpoint) + && breakpoint_enabled (loc->owner)) { - b->enable_state = bp_enabled; found = 1; + break; } } @@ -7215,7 +7219,6 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch, b->ignore_count = ignore_count; b->enable_state = enabled ? bp_enabled : bp_disabled; b->disposition = disposition; - b->pspace = sals.sals[0].pspace; if (type == bp_static_tracepoint) { @@ -7256,11 +7259,6 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch, "tracepoint marker to probe")); } - if (enabled && b->pspace->executing_startup - && (b->type == bp_breakpoint - || b->type == bp_hardware_breakpoint)) - b->enable_state = bp_startup_disabled; - loc = b->loc; } else @@ -7980,14 +7978,10 @@ create_breakpoint (struct gdbarch *gdbarch, b->disposition = tempflag ? disp_del : disp_donttouch; b->condition_not_parsed = 1; b->enable_state = enabled ? bp_enabled : bp_disabled; - b->pspace = current_program_space; + if (type_wanted != bp_breakpoint && type_wanted != bp_hardware_breakpoint) + b->pspace = current_program_space; b->py_bp_object = NULL; - if (enabled && b->pspace->executing_startup - && (b->type == bp_breakpoint - || b->type == bp_hardware_breakpoint)) - b->enable_state = bp_startup_disabled; - if (!internal) /* Do not mention breakpoints with a negative number, but do notify observers. */ @@ -10607,13 +10601,8 @@ update_global_location_list (int should_insert) struct breakpoint *b = loc->owner; struct bp_location **loc_first_p; - if (b->enable_state == bp_disabled - || b->enable_state == bp_call_disabled - || b->enable_state == bp_startup_disabled - || !loc->enabled - || loc->shlib_disabled - || !breakpoint_address_is_meaningful (b) - || is_tracepoint (b)) + if (!should_be_inserted (loc) + || !breakpoint_address_is_meaningful (b)) continue; /* Permanent breakpoint should always be inserted. */ @@ -10891,10 +10880,6 @@ static struct breakpoint_ops base_breakpoint_ops = static void bkpt_re_set (struct breakpoint *b) { - /* Do not attempt to re-set breakpoints disabled during startup. */ - if (b->enable_state == bp_startup_disabled) - return; - /* FIXME: is this still reachable? */ if (b->addr_string == NULL) { @@ -11825,6 +11810,7 @@ addr_string_to_sals (struct breakpoint *b, char *addr_string, int *found) if (e.error == NOT_FOUND_ERROR && (b->condition_not_parsed || (b->loc && b->loc->shlib_disabled) + || (b->loc && b->loc->pspace->executing_startup) || b->enable_state == bp_disabled)) not_found_and_ok = 1; @@ -11913,7 +11899,8 @@ prepare_re_set_context (struct breakpoint *b) input_radix = b->input_radix; cleanups = save_current_space_and_thread (); - switch_to_program_space_and_thread (b->pspace); + if (b->pspace != NULL) + switch_to_program_space_and_thread (b->pspace); set_language (b->language); return cleanups; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index fe381df..94e324a 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -186,14 +186,6 @@ enum enable_state automatically enabled and reset when the call "lands" (either completes, or stops at another eventpoint). */ - bp_startup_disabled, /* The eventpoint has been disabled during - inferior startup. This is necessary on - some targets where the main executable - will get relocated during startup, making - breakpoint addresses invalid. The - eventpoint will be automatically enabled - and reset once inferior startup is - complete. */ bp_permanent /* There is a breakpoint instruction hard-wired into the target's code. Don't try to write another breakpoint @@ -571,7 +563,9 @@ struct breakpoint equals this. */ struct frame_id frame_id; - /* The program space used to set the breakpoint. */ + /* The program space used to set the breakpoint. This is only set + for breakpoints which are specific to a program space; for + ordinary breakpoints this is NULL. */ struct program_space *pspace; /* String we used to set the breakpoint (malloc'd). */ diff --git a/gdb/elfread.c b/gdb/elfread.c index a309a2c..067c77f 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -1032,7 +1032,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b) } gdb_assert (b->type == bp_gnu_ifunc_resolver); - gdb_assert (current_program_space == b->pspace); + gdb_assert (current_program_space == b->pspace || b->pspace == NULL); elf_gnu_ifunc_record_cache (b->addr_string, resolved_pc); sal = find_pc_line (resolved_pc, 0); -- 1.7.6.4