public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 1/2] Fix running to breakpoint set in inline function by lineno/address
Date: Thu, 28 Jun 2018 14:50:00 -0000	[thread overview]
Message-ID: <20180628145035.24713-1-palves@redhat.com> (raw)
In-Reply-To: <77528608-be2c-5a21-e250-36e7d56ba950@redhat.com>

Commit 61b04dd04ac2 ("Change inline frame breakpoint skipping logic
(fix gdb.gdb/selftest.exp)") caused a GDB crash when you set a
breakpoint by line number in an inline function, and then run to the
breakpoint:

    $ gdb -q test Reading symbols from test...done.
    (gdb) b inline-break.c:32
    Breakpoint 1 at 0x40062f: file inline-break.c, line 32.
    (gdb) run
    Starting program: /[...]/test
    [1]    75618 segmentation fault  /[...]/gdb -q test

The problem occurs because we assume that a bp_location's symbol is
not NULL, which is not true when we set the breakpoint with a linespec
location:

    Program received signal SIGSEGV, Segmentation fault.
    0x00000000006f42bb in stopped_by_user_bp_inline_frame (
        stop_chain=<optimized out>, frame_block=<optimized out>)
        at gdb/inline-frame.c:305
    305		      && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol))
    (gdb) p loc->symbol
    $1 = (const symbol *) 0x0

The same thing happens if you run to a breakpoint set in an inline
function by address:

  (gdb) b *0x40062f
  Breakpoint 3 at 0x40062f: file inline-break.c, line 32.

To fix this, add a null pointer check, to avoid the crash, and make it
so that if there's not symbol for the location, then we present the
stop at the inline function.  This preserves the previous behavior
when e.g., setting a breakpoint by address, with "b *ADDRESS".

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* inline-frame.c (stopped_by_user_bp_inline_frame): Return
	true if the the location has no symbol.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.opt/inline-break.c (func1): Add "break here" marker.
	* gdb.opt/inline-break.exp: Test setting breakpoints by line
	number and address and running to them.
---
 gdb/inline-frame.c                     | 16 +++++++++----
 gdb/testsuite/gdb.opt/inline-break.c   |  2 +-
 gdb/testsuite/gdb.opt/inline-break.exp | 42 ++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index 896b0004e4a..c6caf9d0c6e 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -300,10 +300,18 @@ stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain)
 	  bp_location *loc = s->bp_location_at;
 	  enum bp_loc_type t = loc->loc_type;
 
-	  if ((t == bp_loc_software_breakpoint
-	       || t == bp_loc_hardware_breakpoint)
-	      && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol))
-	    return true;
+	  if (t == bp_loc_software_breakpoint
+	      || t == bp_loc_hardware_breakpoint)
+	    {
+	      /* If the location has a function symbol, check whether
+		 the frame was for that inlined function.  If it has
+		 no function symbol, then assume it is.  I.e., default
+		 to presenting the stop at the innermost inline
+		 function.  */
+	      if (loc->symbol == nullptr
+		  || frame_block == SYMBOL_BLOCK_VALUE (loc->symbol))
+		return true;
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.opt/inline-break.c b/gdb/testsuite/gdb.opt/inline-break.c
index f64a81af939..d4779299fa2 100644
--- a/gdb/testsuite/gdb.opt/inline-break.c
+++ b/gdb/testsuite/gdb.opt/inline-break.c
@@ -29,7 +29,7 @@
 static inline ATTR int
 func1 (int x)
 {
-  return x * 23;
+  return x * 23; /* break here */
 }
 
 /* A non-static inlined function that is called once.  */
diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp
index bae76254905..68b7b89ef31 100644
--- a/gdb/testsuite/gdb.opt/inline-break.exp
+++ b/gdb/testsuite/gdb.opt/inline-break.exp
@@ -256,4 +256,46 @@ foreach_with_prefix func {
 	"breakpoint hit presents stop at breakpointed function"
 }
 
+# Test setting a breakpoint in an inline function by line number and
+# by address, and that GDB presents the stop there.
+
+set line [gdb_get_line_number "break here"]
+
+with_test_prefix "line number" {
+    clean_restart $binfile
+
+    if {![runto main]} {
+	untested "could not run to main"
+	continue
+    }
+
+    # Set the breakpoint by line number, and check that GDB reports
+    # the breakpoint location being the inline function.
+    gdb_breakpoint "$srcfile:$line"
+    gdb_test "continue" "Breakpoint .*, func1 \\(x=1\\) at .*$srcfile:$line.*break here.*" \
+	"breakpoint hit presents stop at inlined function"
+
+    # Save the PC for the following by-address test.
+    set address [get_hexadecimal_valueof "\$pc" "0"]
+}
+
+# Test setting a breakpoint in an inline function by address, and that
+# GDB presents the stop there.
+
+with_test_prefix "address" {
+
+    clean_restart $binfile
+
+    if {![runto main]} {
+	untested "could not run to main"
+	continue
+    }
+
+    # Set the breakpoint by address, and check that GDB reports the
+    # breakpoint location being the inline function.
+    gdb_test "break *$address" ".*Breakpoint .* at $address: file .*$srcfile, line $line."
+    gdb_test "continue" "Breakpoint .*, func1 \\(x=1\\) at .*$srcfile:$line.*break here.*" \
+	"breakpoint hit presents stop at inlined function"
+}
+
 unset -nocomplain results
-- 
2.14.4

  parent reply	other threads:[~2018-06-28 14:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-12 15:06 [PATCH] Ensure captured_main has unique address Tom de Vries
2018-06-12 17:38 ` Change inline frame breakpoint skipping logic (Re: [PATCH] Ensure captured_main has unique address) Pedro Alves
2018-06-14 13:22   ` Tom de Vries
2018-06-19 16:36     ` [pushed] Change inline frame breakpoint skipping logic (fix gdb.gdb/selftest.exp) Pedro Alves
2018-06-25 21:04       ` Joel Brobecker
2018-06-26 19:02         ` Pedro Alves
2018-06-26 22:02           ` Joel Brobecker
2018-06-27 16:28             ` Pedro Alves
2018-06-28 14:48               ` Pedro Alves
2018-06-28 14:50                 ` [PATCH 2/2] "break LINENO/*ADDRESS", inline functions and "info break" output Pedro Alves
2018-06-28 17:42                   ` Joel Brobecker
2018-06-29 18:43                     ` Pedro Alves
2018-06-28 14:50                 ` Pedro Alves [this message]
2018-06-28 17:32                   ` [PATCH 1/2] Fix running to breakpoint set in inline function by lineno/address Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180628145035.24713-1-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).