From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31594 invoked by alias); 27 Oct 2011 15:21:10 -0000 Received: (qmail 31419 invoked by uid 22791); 27 Oct 2011 15:21:00 -0000 X-SWARE-Spam-Status: No, hits=-7.2 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; Thu, 27 Oct 2011 15:20:41 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p9RFKfEk024705 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 Oct 2011 11:20:41 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p9RFKeII025213; Thu, 27 Oct 2011 11:20:40 -0400 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 p9RFKcHx020223; Thu, 27 Oct 2011 11:20:39 -0400 From: Tom Tromey To: gdb-patches@sourceware.org Subject: RFC: don't set the pspace on ordinary breakpoints Date: Thu, 27 Oct 2011 15:32:00 -0000 Message-ID: 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-10/txt/msg00731.txt.bz2 I would appreciate comments on this patch. This is the second of two patches in preparation for my "ambiguous linespec" patch. I think they are reasonably independent so I am sending them separately. This patch does two things. First, it changes ordinary breakpoints to set leave their 'pspace' field NULL. The rationale for this is that, with the ambiguous linespec patch, a linespec may apply to different program spaces, not just the one which was selected when the breakpoint was created. For these breakpoints, we do not want breakpoint_program_space_exit to remove the breakpoint. I looked at removing the pspace field entirely. Internal breakpoints often set this field, though, and I chose not to touch all this code in hopes of keeping the patch down to a reasonable size. So, I left the field and added a special meaning for NULL. Second, this patch removes bp_startup_disabled. Instead, I changed should_be_inserted to directly examine the location's pspace, and I changed one spot (update_global_location_list) to use should_be_inserted to pick this up. Built and regtested on x86-64 F15, but only when applied on top of the previous patch. Tom 2011-10-26 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. 2011-10-26 Tom Tromey >From b7cf18b3ee2b13ef0855fd196fa242d238b03949 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 7 Oct 2011 10:56:00 -0600 Subject: [PATCH 2/2] don't set the pspace on ordinary breakpoints also remove bp_startup_disabled --- gdb/ChangeLog | 21 +++++++++++++++ gdb/breakpoint.c | 76 ++++++++++++++++++++++-------------------------------- gdb/breakpoint.h | 12 ++------ gdb/elfread.c | 2 +- 4 files changed, 56 insertions(+), 55 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 3ae7508..1c6a43b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1578,6 +1578,9 @@ should_be_inserted (struct bp_location *bl) if (!bl->enabled || bl->shlib_disabled || bl->duplicate) return 0; + if (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 @@ -5323,8 +5326,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)" @@ -5817,9 +5819,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; @@ -6960,48 +6964,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; } } @@ -7241,7 +7245,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) { @@ -7282,11 +7285,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 @@ -8001,14 +7999,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. */ @@ -10625,13 +10619,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. */ @@ -10909,10 +10898,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) { @@ -11931,7 +11916,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 dba5392..e038999 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