Change inline frame breakpoint skipping logic (fix gdb.gdb/selftest.exp)
authorPedro Alves <palves@redhat.com>
Tue, 19 Jun 2018 15:30:13 +0000 (16:30 +0100)
committerPedro Alves <palves@redhat.com>
Tue, 19 Jun 2018 15:30:13 +0000 (16:30 +0100)
Currently, gdb.gdb/selftest.exp fails if you build GDB with
optimization (-O2, etc.).

The reason is that after setting a breakpoint in captured_main, we
stop at:
 ...
 Breakpoint 1, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
 ...
while selftest_setup expects a stop at captured_main.

Here, captured_main_1 has been inlined into captured_main, and
captured_main has been inlined into gdb_main:

 ...
 $ nm ./build/gdb/gdb | egrep ' [tT] .*captured_main|gdb_main' | c++filt
 000000000061b950 T gdb_main(captured_main_args*)
 ...

Indeed, the two inlined functions show up in the backtrace:

 ...
 (gdb) bt
 #0  captured_main_1 (context=<optimized out>) at main.c:492
 #1  captured_main (data=<optimized out>) at main.c:1147
 #2  gdb_main (args=args@entry=0x7fffffffdb80) at main.c:1173
 #3  0x000000000040fea5 in main (argc=<optimized out>, argv=<optimized out>)
     at gdb.c:32
 ...

We're now stopping at captured_main_1 because commit ddfe970e6bec
("Don't elide all inlined frames") makes GDB present a stop at the
innermost inlined frame if the program stopped by a user breakpoint.

Now, the selftest.exp testcase explicitly asks to stop at
"captured_main", not "captured_main_1", so I'm thinking that it's
GDB'S behavior that should be improved.  That is what this commit
does, by only showing a stop at an inline frame if the user breakpoint
was set in that frame's block.

Before this commit:

 (top-gdb) b captured_main
 Breakpoint 1 at 0x792f99: file src/gdb/main.c, line 492.
 (top-gdb) r
 Starting program: build/gdb/gdb

 Breakpoint 1, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
 492       lim_at_start = (char *) sbrk (0);
 (top-gdb)

After this commit, we now instead get:

 (top-gdb) b captured_main
 Breakpoint 1 at 0x791339: file src/gdb/main.c, line 492.
 (top-gdb) r
 Starting program: build/gdb/gdb

 Breakpoint 1, captured_main (data=<optimized out>) at src/gdb/main.c:1147
 1147      captured_main_1 (context);
 (top-gdb)

and:

 (top-gdb) b captured_main_1
 Breakpoint 2 at 0x791339: file src/gdb/main.c, line 492.
 (top-gdb) r
 Starting program: build/gdb/gdb
 Breakpoint 2, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
 492       lim_at_start = (char *) sbrk (0);
 (top-gdb)

Note that both captured_main and captured_main_1 resolved to the same
address, 0x791339.  That is necessary to trigger the issue in
question.  The gdb.base/inline-break.exp testcase currently does not
exercise that, but the new test added by this commit does.  That new
test fails without the GDB fix and passes with the fix.  No
regressions on x86-64 GNU/Linux.

While at it, the THIS_PC comparison in stopped_by_user_bp_inline_frame
is basically a nop, so just remove it -- if a software or hardware
breakpoint explains the stop, then it must be that it was installed at
the current PC.

gdb/ChangeLog:
2018-06-19  Pedro Alves  <palves@redhat.com>

* inline-frame.c (stopped_by_user_bp_inline_frame): Replace PC
parameter with a block parameter.  Compare location's block symbol
with the frame's block instead of addresses.
(skip_inline_frames): Pass the current block instead of the
frame's address.  Break out as soon as we determine the frame
should not be skipped.

gdb/testsuite/ChangeLog:
2018-06-19  Pedro Alves  <palves@redhat.com>

* gdb.opt/inline-break.c (func_inline_callee, func_inline_caller)
(func_extern_caller): New.
(main): Call func_extern_caller.
* gdb.opt/inline-break.exp: Add tests for inline frame skipping
logic change.

gdb/ChangeLog
gdb/inline-frame.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.opt/inline-break.c
gdb/testsuite/gdb.opt/inline-break.exp

index 7b108bf5b5d452df7de34f06938eaf4f9322a50d..93703fdcb35d919d5ca6a6afcda41fef2a05618f 100644 (file)
@@ -1,3 +1,12 @@
+2018-06-19  Pedro Alves  <palves@redhat.com>
+
+       * inline-frame.c (stopped_by_user_bp_inline_frame): Replace PC
+       parameter with a block parameter.  Compare location's block symbol
+       with the frame's block instead of addresses.
+       (skip_inline_frames): Pass the current block instead of the
+       frame's address.  Break out as soon as we determine the frame
+       should not be skipped.
+
 2018-06-18  Tom Tromey  <tom@tromey.com>
 
        * solib-aix.c (solib_aix_get_section_offsets): Return
index 1ac5835438da4577fb45bea560b7e03296824d63..3edd5b2b20b1728288a7f36ec7eb718c26c57ebb 100644 (file)
@@ -286,11 +286,10 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block)
 }
 
 /* Loop over the stop chain and determine if execution stopped in an
-   inlined frame because of a user breakpoint.  THIS_PC is the current
-   frame's PC.  */
+   inlined frame because of a user breakpoint set at FRAME_BLOCK.  */
 
 static bool
-stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain)
+stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain)
 {
   for (bpstat s = stop_chain; s != NULL; s = s->next)
     {
@@ -301,9 +300,9 @@ stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain)
          bp_location *loc = s->bp_location_at;
          enum bp_loc_type t = loc->loc_type;
 
-         if (loc->address == this_pc
-             && (t == bp_loc_software_breakpoint
-                 || t == bp_loc_hardware_breakpoint))
+         if ((t == bp_loc_software_breakpoint
+              || t == bp_loc_hardware_breakpoint)
+             && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol))
            return true;
        }
     }
@@ -340,12 +339,12 @@ skip_inline_frames (ptid_t ptid, bpstat stop_chain)
                {
                  /* Do not skip the inlined frame if execution
                     stopped in an inlined frame because of a user
-                    breakpoint.  */
-                 if (!stopped_by_user_bp_inline_frame (this_pc, stop_chain))
-                   {
-                     skip_count++;
-                     last_sym = BLOCK_FUNCTION (cur_block);
-                   }
+                    breakpoint for this inline function.  */
+                 if (stopped_by_user_bp_inline_frame (cur_block, stop_chain))
+                   break;
+
+                 skip_count++;
+                 last_sym = BLOCK_FUNCTION (cur_block);
                }
              else
                break;
index 358474b1e33d19440d364bbdcfd7668848049ef1..a8f99b8c700155054e1a20b09fa45cb1a72d8093 100644 (file)
@@ -1,3 +1,11 @@
+2018-06-19  Pedro Alves  <palves@redhat.com>
+
+       * gdb.opt/inline-break.c (func_inline_callee, func_inline_caller)
+       (func_extern_caller): New.
+       (main): Call func_extern_caller.
+       * gdb.opt/inline-break.exp: Add tests for inline frame skipping
+       logic change.
+
 2018-06-18  Weimin Pan  <weimin.pan@oracle.com>
 
        PR gdb/16841
index 922102debb696791f53e32d11c4a3e2d8ef5082b..f64a81af93904452a73537691c9ef75b50b406c6 100644 (file)
@@ -176,6 +176,38 @@ not_inline_func3 (int x)
   return y + inline_func3 (x);
 }
 
+/* The following three functions serve to exercise GDB's inline frame
+   skipping logic when setting a user breakpoint on an inline function
+   by name.  */
+
+/* A static inlined function that is called by another static inlined
+   function.  */
+
+static inline ATTR int
+func_inline_callee (int x)
+{
+  return x * 23;
+}
+
+/* A static inlined function that calls another static inlined
+   function.  The body of the function is as simple as possible so
+   that both functions are inlined to the same PC address.  */
+
+static inline ATTR int
+func_inline_caller (int x)
+{
+  return func_inline_callee (x);
+}
+
+/* An extern not-inline function that calls a static inlined
+   function.  */
+
+int
+func_extern_caller (int x)
+{
+  return func_inline_caller (x);
+}
+
 /* Entry point.  */
 
 int
@@ -205,5 +237,7 @@ main (int argc, char *argv[])
 
   x = not_inline_func3 (-21);
 
+  func_extern_caller (1);
+
   return x;
 }
index 008ff1ac33ac0cf2d9601eff6b04126a8961466f..bae7625490584c6a91c3493b2b2fab75170521c8 100644 (file)
@@ -231,4 +231,29 @@ foreach_with_prefix cmd [list "break" "tbreak"] {
     }
 }
 
+# func_extern_caller calls func_inline_caller which calls
+# func_inline_callee.  The latter two are both inline functions.  Test
+# that setting a breakpoint on each of the functions reports a stop at
+# that function.  This exercises the inline frame skipping logic.  If
+# we set a breakpoint at function A, we want to present the stop at A,
+# even if A's entry code is an inlined call to another inline function
+# B.
+
+foreach_with_prefix func {
+    "func_extern_caller"
+    "func_inline_caller"
+    "func_inline_callee"
+} {
+    clean_restart $binfile
+
+    if {![runto main]} {
+       untested "could not run to main"
+       continue
+    }
+
+    gdb_breakpoint $func
+    gdb_test "continue" "Breakpoint .* $func .*at .*$srcfile.*" \
+       "breakpoint hit presents stop at breakpointed function"
+}
+
 unset -nocomplain results