From patchwork Wed Sep 3 19:31:55 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 2640 Received: (qmail 4565 invoked by alias); 3 Sep 2014 19:32:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4527 invoked by uid 89); 3 Sep 2014 19:32:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, FILL_THIS_FORM_SHORT, KAM_STOCKGEN, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 03 Sep 2014 19:31:59 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s83JVuIn008775 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 3 Sep 2014 15:31:56 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s83JVtIP005724 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 3 Sep 2014 15:31:56 -0400 Message-ID: <54076CAB.2090407@redhat.com> Date: Wed, 03 Sep 2014 12:31:55 -0700 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Doug Evans CC: "gdb-patches@sourceware.org ml" Subject: Re: [RFA 4/9] Explicit locations v2 - Add address locations References: <536BC678.1010309@redhat.com> <5388CB81.4030409@redhat.com> <5395F91D.1030905@redhat.com> In-Reply-To: <5395F91D.1030905@redhat.com> X-IsSubscribed: yes On 06/09/2014 11:12 AM, Keith Seitz wrote: > On 06/08/2014 02:58 PM, Doug Evans wrote: >> I can think of two solutions. Maybe there's another even better one. >> >> 1) Have a "placement new"-like constructor akin to the existing >> new_foo_location constructors we have that takes a pointer to the >> object to initialize. >> 2) Always use the existing new_foo_location constructors and free the >> object when done. > As I mentioned, I've implemented #2. Keith Changes since last revision: - Use functions instead of EVENT_LOCATION_* macros. - new_address_location/get_address_location. gdb/ChangeLog: * breakpoint.c (create_thread_event_breakpoint): Convert linespec to address location. (init_breakpoint_sal): Likewise. * linespec.c (canonicalize_linespec): Do not handle address locations here. (convert_address_location_to_sals): New function; contents moved from ... (convert_linespc_to_sals): ... here. (parse_linespec): Remove address locations from linespec grammar. Remove handling of address locations. (event_location_to_sals): Handle ADDRESS_LOCATION. (linespec_expression_to_pc): Export. * linespec.h (linespec_expression_to_pc): Add declaration. * location.c (struct event_location.u)
: New member. (copy_event_location): Handle address locations. (delete_event_location): Likewise. (event_location_to_string): Likewise. (string_to_event_location): Likewise. (event_location_empty_p): Likewise. * location.h (enum event_location_type): Add ADDRESS_LOCATION. (new_address_location, get_address_location): New functions. * python/py-finishbreakpoint.c (bpfinishpy_init): Remove local variable `finish_pc'. Convert linespec to address location. * spu-tdep.c (spu_catch_start): Convert linespec to address location. --- gdb/breakpoint.c | 13 --- gdb/linespec.c | 177 +++++++++++++++++--------------------- gdb/linespec.h | 4 + gdb/location.c | 64 +++++++++++++- gdb/location.h | 17 +++- gdb/python/py-finishbreakpoint.c | 7 -- gdb/spu-tdep.c | 2 7 files changed, 164 insertions(+), 120 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 00046b0..395e0f4 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -7531,7 +7531,6 @@ delete_std_terminate_breakpoint (void) struct breakpoint * create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) { - char *tmp; struct breakpoint *b; b = create_internal_breakpoint (gdbarch, address, bp_thread_event, @@ -7539,9 +7538,7 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) b->enable_state = bp_enabled; /* location has to be used or breakpoint_re_set will delete me. */ - tmp = xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address)); - b->location = new_linespec_location (tmp); - xfree (tmp); + b->location = new_address_location (b->loc->address); update_global_location_list_nothrow (1); @@ -9424,13 +9421,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch, if (location != NULL) b->location = location; else - { - char *tmp = xstrprintf ("*%s", - paddress (b->loc->gdbarch, b->loc->address)); - - b->location = new_linespec_location (tmp); - xfree (tmp); - } + b->location = new_address_location (b->loc->address); b->filter = filter; } diff --git a/gdb/linespec.c b/gdb/linespec.c index 6c2467e..7048d02 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -317,7 +317,7 @@ static void iterate_over_file_blocks (struct symtab *symtab, static void initialize_defaults (struct symtab **default_symtab, int *default_line); -static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr); +CORE_ADDR linespec_expression_to_pc (const char **exp_ptr); static struct symtabs_and_lines decode_objc (struct linespec_state *self, linespec_p ls, @@ -1767,77 +1767,67 @@ static void canonicalize_linespec (struct linespec_state *state, linespec_p ls) { char *tmp; + struct ui_file *buf; + int need_colon = 0; /* If canonicalization was not requested, no need to do anything. */ if (!state->canonical) return; - /* Shortcut expressions, which can only appear by themselves. */ - if (ls->expression != NULL) + buf = mem_fileopen (); + + if (ls->source_filename) { - state->canonical->location - = new_linespec_location (ls->expression); + fputs_unfiltered (ls->source_filename, buf); + need_colon = 1; } - else - { - struct ui_file *buf; - int need_colon = 0; - buf = mem_fileopen (); - - if (ls->source_filename) - { - fputs_unfiltered (ls->source_filename, buf); - need_colon = 1; - } + if (ls->function_name) + { + if (need_colon) + fputc_unfiltered (':', buf); + fputs_unfiltered (ls->function_name, buf); + need_colon = 1; + } - if (ls->function_name) - { - if (need_colon) - fputc_unfiltered (':', buf); - fputs_unfiltered (ls->function_name, buf); - need_colon = 1; - } + if (ls->label_name) + { + if (need_colon) + fputc_unfiltered (':', buf); - if (ls->label_name) + if (ls->function_name == NULL) { - if (need_colon) - fputc_unfiltered (':', buf); - - if (ls->function_name == NULL) - { - struct symbol *s; - - /* No function was specified, so add the symbol name. */ - gdb_assert (ls->labels.function_symbols != NULL - && (VEC_length (symbolp, ls->labels.function_symbols) - == 1)); - s = VEC_index (symbolp, ls->labels.function_symbols, 0); - fputs_unfiltered (SYMBOL_NATURAL_NAME (s), buf); - fputc_unfiltered (':', buf); - } - - fputs_unfiltered (ls->label_name, buf); - need_colon = 1; - state->canonical->special_display = 1; + struct symbol *s; + + /* No function was specified, so add the symbol name. */ + gdb_assert (ls->labels.function_symbols != NULL + && (VEC_length (symbolp, ls->labels.function_symbols) + == 1)); + s = VEC_index (symbolp, ls->labels.function_symbols, 0); + fputs_unfiltered (SYMBOL_NATURAL_NAME (s), buf); + fputc_unfiltered (':', buf); } - if (ls->line_offset.sign != LINE_OFFSET_UNKNOWN) - { - if (need_colon) - fputc_unfiltered (':', buf); - fprintf_filtered (buf, "%s%d", - (ls->line_offset.sign == LINE_OFFSET_NONE ? "" - : (ls->line_offset.sign - == LINE_OFFSET_PLUS ? "+" : "-")), - ls->line_offset.offset); - } + fputs_unfiltered (ls->label_name, buf); + need_colon = 1; + state->canonical->special_display = 1; + } - tmp = ui_file_xstrdup (buf, NULL); - state->canonical->location = new_linespec_location (tmp); - xfree (tmp); - ui_file_delete (buf); + if (ls->line_offset.sign != LINE_OFFSET_UNKNOWN) + { + if (need_colon) + fputc_unfiltered (':', buf); + fprintf_filtered (buf, "%s%d", + (ls->line_offset.sign == LINE_OFFSET_NONE ? "" + : (ls->line_offset.sign + == LINE_OFFSET_PLUS ? "+" : "-")), + ls->line_offset.offset); } + + tmp = ui_file_xstrdup (buf, NULL); + state->canonical->location = new_linespec_location (tmp); + xfree (tmp); + ui_file_delete (buf); } /* Given a line offset in LS, construct the relevant SALs. */ @@ -1991,6 +1981,27 @@ create_sals_line_offset (struct linespec_state *self, return values; } +/* Convert the given ADDRESS into SaLs. */ + +static struct symtabs_and_lines +convert_address_location_to_sals (struct linespec_state *self, + CORE_ADDR address) +{ + struct symtab_and_line sal; + struct symtabs_and_lines sals = {NULL, 0}; + + sal = find_pc_line (address, 0); + sal.pc = address; + sal.section = find_pc_overlay (address); + sal.explicit_pc = 1; + add_sal_to_sals (self, &sals, &sal, core_addr_to_string (address), 1); + + if (self->canonical != NULL) + self->canonical->location = new_address_location (address); + + return sals; +} + /* Create and return SALs from the linespec LS. */ static struct symtabs_and_lines @@ -1998,18 +2009,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) { struct symtabs_and_lines sals = {NULL, 0}; - if (ls->expression != NULL) - { - struct symtab_and_line sal; - - /* We have an expression. No other attribute is allowed. */ - sal = find_pc_line (ls->expr_pc, 0); - sal.pc = ls->expr_pc; - sal.section = find_pc_overlay (ls->expr_pc); - sal.explicit_pc = 1; - add_sal_to_sals (state, &sals, &sal, ls->expression, 1); - } - else if (ls->labels.label_symbols != NULL) + if (ls->labels.label_symbols != NULL) { /* We have just a bunch of functions/methods or labels. */ int i; @@ -2109,8 +2109,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) The basic grammar of linespecs: - linespec -> expr_spec | var_spec | basic_spec - expr_spec -> '*' STRING + linespec -> var_spec | basic_spec var_spec -> '$' (STRING | NUMBER) basic_spec -> file_offset_spec | function_spec | label_spec @@ -2206,33 +2205,7 @@ parse_linespec (linespec_parser *parser, const char **argptr) token = linespec_lexer_lex_one (parser); /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER. */ - if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*') - { - char *expr; - const char *copy; - - /* User specified an expression, *EXPR. */ - copy = expr = copy_token_string (token); - cleanup = make_cleanup (xfree, expr); - PARSER_RESULT (parser)->expr_pc = linespec_expression_to_pc (©); - discard_cleanups (cleanup); - PARSER_RESULT (parser)->expression = expr; - - /* This is a little hacky/tricky. If linespec_expression_to_pc - did not evaluate the entire token, then we must find the - string COPY inside the original token buffer. */ - if (*copy != '\0') - { - PARSER_STREAM (parser) = strstr (parser->lexer.saved_arg, copy); - gdb_assert (PARSER_STREAM (parser) != NULL); - } - - /* Consume the token. */ - linespec_lexer_consume_token (parser); - - goto convert_to_sals; - } - else if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$') + if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$') { char *var; @@ -2454,6 +2427,12 @@ event_location_to_sals (linespec_parser *parser, } break; + case ADDRESS_LOCATION: + result + = convert_address_location_to_sals (PARSER_STATE (parser), + get_address_location (location)); + break; + default: gdb_assert_not_reached ("unhandled event location type"); } @@ -2648,7 +2627,7 @@ initialize_defaults (struct symtab **default_symtab, int *default_line) /* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR, advancing EXP_PTR past any parsed text. */ -static CORE_ADDR +CORE_ADDR linespec_expression_to_pc (const char **exp_ptr) { if (current_program_space->executing_startup) diff --git a/gdb/linespec.h b/gdb/linespec.h index 658f2a1..df5820a 100644 --- a/gdb/linespec.h +++ b/gdb/linespec.h @@ -152,4 +152,8 @@ extern struct symtabs_and_lines decode_line_with_current_source (char *, int); extern struct symtabs_and_lines decode_line_with_last_displayed (char *, int); +/* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR, + advancing EXP_PTR past any parsed text. */ + +extern CORE_ADDR linespec_expression_to_pc (const char **exp_ptr); #endif /* defined (LINESPEC_H) */ diff --git a/gdb/location.c b/gdb/location.c index 8111d7c..9e86107 100644 --- a/gdb/location.c +++ b/gdb/location.c @@ -45,6 +45,10 @@ struct event_location probes. */ char *addr_string; #define EL_LINESPEC(PTR) ((PTR)->u.addr_string) + + /* An address in the inferior. */ + CORE_ADDR address; +#define EL_ADDRESS(PTR) (PTR)->u.address } u; /* Cached string representation of this location. This is used, e.g., to @@ -87,6 +91,28 @@ get_linespec_location (struct event_location *location) /* See description in location.h. */ struct event_location * +new_address_location (CORE_ADDR addr) +{ + struct event_location *location; + + location = XCNEW (struct event_location); + EL_TYPE (location) = ADDRESS_LOCATION; + EL_ADDRESS (location) = addr; + return location; +} + +/* See description in location.h. */ + +CORE_ADDR +get_address_location (struct event_location *location) +{ + gdb_assert (EL_TYPE (location) == ADDRESS_LOCATION); + return EL_ADDRESS (location); +} + +/* See description in location.h. */ + +struct event_location * copy_event_location (const struct event_location *src) { struct event_location *dst; @@ -103,6 +129,10 @@ copy_event_location (const struct event_location *src) EL_LINESPEC (dst) = xstrdup (EL_LINESPEC (src)); break; + case ADDRESS_LOCATION: + EL_ADDRESS (dst) = EL_ADDRESS (src); + break; + default: gdb_assert_not_reached ("unknown event location type"); } @@ -143,6 +173,10 @@ delete_event_location (struct event_location *location) xfree (EL_LINESPEC (location)); break; + case ADDRESS_LOCATION: + /* Nothing to do. */ + break; + default: gdb_assert_not_reached ("unknown event location type"); } @@ -179,6 +213,12 @@ event_location_to_string_const (const struct event_location *location) result = xstrdup (EL_LINESPEC (location)); break; + case ADDRESS_LOCATION: + result + = xstrprintf ("*%s", + core_addr_to_string (EL_ADDRESS (location))); + break; + default: gdb_assert_not_reached ("unknown event location type"); } @@ -206,9 +246,24 @@ string_to_event_location (char **stringp, { struct event_location *location; - location = new_linespec_location (*stringp); - if (*stringp != NULL) - *stringp += strlen (*stringp); + /* First, check if the string is an address location. */ + if (*stringp != NULL && **stringp == '*') + { + const char *arg, *orig; + CORE_ADDR addr; + + orig = arg = *stringp; + addr = linespec_expression_to_pc (&arg); + location = new_address_location (addr); + *stringp += arg - orig; + } + else + { + /* Everything else is a linespec. */ + location = new_linespec_location (*stringp); + if (*stringp != NULL) + *stringp += strlen (*stringp); + } return location; } @@ -224,6 +279,9 @@ event_location_empty_p (const struct event_location *location) /* Linespecs are never "empty." (NULL is a valid linespec) */ return 0; + case ADDRESS_LOCATION: + return 0; + default: gdb_assert_not_reached ("unknown event location type"); } diff --git a/gdb/location.h b/gdb/location.h index 705c83b..14aaffa 100644 --- a/gdb/location.h +++ b/gdb/location.h @@ -28,7 +28,10 @@ struct event_location; enum event_location_type { /* A traditional linespec. */ - LINESPEC_LOCATION + LINESPEC_LOCATION, + + /* An address in the inferior. */ + ADDRESS_LOCATION }; /* Return the type of the given event location. */ @@ -64,6 +67,18 @@ extern struct event_location * extern char * get_linespec_location (struct event_location *location); +/* Create a new address location. The return result is malloc'd + and should be freed with delete_event_location. */ + +extern struct event_location * + new_address_location (CORE_ADDR addr); + +/* Return the address location (a CORE_ADDR) of the given event_location + (which must be of type ADDRESS_LOCATION). */ + +extern CORE_ADDR + get_address_location (struct event_location *location); + /* Free an event location and any associated data. */ extern void delete_event_location (struct event_location *location); diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c index d08e13a..f52b1db 100644 --- a/gdb/python/py-finishbreakpoint.c +++ b/gdb/python/py-finishbreakpoint.c @@ -167,9 +167,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) struct frame_id frame_id; PyObject *internal = NULL; int internal_bp = 0; - CORE_ADDR finish_pc, pc; + CORE_ADDR pc; volatile struct gdb_exception except; - char small_buf[100]; struct symbol *function; if (!PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords, @@ -292,9 +291,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) struct cleanup *back_to; /* Set a breakpoint on the return address. */ - finish_pc = get_frame_pc (prev_frame); - xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string (finish_pc)); - location = new_linespec_location (small_buf); + location = new_address_location (get_frame_pc (prev_frame)); back_to = make_cleanup_delete_event_location (location); create_breakpoint (python_gdbarch, location, NULL, thread, NULL, diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c index e20fea3..e566e9f 100644 --- a/gdb/spu-tdep.c +++ b/gdb/spu-tdep.c @@ -2002,7 +2002,7 @@ spu_catch_start (struct objfile *objfile) /* Use a numerical address for the set_breakpoint command to avoid having the breakpoint re-set incorrectly. */ - location = new_linespec_location (core_addr_to_string (pc)); + location = new_address_location (pc); back_to = make_cleanup_delete_event_location (location); create_breakpoint (get_objfile_arch (objfile), location, NULL /* cond_string */, -1 /* thread */,