public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations")
@ 2016-01-20 23:39 Doug Evans
  2016-01-21 10:40 ` Joel Brobecker
  0 siblings, 1 reply; 3+ messages in thread
From: Doug Evans @ 2016-01-20 23:39 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Keith Seitz, gdb-patches

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations")
  2016-01-20 23:39 [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations") Doug Evans
@ 2016-01-21 10:40 ` Joel Brobecker
  0 siblings, 0 replies; 3+ messages in thread
From: Joel Brobecker @ 2016-01-21 10:40 UTC (permalink / raw)
  To: Doug Evans; +Cc: Keith Seitz, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2805 bytes --]

>  > 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.
>  > +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.

I am not sure what my exact thinking was, at the time, and
whether it was flawed or not, but I chose the naming at the time
to be consistent with the other functions that looked similar in
they functionality. The ideal name for the function would probably
be something like "get_address_string_from_location", in other
words, what I meant is to return the "address string" (as opposed
to the address itself).

For now, I pushed the patch as is, but if you think the name
should be changed, here is a patch that does that. So, it's ready
to go in, if you give the signal.

What I would (counter-)propose, on the other hand, is that we
look at renaming the function to use "_from_location". Or, maybe
have the functions start with "location_" (Eg:
location_get_address_string, which actually might be closer
to our typical namespace strategy).

-- 
Joel

[-- Attachment #2: 0001-rename-get_address_string_location-into-get_address_.patch --]
[-- Type: text/x-diff, Size: 2533 bytes --]

From 493d020497093787700e1a055a25f8d6471ea416 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 21 Jan 2016 14:26:33 +0400
Subject: [PATCH] rename get_address_string_location into
 get_address_location_string

The new function name that sounds more correct.

gdb/ChangeLog:

        * location.h, linespec.c, location.c (get_address_location_string):
        Renames get_address_string_location.
---
 gdb/ChangeLog  | 5 +++++
 gdb/linespec.c | 2 +-
 gdb/location.c | 2 +-
 gdb/location.h | 2 +-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a377a32..598ac9d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-01-21  Joel Brobecker  <brobecker@adacore.com>
 
+	* location.h, linespec.c, location.c (get_address_location_string):
+	Renames get_address_string_location.
+
+2016-01-21  Joel Brobecker  <brobecker@adacore.com>
+
 	* location.h (new_address_location): Add new parameters
 	"addr_string" and "addr_string_len".
 	(get_address_string_location): Add declaration.
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 2360cc1..9c8a27a 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2513,7 +2513,7 @@ event_location_to_sals (linespec_parser *parser,
 
     case ADDRESS_LOCATION:
       {
-	const char *addr_string = get_address_string_location (location);
+	const char *addr_string = get_address_location_string (location);
 	CORE_ADDR addr = get_address_location (location);
 
 	if (addr_string != NULL)
diff --git a/gdb/location.c b/gdb/location.c
index e43ebf1..88ae61d 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -138,7 +138,7 @@ get_address_location (const struct event_location *location)
 /* See description in location.h.  */
 
 const char *
-get_address_string_location (const struct event_location *location)
+get_address_location_string (const struct event_location *location)
 {
   gdb_assert (EL_TYPE (location) == ADDRESS_LOCATION);
   return EL_STRING (location);
diff --git a/gdb/location.h b/gdb/location.h
index b2cf45e..2b04def 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -145,7 +145,7 @@ extern CORE_ADDR
    of the given event_location (which must be of type ADDRESS_LOCATION).  */
 
 extern const char *
-  get_address_string_location (const struct event_location *location);
+  get_address_location_string (const struct event_location *location);
 
 /* Create a new probe location.  The return result is malloc'd
    and should be freed with delete_event_location.  */
-- 
2.5.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations")
  2015-12-15 13:40       ` Joel Brobecker
@ 2016-01-17 15:32         ` Joel Brobecker
  0 siblings, 0 replies; 3+ messages in thread
From: Joel Brobecker @ 2016-01-17 15:32 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

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.

Thanks!
-- 
Joel

[-- Attachment #2: 0001-Fix-regression-introduced-in-break-EXPR-by-explicit-.patch --]
[-- Type: text/x-diff, Size: 16498 bytes --]

From 56921ae04d132975387a2b40d376c55fde37bfd3 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 8 Dec 2015 19:04:56 +0100
Subject: [PATCH] Fix regression introduced in "break *<EXPR>" by explicit
 location patches.

A relatively recent patch support for explicit locations, and part
of that patch cleaned up the way we parse breakpoint locations.
Unfortunatly, a small regression crept in for "*<EXPR>" breakpoint
locations. In particular, on PIE programs, one can see the issue by
doing the following, with any program:

    (gdb) b *main
    Breakpoint 1 at 0x51a: file hello.c, line 3.
    (gdb) run
    Starting program: /[...]/hello
    Error in re-setting breakpoint 1: Warning:
    Cannot insert breakpoint 1.
    Cannot access memory at address 0x51a

    Warning:
    Cannot insert breakpoint 1.
    Cannot access memory at address 0x51a

Just for the record, this regression was introduced by:

    commit a06efdd6effd149a1d392df8d62824e44872003a
    Date:   Tue Aug 11 17:09:35 2015 -0700
    Subject: Explicit locations: introduce address locations

What happens is that the patch makes the implicit assumption that
the address computed the first time is static, as if it was designed
to only support litteral expressions (Eg. "*0x1234"). This allows
the shortcut of not re-computing the breakpoint location's address
when re-setting breakpoints.

However, this does not work in general, as demonstrated in the example
above.

This patch plugs that hole simply by saving the original expression
used to compute the address as part of the address location, so as
to then re-evaluate that expression during breakpoint re-set.

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.

---
 gdb/breakpoint.c                          | 15 +++++-
 gdb/config/djgpp/fnchange.lst             |  2 +
 gdb/linespec.c                            | 24 +++++++--
 gdb/location.c                            | 16 +++++-
 gdb/location.h                            | 15 ++++--
 gdb/python/py-finishbreakpoint.c          |  2 +-
 gdb/spu-tdep.c                            |  2 +-
 gdb/testsuite/gdb.base/break-fun-addr.exp | 84 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/break-fun-addr1.c  | 22 ++++++++
 gdb/testsuite/gdb.base/break-fun-addr2.c  | 28 +++++++++++
 10 files changed, 198 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/break-fun-addr.exp
 create mode 100644 gdb/testsuite/gdb.base/break-fun-addr1.c
 create mode 100644 gdb/testsuite/gdb.base/break-fun-addr2.c

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 72da4ef..1c637a7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7766,7 +7766,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.  */
-  b->location = new_address_location (b->loc->address);
+  b->location = new_address_location (b->loc->address, NULL, 0);
 
   update_global_location_list_nothrow (UGLL_MAY_INSERT);
 
@@ -9333,7 +9333,18 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
   if (location != NULL)
     b->location = location;
   else
-    b->location = new_address_location (b->loc->address);
+    {
+      const char *addr_string = NULL;
+      int addr_string_len = 0;
+
+      if (location != NULL)
+	addr_string = event_location_to_string (location);
+      if (addr_string != NULL)
+	addr_string_len = strlen (addr_string);
+
+      b->location = new_address_location (b->loc->address,
+					  addr_string, addr_string_len);
+    }
   b->filter = filter;
 }
 
diff --git a/gdb/config/djgpp/fnchange.lst b/gdb/config/djgpp/fnchange.lst
index 5495308..21a8071 100644
--- a/gdb/config/djgpp/fnchange.lst
+++ b/gdb/config/djgpp/fnchange.lst
@@ -407,6 +407,8 @@
 @V@/gdb/testsuite/gdb.base/bitfields2.c @V@/gdb/testsuite/gdb.base/bitfiel2.c
 @V@/gdb/testsuite/gdb.base/bitfields2.exp @V@/gdb/testsuite/gdb.base/bitfiel2.exp
 @V@/gdb/testsuite/gdb.base/break-entry.exp @V@/gdb/testsuite/gdb.base/brkentry.exp
+@V@/gdb/testsuite/gdb.base/break-fun-addr1.c @V@/gdb/testsuite/gdb.base/b-f-a1.c
+@V@/gdb/testsuite/gdb.base/break-fun-addr2.c @V@/gdb/testsuite/gdb.base/b-f-a2.c
 @V@/gdb/testsuite/gdb.base/coremaker2.c @V@/gdb/testsuite/gdb.base/core2maker.c
 @V@/gdb/testsuite/gdb.base/hashline1.exp @V@/gdb/testsuite/gdb.base/hash1line.exp
 @V@/gdb/testsuite/gdb.base/hashline2.exp @V@/gdb/testsuite/gdb.base/hash2line.exp
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 50951bd..4c7dab3 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2498,9 +2498,27 @@ event_location_to_sals (linespec_parser *parser,
       break;
 
     case ADDRESS_LOCATION:
-      result
-	= convert_address_location_to_sals (PARSER_STATE (parser),
-					    get_address_location (location));
+      {
+	const char *addr_string = get_address_string_location (location);
+	CORE_ADDR addr = get_address_location (location);
+
+	if (addr_string != NULL)
+	  {
+	    char *expr = xstrdup (addr_string);
+	    const char *const_expr = expr;
+	    struct cleanup *cleanup = make_cleanup (xfree, expr);
+
+	    addr = linespec_expression_to_pc (&const_expr);
+	    if (PARSER_STATE (parser)->canonical != NULL)
+	      PARSER_STATE (parser)->canonical->location
+		= copy_event_location (location);
+
+	    do_cleanups (cleanup);
+	  }
+
+	result = convert_address_location_to_sals (PARSER_STATE (parser),
+						   addr);
+      }
       break;
 
     case EXPLICIT_LOCATION:
diff --git a/gdb/location.c b/gdb/location.c
index 37285f3..e43ebf1 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -113,13 +113,16 @@ get_linespec_location (const struct event_location *location)
 /* See description in location.h.  */
 
 struct event_location *
-new_address_location (CORE_ADDR addr)
+new_address_location (CORE_ADDR addr, const char *addr_string,
+		      int addr_string_len)
 {
   struct event_location *location;
 
   location = XCNEW (struct event_location);
   EL_TYPE (location) = ADDRESS_LOCATION;
   EL_ADDRESS (location) = addr;
+  if (addr_string != NULL)
+    EL_STRING (location) = xstrndup (addr_string, addr_string_len);
   return location;
 }
 
@@ -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.  */
+
 struct event_location *
 new_probe_location (const char *probe)
 {
@@ -635,7 +647,7 @@ string_to_event_location (char **stringp,
 
       orig = arg = *stringp;
       addr = linespec_expression_to_pc (&arg);
-      location = new_address_location (addr);
+      location = new_address_location (addr, orig, arg - orig);
       *stringp += arg - orig;
     }
   else
diff --git a/gdb/location.h b/gdb/location.h
index bc53884..b2cf45e 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -126,11 +126,14 @@ extern struct event_location *
 extern const char *
   get_linespec_location (const struct event_location *location);
 
-/* Create a new address location.  The return result is malloc'd
-   and should be freed with delete_event_location.  */
+/* Create a new address location.
+   ADDR is the address corresponding to this event_location.
+   ADDR_STRING, a string of ADDR_STRING_LEN characters, is
+   the expression that was parsed to determine the address ADDR.  */
 
 extern struct event_location *
-  new_address_location (CORE_ADDR addr);
+  new_address_location (CORE_ADDR addr, const char *addr_string,
+			int addr_string_len);
 
 /* Return the address location (a CORE_ADDR) of the given event_location
    (which must be of type ADDRESS_LOCATION).  */
@@ -138,6 +141,12 @@ extern struct event_location *
 extern CORE_ADDR
   get_address_location (const struct event_location *location);
 
+/* Return the expression (a string) that was used to compute the address
+   of the given event_location (which must be of type ADDRESS_LOCATION).  */
+
+extern const char *
+  get_address_string_location (const struct event_location *location);
+
 /* Create a new probe location.  The return result is malloc'd
    and should be freed with delete_event_location.  */
 
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index bff6dba..a0513b5 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -297,7 +297,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
       struct cleanup *back_to;
 
       /* Set a breakpoint on the return address.  */
-      location = new_address_location (get_frame_pc (prev_frame));
+      location = new_address_location (get_frame_pc (prev_frame), NULL, 0);
       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 95ca8ab..bf3b289 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2001,7 +2001,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_address_location (pc);
+  location = new_address_location (pc, NULL, 0);
   back_to = make_cleanup_delete_event_location (location);
   create_breakpoint (get_objfile_arch (objfile), location,
 		     NULL /* cond_string */, -1 /* thread */,
diff --git a/gdb/testsuite/gdb.base/break-fun-addr.exp b/gdb/testsuite/gdb.base/break-fun-addr.exp
new file mode 100644
index 0000000..e8bed3f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-fun-addr.exp
@@ -0,0 +1,84 @@
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# The purpose of this testcase is to verify that, when using a breakpoint
+# location of the form "*<EXPR>" (Eg: "*main"), GDB is able to start
+# the program and stop at the correct location.  With programs built
+# as PIE, this means that GDB needs to re-evaluate the location once
+# the program as started, since PIE ensures that the address of all
+# symbols have changed after load.
+#
+# PIE is not always supported by the target system, so instead of
+# creating a testcase building executables with PIE, this testcase
+# takes a slightly different approach.  It builds a first program,
+# breaks on *main, and then runs to that breakpoint. It then builds
+# a second program, different from the first one, and loads that
+# executable within the same GDB session.  Similarly to the PIE case,
+# the address of main should be different, and therefore GDB should
+# recalculate it.  We verify that by checking that running to that
+# breakpoint still works, and that we land at the first instruction
+# of that function in both cases.
+
+set testfile1 "break-fun-addr1"
+set srcfile1 ${testfile1}.c
+set binfile1 [standard_output_file ${testfile1}]
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" executable {debug}] != "" } {
+    untested "Couldn't compile ${srcfile1}"
+    return -1
+}
+
+# Start the debugger with the first executable, put a breakpoint
+# on the first instruction of function "main" ("*main"), then
+# run to that breakpoint.
+
+clean_restart ${binfile1}
+
+with_test_prefix "${binfile1}" {
+
+    gdb_test "break *main" \
+        "Breakpoint.*at.* file .*$srcfile1, line .*" \
+
+    gdb_run_cmd
+    gdb_test "" \
+             "Breakpoint.* main \\(\\) at .*$srcfile1:.*" \
+             "run to breakpoint at *main"
+
+    # Verify also that we stopped at the start of the function...
+    gdb_test "p \$pc == main" " = 1"
+}
+
+set testfile2 "break-fun-addr2"
+set srcfile2 ${testfile2}.c
+set binfile2 [standard_output_file ${testfile2}]
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug}] != "" } {
+    untested "Couldn't compile ${srcfile2}"
+    return -1
+}
+
+# Now, keeping the same GDB process (so as to keep the same breakpoint),
+# start a new debugging session with a different executable.
+gdb_load ${binfile2}
+
+with_test_prefix "${binfile2}" {
+
+    gdb_run_cmd
+    gdb_test "" \
+             "Breakpoint.* main \\(\\) at .*$srcfile2:.*" \
+             "run to breakpoint at *main"
+
+    gdb_test "p \$pc == main" " = 1"
+}
diff --git a/gdb/testsuite/gdb.base/break-fun-addr1.c b/gdb/testsuite/gdb.base/break-fun-addr1.c
new file mode 100644
index 0000000..1545b21
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-fun-addr1.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/break-fun-addr2.c b/gdb/testsuite/gdb.base/break-fun-addr2.c
new file mode 100644
index 0000000..13eec05
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-fun-addr2.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+compute_something (int i)
+{
+  return i - 1;
+}
+
+int
+main (void)
+{
+  return compute_something (1);
+}
-- 
2.5.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-01-21 10:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-20 23:39 [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations") Doug Evans
2016-01-21 10:40 ` Joel Brobecker
  -- strict thread matches above, loose matches on Subject: below --
2015-08-05 23:28 [PATCH v6 0/9] Series short description Keith Seitz
2015-08-05 23:30 ` [PATCH v6 4/9] Explicit locations: introduce address locations Keith Seitz
2015-12-14  7:11   ` Joel Brobecker
2015-12-14 20:56     ` Keith Seitz
2015-12-15 13:40       ` Joel Brobecker
2016-01-17 15:32         ` [RFA] Fix regression introduced in "break *<EXPR>" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations") Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).