[PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

This patch set changes the current behavior of breakpoints placed on
while loops. (It actually restores past behavior; see below.)

Consider the following code:

7         v = 0;
8
9         while (v < 3)                         /* Loop 1 condition */
10          {
11            v++;                              /* Loop 1 increment */
12          }

This example is taken from the new test case, loop-break.c.

Let's place breakpoints at lines 9 and 11:

(gdb) b 9
Breakpoint 1 at 0x4005af: file gdb.base/loop-break.c, line 9.
(gdb) b 11
Breakpoint 2 at 0x4005a0: file gdb.base/loop-break.c, line 11.

We'll run the program and then continue to get to breakpoint #2:

(gdb) run
Starting program: gdb.base/loop-break

Breakpoint 1, loop_test ()
    at gdb.base/loop-break.c:9
9         while (v < 3)                         /* Loop 1 condition */
(gdb) c
Continuing.

Breakpoint 2, loop_test ()
    at gdb.base/loop-break.c:11
11            v++;                              /* Loop 1 increment */

So far, so good.  Now, watch what happens when we continue again:

(gdb) c
Continuing.

Breakpoint 2, loop_test ()
    at /ironwood1/sourceware-git/mesquite-native-5509943/bld/../../binutils-gdb/gdb/testsuite/gdb.base/loop-break.c:11
11            v++;                              /* Loop 1 increment */

GDB has somehow missed the breakpoint placed on line 9.  The user is
unable to examine state prior to evaluation of the loop condition.

The compiler is implementing this looping construct in the following
fashion.  An unconditional branch is placed at the start of the loop,
branching to an address immediately after the loop body.  At that point,
the condition is evaluated.  If the condition evaluates as true, a
conditional branch causes execution to continue at the first instruction
of the loop body, placed immediately after the unconditional branch.

GDB is placing its breakpoint on the unconditional branch. This is fine
for the first iteration of the loop, but does not work for subsequent
iterations.

This is the code that gcc generates on x86_64:

0x000000000040059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
0x00000000004005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax # 0x601030 <v>
0x00000000004005a6 <loop_test+22>:   add    $0x1,%eax
0x00000000004005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip) # 0x601030 <v>
0x00000000004005af <loop_test+31>:   mov    0x200a7b(%rip),%eax # 0x601030 <v>
0x00000000004005b5 <loop_test+37>:   cmp    $0x2,%eax
0x00000000004005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>

The breakpoint is being placed on 0x40059e (loop_test+14).  As such, it
gets hit only once.  If we could arrange to have the breakpoint placed at
the branch target, it would stop at each iteration of the loop. I.e.
it would behave like this:

(gdb) b 9
Breakpoint 1 at 0x4005af: file gdb.base/loop-break.c, line 9.
(gdb) b 11
Breakpoint 2 at 0x4005a0: file gdb.base/loop-break.c, line 11.
(gdb) command 1
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
>p v
>end
(gdb) run
Starting program: gdb.base/loop-break

Breakpoint 1, loop_test ()
    at gdb.base/loop-break.c:9
9         while (v < 3)                         /* Loop 1 condition */
$1 = 0
(gdb) c
Continuing.

Breakpoint 2, loop_test ()
    at gdb.base/loop-break.c:11
11            v++;                              /* Loop 1 increment */
(gdb) c
Continuing.

Breakpoint 1, loop_test ()
    at gdb.base/loop-break.c:9
9         while (v < 3)                         /* Loop 1 condition */
$2 = 1

This change introduces a new gdbarch method for potentially following
an unconditional branch.  If the circumstances are right, it uses
this method to cause the breakpoint to be placed on the branch target
instead of the branch itself.

This fixes the problem described above and causes no regressions.
The new test case includes the loop shown above.  It also contains
several other loops which verify that GDB stops at the the correct
places in each.  Only while loops of the form shown above are
affected by this patch; other looping constructs continue to work
as they had before.

Of particular interest is the test which uses an explicit goto; this
mimics the code that the compiler generates for a while loop.
However, in this example, placing a breakpoint on the goto should
cause execution to stop on the goto.  My initial attempt at a fix
for this problem caused that explicit goto to be followed, which is
definitely not correct.

Lastly, I'll note that, in the distant past, GCC used to output code
for the condition at the top of the loop.  GDB from either then or now
will stop at the evaluation of the condition each time through the
loop.

I was able to locate a compiler that I could run from circa 1998. While
I was able to run the compiler, I was not able to run the linker; it
died with an internal error in collect2, undoubtedly due to not having
compatible libraries.  But I was able to use -S in order to see
what the assembly code looked like.  Here is the assembly code for
our example loop, compiled by gcc 2.9, targeting i386-pc-linux-gnu:

	    movl $0,v
    .stabn 68,0,9,.LM3-loop_test
    .LM3:
	    .align 4
    .L2:
	    movl v,%eax
	    cmpl $2,%eax
	    jle .L4
	    jmp .L3
	    .align 4
    .L4:
    .stabn 68,0,11,.LM4-loop_test
    .LM4:
	    movl v,%eax
	    leal 1(%eax),%edx
	    movl %edx,v
    .stabn 68,0,12,.LM5-loop_test
    .LM5:
	    jmp .L2
    .L3:

The loop begins with the evaluation of the condition at .L2.  The jle
branches to instructions forming the body of the loop; the jmp instruction
immediately following the jle gets us out of the loop.

This code isn't very efficient, but it is a straightforward translation
of the corresponding C code.  A breakpoint placed at the beginning of
line 9 (which is both .LM3 and .L2) will stop on each iteration through
the loop.

My patch-set restores this behavior for while loops implemented in
a more efficient manner.

[PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Kevin Buettner]

This test places breakpoints at various points on several different
looping constructs, making sure that GDB behaves as expected.

gdb/testsuite/ChangeLog:

    	* gdb.base/loop-break.c, gdb.base/loop-break.exp: New files.
---
 gdb/testsuite/gdb.base/loop-break.c   |  42 +++++++++++++
 gdb/testsuite/gdb.base/loop-break.exp | 113 ++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+)

diff --git a/gdb/testsuite/gdb.base/loop-break.c b/gdb/testsuite/gdb.base/loop-break.c
new file mode 100644
index 0000000..3e4b5ca
--- /dev/null
+++ b/gdb/testsuite/gdb.base/loop-break.c
@@ -0,0 +1,42 @@
+volatile int v;
+volatile int w;
+
+void
+loop_test (void)
+{
+  v = 0;
+
+  while (v < 3)				/* Loop 1 condition */
+    {
+      v++;				/* Loop 1 increment */
+    }
+
+  v = -42;
+
+  for (v = 0; v < 3; )			/* Loop 2 */
+    {
+      v++;				/* Loop 2 increment */
+    }
+
+  v = -42;
+  w = 42;
+
+  for (v = 0;				/* Loop 3 initialization */
+       v < 3;				/* Loop 3 condition */
+       v++)				/* Loop 3 increment */
+     {
+      w = w - v;	
+     }
+
+  v = 0;
+  goto b;				/* Loop 4 initial goto */
+a:  v++;
+b:  if (v < 3) goto a;			/* Loop 4 condition */
+}
+
+int main (int argc, char **argv)
+{
+  loop_test ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/loop-break.exp b/gdb/testsuite/gdb.base/loop-break.exp
new file mode 100644
index 0000000..fef3fcb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/loop-break.exp
@@ -0,0 +1,113 @@
+# Copyright 2015 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/>.
+
+# Place breakpoints at various points on several different looping
+# constructs.  Make sure that GDB correctly runs to each of these
+# breakpoints and that computed values are correct at each point along
+# the way.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then { fail "loop-break tests suppressed" }
+
+proc break_at { search_string } {
+	global srcfile
+	set bp_location [gdb_get_line_number $search_string]
+	gdb_test "break $bp_location" \
+	    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+	    "break $search_string"
+
+	return $bp_location;
+}
+
+proc continue_to { linenum testname iter } {
+    global srcfile
+    global gdb_prompt
+    set full_name "continue to $testname, $iter"
+
+    send_gdb "continue\n"
+    gdb_expect {
+	-re "Continuing.*Breakpoint.*$srcfile:$linenum\r\n.*\r\n$gdb_prompt $" {
+	    pass $full_name
+	}
+	-re ".*$gdb_prompt $" {
+	    fail $full_name
+	}
+	timeout { 
+	    fail "$full_name (timeout)"
+	}
+    }
+}
+
+set bp_location1a [break_at "Loop 1 condition"]
+set bp_location1b [break_at "Loop 1 increment"]
+set bp_location2a [break_at "Loop 2"]
+set bp_location2b [break_at "Loop 2 increment"]
+set bp_location3a [break_at "Loop 3 initialization"]
+set bp_location3b [break_at "Loop 3 condition"]
+set bp_location3c [break_at "Loop 3 increment"]
+set bp_location4a [break_at "Loop 4 initial goto"]
+set bp_location4b [break_at "Loop 4 condition"]
+
+continue_to $bp_location1a "Loop 1 condition" 0
+gdb_test "p v" "= 0" "Loop 1 value check at condition 0"
+continue_to $bp_location1b "Loop 1 increment" 0
+gdb_test "p v" "= 0" "Loop 1 value check at increment 0"
+
+continue_to $bp_location1a "Loop 1 condition" 1
+gdb_test "p v" "= 1" "Loop 1 value check at condition 1"
+continue_to $bp_location1b "Loop 1 increment" 1
+gdb_test "p v" "= 1" "Loop 1 value check at increment 1"
+
+continue_to $bp_location1a "Loop 1 condition" 2
+continue_to $bp_location1b "Loop 1 increment" 2
+
+continue_to $bp_location1a "Loop 1 condition" 3
+
+continue_to $bp_location2a "Loop 2" 0
+gdb_test "p v" "= -42" "Loop 2 value check at loop start"
+continue_to $bp_location2b "Loop 2 increment" 0
+gdb_test "p v" "= 0" "Loop 2 value check at increment 0"
+continue_to $bp_location2b "Loop 2 increment" 1
+gdb_test "p v" "= 1" "Loop 2 value check at increment 1"
+continue_to $bp_location2b "Loop 2 increment" 2
+gdb_test "p v" "= 2" "Loop 2 value check at increment 2"
+
+continue_to $bp_location3a "Loop 3 initialization" 0
+gdb_test "p v" "= -42" "Loop 3 value check at initialization"
+continue_to $bp_location3b "Loop 3 condition" 0
+gdb_test "p v" "= 0" "Loop 3 value check at condition 0"
+continue_to $bp_location3c "Loop 3 increment" 0
+gdb_test "p v" "= 0" "Loop 3 value check at increment 0"
+continue_to $bp_location3b "Loop 3 condition" 1
+continue_to $bp_location3c "Loop 3 increment" 1
+continue_to $bp_location3b "Loop 3 condition" 2
+continue_to $bp_location3c "Loop 3 increment" 2
+continue_to $bp_location3b "Loop 3 condition" 3
+
+continue_to $bp_location4a "Loop 4 initial goto" 0
+gdb_test "p v" "= 0" "Loop 4 value check at initial goto"
+continue_to $bp_location4b "Loop 4 condition" 0
+gdb_test "p v" "= 0" "Loop 4 value check at condition 0"
+continue_to $bp_location4b "Loop 4 condition" 1
+gdb_test "p v" "= 1" "Loop 4 value check at condition 1"
+continue_to $bp_location4b "Loop 4 condition" 2
+gdb_test "p v" "= 2" "Loop 4 value check at condition 2"
+continue_to $bp_location4b "Loop 4 condition" 3
+gdb_test "p v" "= 3" "Loop 4 value check at condition 3"

[PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Kevin Buettner]

Add new gdbarch method, unconditional_branch_address.

gdb/ChangeLog:

    	* gdbarch.sh (unconditional_branch_address): New gdbarch method.
    	* gdbarch.h, gdbarch.c: Regenerate.
    	* arch-utils.h (default_unconditional_branch_address): Declare.
    	* arch-utils.c (default_unconditional_branch_address): New function.
---
 gdb/arch-utils.c |  6 ++++++
 gdb/arch-utils.h |  2 ++
 gdb/gdbarch.c    | 23 +++++++++++++++++++++++
 gdb/gdbarch.h    | 12 +++++++++++-
 gdb/gdbarch.sh   |  7 +++++++
 5 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 7df5570..fbbb94c 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -820,6 +820,12 @@ default_gen_return_address (struct gdbarch *gdbarch,
   error (_("This architecture has no method to collect a return address."));
 }
 
+CORE_ADDR
+default_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  return 0;
+}
+
 int
 default_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
 					struct type *type)
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 18e2290..1bad2c0 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -204,4 +204,6 @@ extern char *default_gcc_target_options (struct gdbarch *gdbarch);
 extern const char *default_gnu_triplet_regexp (struct gdbarch *gdbarch);
 extern int default_addressable_memory_unit_size (struct gdbarch *gdbarch);
 
+extern CORE_ADDR default_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc);
+
 #endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 07b38a3..741dc10 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -330,6 +330,7 @@ struct gdbarch
   gdbarch_gcc_target_options_ftype *gcc_target_options;
   gdbarch_gnu_triplet_regexp_ftype *gnu_triplet_regexp;
   gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size;
+  gdbarch_unconditional_branch_address_ftype *unconditional_branch_address;
 };
 
 /* Create a new ``struct gdbarch'' based on information provided by
@@ -432,6 +433,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->gcc_target_options = default_gcc_target_options;
   gdbarch->gnu_triplet_regexp = default_gnu_triplet_regexp;
   gdbarch->addressable_memory_unit_size = default_addressable_memory_unit_size;
+  gdbarch->unconditional_branch_address = default_unconditional_branch_address;
   /* gdbarch_alloc() */
 
   return gdbarch;
@@ -666,6 +668,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of gcc_target_options, invalid_p == 0 */
   /* Skip verify of gnu_triplet_regexp, invalid_p == 0 */
   /* Skip verify of addressable_memory_unit_size, invalid_p == 0 */
+  /* Skip verify of unconditional_branch_address, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &length);
   make_cleanup (xfree, buf);
   if (length > 0)
@@ -1356,6 +1359,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: target_desc = %s\n",
                       host_address_to_string (gdbarch->target_desc));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: unconditional_branch_address = <%s>\n",
+                      host_address_to_string (gdbarch->unconditional_branch_address));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_unwind_pc_p() = %d\n",
                       gdbarch_unwind_pc_p (gdbarch));
   fprintf_unfiltered (file,
@@ -4753,6 +4759,23 @@ set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch,
   gdbarch->addressable_memory_unit_size = addressable_memory_unit_size;
 }
 
+CORE_ADDR
+gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->unconditional_branch_address != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_unconditional_branch_address called\n");
+  return gdbarch->unconditional_branch_address (gdbarch, pc);
+}
+
+void
+set_gdbarch_unconditional_branch_address (struct gdbarch *gdbarch,
+                                          gdbarch_unconditional_branch_address_ftype unconditional_branch_address)
+{
+  gdbarch->unconditional_branch_address = unconditional_branch_address;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index c1e2c1a..1770960 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -924,7 +924,7 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
    If your architecture doesn't need to adjust instructions before
    single-stepping them, consider using simple_displaced_step_copy_insn
    here.
-
+  
    If the instruction cannot execute out of line, return NULL.  The
    core falls back to stepping past the instruction in-line instead in
    that case. */
@@ -1478,6 +1478,16 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
 extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
 extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
 
+/* Examine instruction at PC.  If instruction at PC is an unconditional
+   branch, return the address to which control is transferred when the
+   branch is taken.  Return 0 when this method is not implemented by
+   architecture, PC refers to an invalid address, or instruction at PC
+   is not an unconditional branch. */
+
+typedef CORE_ADDR (gdbarch_unconditional_branch_address_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc);
+extern CORE_ADDR gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc);
+extern void set_gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, gdbarch_unconditional_branch_address_ftype *unconditional_branch_address);
+
 /* Definition for an unknown syscall, used basically in error-cases.  */
 #define UNKNOWN_SYSCALL (-1)
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 994a87b..c70d241 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1123,6 +1123,13 @@ m:const char *:gnu_triplet_regexp:void:::default_gnu_triplet_regexp::0
 # each address in memory.
 m:int:addressable_memory_unit_size:void:::default_addressable_memory_unit_size::0
 
+# Examine instruction at PC.  If instruction at PC is an unconditional
+# branch, return the address to which control is transferred when the
+# branch is taken.  Return 0 when this method is not implemented by
+# architecture, PC refers to an invalid address, or instruction at PC
+# is not an unconditional branch.
+m:CORE_ADDR:unconditional_branch_address:CORE_ADDR pc:pc::default_unconditional_branch_address::0
+
 EOF
 }
 

[PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

This patch changes create_sals_line_offset() in linespec.c so that, for
a given SAL, if that SAL's address (pc) refers to an unconditional
branch instruction whose branch target also refers to the same SAL, then
the branch target is used for the SAL instead.

The pratical effect of this is that a breakpoint placed on a while
loop will break at the evaluation of the condition instead of at the
unconditional branch which transfers control to the starting address
for the evaluation of the condition.

Consider the following code snippet (which is taken from one of the
new tests for this patch set):

    9         while (v < 3)                         /* Loop 1 condition */
    10          {
    11            v++;                              /* Loop 1 increment */
    12          }

This is compiled as the following x86_64 code:

    0x000000000040059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
    0x00000000004005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax # 0x601030 <v>
    0x00000000004005a6 <loop_test+22>:   add    $0x1,%eax
    0x00000000004005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip) # 0x601030 <v>
    0x00000000004005af <loop_test+31>:   mov    0x200a7b(%rip),%eax # 0x601030 <v>
    0x00000000004005b5 <loop_test+37>:   cmp    $0x2,%eax
    0x00000000004005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>

If a breakpoint is placed on line 9, which begins at loop_test+14, this
change/patch causes the breakpoint to be placed on loop_test+31, which is
the starting address for the evaluation of the condition.

In order for this to work, an architecture specific method,
unconditional_branch_address, was introduced in an earlier patch in
the set.  I've implemented this method for x86_64, i386, arm, thumb,
powerpc, rx, and rl78.

I've tested on each of these architectures and see no regressions.

gdb/ChangeLog:

    	* linespec.c (addr_in_sals): New function.
    	(create_sals_line_offset): Adjust SAL whose pc refers to an
	unconditional branch whose target is the same line.
---
 gdb/linespec.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 00fa4ba..2e0146d 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1808,6 +1808,29 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
     }
 }
 
+/* Return 1 if one of the SALS between 0 and NELTS contains ADDR. 
+   Return 0 otherwise.  */
+
+static int
+addr_in_sals (CORE_ADDR addr, int nelts, struct symtab_and_line *sals)
+{
+  int i;
+
+  for (i = 0; i < nelts; i++)
+    {
+      struct symtab_and_line sal;
+
+      if (sals[i].end == 0)
+        sal = find_pc_sect_line (sals[i].pc, sals[i].section, 0);
+      else
+        sal = sals[i];
+
+      if (sal.pc <= addr && addr < sal.end)
+	return 1;
+    }
+  return 0;
+}
+
 /* Given a line offset in LS, construct the relevant SALs.  */
 
 static struct symtabs_and_lines
@@ -1933,6 +1956,18 @@ create_sals_line_offset (struct linespec_state *self,
 	    struct symbol *sym = (blocks[i]
 				  ? block_containing_function (blocks[i])
 				  : NULL);
+	    CORE_ADDR branch_addr = gdbarch_unconditional_branch_address
+	      (get_current_arch (), intermediate_results.sals[i].pc);
+
+	    /* Only use branch if it's in the same block and is also
+	       within one of the sals from the initial list.  */
+	    if (branch_addr != 0 && blocks[i]->startaddr <= branch_addr
+	        && branch_addr < blocks[i]->endaddr
+		&& addr_in_sals (branch_addr, intermediate_results.nelts,
+		                 intermediate_results.sals))
+	      {
+		intermediate_results.sals[i].pc = branch_addr;
+	      }
 
 	    if (self->funfirstline)
 	      skip_prologue_sal (&intermediate_results.sals[i]);

[PATCH 4/8] Implement unconditional_branch_address method for x86-64 and i386

[Kevin Buettner]

Implement unconditional_branch_address method for x86-64 and i386.

gdb/ChangeLog:
    
    	* i386-tdep.c (i386_unconditional_branch_address): New function.
    	(i386_gdbarch_init): Register i386_unconditional_branch_address.
---
 gdb/i386-tdep.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 9d52d4a..60cd93e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8237,6 +8237,19 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep,
   return valid_p;
 }
 
+/* Implement the unconditional_branch_address gdbarch method.  */
+
+static CORE_ADDR
+i386_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  CORE_ADDR new_pc = i386_follow_jump (gdbarch, pc);
+
+  if (new_pc == pc)
+    return 0;
+  else
+    return new_pc;
+}
+
 \f
 static struct gdbarch *
 i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -8582,6 +8595,10 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_fast_tracepoint_valid_at (gdbarch,
 					i386_fast_tracepoint_valid_at);
 
+  /* Unconditional Branch.  */
+  set_gdbarch_unconditional_branch_address (gdbarch,
+                                            i386_unconditional_branch_address);
+
   return gdbarch;
 }
 

[PATCH 5/8] Implement unconditional_branch_address method for arm and thumb

[Kevin Buettner]

Implement unconditional_branch_address method for arm and thumb.

gdb/ChangeLog:
    
    	* arm-tdep.c (arm_unconditional_branch_address): New function.
    	(arm_gdbarch_init): Register arm_unconditional_branch_address.
---
 gdb/arm-tdep.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 73f26b9..2e16a28 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9902,7 +9902,32 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
 
   /* Otherwise we don't have a useful guess.  */
 }
+\f
+/* Implement the unconditional_branch_address gdbarch method.  */
+
+static CORE_ADDR
+arm_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+  unsigned int insn;
+
+  if (arm_pc_is_thumb (gdbarch, pc))
+    {
+      insn = read_memory_unsigned_integer (pc, 2, byte_order_for_code);
 
+      if ((insn & 0xf800) == 0xe000)
+	return pc + 4 + (sbits (insn, 0, 10) << 1);
+    }
+  else
+    {
+      insn = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
+
+      if (bits (insn, 28, 31) == INST_AL && bits (insn, 24, 27) == 0xa)
+	return BranchDest (pc, insn);
+    }
+
+  return 0;
+}
 \f
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
@@ -10499,6 +10524,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     user_reg_add (gdbarch, arm_register_aliases[i].name,
 		  value_of_arm_user_reg, &arm_register_aliases[i].regnum);
 
+  /* Unconditional Branch.  */
+  set_gdbarch_unconditional_branch_address (gdbarch,
+                                            arm_unconditional_branch_address);
+
   return gdbarch;
 }
 

[PATCH 6/8] Implement unconditional_branch_address method for powerpc / rs6000

[Kevin Buettner]

Implement unconditional_branch_address method for powerpc / rs6000.

gdb/ChangeLog:
    
    	* rs6000-tdep.c (rs6000_unconditional_branch_address): New function.
    	(rs6000_gdbarch_init): Register rs6000_unconditional_branch_address.
---
 gdb/rs6000-tdep.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 785f451..f1c33e0 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3578,6 +3578,20 @@ ppc_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
     reg->how = DWARF2_FRAME_REG_CFA;
 }
 
+/* Implement the unconditional_branch_address gdbarch method.  */
+
+static CORE_ADDR
+rs6000_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  unsigned long insn = read_memory_unsigned_integer (pc, 4, byte_order);
+
+  if ((insn & 0xfc000003) == 0x48000000)	/* b instruction.  */
+    return pc + (((insn & 0x02000000) ? ~((CORE_ADDR) 0x03fffffc) : 0)
+                   | (insn & 0x03fffffc));
+  else
+    return 0;
+}
 
 /* Return true if a .gnu_attributes section exists in BFD and it
    indicates we are using SPE extensions OR if a .PPC.EMB.apuinfo
@@ -6051,6 +6065,10 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   else
     register_ppc_ravenscar_ops (gdbarch);
 
+  /* Unconditional Branch.  */
+  set_gdbarch_unconditional_branch_address
+    (gdbarch, rs6000_unconditional_branch_address);
+
   return gdbarch;
 }
 

[PATCH 7/8] Implement unconditional_branch_address method for rl78

[Kevin Buettner]

Implement unconditional_branch_address method for rl78.

gdb/ChangeLog:
    
    	* rl78-tdep.c (rl78_unconditional_branch_address): New function.
    	(rl78_gdbarch_init): Register rl78_unconditional_branch_address.
---
 gdb/rl78-tdep.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/gdb/rl78-tdep.c b/gdb/rl78-tdep.c
index a5861d8..98129cc 100644
--- a/gdb/rl78-tdep.c
+++ b/gdb/rl78-tdep.c
@@ -1365,6 +1365,31 @@ rl78_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   return rl78_make_data_address (sp + 4);
 }
 
+/* Implement the unconditional_branch_address gdbarch method.  */
+
+static CORE_ADDR
+rl78_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  int bytes_read;
+  RL78_Opcode_Decoded opc;
+  struct rl78_get_opcode_byte_handle opcode_handle;
+
+  opcode_handle.pc = pc;
+
+  bytes_read = rl78_decode_opcode (pc, &opc, rl78_get_opcode_byte,
+                                   &opcode_handle);
+
+  if (bytes_read > 0
+      && opc.id == RLO_branch
+      && opc.op[0].type == RL78_Operand_Immediate)
+    {
+      LONGEST l = opc.op[0].addend;
+
+      return (CORE_ADDR) l;
+    }
+
+  return 0;
+}
 /* Allocate and initialize a gdbarch object.  */
 
 static struct gdbarch *
@@ -1491,6 +1516,10 @@ rl78_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Virtual tables.  */
   set_gdbarch_vbit_in_delta (gdbarch, 1);
 
+  /* Unconditional Branch.  */
+  set_gdbarch_unconditional_branch_address (gdbarch,
+                                            rl78_unconditional_branch_address);
+
   return gdbarch;
 }
 

[PATCH 8/8] Implement unconditional_branch_address method for rx

[Kevin Buettner]

Implement unconditional_branch_address method for rx.

gdb/ChangeLog:
    	* rx-tdep.c (rx_unconditional_branch_address): New function.
    	(rx_gdbarch_init): Register rx_unconditional_branch_address.
---
 gdb/rx-tdep.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
index 0bd91ff..f7ba2fb 100644
--- a/gdb/rx-tdep.c
+++ b/gdb/rx-tdep.c
@@ -1015,6 +1015,38 @@ rx_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
 		    reg);
 }
 
+/* Implement the unconditional_branch_address gdbarch method.  */
+
+static CORE_ADDR
+rx_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  int bytes_read;
+  RX_Opcode_Decoded opc;
+  struct rx_get_opcode_byte_handle opcode_handle;
+
+  opcode_handle.pc = pc;
+
+  bytes_read = rx_decode_opcode (pc, &opc, rx_get_opcode_byte,
+               &opcode_handle);
+
+  if (bytes_read > 0
+      && (opc.id == RXO_branch || opc.id == RXO_branchrel)
+      && (opc.op[1].type == RX_Operand_None 
+          || (opc.op[1].type == RX_Operand_Condition 
+	      && opc.op[1].reg == RXC_always))
+      && opc.op[0].type == RX_Operand_Immediate)
+    {
+      LONGEST l = opc.op[0].addend;
+
+      if (opc.id == RXO_branch)
+        return (CORE_ADDR) l;
+      else
+	return pc + l;
+    }
+
+  return 0;
+}
+
 /* Allocate and initialize a gdbarch object.  */
 static struct gdbarch *
 rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -1146,6 +1178,10 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Virtual tables.  */
   set_gdbarch_vbit_in_delta (gdbarch, 1);
 
+  /* Unconditional Branch.  */
+  set_gdbarch_unconditional_branch_address (gdbarch,
+                                            rx_unconditional_branch_address);
+
   return gdbarch;
 }
 

Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Pedro Alves]

On 08/19/2015 08:03 AM, Kevin Buettner wrote:

> @@ -1933,6 +1956,18 @@ create_sals_line_offset (struct linespec_state *self,
>  	    struct symbol *sym = (blocks[i]
>  				  ? block_containing_function (blocks[i])
>  				  : NULL);
> +	    CORE_ADDR branch_addr = gdbarch_unconditional_branch_address
> +	      (get_current_arch (), intermediate_results.sals[i].pc);

It'd be nice to have a comment here explaining why we do this.  You have
it in the commit log, but it'd be useful going forward to have it in
the sources.

Nit, this is about the branch _destination_ not the branch instruction's
address, right?  I'd suggest
s/gdbarch_unconditional_branch_address/gdbarch_unconditional_branch_destination/

Also, there should be a better gdbarch to use than get_current_arch().
E.g., get_objfile_arch (SYMTAB_OBJFILE (sals[i].symtab)), the sal's pspace's
gdbarch, etc.

> +
> +	    /* Only use branch if it's in the same block and is also

And here "use branch destination".

> +	       within one of the sals from the initial list.  */
> +	    if (branch_addr != 0 && blocks[i]->startaddr <= branch_addr
> +	        && branch_addr < blocks[i]->endaddr
> +		&& addr_in_sals (branch_addr, intermediate_results.nelts,
> +		                 intermediate_results.sals))
> +	      {
> +		intermediate_results.sals[i].pc = branch_addr;
> +	      }

Also, we really shouldn't be adding code that assumes 0 to mean invalid
address, as on some ports, it is a valid address.  You could e.g.,
change gdbarch_unconditional_branch_address to return an int and
take an output CORE_ADDR * parameter, and/or make it an M gdbarch
method, and check gdbarch_unconditional_branch_address_p before
use.

I wonder whether we have to worry about the case of a goto
jumping to the same line?  Like:

  goto label; foo (); label: bar ();

Not the most usual or beautiful code to write, though I'd guess code
generators could output things like that.  Writing it in several lines
but behind a #define might be another way to get everything in
the same line.

Thanks,
Pedro Alves

Re: [PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Pedro Alves]

On 08/19/2015 07:57 AM, Kevin Buettner wrote:

> index 0000000..3e4b5ca
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/loop-break.c
> @@ -0,0 +1,42 @@
> +volatile int v;
> +volatile int w;

Copyright header missing.

> +  v = 0;
> +  goto b;				/* Loop 4 initial goto */
> +a:  v++;
> +b:  if (v < 3) goto a;			/* Loop 4 condition */
> +}
> +
> +int main (int argc, char **argv)

Line break after first int.

> +{
> +  loop_test ();
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/loop-break.exp b/gdb/testsuite/gdb.base/loop-break.exp
> new file mode 100644
> index 0000000..fef3fcb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/loop-break.exp
> @@ -0,0 +1,113 @@
> +# Copyright 2015 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/>.
> +
> +# Place breakpoints at various points on several different looping
> +# constructs.  Make sure that GDB correctly runs to each of these
> +# breakpoints and that computed values are correct at each point along
> +# the way.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> +    return -1
> +}
> +
> +if ![runto_main] then { fail "loop-break tests suppressed" }

This should return rather than continue into a cascade of failures:

if ![runto_main] then {
    fail "Can't run to main"
    return 0
}

> +
> +proc break_at { search_string } {
> +	global srcfile
> +	set bp_location [gdb_get_line_number $search_string]
> +	gdb_test "break $bp_location" \
> +	    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> +	    "break $search_string"
> +
> +	return $bp_location;
> +}
> +
> +proc continue_to { linenum testname iter } {
> +    global srcfile
> +    global gdb_prompt
> +    set full_name "continue to $testname, $iter"
> +
> +    send_gdb "continue\n"
> +    gdb_expect {
> +	-re "Continuing.*Breakpoint.*$srcfile:$linenum\r\n.*\r\n$gdb_prompt $" {
> +	    pass $full_name
> +	}
> +	-re ".*$gdb_prompt $" {
> +	    fail $full_name
> +	}
> +	timeout { 
> +	    fail "$full_name (timeout)"
> +	}

Use gdb_test_multiple.  Or even, gdb_test ?

Thanks,
Pedro Alves

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Pedro Alves]

On 08/19/2015 08:00 AM, Kevin Buettner wrote:

> +# Examine instruction at PC.  If instruction at PC is an unconditional
> +# branch, return the address to which control is transferred when the
> +# branch is taken.  Return 0 when this method is not implemented by
> +# architecture, PC refers to an invalid address, or instruction at PC
> +# is not an unconditional branch.
> +m:CORE_ADDR:unconditional_branch_address:CORE_ADDR pc:pc::default_unconditional_branch_address::0
> +

In addition to the comments in the other patch, I wonder if we would
better name this in terms of a higher level concept, around
"breakpoint address" instead of a "low level branch destination"?

Thanks,
Pedro Alves

Re: [PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Kevin Buettner]

On Tue, 25 Aug 2015 13:10:06 +0100
Pedro Alves <palves@redhat.com> wrote:

> On 08/19/2015 07:57 AM, Kevin Buettner wrote:
> 
> > +++ b/gdb/testsuite/gdb.base/loop-break.c
> 
> Copyright header missing.
> 
> > +int main (int argc, char **argv)
> 
> Line break after first int.
> 
> > +if ![runto_main] then { fail "loop-break tests suppressed" }
> 
> This should return rather than continue into a cascade of failures:
> 
> if ![runto_main] then {
>     fail "Can't run to main"
>     return 0
> }
> 
> > +proc continue_to { linenum testname iter } {
> > +    global srcfile
> > +    global gdb_prompt
> > +    set full_name "continue to $testname, $iter"
> > +
> > +    send_gdb "continue\n"
> > +    gdb_expect {
> > +	-re "Continuing.*Breakpoint.*$srcfile:$linenum\r\n.*\r\n$gdb_prompt $" {
> > +	    pass $full_name
> > +	}
> > +	-re ".*$gdb_prompt $" {
> > +	    fail $full_name
> > +	}
> > +	timeout { 
> > +	    fail "$full_name (timeout)"
> > +	}
> 
> Use gdb_test_multiple.  Or even, gdb_test ?

Hi Pedro,

Thanks for the review.  The version below fixes the problems you identified...



Add new test, gdb.base/loop-break.exp.

This test places breakpoints at various points on several different
looping constructs, making sure that GDB behaves as expected.

gdb/testsuite/ChangeLog:
    
    	* gdb.base/loop-break.c, gdb.base/loop-break.exp: New files.
---
 gdb/testsuite/gdb.base/loop-break.c   |  60 ++++++++++++++++++++
 gdb/testsuite/gdb.base/loop-break.exp | 104 ++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+)

diff --git a/gdb/testsuite/gdb.base/loop-break.c b/gdb/testsuite/gdb.base/loop-break.c
new file mode 100644
index 0000000..7b7d0f7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/loop-break.c
@@ -0,0 +1,60 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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/>.  */
+
+volatile int v;
+volatile int w;
+
+void
+loop_test (void)
+{
+  v = 0;
+
+  while (v < 3)				/* Loop 1 condition */
+    {
+      v++;				/* Loop 1 increment */
+    }
+
+  v = -42;
+
+  for (v = 0; v < 3; )			/* Loop 2 */
+    {
+      v++;				/* Loop 2 increment */
+    }
+
+  v = -42;
+  w = 42;
+
+  for (v = 0;				/* Loop 3 initialization */
+       v < 3;				/* Loop 3 condition */
+       v++)				/* Loop 3 increment */
+     {
+      w = w - v;	
+     }
+
+  v = 0;
+  goto b;				/* Loop 4 initial goto */
+a:  v++;
+b:  if (v < 3) goto a;			/* Loop 4 condition */
+}
+
+int
+main (int argc, char **argv)
+{
+  loop_test ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/loop-break.exp b/gdb/testsuite/gdb.base/loop-break.exp
new file mode 100644
index 0000000..b573a7b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/loop-break.exp
@@ -0,0 +1,104 @@
+# Copyright 2015 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/>.
+
+# Place breakpoints at various points on several different looping
+# constructs.  Make sure that GDB correctly runs to each of these
+# breakpoints and that computed values are correct at each point along
+# the way.
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+	fail "Can't run to main"
+	return 0
+}
+
+proc break_at { search_string } {
+	global srcfile
+	set bp_location [gdb_get_line_number $search_string]
+	gdb_test "break $bp_location" \
+	    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+	    "break $search_string"
+
+	return $bp_location;
+}
+
+proc continue_to { linenum testname iter } {
+	global srcfile
+	gdb_test "continue" \
+	         "Continuing.*Breakpoint.*$srcfile:$linenum.*" \
+		 "continue to $testname, $iter"
+}
+
+set bp_location1a [break_at "Loop 1 condition"]
+set bp_location1b [break_at "Loop 1 increment"]
+set bp_location2a [break_at "Loop 2"]
+set bp_location2b [break_at "Loop 2 increment"]
+set bp_location3a [break_at "Loop 3 initialization"]
+set bp_location3b [break_at "Loop 3 condition"]
+set bp_location3c [break_at "Loop 3 increment"]
+set bp_location4a [break_at "Loop 4 initial goto"]
+set bp_location4b [break_at "Loop 4 condition"]
+
+continue_to $bp_location1a "Loop 1 condition" 0
+gdb_test "p v" "= 0" "Loop 1 value check at condition 0"
+continue_to $bp_location1b "Loop 1 increment" 0
+gdb_test "p v" "= 0" "Loop 1 value check at increment 0"
+
+continue_to $bp_location1a "Loop 1 condition" 1
+gdb_test "p v" "= 1" "Loop 1 value check at condition 1"
+continue_to $bp_location1b "Loop 1 increment" 1
+gdb_test "p v" "= 1" "Loop 1 value check at increment 1"
+
+continue_to $bp_location1a "Loop 1 condition" 2
+continue_to $bp_location1b "Loop 1 increment" 2
+
+continue_to $bp_location1a "Loop 1 condition" 3
+
+continue_to $bp_location2a "Loop 2" 0
+gdb_test "p v" "= -42" "Loop 2 value check at loop start"
+continue_to $bp_location2b "Loop 2 increment" 0
+gdb_test "p v" "= 0" "Loop 2 value check at increment 0"
+continue_to $bp_location2b "Loop 2 increment" 1
+gdb_test "p v" "= 1" "Loop 2 value check at increment 1"
+continue_to $bp_location2b "Loop 2 increment" 2
+gdb_test "p v" "= 2" "Loop 2 value check at increment 2"
+
+continue_to $bp_location3a "Loop 3 initialization" 0
+gdb_test "p v" "= -42" "Loop 3 value check at initialization"
+continue_to $bp_location3b "Loop 3 condition" 0
+gdb_test "p v" "= 0" "Loop 3 value check at condition 0"
+continue_to $bp_location3c "Loop 3 increment" 0
+gdb_test "p v" "= 0" "Loop 3 value check at increment 0"
+continue_to $bp_location3b "Loop 3 condition" 1
+continue_to $bp_location3c "Loop 3 increment" 1
+continue_to $bp_location3b "Loop 3 condition" 2
+continue_to $bp_location3c "Loop 3 increment" 2
+continue_to $bp_location3b "Loop 3 condition" 3
+
+continue_to $bp_location4a "Loop 4 initial goto" 0
+gdb_test "p v" "= 0" "Loop 4 value check at initial goto"
+continue_to $bp_location4b "Loop 4 condition" 0
+gdb_test "p v" "= 0" "Loop 4 value check at condition 0"
+continue_to $bp_location4b "Loop 4 condition" 1
+gdb_test "p v" "= 1" "Loop 4 value check at condition 1"
+continue_to $bp_location4b "Loop 4 condition" 2
+gdb_test "p v" "= 2" "Loop 4 value check at condition 2"
+continue_to $bp_location4b "Loop 4 condition" 3
+gdb_test "p v" "= 3" "Loop 4 value check at condition 3"

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Kevin Buettner]

On Tue, 25 Aug 2015 13:13:49 +0100
Pedro Alves <palves@redhat.com> wrote:

> > +# Examine instruction at PC.  If instruction at PC is an unconditional
> > +# branch, return the address to which control is transferred when the
> > +# branch is taken.  Return 0 when this method is not implemented by
> > +# architecture, PC refers to an invalid address, or instruction at PC
> > +# is not an unconditional branch.
> > +m:CORE_ADDR:unconditional_branch_address:CORE_ADDR pc:pc::default_unconditional_branch_address::0
> > +
> 
> In addition to the comments in the other patch, I wonder if we would
> better name this in terms of a higher level concept, around
> "breakpoint address" instead of a "low level branch destination"?

I gave this some thought, but could not come up with a name that was
meaningful yet not overly long.  If you have a name that you like, I'm
quite willing to revise the patches again.

In the midst of adding a parameter for holding the branch destination,
I noticed that gdbarch.sh has a method called insn_is_jump, which is
used in btrace.c.  (There is also insn_is_call and insn_is_ret.)  I
thought about using insn_is_jump, but I would need to modify it
to return the branch destination.

I decided that the work I'm doing here should have it's own method.
If GCC should someday emit DWARF which sets is_stmt to 0 for the
branch instruction at the beginning of a while loop, this work might
not be needed any longer.  It'll be easier to rip it out and clean things
up again if I don't modify gdbarch methods that were added for
other purposes.

In the end, I decided to go with the name that you suggested in your
review of patch #3, unconditional_branch_destination.

Here's the revised patch:


    Add new gdbarch method, unconditional_branch_destination.
    
    gdb/ChangeLog:
    
    	* gdbarch.sh (unconditional_branch_destination): New gdbarch method.
    	* gdbarch.h, gdbarch.c: Regenerate.
    	* arch-utils.h (default_unconditional_branch_destination): Declare.
    	* arch-utils.c (default_unconditional_branch_destination): New
    	function.
---
 gdb/arch-utils.c |  7 +++++++
 gdb/arch-utils.h |  4 ++++
 gdb/gdbarch.c    | 23 +++++++++++++++++++++++
 gdb/gdbarch.h    |  9 +++++++++
 gdb/gdbarch.sh   |  6 ++++++
 5 files changed, 49 insertions(+)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 7df5570..23f9e69 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -821,6 +821,13 @@ default_gen_return_address (struct gdbarch *gdbarch,
 }
 
 int
+default_unconditional_branch_destination (struct gdbarch *gdbarch, CORE_ADDR pc,
+                                          CORE_ADDR *dest)
+{
+  return 0;
+}
+
+int
 default_return_in_first_hidden_param_p (struct gdbarch *gdbarch,
 					struct type *type)
 {
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 18e2290..e3a9a7d 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -204,4 +204,8 @@ extern char *default_gcc_target_options (struct gdbarch *gdbarch);
 extern const char *default_gnu_triplet_regexp (struct gdbarch *gdbarch);
 extern int default_addressable_memory_unit_size (struct gdbarch *gdbarch);
 
+extern int default_unconditional_branch_destination (struct gdbarch *gdbarch,
+                                                     CORE_ADDR pc,
+					             CORE_ADDR *dest);
+
 #endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index f04eef9..a2e7dfb 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -330,6 +330,7 @@ struct gdbarch
   gdbarch_gcc_target_options_ftype *gcc_target_options;
   gdbarch_gnu_triplet_regexp_ftype *gnu_triplet_regexp;
   gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size;
+  gdbarch_unconditional_branch_destination_ftype *unconditional_branch_destination;
 };
 
 /* Create a new ``struct gdbarch'' based on information provided by
@@ -432,6 +433,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->gcc_target_options = default_gcc_target_options;
   gdbarch->gnu_triplet_regexp = default_gnu_triplet_regexp;
   gdbarch->addressable_memory_unit_size = default_addressable_memory_unit_size;
+  gdbarch->unconditional_branch_destination = default_unconditional_branch_destination;
   /* gdbarch_alloc() */
 
   return gdbarch;
@@ -674,6 +676,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of gcc_target_options, invalid_p == 0 */
   /* Skip verify of gnu_triplet_regexp, invalid_p == 0 */
   /* Skip verify of addressable_memory_unit_size, invalid_p == 0 */
+  /* Skip verify of unconditional_branch_destination, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &length);
   make_cleanup (xfree, buf);
   if (length > 0)
@@ -1364,6 +1367,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: target_desc = %s\n",
                       host_address_to_string (gdbarch->target_desc));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: unconditional_branch_destination = <%s>\n",
+                      host_address_to_string (gdbarch->unconditional_branch_destination));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_unwind_pc_p() = %d\n",
                       gdbarch_unwind_pc_p (gdbarch));
   fprintf_unfiltered (file,
@@ -4761,6 +4767,23 @@ set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch,
   gdbarch->addressable_memory_unit_size = addressable_memory_unit_size;
 }
 
+int
+gdbarch_unconditional_branch_destination (struct gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR *dest)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->unconditional_branch_destination != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_unconditional_branch_destination called\n");
+  return gdbarch->unconditional_branch_destination (gdbarch, pc, dest);
+}
+
+void
+set_gdbarch_unconditional_branch_destination (struct gdbarch *gdbarch,
+                                              gdbarch_unconditional_branch_destination_ftype unconditional_branch_destination)
+{
+  gdbarch->unconditional_branch_destination = unconditional_branch_destination;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 82e0259..2d0cf8d 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1478,6 +1478,15 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
 extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
 extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
 
+/* Examine instruction at PC.  If instruction at PC is an unconditional
+   branch, return 1 and set *DEST to the branch destination address.
+   Return 0 when this method is not implemented by architecture, PC refers
+   to an invalid address, or instruction at PC is not an unconditional branch. */
+
+typedef int (gdbarch_unconditional_branch_destination_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR *dest);
+extern int gdbarch_unconditional_branch_destination (struct gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR *dest);
+extern void set_gdbarch_unconditional_branch_destination (struct gdbarch *gdbarch, gdbarch_unconditional_branch_destination_ftype *unconditional_branch_destination);
+
 /* Definition for an unknown syscall, used basically in error-cases.  */
 #define UNKNOWN_SYSCALL (-1)
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 388920f..b87fd19 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1123,6 +1123,12 @@ m:const char *:gnu_triplet_regexp:void:::default_gnu_triplet_regexp::0
 # each address in memory.
 m:int:addressable_memory_unit_size:void:::default_addressable_memory_unit_size::0
 
+# Examine instruction at PC.  If instruction at PC is an unconditional
+# branch, return 1 and set *DEST to the branch destination address.
+# Return 0 when this method is not implemented by architecture, PC refers
+# to an invalid address, or instruction at PC is not an unconditional branch.
+m:int:unconditional_branch_destination:CORE_ADDR pc, CORE_ADDR *dest:pc, dest::default_unconditional_branch_destination::0
+
 EOF
 }
 

Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

On Tue, 25 Aug 2015 13:09:55 +0100
Pedro Alves <palves@redhat.com> wrote:

> On 08/19/2015 08:03 AM, Kevin Buettner wrote:
> 
> > @@ -1933,6 +1956,18 @@ create_sals_line_offset (struct linespec_state *self,
> >  	    struct symbol *sym = (blocks[i]
> >  				  ? block_containing_function (blocks[i])
> >  				  : NULL);
> > +	    CORE_ADDR branch_addr = gdbarch_unconditional_branch_address
> > +	      (get_current_arch (), intermediate_results.sals[i].pc);
> 
> It'd be nice to have a comment here explaining why we do this.  You have
> it in the commit log, but it'd be useful going forward to have it in
> the sources.
> 
> Nit, this is about the branch _destination_ not the branch instruction's
> address, right?  I'd suggest
> s/gdbarch_unconditional_branch_address/gdbarch_unconditional_branch_destination/
> 
> Also, there should be a better gdbarch to use than get_current_arch().
> E.g., get_objfile_arch (SYMTAB_OBJFILE (sals[i].symtab)), the sal's pspace's
> gdbarch, etc.
> 
> > +
> > +	    /* Only use branch if it's in the same block and is also
> 
> And here "use branch destination".
> 
> > +	       within one of the sals from the initial list.  */
> > +	    if (branch_addr != 0 && blocks[i]->startaddr <= branch_addr
> > +	        && branch_addr < blocks[i]->endaddr
> > +		&& addr_in_sals (branch_addr, intermediate_results.nelts,
> > +		                 intermediate_results.sals))
> > +	      {
> > +		intermediate_results.sals[i].pc = branch_addr;
> > +	      }
> 
> Also, we really shouldn't be adding code that assumes 0 to mean invalid
> address, as on some ports, it is a valid address.  You could e.g.,
> change gdbarch_unconditional_branch_address to return an int and
> take an output CORE_ADDR * parameter, and/or make it an M gdbarch
> method, and check gdbarch_unconditional_branch_address_p before
> use.

I've fixed each of these in the new patch.  See below.

> I wonder whether we have to worry about the case of a goto
> jumping to the same line?  Like:
> 
>   goto label; foo (); label: bar ();
> 
> Not the most usual or beautiful code to write, though I'd guess code
> generators could output things like that.  Writing it in several lines
> but behind a #define might be another way to get everything in
> the same line.

Here is what I tried:

volatile int v;
...
  v = 0;
  goto d; c: v++; d: if (v < 3) goto c;	/* Loop 5 */

Here's what gcc generates for x86_64 (on my machine, anyway):

=> 0x4005c2 <loop_test+210>:    movl   $0x0,0x200a64(%rip)        # 0x601030 <v>
   0x4005cc <loop_test+220>:    nop
   0x4005cd <loop_test+221>:    mov    0x200a5d(%rip),%eax        # 0x601030 <v>
   0x4005d3 <loop_test+227>:    cmp    $0x2,%eax
   0x4005d6 <loop_test+230>:    jg     0x4005ea <loop_test+250>
   0x4005d8 <loop_test+232>:    nop
   0x4005d9 <loop_test+233>:    mov    0x200a51(%rip),%eax        # 0x601030 <v>
   0x4005df <loop_test+239>:    add    $0x1,%eax
   0x4005e2 <loop_test+242>:    mov    %eax,0x200a48(%rip)        # 0x601030 <v>
   0x4005e8 <loop_test+248>:    jmp    0x4005cd <loop_test+221>

If I place a breakpoint on the Loop 5 line, it ends up on on 0x4005cc,
which is a nop.  It appears that gcc has rewritten this code so that
the the comparison comes first, which exactly the opposite of what
happens when one writes a while loop.

The presence of the nop means that things work as expected when we
place a breakpoint on that line.  It will stop just once on the
statement instead of once for each time through the loop.

This was an interesting exercise, but it doesn't really answer the
question of what would happen if gcc would instead place an
actual branch at the beginning.  I would expect my patch
to cause the breakpoint to be placed within the loop instead
of at the beginning of the statement.  I don't think this is
desirable, but I can't think of a way (within the current work) to
stop it from happening.

Here's the new patch:

    Break at each iteration for breakpoints placed on a while statement.
    
    This patch changes create_sals_line_offset() in linespec.c so that, for
    a given SAL, if that SAL's address (pc) refers to an unconditional
    branch instruction whose branch target also refers to the same SAL, then
    the branch target is used for the SAL instead.
    
    The pratical effect of this is that a breakpoint placed on a while
    loop will break at the evaluation of the condition instead of at the
    unconditional branch which transfers control to the starting address
    for the evaluation of the condition.
    
    Consider the following code snippet (which is taken from one of the
    new tests for this patch set):
    
        9         while (v < 3)                         /* Loop 1 condition */
        10          {
        11            v++;                              /* Loop 1 increment */
        12          }
    
    This is compiled as the following x86_64 code:
    
        0x000000000040059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
        0x00000000004005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax # 0x601030 <v>
        0x00000000004005a6 <loop_test+22>:   add    $0x1,%eax
        0x00000000004005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip) # 0x601030 <v>
        0x00000000004005af <loop_test+31>:   mov    0x200a7b(%rip),%eax # 0x601030 <v>
        0x00000000004005b5 <loop_test+37>:   cmp    $0x2,%eax
        0x00000000004005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
    
    If a breakpoint is placed on line 9, which begins at loop_test+14, this
    change/patch causes the breakpoint to be placed on loop_test+31, which is
    the starting address for the evaluation of the condition.
    
    In order for this to work, an architecture specific method,
    unconditional_branch_destination, was introduced in an earlier patch in
    the set.  I've implemented this method for x86_64, i386, arm, thumb,
    powerpc, rx, and rl78.
    
    I've tested on each of these architectures and see no regressions.
    
    gdb/ChangeLog:
    	* linespec.c (addr_in_sals): New function.
    	(create_sals_line_offset): Adjust SAL whose pc refers to an
    	unconditional branch whose destination is the same line.
---
 gdb/linespec.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4c29c12..4b5cde9 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1808,6 +1808,29 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
     }
 }
 
+/* Return 1 if one of the SALS between 0 and NELTS contains ADDR. 
+   Return 0 otherwise.  */
+
+static int
+addr_in_sals (CORE_ADDR addr, int nelts, struct symtab_and_line *sals)
+{
+  int i;
+
+  for (i = 0; i < nelts; i++)
+    {
+      struct symtab_and_line sal;
+
+      if (sals[i].end == 0)
+        sal = find_pc_sect_line (sals[i].pc, sals[i].section, 0);
+      else
+        sal = sals[i];
+
+      if (sal.pc <= addr && addr < sal.end)
+	return 1;
+    }
+  return 0;
+}
+
 /* Given a line offset in LS, construct the relevant SALs.  */
 
 static struct symtabs_and_lines
@@ -1933,6 +1956,59 @@ create_sals_line_offset (struct linespec_state *self,
 	    struct symbol *sym = (blocks[i]
 				  ? block_containing_function (blocks[i])
 				  : NULL);
+	    int is_branch;
+	    CORE_ADDR branch_dest;
+	    
+	    /* This next bit of code examines the instruction at the
+	       SAL's address (pc).  If the instruction is an
+	       unconditional branch whose branch destination also
+	       refers to the same SAL, then the branch destination is
+	       used for the SAL's pc value instead.
+
+	       The pratical effect of this is that a breakpoint placed
+	       on a while loop will break at the evaluation of the
+	       condition instead of at an unconditional branch which
+	       transfers control to the starting address for the
+	       evaluation of the condition.
+
+	       Consider the following code snippet (which is taken
+	       from the test case for gdb.base/loop-break.exp.)
+
+		 while (v < 3)
+                  {
+                    v++; 
+                  }
+
+	       On x86_64, this might be compiled as follows:
+
+		 0x40059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
+		 0x4005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax
+		 0x4005a6 <loop_test+22>:   add    $0x1,%eax
+		 0x4005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip)
+		 0x4005af <loop_test+31>:   mov    0x200a7b(%rip),%eax
+		 0x4005b5 <loop_test+37>:   cmp    $0x2,%eax
+		 0x4005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
+
+	       If a breakpoint is placed on the while statement line
+	       which begins at loop_test+14, the following code causes
+	       the breakpoint to be (instead) placed on loop_test+31, which
+	       is the starting address for the evaluation of the condition.  */
+
+	    is_branch = gdbarch_unconditional_branch_destination
+	      (get_objfile_arch
+	        (SYMTAB_OBJFILE (intermediate_results.sals[i].symtab)),
+	       intermediate_results.sals[i].pc,
+	       &branch_dest);
+
+	    /* Only use branch destination if it's in the same block and
+	       is also within one of the sals from the initial list.  */
+	    if (is_branch && blocks[i]->startaddr <= branch_dest
+	        && branch_dest < blocks[i]->endaddr
+		&& addr_in_sals (branch_dest, intermediate_results.nelts,
+		                 intermediate_results.sals))
+	      {
+		intermediate_results.sals[i].pc = branch_dest;
+	      }
 
 	    if (self->funfirstline)
 	      skip_prologue_sal (&intermediate_results.sals[i]);

Re: [PATCH 4/8] Implement unconditional_branch_address method for x86-64 and i386

[Kevin Buettner]

Here is an updated patch for x86_64 and i386.

    Implement unconditional_branch_destination method for x86-64 and i386.
    
    gdb/ChangeLog:
    
    	* i386-tdep.c (i386_unconditional_branch_destination): New function.
    	(i386_gdbarch_init): Register i386_unconditional_branch_destination.
---
 gdb/i386-tdep.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 9d52d4a..4fd9f6e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8237,6 +8237,23 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep,
   return valid_p;
 }
 
+/* Implement the unconditional_branch_destination gdbarch method.  */
+
+static int
+i386_unconditional_branch_destination (struct gdbarch *gdbarch, CORE_ADDR pc,
+                                       CORE_ADDR *dest)
+{
+  CORE_ADDR new_pc = i386_follow_jump (gdbarch, pc);
+
+  if (new_pc == pc)
+    return 0;
+  else
+    {
+      *dest = new_pc;
+      return 1;
+    }
+}
+
 \f
 static struct gdbarch *
 i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -8582,6 +8599,10 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_fast_tracepoint_valid_at (gdbarch,
 					i386_fast_tracepoint_valid_at);
 
+  /* Unconditional Branch.  */
+  set_gdbarch_unconditional_branch_destination
+    (gdbarch, i386_unconditional_branch_destination);
+
   return gdbarch;
 }
 

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Andrew Burgess]

* Kevin Buettner <kevinb@redhat.com> [2015-08-19 00:00:02 -0700]:

> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index c1e2c1a..1770960 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -924,7 +924,7 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
>     If your architecture doesn't need to adjust instructions before
>     single-stepping them, consider using simple_displaced_step_copy_insn
>     here.
> -
> +  
>     If the instruction cannot execute out of line, return NULL.  The
>     core falls back to stepping past the instruction in-line instead in
>     that case. */
> @@ -1478,6 +1478,16 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
>  extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
>  extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
>  
> +/* Examine instruction at PC.  If instruction at PC is an unconditional
> +   branch, return the address to which control is transferred when the
> +   branch is taken.  Return 0 when this method is not implemented by
> +   architecture, PC refers to an invalid address, or instruction at PC
> +   is not an unconditional branch. */
> +
> +typedef CORE_ADDR (gdbarch_unconditional_branch_address_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc);
> +extern CORE_ADDR gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc);
> +extern void set_gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, gdbarch_unconditional_branch_address_ftype *unconditional_branch_address);

Personally I'm not a fan of overloading the return values on these
functions, especially when the return value is an address, on some
targets 0 is a valid address.  I know there are lots of other places
where we use 0 as a special address in gdb, so this would be just one
more... but...

How would you feel about changing the function so that it returned a
bool and placed the address into a CORE_ADDRESS passed by pointer?

Just a thought,
thanks
Andrew

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2015-09-18 13:01:57 +0100]:

> * Kevin Buettner <kevinb@redhat.com> [2015-08-19 00:00:02 -0700]:
> 
> > diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> > index c1e2c1a..1770960 100644
> > --- a/gdb/gdbarch.h
> > +++ b/gdb/gdbarch.h
> > @@ -924,7 +924,7 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
> >     If your architecture doesn't need to adjust instructions before
> >     single-stepping them, consider using simple_displaced_step_copy_insn
> >     here.
> > -
> > +  
> >     If the instruction cannot execute out of line, return NULL.  The
> >     core falls back to stepping past the instruction in-line instead in
> >     that case. */
> > @@ -1478,6 +1478,16 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
> >  extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
> >  extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
> >  
> > +/* Examine instruction at PC.  If instruction at PC is an unconditional
> > +   branch, return the address to which control is transferred when the
> > +   branch is taken.  Return 0 when this method is not implemented by
> > +   architecture, PC refers to an invalid address, or instruction at PC
> > +   is not an unconditional branch. */
> > +
> > +typedef CORE_ADDR (gdbarch_unconditional_branch_address_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc);
> > +extern CORE_ADDR gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc);
> > +extern void set_gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, gdbarch_unconditional_branch_address_ftype *unconditional_branch_address);

<snip>

> How would you feel about changing the function so that it returned a
> bool and placed the address into a CORE_ADDRESS passed by pointer?

Or I could actually spot your revised patch and see you've already
done this :-/

Sorry for the noise.
Andrew

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Kevin Buettner]

On Fri, 18 Sep 2015 13:01:57 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> * Kevin Buettner <kevinb@redhat.com> [2015-08-19 00:00:02 -0700]:
> 
> > diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> > index c1e2c1a..1770960 100644
> > --- a/gdb/gdbarch.h
> > +++ b/gdb/gdbarch.h
> > @@ -924,7 +924,7 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
> >     If your architecture doesn't need to adjust instructions before
> >     single-stepping them, consider using simple_displaced_step_copy_insn
> >     here.
> > -
> > +  
> >     If the instruction cannot execute out of line, return NULL.  The
> >     core falls back to stepping past the instruction in-line instead in
> >     that case. */
> > @@ -1478,6 +1478,16 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
> >  extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
> >  extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
> >  
> > +/* Examine instruction at PC.  If instruction at PC is an unconditional
> > +   branch, return the address to which control is transferred when the
> > +   branch is taken.  Return 0 when this method is not implemented by
> > +   architecture, PC refers to an invalid address, or instruction at PC
> > +   is not an unconditional branch. */
> > +
> > +typedef CORE_ADDR (gdbarch_unconditional_branch_address_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc);
> > +extern CORE_ADDR gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc);
> > +extern void set_gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, gdbarch_unconditional_branch_address_ftype *unconditional_branch_address);
> 
> Personally I'm not a fan of overloading the return values on these
> functions, especially when the return value is an address, on some
> targets 0 is a valid address.  I know there are lots of other places
> where we use 0 as a special address in gdb, so this would be just one
> more... but...
> 
> How would you feel about changing the function so that it returned a
> bool and placed the address into a CORE_ADDRESS passed by pointer?

Pedro had a similar observation. The new patch that I posted does this.
(Though it uses int instead of bool.)

Kevin

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Kevin Buettner]

On Fri, 18 Sep 2015 13:06:06 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> * Andrew Burgess <andrew.burgess@embecosm.com> [2015-09-18 13:01:57 +0100]:
> 
> > * Kevin Buettner <kevinb@redhat.com> [2015-08-19 00:00:02 -0700]:
> > 
> > > diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> > > index c1e2c1a..1770960 100644
> > > --- a/gdb/gdbarch.h
> > > +++ b/gdb/gdbarch.h
> > > @@ -924,7 +924,7 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
> > >     If your architecture doesn't need to adjust instructions before
> > >     single-stepping them, consider using simple_displaced_step_copy_insn
> > >     here.
> > > -
> > > +  
> > >     If the instruction cannot execute out of line, return NULL.  The
> > >     core falls back to stepping past the instruction in-line instead in
> > >     that case. */
> > > @@ -1478,6 +1478,16 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
> > >  extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
> > >  extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
> > >  
> > > +/* Examine instruction at PC.  If instruction at PC is an unconditional
> > > +   branch, return the address to which control is transferred when the
> > > +   branch is taken.  Return 0 when this method is not implemented by
> > > +   architecture, PC refers to an invalid address, or instruction at PC
> > > +   is not an unconditional branch. */
> > > +
> > > +typedef CORE_ADDR (gdbarch_unconditional_branch_address_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc);
> > > +extern CORE_ADDR gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, CORE_ADDR pc);
> > > +extern void set_gdbarch_unconditional_branch_address (struct gdbarch *gdbarch, gdbarch_unconditional_branch_address_ftype *unconditional_branch_address);
> 
> <snip>
> 
> > How would you feel about changing the function so that it returned a
> > bool and placed the address into a CORE_ADDRESS passed by pointer?
> 
> Or I could actually spot your revised patch and see you've already
> done this :-/

Oops.  I'm guilty of this too. (I should have looked ahead to see this email.)

Kevin

Re: [PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Kevin Buettner]

The test case for loop-break.exp contains some code which performs
looping via gotos:

    volatile int v;
    ...
48        v = 0;
49        goto b;                               /* Loop 4 initial goto */
50      a:  v++;
51      b:  if (v < 3) goto a;                  /* Loop 4 condition */

While testing on arm and powerpc, I'm seeing this failure:

FAIL: gdb.base/loop-break.exp: continue to Loop 4 condition, 3 (the program exited)

The test places a breakpoint on lines 49 and 51. We expect the breakpoint
at line 49 to be hit once and the breakpoint at line 51 to be hit four
times, with v assuming the values 0, 1, 2, and 3 at successive stops
at this breakpoint.

For both arm and powerpc, the breakpoint on line 51 is being hit only
three times; the breakpoint is not being hit when v = 3.

In order to figure out what's going on, I placed breakpoints on lines
48, 49, 50, and 51.  Here's the assembly code for arm with annotations
showing the location of each breakpoint.  I've adjusted GDB's output
somewhat to better fit in 80 columns (without wrapping) along with
providing a more informative comment regarding v.

0x833c <loop_test+228>:   ldr r3, [pc, #64]   ; v		# line 48 bkpt
0x8340 <loop_test+232>:   mov     r2, #0
0x8344 <loop_test+236>:   str     r2, [r3]
0x8348 <loop_test+240>:   b       0x8364 <loop_test+268>	# line 49 bkpt
0x834c <loop_test+244>:   nop					# line 51 bkpt
0x8350 <loop_test+248>:   ldr r3, [pc, #44]   ; v		# line 50 bkpt
0x8354 <loop_test+252>:   ldr     r3, [r3]
0x8358 <loop_test+256>:   add     r3, r3, #1
0x835c <loop_test+260>:   ldr r2, [pc, #32]   ; v
0x8360 <loop_test+264>:   str     r3, [r2]
0x8364 <loop_test+268>:   ldr r3, [pc, #24]   ; v		# Loop 4 Cond
0x8368 <loop_test+272>:   ldr     r3, [r3]
0x836c <loop_test+276>:   cmp     r3, #2
0x8370 <loop_test+280>:   ble     0x834c <loop_test+244>
0x8374 <loop_test+284>:   nop

The locations for breakpoints for line 48, 49, and 50 are not surprising;
these are exactly where I would expect them to be.

The breakpoint for line 51 is placed on the nop instruction
immediately following the branch.  This nop is the branch destination
for the conditional branch at the bottom of the loop.  As such, the
breakpoint at line 51 will only be hit after evaluation of the condition,
but immediately before the increment on line 50.

The correct location for a breakpoint on line 51 is 0x8364, which I've
annotated with "Loop 4 Cond".

I've performed an analysis for powerpc too; the code being generated
is the same (modulo the differences in instruction sets) as that of
arm.

The .debug_line section contains these statements for this section of
code:

[0x0000016c]  Special opcode 68: advance Address by 8 to 0x833c and Line by 7 to 48
[0x0000016d]  Special opcode 90: advance Address by 12 to 0x8348 and Line by 1 to 49
[0x0000016e]  Special opcode 35: advance Address by 4 to 0x834c and Line by 2 to 51
[0x0000016f]  Special opcode 32: advance Address by 4 to 0x8350 and Line by -1 to 50
[0x00000170]  Special opcode 146: advance Address by 20 to 0x8364 and Line by 1 to 51

It's not clear to me why the nop is considered to be part of line 51. 
This might make (some) sense if either architecture had branch delay
slots, but to the best of my knowledge they do not.

In any case, things would work correctly if the statement at
0x0000016e were removed.

Kevin

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Yao Qi]

Kevin Buettner <kevinb@redhat.com> writes:

> Add new gdbarch method, unconditional_branch_address.
>
> gdb/ChangeLog:
>
>     	* gdbarch.sh (unconditional_branch_address): New gdbarch method.
>     	* gdbarch.h, gdbarch.c: Regenerate.
>     	* arch-utils.h (default_unconditional_branch_address): Declare.
>     	* arch-utils.c (default_unconditional_branch_address): New function.

Hi Kevin,
Did you consider using existing gdbarch method
adjust_breakpoint_address when you wrote the patch?  Can we use
adjust_breakpoint_address here rather than adding a new gdbarch method?

-- 
Yao (齐尧)

Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Kevin Buettner]

On Tue, 22 Sep 2015 17:09:17 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> Kevin Buettner <kevinb@redhat.com> writes:
> 
> > Add new gdbarch method, unconditional_branch_address.
> >
> > gdb/ChangeLog:
> >
> >     	* gdbarch.sh (unconditional_branch_address): New gdbarch method.
> >     	* gdbarch.h, gdbarch.c: Regenerate.
> >     	* arch-utils.h (default_unconditional_branch_address): Declare.
> >     	* arch-utils.c (default_unconditional_branch_address): New function.
> 
> Did you consider using existing gdbarch method
> adjust_breakpoint_address when you wrote the patch?  Can we use
> adjust_breakpoint_address here rather than adding a new gdbarch method?

Hi Yao,

The adjust_breakpoint_address method is not suitable for this patch
set.  It was originally added for the FR-V architecture.  FR-V is a
VLIW architecture which places limits on where a breakpoint may be
placed within a bundle.  For FR-V, this method ensures that a breakpoint
is placed at the beginning of the VLIW bundle.  I see now that it has
also been used for MIPS to avoid placing breakpoints on branch delay
slots and also for ARM to avoid placing breakpoints within IT
(if-then) blocks.  In each of these cases, the breakpoint is moved
because, if is not, the breakpoint might not be hit.

The gdbarch method that I'm introducing in this patch set needs to
limit itself to checking to see if the instruction at a given address
is an unconditional branch; if it is, it will return true and also
provide the branch destination address via an output parameter.  An
existing gdbarch method that might be suitable (with some
modification) for this purpose is insn_is_jump, which is presently
used by the btrace code.

The insn_is_jump method is a predicate only; at the moment, it does
not provide the branch destination.  It could be changed to do so,
but I think it makes sense to limit use of the insn_is_{jump,ret,call}
methods to btrace.  This is the justification that I provided in
another reply to this thread:

    I decided that the work I'm doing here should have it's own
    method.  If GCC should someday emit DWARF which sets is_stmt to 0
    for the branch instruction at the beginning of a while loop, this
    work might not be needed any longer.  It'll be easier to rip it
    out and clean things up again if I don't modify gdbarch methods
    that were added for other purposes.

Kevin

Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Pedro Alves]

Hi Kevin,

Sorry for the delay.

On 09/18/2015 02:57 AM, Kevin Buettner wrote:

> If I place a breakpoint on the Loop 5 line, it ends up on on 0x4005cc,
> which is a nop.  It appears that gcc has rewritten this code so that
> the the comparison comes first, which exactly the opposite of what
> happens when one writes a while loop.
> 
> The presence of the nop means that things work as expected when we
> place a breakpoint on that line.  It will stop just once on the
> statement instead of once for each time through the loop.
> 
> This was an interesting exercise, but it doesn't really answer the
> question of what would happen if gcc would instead place an
> actual branch at the beginning.  I would expect my patch
> to cause the breakpoint to be placed within the loop instead
> of at the beginning of the statement.  I don't think this is
> desirable, but I can't think of a way (within the current work) to
> stop it from happening.

Yeah, me neither.  This is always going to be brittle.  :-/
Really the best would be DWARF is_stmt.

> 
> Here's the new patch:
> 
>     Break at each iteration for breakpoints placed on a while statement.
>     
>     This patch changes create_sals_line_offset() in linespec.c so that, for
>     a given SAL, if that SAL's address (pc) refers to an unconditional
>     branch instruction whose branch target also refers to the same SAL, then
>     the branch target is used for the SAL instead.
>     
>     The pratical effect of this is that a breakpoint placed on a while

"practical"

>     loop will break at the evaluation of the condition instead of at the
>     unconditional branch which transfers control to the starting address
>     for the evaluation of the condition.
>     
>     Consider the following code snippet (which is taken from one of the
>     new tests for this patch set):
>     
>         9         while (v < 3)                         /* Loop 1 condition */
>         10          {
>         11            v++;                              /* Loop 1 increment */
>         12          }
>     
>     This is compiled as the following x86_64 code:
>     
>         0x000000000040059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
>         0x00000000004005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax # 0x601030 <v>
>         0x00000000004005a6 <loop_test+22>:   add    $0x1,%eax
>         0x00000000004005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip) # 0x601030 <v>
>         0x00000000004005af <loop_test+31>:   mov    0x200a7b(%rip),%eax # 0x601030 <v>
>         0x00000000004005b5 <loop_test+37>:   cmp    $0x2,%eax
>         0x00000000004005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
>     
>     If a breakpoint is placed on line 9, which begins at loop_test+14, this
>     change/patch causes the breakpoint to be placed on loop_test+31, which is
>     the starting address for the evaluation of the condition.
>     
>     In order for this to work, an architecture specific method,
>     unconditional_branch_destination, was introduced in an earlier patch in
>     the set.  I've implemented this method for x86_64, i386, arm, thumb,
>     powerpc, rx, and rl78.
>     
>     I've tested on each of these architectures and see no regressions.
>     
>     gdb/ChangeLog:
>     	* linespec.c (addr_in_sals): New function.
>     	(create_sals_line_offset): Adjust SAL whose pc refers to an
>     	unconditional branch whose destination is the same line.
> ---
>  gdb/linespec.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 4c29c12..4b5cde9 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -1808,6 +1808,29 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
>      }
>  }
>  
> +/* Return 1 if one of the SALS between 0 and NELTS contains ADDR. 
> +   Return 0 otherwise.  */
> +
> +static int
> +addr_in_sals (CORE_ADDR addr, int nelts, struct symtab_and_line *sals)
> +{
> +  int i;
> +
> +  for (i = 0; i < nelts; i++)
> +    {
> +      struct symtab_and_line sal;
> +
> +      if (sals[i].end == 0)
> +        sal = find_pc_sect_line (sals[i].pc, sals[i].section, 0);
> +      else
> +        sal = sals[i];
> +
> +      if (sal.pc <= addr && addr < sal.end)
> +	return 1;
> +    }
> +  return 0;
> +}
> +
>  /* Given a line offset in LS, construct the relevant SALs.  */
>  
>  static struct symtabs_and_lines
> @@ -1933,6 +1956,59 @@ create_sals_line_offset (struct linespec_state *self,
>  	    struct symbol *sym = (blocks[i]
>  				  ? block_containing_function (blocks[i])
>  				  : NULL);
> +	    int is_branch;
> +	    CORE_ADDR branch_dest;
> +	    
> +	    /* This next bit of code examines the instruction at the
> +	       SAL's address (pc).  If the instruction is an
> +	       unconditional branch whose branch destination also
> +	       refers to the same SAL, then the branch destination is
> +	       used for the SAL's pc value instead.
> +
> +	       The pratical effect of this is that a breakpoint placed

"practical".

> +	       on a while loop will break at the evaluation of the
> +	       condition instead of at an unconditional branch which
> +	       transfers control to the starting address for the
> +	       evaluation of the condition.
> +
> +	       Consider the following code snippet (which is taken
> +	       from the test case for gdb.base/loop-break.exp.)
> +
> +		 while (v < 3)
> +                  {
> +                    v++; 
> +                  }
> +
> +	       On x86_64, this might be compiled as follows:
> +
> +		 0x40059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
> +		 0x4005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax
> +		 0x4005a6 <loop_test+22>:   add    $0x1,%eax
> +		 0x4005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip)
> +		 0x4005af <loop_test+31>:   mov    0x200a7b(%rip),%eax
> +		 0x4005b5 <loop_test+37>:   cmp    $0x2,%eax
> +		 0x4005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
> +
> +	       If a breakpoint is placed on the while statement line
> +	       which begins at loop_test+14, the following code causes
> +	       the breakpoint to be (instead) placed on loop_test+31, which
> +	       is the starting address for the evaluation of the condition.  */
> +
> +	    is_branch = gdbarch_unconditional_branch_destination
> +	      (get_objfile_arch
> +	        (SYMTAB_OBJFILE (intermediate_results.sals[i].symtab)),
> +	       intermediate_results.sals[i].pc,
> +	       &branch_dest);
> +
> +	    /* Only use branch destination if it's in the same block and
> +	       is also within one of the sals from the initial list.  */
> +	    if (is_branch && blocks[i]->startaddr <= branch_dest
> +	        && branch_dest < blocks[i]->endaddr
> +		&& addr_in_sals (branch_dest, intermediate_results.nelts,
> +		                 intermediate_results.sals))

I believe intermediate_results can contain sals of different
pspaces / inferiors.  Shouldn't we take that into account somehow?

> +	      {
> +		intermediate_results.sals[i].pc = branch_dest;
> +	      }

Unnecessary braces.

>  
>  	    if (self->funfirstline)
>  	      skip_prologue_sal (&intermediate_results.sals[i]);
> 

Thanks,
Pedro Alves

Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

Hi Pedro,

Thanks again for the review.

On Wed, 30 Sep 2015 13:16:55 +0100
Pedro Alves <palves@redhat.com> wrote:

> >     The pratical effect of this is that a breakpoint placed on a while
> 
> "practical"

> > +	       The pratical effect of this is that a breakpoint placed
> 
> "practical".

Both fixed in new patch.

> > +	    /* Only use branch destination if it's in the same block and
> > +	       is also within one of the sals from the initial list.  */
> > +	    if (is_branch && blocks[i]->startaddr <= branch_dest
> > +	        && branch_dest < blocks[i]->endaddr
> > +		&& addr_in_sals (branch_dest, intermediate_results.nelts,
> > +		                 intermediate_results.sals))
> 
> I believe intermediate_results can contain sals of different
> pspaces / inferiors.  Shouldn't we take that into account somehow?

Good catch.  I had not considered this.

In the new patch, I've added a `pspace' paramater to addr_in_sals()
and have added the necessary code to that function.  The end result is
that only sals in the program space of the sal that we might
potentially fix up will be considered.  (I hope that makes sense.)

> 
> > +	      {
> > +		intermediate_results.sals[i].pc = branch_dest;
> > +	      }
> 
> Unnecessary braces.

I took them out.

Here's the revised patch:

commit 6018c9ae5ea2bc5fcf723ce9a9beeb315d3fb3c0
Author: Kevin Buettner <kevinb@redhat.com>
Date:   Tue Aug 18 22:06:40 2015 -0700

    Break at each iteration for breakpoints placed on a while statement.
    
    This patch changes create_sals_line_offset() in linespec.c so that, for
    a given SAL, if that SAL's address (pc) refers to an unconditional
    branch instruction whose branch target also refers to the same SAL, then
    the branch target is used for the SAL instead.
    
    The practical effect of this is that a breakpoint placed on a while
    loop will break at the evaluation of the condition instead of at the
    unconditional branch which transfers control to the starting address
    for the evaluation of the condition.
    
    Consider the following code snippet (which is taken from one of the
    new tests for this patch set):
    
        9         while (v < 3)                         /* Loop 1 condition */
        10          {
        11            v++;                              /* Loop 1 increment */
        12          }
    
    This is compiled as the following x86_64 code:
    
        0x000000000040059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
        0x00000000004005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax # 0x601030 <v>
        0x00000000004005a6 <loop_test+22>:   add    $0x1,%eax
        0x00000000004005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip) # 0x601030 <v>
        0x00000000004005af <loop_test+31>:   mov    0x200a7b(%rip),%eax # 0x601030 <v>
        0x00000000004005b5 <loop_test+37>:   cmp    $0x2,%eax
        0x00000000004005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
    
    If a breakpoint is placed on line 9, which begins at loop_test+14, this
    change/patch causes the breakpoint to be placed on loop_test+31, which is
    the starting address for the evaluation of the condition.
    
    In order for this to work, an architecture specific method,
    unconditional_branch_destination, was introduced in an earlier patch in
    the set.  I've implemented this method for x86_64, i386, arm, thumb,
    powerpc, rx, and rl78.
    
    I've tested on each of these architectures and see no regressions.
    
    gdb/ChangeLog:
    	* linespec.c (addr_in_sals): New function.
    	(create_sals_line_offset): Adjust SAL whose pc refers to an
    	unconditional branch whose destination is the same line.
---
 gdb/linespec.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index b2233b9..1ba0e67 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1809,6 +1809,37 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
     }
 }
 
+/* Return 1 if one of the SALS between 0 and NELTS contains ADDR. 
+   Return 0 otherwise.  */
+
+static int
+addr_in_sals (CORE_ADDR addr, struct program_space *pspace, int nelts,
+              struct symtab_and_line *sals)
+{
+  int i;
+
+  /* Make sure we're in the correct program space for a potential
+     call to find_pc_sect_line, below.  */
+  set_current_program_space (pspace);
+
+  for (i = 0; i < nelts; i++)
+    {
+      struct symtab_and_line sal;
+
+      if (pspace != sals[i].pspace)
+	continue;
+
+      if (sals[i].end == 0)
+        sal = find_pc_sect_line (sals[i].pc, sals[i].section, 0);
+      else
+        sal = sals[i];
+
+      if (sal.pc <= addr && addr < sal.end)
+	return 1;
+    }
+  return 0;
+}
+
 /* Given a line offset in LS, construct the relevant SALs.  */
 
 static struct symtabs_and_lines
@@ -1934,6 +1965,60 @@ create_sals_line_offset (struct linespec_state *self,
 	    struct symbol *sym = (blocks[i]
 				  ? block_containing_function (blocks[i])
 				  : NULL);
+	    int is_branch;
+	    CORE_ADDR branch_dest;
+	    
+	    /* This next bit of code examines the instruction at the
+	       SAL's address (pc).  If the instruction is an
+	       unconditional branch whose branch destination also
+	       refers to the same SAL, then the branch destination is
+	       used for the SAL's pc value instead.
+
+	       The practical effect of this is that a breakpoint placed
+	       on a while loop will break at the evaluation of the
+	       condition instead of at an unconditional branch which
+	       transfers control to the starting address for the
+	       evaluation of the condition.
+
+	       Consider the following code snippet (which is taken
+	       from the test case for gdb.base/loop-break.exp.)
+
+		 while (v < 3)
+                  {
+                    v++; 
+                  }
+
+	       On x86_64, this might be compiled as follows:
+
+		 0x40059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
+		 0x4005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax
+		 0x4005a6 <loop_test+22>:   add    $0x1,%eax
+		 0x4005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip)
+		 0x4005af <loop_test+31>:   mov    0x200a7b(%rip),%eax
+		 0x4005b5 <loop_test+37>:   cmp    $0x2,%eax
+		 0x4005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
+
+	       If a breakpoint is placed on the while statement line
+	       which begins at loop_test+14, the following code causes
+	       the breakpoint to be (instead) placed on loop_test+31, which
+	       is the starting address for the evaluation of the condition.  */
+
+	    is_branch = gdbarch_unconditional_branch_destination
+	      (get_objfile_arch
+	        (SYMTAB_OBJFILE (intermediate_results.sals[i].symtab)),
+	       intermediate_results.sals[i].pc,
+	       &branch_dest);
+
+	    /* Only use branch destination if it's in the same block and
+	       is also within one of the sals (within the same program
+	       space) from the initial list.  */
+	    if (is_branch && blocks[i]->startaddr <= branch_dest
+	        && branch_dest < blocks[i]->endaddr
+		&& addr_in_sals (branch_dest,
+		                 intermediate_results.sals[i].pspace,
+		                 intermediate_results.nelts,
+		                 intermediate_results.sals))
+	      intermediate_results.sals[i].pc = branch_dest;
 
 	    if (self->funfirstline)
 	      skip_prologue_sal (&intermediate_results.sals[i]);

Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Doug Evans]

On Wed, Sep 30, 2015 at 5:16 AM, Pedro Alves <palves@redhat.com> wrote:
> Hi Kevin,
>
> Sorry for the delay.
>
> On 09/18/2015 02:57 AM, Kevin Buettner wrote:
>
>> If I place a breakpoint on the Loop 5 line, it ends up on on 0x4005cc,
>> which is a nop.  It appears that gcc has rewritten this code so that
>> the the comparison comes first, which exactly the opposite of what
>> happens when one writes a while loop.
>>
>> The presence of the nop means that things work as expected when we
>> place a breakpoint on that line.  It will stop just once on the
>> statement instead of once for each time through the loop.
>>
>> This was an interesting exercise, but it doesn't really answer the
>> question of what would happen if gcc would instead place an
>> actual branch at the beginning.  I would expect my patch
>> to cause the breakpoint to be placed within the loop instead
>> of at the beginning of the statement.  I don't think this is
>> desirable, but I can't think of a way (within the current work) to
>> stop it from happening.
>
> Yeah, me neither.  This is always going to be brittle.  :-/
> Really the best would be DWARF is_stmt.

I hate to enter into this late, but holy cow,
is this really what the dwarf committee and gcc developers
expect gdb to do?

Assuming, for the moment, that dwarf handles this just fine,
can't we push back on gcc?

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

Ping.

On Tue, 18 Aug 2015 23:53:34 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> This patch set changes the current behavior of breakpoints placed on
> while loops. (It actually restores past behavior; see below.)
> 
> Consider the following code:
> 
> 7         v = 0;
> 8
> 9         while (v < 3)                         /* Loop 1 condition */
> 10          {
> 11            v++;                              /* Loop 1 increment */
> 12          }
> 
> This example is taken from the new test case, loop-break.c.
> 
> Let's place breakpoints at lines 9 and 11:
> 
> (gdb) b 9
> Breakpoint 1 at 0x4005af: file gdb.base/loop-break.c, line 9.
> (gdb) b 11
> Breakpoint 2 at 0x4005a0: file gdb.base/loop-break.c, line 11.
> 
> We'll run the program and then continue to get to breakpoint #2:
> 
> (gdb) run
> Starting program: gdb.base/loop-break
> 
> Breakpoint 1, loop_test ()
>     at gdb.base/loop-break.c:9
> 9         while (v < 3)                         /* Loop 1 condition */
> (gdb) c
> Continuing.
> 
> Breakpoint 2, loop_test ()
>     at gdb.base/loop-break.c:11
> 11            v++;                              /* Loop 1 increment */
> 
> So far, so good.  Now, watch what happens when we continue again:
> 
> (gdb) c
> Continuing.
> 
> Breakpoint 2, loop_test ()
>     at /ironwood1/sourceware-git/mesquite-native-5509943/bld/../../binutils-gdb/gdb/testsuite/gdb.base/loop-break.c:11
> 11            v++;                              /* Loop 1 increment */
> 
> GDB has somehow missed the breakpoint placed on line 9.  The user is
> unable to examine state prior to evaluation of the loop condition.
> 
> The compiler is implementing this looping construct in the following
> fashion.  An unconditional branch is placed at the start of the loop,
> branching to an address immediately after the loop body.  At that point,
> the condition is evaluated.  If the condition evaluates as true, a
> conditional branch causes execution to continue at the first instruction
> of the loop body, placed immediately after the unconditional branch.
> 
> GDB is placing its breakpoint on the unconditional branch. This is fine
> for the first iteration of the loop, but does not work for subsequent
> iterations.
> 
> This is the code that gcc generates on x86_64:
> 
> 0x000000000040059e <loop_test+14>:   jmp    0x4005af <loop_test+31>
> 0x00000000004005a0 <loop_test+16>:   mov    0x200a8a(%rip),%eax # 0x601030 <v>
> 0x00000000004005a6 <loop_test+22>:   add    $0x1,%eax
> 0x00000000004005a9 <loop_test+25>:   mov    %eax,0x200a81(%rip) # 0x601030 <v>
> 0x00000000004005af <loop_test+31>:   mov    0x200a7b(%rip),%eax # 0x601030 <v>
> 0x00000000004005b5 <loop_test+37>:   cmp    $0x2,%eax
> 0x00000000004005b8 <loop_test+40>:   jle    0x4005a0 <loop_test+16>
> 
> The breakpoint is being placed on 0x40059e (loop_test+14).  As such, it
> gets hit only once.  If we could arrange to have the breakpoint placed at
> the branch target, it would stop at each iteration of the loop. I.e.
> it would behave like this:
> 
> (gdb) b 9
> Breakpoint 1 at 0x4005af: file gdb.base/loop-break.c, line 9.
> (gdb) b 11
> Breakpoint 2 at 0x4005a0: file gdb.base/loop-break.c, line 11.
> (gdb) command 1
> Type commands for breakpoint(s) 1, one per line.
> End with a line saying just "end".
> >p v
> >end
> (gdb) run
> Starting program: gdb.base/loop-break
> 
> Breakpoint 1, loop_test ()
>     at gdb.base/loop-break.c:9
> 9         while (v < 3)                         /* Loop 1 condition */
> $1 = 0
> (gdb) c
> Continuing.
> 
> Breakpoint 2, loop_test ()
>     at gdb.base/loop-break.c:11
> 11            v++;                              /* Loop 1 increment */
> (gdb) c
> Continuing.
> 
> Breakpoint 1, loop_test ()
>     at gdb.base/loop-break.c:9
> 9         while (v < 3)                         /* Loop 1 condition */
> $2 = 1
> 
> This change introduces a new gdbarch method for potentially following
> an unconditional branch.  If the circumstances are right, it uses
> this method to cause the breakpoint to be placed on the branch target
> instead of the branch itself.
> 
> This fixes the problem described above and causes no regressions.
> The new test case includes the loop shown above.  It also contains
> several other loops which verify that GDB stops at the the correct
> places in each.  Only while loops of the form shown above are
> affected by this patch; other looping constructs continue to work
> as they had before.
> 
> Of particular interest is the test which uses an explicit goto; this
> mimics the code that the compiler generates for a while loop.
> However, in this example, placing a breakpoint on the goto should
> cause execution to stop on the goto.  My initial attempt at a fix
> for this problem caused that explicit goto to be followed, which is
> definitely not correct.
> 
> Lastly, I'll note that, in the distant past, GCC used to output code
> for the condition at the top of the loop.  GDB from either then or now
> will stop at the evaluation of the condition each time through the
> loop.
> 
> I was able to locate a compiler that I could run from circa 1998. While
> I was able to run the compiler, I was not able to run the linker; it
> died with an internal error in collect2, undoubtedly due to not having
> compatible libraries.  But I was able to use -S in order to see
> what the assembly code looked like.  Here is the assembly code for
> our example loop, compiled by gcc 2.9, targeting i386-pc-linux-gnu:
> 
> 	    movl $0,v
>     .stabn 68,0,9,.LM3-loop_test
>     .LM3:
> 	    .align 4
>     .L2:
> 	    movl v,%eax
> 	    cmpl $2,%eax
> 	    jle .L4
> 	    jmp .L3
> 	    .align 4
>     .L4:
>     .stabn 68,0,11,.LM4-loop_test
>     .LM4:
> 	    movl v,%eax
> 	    leal 1(%eax),%edx
> 	    movl %edx,v
>     .stabn 68,0,12,.LM5-loop_test
>     .LM5:
> 	    jmp .L2
>     .L3:
> 
> The loop begins with the evaluation of the condition at .L2.  The jle
> branches to instructions forming the body of the loop; the jmp instruction
> immediately following the jle gets us out of the loop.
> 
> This code isn't very efficient, but it is a straightforward translation
> of the corresponding C code.  A breakpoint placed at the beginning of
> line 9 (which is both .LM3 and .L2) will stop on each iteration through
> the loop.
> 
> My patch-set restores this behavior for while loops implemented in
> a more efficient manner.

Re: [PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Kevin Buettner]

Ping.

I started to sent a ping for the 0/8 post, but decided instead to send
a ping for the latest version of the 1/8 patch, which is the test case.

I'd really like to see this test case go in, even if the rest of it
doesn't.

I should note that if the rest doesn't go in, I ought to rewrite the
test case somewhat to use KFAIL on the part that's known to fail (and
for which we're waiting for help from the compiler) in addition to
structuring it so that we don't get cascade failures from the first
part of the test.

That said, if the rest of the patch series goes in, along with the
test case, I won't need modify the test case.

Thoughts?

Kevin

On Thu, 17 Sep 2015 17:50:08 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> On Tue, 25 Aug 2015 13:10:06 +0100
> Pedro Alves <palves@redhat.com> wrote:
> 
> > On 08/19/2015 07:57 AM, Kevin Buettner wrote:
> > 
> > > +++ b/gdb/testsuite/gdb.base/loop-break.c
> > 
> > Copyright header missing.
> > 
> > > +int main (int argc, char **argv)
> > 
> > Line break after first int.
> > 
> > > +if ![runto_main] then { fail "loop-break tests suppressed" }
> > 
> > This should return rather than continue into a cascade of failures:
> > 
> > if ![runto_main] then {
> >     fail "Can't run to main"
> >     return 0
> > }
> > 
> > > +proc continue_to { linenum testname iter } {
> > > +    global srcfile
> > > +    global gdb_prompt
> > > +    set full_name "continue to $testname, $iter"
> > > +
> > > +    send_gdb "continue\n"
> > > +    gdb_expect {
> > > +	-re "Continuing.*Breakpoint.*$srcfile:$linenum\r\n.*\r\n$gdb_prompt $" {
> > > +	    pass $full_name
> > > +	}
> > > +	-re ".*$gdb_prompt $" {
> > > +	    fail $full_name
> > > +	}
> > > +	timeout { 
> > > +	    fail "$full_name (timeout)"
> > > +	}
> > 
> > Use gdb_test_multiple.  Or even, gdb_test ?
> 
> Hi Pedro,
> 
> Thanks for the review.  The version below fixes the problems you identified...
> 
> 
> 
> Add new test, gdb.base/loop-break.exp.
> 
> This test places breakpoints at various points on several different
> looping constructs, making sure that GDB behaves as expected.
> 
> gdb/testsuite/ChangeLog:
>     
>     	* gdb.base/loop-break.c, gdb.base/loop-break.exp: New files.
> ---
>  gdb/testsuite/gdb.base/loop-break.c   |  60 ++++++++++++++++++++
>  gdb/testsuite/gdb.base/loop-break.exp | 104 ++++++++++++++++++++++++++++++++++
>  2 files changed, 164 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/loop-break.c b/gdb/testsuite/gdb.base/loop-break.c
> new file mode 100644
> index 0000000..7b7d0f7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/loop-break.c
> @@ -0,0 +1,60 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2015 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/>.  */
> +
> +volatile int v;
> +volatile int w;
> +
> +void
> +loop_test (void)
> +{
> +  v = 0;
> +
> +  while (v < 3)				/* Loop 1 condition */
> +    {
> +      v++;				/* Loop 1 increment */
> +    }
> +
> +  v = -42;
> +
> +  for (v = 0; v < 3; )			/* Loop 2 */
> +    {
> +      v++;				/* Loop 2 increment */
> +    }
> +
> +  v = -42;
> +  w = 42;
> +
> +  for (v = 0;				/* Loop 3 initialization */
> +       v < 3;				/* Loop 3 condition */
> +       v++)				/* Loop 3 increment */
> +     {
> +      w = w - v;	
> +     }
> +
> +  v = 0;
> +  goto b;				/* Loop 4 initial goto */
> +a:  v++;
> +b:  if (v < 3) goto a;			/* Loop 4 condition */
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  loop_test ();
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/loop-break.exp b/gdb/testsuite/gdb.base/loop-break.exp
> new file mode 100644
> index 0000000..b573a7b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/loop-break.exp
> @@ -0,0 +1,104 @@
> +# Copyright 2015 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/>.
> +
> +# Place breakpoints at various points on several different looping
> +# constructs.  Make sure that GDB correctly runs to each of these
> +# breakpoints and that computed values are correct at each point along
> +# the way.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +	fail "Can't run to main"
> +	return 0
> +}
> +
> +proc break_at { search_string } {
> +	global srcfile
> +	set bp_location [gdb_get_line_number $search_string]
> +	gdb_test "break $bp_location" \
> +	    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> +	    "break $search_string"
> +
> +	return $bp_location;
> +}
> +
> +proc continue_to { linenum testname iter } {
> +	global srcfile
> +	gdb_test "continue" \
> +	         "Continuing.*Breakpoint.*$srcfile:$linenum.*" \
> +		 "continue to $testname, $iter"
> +}
> +
> +set bp_location1a [break_at "Loop 1 condition"]
> +set bp_location1b [break_at "Loop 1 increment"]
> +set bp_location2a [break_at "Loop 2"]
> +set bp_location2b [break_at "Loop 2 increment"]
> +set bp_location3a [break_at "Loop 3 initialization"]
> +set bp_location3b [break_at "Loop 3 condition"]
> +set bp_location3c [break_at "Loop 3 increment"]
> +set bp_location4a [break_at "Loop 4 initial goto"]
> +set bp_location4b [break_at "Loop 4 condition"]
> +
> +continue_to $bp_location1a "Loop 1 condition" 0
> +gdb_test "p v" "= 0" "Loop 1 value check at condition 0"
> +continue_to $bp_location1b "Loop 1 increment" 0
> +gdb_test "p v" "= 0" "Loop 1 value check at increment 0"
> +
> +continue_to $bp_location1a "Loop 1 condition" 1
> +gdb_test "p v" "= 1" "Loop 1 value check at condition 1"
> +continue_to $bp_location1b "Loop 1 increment" 1
> +gdb_test "p v" "= 1" "Loop 1 value check at increment 1"
> +
> +continue_to $bp_location1a "Loop 1 condition" 2
> +continue_to $bp_location1b "Loop 1 increment" 2
> +
> +continue_to $bp_location1a "Loop 1 condition" 3
> +
> +continue_to $bp_location2a "Loop 2" 0
> +gdb_test "p v" "= -42" "Loop 2 value check at loop start"
> +continue_to $bp_location2b "Loop 2 increment" 0
> +gdb_test "p v" "= 0" "Loop 2 value check at increment 0"
> +continue_to $bp_location2b "Loop 2 increment" 1
> +gdb_test "p v" "= 1" "Loop 2 value check at increment 1"
> +continue_to $bp_location2b "Loop 2 increment" 2
> +gdb_test "p v" "= 2" "Loop 2 value check at increment 2"
> +
> +continue_to $bp_location3a "Loop 3 initialization" 0
> +gdb_test "p v" "= -42" "Loop 3 value check at initialization"
> +continue_to $bp_location3b "Loop 3 condition" 0
> +gdb_test "p v" "= 0" "Loop 3 value check at condition 0"
> +continue_to $bp_location3c "Loop 3 increment" 0
> +gdb_test "p v" "= 0" "Loop 3 value check at increment 0"
> +continue_to $bp_location3b "Loop 3 condition" 1
> +continue_to $bp_location3c "Loop 3 increment" 1
> +continue_to $bp_location3b "Loop 3 condition" 2
> +continue_to $bp_location3c "Loop 3 increment" 2
> +continue_to $bp_location3b "Loop 3 condition" 3
> +
> +continue_to $bp_location4a "Loop 4 initial goto" 0
> +gdb_test "p v" "= 0" "Loop 4 value check at initial goto"
> +continue_to $bp_location4b "Loop 4 condition" 0
> +gdb_test "p v" "= 0" "Loop 4 value check at condition 0"
> +continue_to $bp_location4b "Loop 4 condition" 1
> +gdb_test "p v" "= 1" "Loop 4 value check at condition 1"
> +continue_to $bp_location4b "Loop 4 condition" 2
> +gdb_test "p v" "= 2" "Loop 4 value check at condition 2"
> +continue_to $bp_location4b "Loop 4 condition" 3
> +gdb_test "p v" "= 3" "Loop 4 value check at condition 3"
> 

Re: [PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Kevin Buettner]

Ping.

On Mon, 1 Feb 2016 13:00:23 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> Ping.
> 
> I started to sent a ping for the 0/8 post, but decided instead to send
> a ping for the latest version of the 1/8 patch, which is the test case.
> 
> I'd really like to see this test case go in, even if the rest of it
> doesn't.
> 
> I should note that if the rest doesn't go in, I ought to rewrite the
> test case somewhat to use KFAIL on the part that's known to fail (and
> for which we're waiting for help from the compiler) in addition to
> structuring it so that we don't get cascade failures from the first
> part of the test.
> 
> That said, if the rest of the patch series goes in, along with the
> test case, I won't need modify the test case.
> 
> Thoughts?
> 
> Kevin
> 
> On Thu, 17 Sep 2015 17:50:08 -0700
> Kevin Buettner <kevinb@redhat.com> wrote:
> 
> > On Tue, 25 Aug 2015 13:10:06 +0100
> > Pedro Alves <palves@redhat.com> wrote:
> > 
> > > On 08/19/2015 07:57 AM, Kevin Buettner wrote:
> > > 
> > > > +++ b/gdb/testsuite/gdb.base/loop-break.c
> > > 
> > > Copyright header missing.
> > > 
> > > > +int main (int argc, char **argv)
> > > 
> > > Line break after first int.
> > > 
> > > > +if ![runto_main] then { fail "loop-break tests suppressed" }
> > > 
> > > This should return rather than continue into a cascade of failures:
> > > 
> > > if ![runto_main] then {
> > >     fail "Can't run to main"
> > >     return 0
> > > }
> > > 
> > > > +proc continue_to { linenum testname iter } {
> > > > +    global srcfile
> > > > +    global gdb_prompt
> > > > +    set full_name "continue to $testname, $iter"
> > > > +
> > > > +    send_gdb "continue\n"
> > > > +    gdb_expect {
> > > > +	-re "Continuing.*Breakpoint.*$srcfile:$linenum\r\n.*\r\n$gdb_prompt $" {
> > > > +	    pass $full_name
> > > > +	}
> > > > +	-re ".*$gdb_prompt $" {
> > > > +	    fail $full_name
> > > > +	}
> > > > +	timeout { 
> > > > +	    fail "$full_name (timeout)"
> > > > +	}
> > > 
> > > Use gdb_test_multiple.  Or even, gdb_test ?
> > 
> > Hi Pedro,
> > 
> > Thanks for the review.  The version below fixes the problems you identified...
> > 
> > 
> > 
> > Add new test, gdb.base/loop-break.exp.
> > 
> > This test places breakpoints at various points on several different
> > looping constructs, making sure that GDB behaves as expected.
> > 
> > gdb/testsuite/ChangeLog:
> >     
> >     	* gdb.base/loop-break.c, gdb.base/loop-break.exp: New files.
> > ---
> >  gdb/testsuite/gdb.base/loop-break.c   |  60 ++++++++++++++++++++
> >  gdb/testsuite/gdb.base/loop-break.exp | 104 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 164 insertions(+)
> > 
> > diff --git a/gdb/testsuite/gdb.base/loop-break.c b/gdb/testsuite/gdb.base/loop-break.c
> > new file mode 100644
> > index 0000000..7b7d0f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/loop-break.c
> > @@ -0,0 +1,60 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2015 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/>.  */
> > +
> > +volatile int v;
> > +volatile int w;
> > +
> > +void
> > +loop_test (void)
> > +{
> > +  v = 0;
> > +
> > +  while (v < 3)				/* Loop 1 condition */
> > +    {
> > +      v++;				/* Loop 1 increment */
> > +    }
> > +
> > +  v = -42;
> > +
> > +  for (v = 0; v < 3; )			/* Loop 2 */
> > +    {
> > +      v++;				/* Loop 2 increment */
> > +    }
> > +
> > +  v = -42;
> > +  w = 42;
> > +
> > +  for (v = 0;				/* Loop 3 initialization */
> > +       v < 3;				/* Loop 3 condition */
> > +       v++)				/* Loop 3 increment */
> > +     {
> > +      w = w - v;	
> > +     }
> > +
> > +  v = 0;
> > +  goto b;				/* Loop 4 initial goto */
> > +a:  v++;
> > +b:  if (v < 3) goto a;			/* Loop 4 condition */
> > +}
> > +
> > +int
> > +main (int argc, char **argv)
> > +{
> > +  loop_test ();
> > +
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.base/loop-break.exp b/gdb/testsuite/gdb.base/loop-break.exp
> > new file mode 100644
> > index 0000000..b573a7b
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/loop-break.exp
> > @@ -0,0 +1,104 @@
> > +# Copyright 2015 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/>.
> > +
> > +# Place breakpoints at various points on several different looping
> > +# constructs.  Make sure that GDB correctly runs to each of these
> > +# breakpoints and that computed values are correct at each point along
> > +# the way.
> > +
> > +standard_testfile
> > +
> > +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> > +    return -1
> > +}
> > +
> > +if ![runto_main] then {
> > +	fail "Can't run to main"
> > +	return 0
> > +}
> > +
> > +proc break_at { search_string } {
> > +	global srcfile
> > +	set bp_location [gdb_get_line_number $search_string]
> > +	gdb_test "break $bp_location" \
> > +	    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> > +	    "break $search_string"
> > +
> > +	return $bp_location;
> > +}
> > +
> > +proc continue_to { linenum testname iter } {
> > +	global srcfile
> > +	gdb_test "continue" \
> > +	         "Continuing.*Breakpoint.*$srcfile:$linenum.*" \
> > +		 "continue to $testname, $iter"
> > +}
> > +
> > +set bp_location1a [break_at "Loop 1 condition"]
> > +set bp_location1b [break_at "Loop 1 increment"]
> > +set bp_location2a [break_at "Loop 2"]
> > +set bp_location2b [break_at "Loop 2 increment"]
> > +set bp_location3a [break_at "Loop 3 initialization"]
> > +set bp_location3b [break_at "Loop 3 condition"]
> > +set bp_location3c [break_at "Loop 3 increment"]
> > +set bp_location4a [break_at "Loop 4 initial goto"]
> > +set bp_location4b [break_at "Loop 4 condition"]
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 0
> > +gdb_test "p v" "= 0" "Loop 1 value check at condition 0"
> > +continue_to $bp_location1b "Loop 1 increment" 0
> > +gdb_test "p v" "= 0" "Loop 1 value check at increment 0"
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 1
> > +gdb_test "p v" "= 1" "Loop 1 value check at condition 1"
> > +continue_to $bp_location1b "Loop 1 increment" 1
> > +gdb_test "p v" "= 1" "Loop 1 value check at increment 1"
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 2
> > +continue_to $bp_location1b "Loop 1 increment" 2
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 3
> > +
> > +continue_to $bp_location2a "Loop 2" 0
> > +gdb_test "p v" "= -42" "Loop 2 value check at loop start"
> > +continue_to $bp_location2b "Loop 2 increment" 0
> > +gdb_test "p v" "= 0" "Loop 2 value check at increment 0"
> > +continue_to $bp_location2b "Loop 2 increment" 1
> > +gdb_test "p v" "= 1" "Loop 2 value check at increment 1"
> > +continue_to $bp_location2b "Loop 2 increment" 2
> > +gdb_test "p v" "= 2" "Loop 2 value check at increment 2"
> > +
> > +continue_to $bp_location3a "Loop 3 initialization" 0
> > +gdb_test "p v" "= -42" "Loop 3 value check at initialization"
> > +continue_to $bp_location3b "Loop 3 condition" 0
> > +gdb_test "p v" "= 0" "Loop 3 value check at condition 0"
> > +continue_to $bp_location3c "Loop 3 increment" 0
> > +gdb_test "p v" "= 0" "Loop 3 value check at increment 0"
> > +continue_to $bp_location3b "Loop 3 condition" 1
> > +continue_to $bp_location3c "Loop 3 increment" 1
> > +continue_to $bp_location3b "Loop 3 condition" 2
> > +continue_to $bp_location3c "Loop 3 increment" 2
> > +continue_to $bp_location3b "Loop 3 condition" 3
> > +
> > +continue_to $bp_location4a "Loop 4 initial goto" 0
> > +gdb_test "p v" "= 0" "Loop 4 value check at initial goto"
> > +continue_to $bp_location4b "Loop 4 condition" 0
> > +gdb_test "p v" "= 0" "Loop 4 value check at condition 0"
> > +continue_to $bp_location4b "Loop 4 condition" 1
> > +gdb_test "p v" "= 1" "Loop 4 value check at condition 1"
> > +continue_to $bp_location4b "Loop 4 condition" 2
> > +gdb_test "p v" "= 2" "Loop 4 value check at condition 2"
> > +continue_to $bp_location4b "Loop 4 condition" 3
> > +gdb_test "p v" "= 3" "Loop 4 value check at condition 3"
> > 

Re: [PATCH 1/8] Add new test, gdb.base/loop-break.exp

[Kevin Buettner]

Ping.

On Mon, 1 Feb 2016 13:00:23 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> Ping.
> 
> I started to sent a ping for the 0/8 post, but decided instead to send
> a ping for the latest version of the 1/8 patch, which is the test case.
> 
> I'd really like to see this test case go in, even if the rest of it
> doesn't.
> 
> I should note that if the rest doesn't go in, I ought to rewrite the
> test case somewhat to use KFAIL on the part that's known to fail (and
> for which we're waiting for help from the compiler) in addition to
> structuring it so that we don't get cascade failures from the first
> part of the test.
> 
> That said, if the rest of the patch series goes in, along with the
> test case, I won't need modify the test case.
> 
> Thoughts?
> 
> Kevin
> 
> On Thu, 17 Sep 2015 17:50:08 -0700
> Kevin Buettner <kevinb@redhat.com> wrote:
> 
> > On Tue, 25 Aug 2015 13:10:06 +0100
> > Pedro Alves <palves@redhat.com> wrote:
> > 
> > > On 08/19/2015 07:57 AM, Kevin Buettner wrote:
> > > 
> > > > +++ b/gdb/testsuite/gdb.base/loop-break.c
> > > 
> > > Copyright header missing.
> > > 
> > > > +int main (int argc, char **argv)
> > > 
> > > Line break after first int.
> > > 
> > > > +if ![runto_main] then { fail "loop-break tests suppressed" }
> > > 
> > > This should return rather than continue into a cascade of failures:
> > > 
> > > if ![runto_main] then {
> > >     fail "Can't run to main"
> > >     return 0
> > > }
> > > 
> > > > +proc continue_to { linenum testname iter } {
> > > > +    global srcfile
> > > > +    global gdb_prompt
> > > > +    set full_name "continue to $testname, $iter"
> > > > +
> > > > +    send_gdb "continue\n"
> > > > +    gdb_expect {
> > > > +	-re "Continuing.*Breakpoint.*$srcfile:$linenum\r\n.*\r\n$gdb_prompt $" {
> > > > +	    pass $full_name
> > > > +	}
> > > > +	-re ".*$gdb_prompt $" {
> > > > +	    fail $full_name
> > > > +	}
> > > > +	timeout { 
> > > > +	    fail "$full_name (timeout)"
> > > > +	}
> > > 
> > > Use gdb_test_multiple.  Or even, gdb_test ?
> > 
> > Hi Pedro,
> > 
> > Thanks for the review.  The version below fixes the problems you identified...
> > 
> > 
> > 
> > Add new test, gdb.base/loop-break.exp.
> > 
> > This test places breakpoints at various points on several different
> > looping constructs, making sure that GDB behaves as expected.
> > 
> > gdb/testsuite/ChangeLog:
> >     
> >     	* gdb.base/loop-break.c, gdb.base/loop-break.exp: New files.
> > ---
> >  gdb/testsuite/gdb.base/loop-break.c   |  60 ++++++++++++++++++++
> >  gdb/testsuite/gdb.base/loop-break.exp | 104 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 164 insertions(+)
> > 
> > diff --git a/gdb/testsuite/gdb.base/loop-break.c b/gdb/testsuite/gdb.base/loop-break.c
> > new file mode 100644
> > index 0000000..7b7d0f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/loop-break.c
> > @@ -0,0 +1,60 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2015 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/>.  */
> > +
> > +volatile int v;
> > +volatile int w;
> > +
> > +void
> > +loop_test (void)
> > +{
> > +  v = 0;
> > +
> > +  while (v < 3)				/* Loop 1 condition */
> > +    {
> > +      v++;				/* Loop 1 increment */
> > +    }
> > +
> > +  v = -42;
> > +
> > +  for (v = 0; v < 3; )			/* Loop 2 */
> > +    {
> > +      v++;				/* Loop 2 increment */
> > +    }
> > +
> > +  v = -42;
> > +  w = 42;
> > +
> > +  for (v = 0;				/* Loop 3 initialization */
> > +       v < 3;				/* Loop 3 condition */
> > +       v++)				/* Loop 3 increment */
> > +     {
> > +      w = w - v;	
> > +     }
> > +
> > +  v = 0;
> > +  goto b;				/* Loop 4 initial goto */
> > +a:  v++;
> > +b:  if (v < 3) goto a;			/* Loop 4 condition */
> > +}
> > +
> > +int
> > +main (int argc, char **argv)
> > +{
> > +  loop_test ();
> > +
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.base/loop-break.exp b/gdb/testsuite/gdb.base/loop-break.exp
> > new file mode 100644
> > index 0000000..b573a7b
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/loop-break.exp
> > @@ -0,0 +1,104 @@
> > +# Copyright 2015 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/>.
> > +
> > +# Place breakpoints at various points on several different looping
> > +# constructs.  Make sure that GDB correctly runs to each of these
> > +# breakpoints and that computed values are correct at each point along
> > +# the way.
> > +
> > +standard_testfile
> > +
> > +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> > +    return -1
> > +}
> > +
> > +if ![runto_main] then {
> > +	fail "Can't run to main"
> > +	return 0
> > +}
> > +
> > +proc break_at { search_string } {
> > +	global srcfile
> > +	set bp_location [gdb_get_line_number $search_string]
> > +	gdb_test "break $bp_location" \
> > +	    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> > +	    "break $search_string"
> > +
> > +	return $bp_location;
> > +}
> > +
> > +proc continue_to { linenum testname iter } {
> > +	global srcfile
> > +	gdb_test "continue" \
> > +	         "Continuing.*Breakpoint.*$srcfile:$linenum.*" \
> > +		 "continue to $testname, $iter"
> > +}
> > +
> > +set bp_location1a [break_at "Loop 1 condition"]
> > +set bp_location1b [break_at "Loop 1 increment"]
> > +set bp_location2a [break_at "Loop 2"]
> > +set bp_location2b [break_at "Loop 2 increment"]
> > +set bp_location3a [break_at "Loop 3 initialization"]
> > +set bp_location3b [break_at "Loop 3 condition"]
> > +set bp_location3c [break_at "Loop 3 increment"]
> > +set bp_location4a [break_at "Loop 4 initial goto"]
> > +set bp_location4b [break_at "Loop 4 condition"]
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 0
> > +gdb_test "p v" "= 0" "Loop 1 value check at condition 0"
> > +continue_to $bp_location1b "Loop 1 increment" 0
> > +gdb_test "p v" "= 0" "Loop 1 value check at increment 0"
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 1
> > +gdb_test "p v" "= 1" "Loop 1 value check at condition 1"
> > +continue_to $bp_location1b "Loop 1 increment" 1
> > +gdb_test "p v" "= 1" "Loop 1 value check at increment 1"
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 2
> > +continue_to $bp_location1b "Loop 1 increment" 2
> > +
> > +continue_to $bp_location1a "Loop 1 condition" 3
> > +
> > +continue_to $bp_location2a "Loop 2" 0
> > +gdb_test "p v" "= -42" "Loop 2 value check at loop start"
> > +continue_to $bp_location2b "Loop 2 increment" 0
> > +gdb_test "p v" "= 0" "Loop 2 value check at increment 0"
> > +continue_to $bp_location2b "Loop 2 increment" 1
> > +gdb_test "p v" "= 1" "Loop 2 value check at increment 1"
> > +continue_to $bp_location2b "Loop 2 increment" 2
> > +gdb_test "p v" "= 2" "Loop 2 value check at increment 2"
> > +
> > +continue_to $bp_location3a "Loop 3 initialization" 0
> > +gdb_test "p v" "= -42" "Loop 3 value check at initialization"
> > +continue_to $bp_location3b "Loop 3 condition" 0
> > +gdb_test "p v" "= 0" "Loop 3 value check at condition 0"
> > +continue_to $bp_location3c "Loop 3 increment" 0
> > +gdb_test "p v" "= 0" "Loop 3 value check at increment 0"
> > +continue_to $bp_location3b "Loop 3 condition" 1
> > +continue_to $bp_location3c "Loop 3 increment" 1
> > +continue_to $bp_location3b "Loop 3 condition" 2
> > +continue_to $bp_location3c "Loop 3 increment" 2
> > +continue_to $bp_location3b "Loop 3 condition" 3
> > +
> > +continue_to $bp_location4a "Loop 4 initial goto" 0
> > +gdb_test "p v" "= 0" "Loop 4 value check at initial goto"
> > +continue_to $bp_location4b "Loop 4 condition" 0
> > +gdb_test "p v" "= 0" "Loop 4 value check at condition 0"
> > +continue_to $bp_location4b "Loop 4 condition" 1
> > +gdb_test "p v" "= 1" "Loop 4 value check at condition 1"
> > +continue_to $bp_location4b "Loop 4 condition" 2
> > +gdb_test "p v" "= 2" "Loop 4 value check at condition 2"
> > +continue_to $bp_location4b "Loop 4 condition" 3
> > +gdb_test "p v" "= 3" "Loop 4 value check at condition 3"
> > 

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Yao Qi]

Kevin Buettner <kevinb@redhat.com> writes:

> This patch set changes the current behavior of breakpoints placed on
> while loops. (It actually restores past behavior; see below.)

Hi Pedro,
your recent change https://sourceware.org/ml/gdb-patches/2016-03/msg00462.html
triggers a test failure in gdb.base/jit.exp on arm-linux that breakpoint
on the loop "WAIT_FOR_GDB" isn't hit.  The problem is also found by
buildbot, https://sourceware.org/ml/gdb-testers/2016-q1/msg10203.html

The problem is what this patch series is trying to fix, so I think we
do need this patch series.  Do you plan to review them? (I ask you this
because you gave comments to one of these patches) or I can review
them if you don't have cycles.

-- 
Yao (齐尧)

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Luis Machado]

On 04/04/2016 10:56 AM, Yao Qi wrote:
> Kevin Buettner <kevinb@redhat.com> writes:
>
>> This patch set changes the current behavior of breakpoints placed on
>> while loops. (It actually restores past behavior; see below.)
>
> Hi Pedro,
> your recent change https://sourceware.org/ml/gdb-patches/2016-03/msg00462.html
> triggers a test failure in gdb.base/jit.exp on arm-linux that breakpoint
> on the loop "WAIT_FOR_GDB" isn't hit.  The problem is also found by
> buildbot, https://sourceware.org/ml/gdb-testers/2016-q1/msg10203.html
>
> The problem is what this patch series is trying to fix, so I think we
> do need this patch series.  Do you plan to review them? (I ask you this
> because you gave comments to one of these patches) or I can review
> them if you don't have cycles.
>

Thinking about the series, isn't this problem related to the compiler 
optimizing things and/or presenting an imprecise addr/line information 
as opposed to GDB doing the wrong thing.

Just wondering if working around the compiler is the best choice or if 
we risk breaking something else depending on the way code is output in 
some cases.

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Yao Qi]

Luis Machado <lgustavo@codesourcery.com> writes:

> Thinking about the series, isn't this problem related to the compiler
> optimizing things and/or presenting an imprecise addr/line information
> as opposed to GDB doing the wrong thing.

I think the address/line information is correct, and compiler does
nothing wrong in this case.  The first instruction of for loop is 
branch/jmp, which is correctly mapped to the right line.  GDB should
insert breakpoint at the place which can be hit in every iteration of
loop on that line, rather than the first instruction of the loop (or line).

-- 
Yao (齐尧)

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

On Fri, 15 Apr 2016 12:59:19 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> Luis Machado <lgustavo@codesourcery.com> writes:
> 
> > Thinking about the series, isn't this problem related to the compiler
> > optimizing things and/or presenting an imprecise addr/line information
> > as opposed to GDB doing the wrong thing.
> 
> I think the address/line information is correct, and compiler does
> nothing wrong in this case.  The first instruction of for loop is 
> branch/jmp, which is correctly mapped to the right line.  GDB should
> insert breakpoint at the place which can be hit in every iteration of
> loop on that line, rather than the first instruction of the loop (or line).

If the compiler could arrange to set `is_stmt' to false for that
inital branch, we could more easily arrange for GDB to not place a
break on that initial branch.  It might even "just work" without any
additional coding on our part.

One of my colleagues within Red Hat looked at this and came up with a
gcc patch, but it turned out to cause breakage elsewhere.  I still
think it makes sense to try to tackle this problem from the compiler
side though.

Kevin

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Pedro Alves]

On 04/15/2016 08:48 PM, Kevin Buettner wrote:

> If the compiler could arrange to set `is_stmt' to false for that
> inital branch, we could more easily arrange for GDB to not place a
> break on that initial branch.  It might even "just work" without any
> additional coding on our part.
> 
> One of my colleagues within Red Hat looked at this and came up with a
> gcc patch, but it turned out to cause breakage elsewhere.  I still
> think it makes sense to try to tackle this problem from the compiler
> side though.

Was a gcc bug ever filed for this?

Thanks,
Pedro Alves

Re: [PATCH 0/8] Break at each iteration for breakpoints placed on a while statement

[Kevin Buettner]

On Fri, 15 Apr 2016 23:34:14 +0100
Pedro Alves <palves@redhat.com> wrote:

> On 04/15/2016 08:48 PM, Kevin Buettner wrote:
> 
> > If the compiler could arrange to set `is_stmt' to false for that
> > inital branch, we could more easily arrange for GDB to not place a
> > break on that initial branch.  It might even "just work" without any
> > additional coding on our part.
> > 
> > One of my colleagues within Red Hat looked at this and came up with a
> > gcc patch, but it turned out to cause breakage elsewhere.  I still
> > think it makes sense to try to tackle this problem from the compiler
> > side though.
> 
> Was a gcc bug ever filed for this?

Not by me.

I'll do some checking; if there's not one, I'll file it.

Kevin