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

Doug Evans dje@google.com
Wed Jan 20 23:39:00 GMT 2016


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.



More information about the Gdb-patches mailing list