This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations")


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]