This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations")
- From: Doug Evans <dje at google dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: Keith Seitz <keiths at redhat dot com>, gdb-patches at sourceware dot org
- Date: Wed, 20 Jan 2016 23:39:52 +0000
- Subject: Re: [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations")
- Authentication-results: sourceware.org; auth=none
Joel Brobecker writes:
> Hi Keith,
>
> What do you think of the attached patch? There is also a testcase,
> which is slightly different from the scenario that triggered this
> exchange, but which also has the advantage of not requiring PIE,
> which makes the test a little more universal, I think.
>
> gdb/ChangeLog:
>
> * location.h (new_address_location): Add new parameters
> "addr_string" and "addr_string_len".
> (get_address_string_location): Add declaration.
> * location.c (new_address_location): Add new parameters
> "addr_string" and "addr_string_len". If not NULL, store
> a copy of the addr_string in the new location as well.
> (get_address_string_location): New function.
> (string_to_event_location): Update call to new_address_location.
> * linespec.c (event_location_to_sals) <ADDRESS_LOCATION>:
> Save the event location in the parser's state before
> passing it to convert_address_location_to_sals.
> * breakpoint.c (create_thread_event_breakpoint): Update call
> to new_address_location.
> (init_breakpoint_sal): Get the event location's string, if any,
> and use it to update call to new_address_location.
> * python/py-finishbreakpoint.c (bpfinishpy_init):
> Update call to new_address_location.
> * spu-tdep.c (spu_catch_start): Likewise.
>
> * config/djgpp/fnchange.lst: Add entries for
> gdb/testsuite/gdb.base/break-fun-addr1.c and
> gdb/testsuite/gdb.base/break-fun-addr2.c.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/break-fun-addr.exp: New file.
> * gdb.base/break-fun-addr1.c: New file.
> * gdb.base/break-fun-addr2.c: New file.
>
> Tested on x86_64-linux.
Hi.
One nit.
>...
> diff --git a/gdb/location.c b/gdb/location.c
> index 37285f3..e43ebf1 100644
> --- a/gdb/location.c
> +++ b/gdb/location.c
>...
> @@ -134,6 +137,15 @@ get_address_location (const struct event_location
*location)
>
> /* See description in location.h. */
>
> +const char *
> +get_address_string_location (const struct event_location *location)
> +{
> + gdb_assert (EL_TYPE (location) == ADDRESS_LOCATION);
> + return EL_STRING (location);
> +}
> +
> +/* See description in location.h. */
> +
Given that the argument must be an address location,
"get_address_location_string" reads better to me.
LGTM with that change.