[PATCH] Improve C++ loop's backward-jump location

Andreas Arnez arnez@linux.vnet.ibm.com
Tue Nov 10 11:23:00 GMT 2015


While fixing PR67192 it turned out that there are multiple potential
issues with the location of the backward-jump statement generated by the
C and C++ front-ends for an unconditional loop.

First, due to a bug the C front-end had sometimes set the location to
the token following the loop.  This is what triggered PR67192.

The C front-end's intention was to set the location to the last line of
the loop instead.  This is not perfect either; it may be slightly
confusing in the special case where the loop body consists of an if-else
statement: Breaking on that line then causes a breakpoint hit on every
iteration, even if the else-path is never executed.

The C++ front-end does not have these issues; it sets the location to
the "while" or "for" token.  But this can cause confusion when setting a
breakpoint on "while (1)": When hitting it for the first time, one loop
iteration will already have executed.

To fix this as well, this patch mimics the fix applied to the C
front-end, making the front-ends behave identically in this regard.

gcc/cp/ChangeLog:

	* cp-gimplify.c (genericize_cp_loop): Change LOOP_EXPR's location
	to start of loop body instead of start of loop.

gcc/testsuite/ChangeLog:

	* g++.dg/guality/pr67192.C: New test.
---
 gcc/cp/cp-gimplify.c                   |  7 ++-
 gcc/testsuite/g++.dg/guality/pr67192.C | 79 ++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/guality/pr67192.C

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index e4b50e5..d9bb708 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -266,7 +266,12 @@ genericize_cp_loop (tree *stmt_p, location_t start_locus, tree cond, tree body,
 	loop = stmt_list;
     }
   else
-    loop = build1_loc (start_locus, LOOP_EXPR, void_type_node, stmt_list);
+    {
+      location_t loc = EXPR_LOCATION (expr_first (body));
+      if (loc == UNKNOWN_LOCATION)
+	loc = start_locus;
+      loop = build1_loc (loc, LOOP_EXPR, void_type_node, stmt_list);
+    }
 
   stmt_list = NULL;
   append_to_statement_list (loop, &stmt_list);
diff --git a/gcc/testsuite/g++.dg/guality/pr67192.C b/gcc/testsuite/g++.dg/guality/pr67192.C
new file mode 100644
index 0000000..c09ecf8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/guality/pr67192.C
@@ -0,0 +1,79 @@
+/* PR debug/67192 */
+/* { dg-do run } */
+/* { dg-options "-x c++ -g -Wmisleading-indentation" } */
+
+volatile int cnt = 0;
+
+__attribute__((noinline, noclone)) static int
+last (void)
+{
+  return ++cnt % 5 == 0;
+}
+
+__attribute__((noinline, noclone)) static void
+do_it (void)
+{
+  asm volatile ("" : : "r" (&cnt) : "memory");
+}
+
+__attribute__((noinline, noclone)) static void
+f1 (void)
+{
+  for (;; do_it())
+    {
+      if (last ())
+	break;
+    }
+  do_it (); /* { dg-final { gdb-test 27 "cnt" "5" } } */
+}
+
+__attribute__((noinline, noclone)) static void
+f2 (void)
+{
+  while (1)
+    {
+      if (last ())
+	break;
+      do_it ();
+    }
+  do_it (); /* { dg-final { gdb-test 39 "cnt" "10" } } */
+}
+
+__attribute__((noinline, noclone)) static void
+f3 (void)
+{
+  for (;; do_it())
+    if (last ())
+      break;
+  do_it (); /* { dg-final { gdb-test 48 "cnt" "15" } } */
+}
+
+__attribute__((noinline, noclone)) static void
+f4 (void)
+{
+  while (1) /* { dg-final { gdb-test 54 "cnt" "15" } } */
+    if (last ())
+      break;
+    else
+      do_it ();
+  do_it (); /* { dg-final { gdb-test 59 "cnt" "20" } } */
+}
+
+void (*volatile fnp1) (void) = f1;
+void (*volatile fnp2) (void) = f2;
+void (*volatile fnp3) (void) = f3;
+void (*volatile fnp4) (void) = f4;
+
+int
+main ()
+{
+  asm volatile ("" : : "r" (&fnp1) : "memory");
+  asm volatile ("" : : "r" (&fnp2) : "memory");
+  asm volatile ("" : : "r" (&fnp3) : "memory");
+  asm volatile ("" : : "r" (&fnp4) : "memory");
+  fnp1 ();
+  fnp2 ();
+  fnp3 ();
+  fnp4 ();
+  return 0;
+}
-- 
2.3.0



More information about the Gcc-patches mailing list