Hi!
The introduction of -gstatement-frontiers (and making it the default)
regressed debug info experience on the attached testcases (not including
in the testsuite, as not sure how to model that in the guality testsuite).
The current behavior is that
b test
r
n
n
n
shows the first iteration and then executes the rest of the loop iteration
in one go and breaks next time on __libc_start_main.
The following patch fixes that by adding DEBUG_BEGIN_STMTs not just at the
start of the loop, but also at the start of the incr-expr and cond-expr;
the DWARF standard says that is_stmt 1 is a recommended breakpoint location,
doesn't necessarily mean full statement, but also a part of it if it is a
recommended breakpoint location there. And the loop expressions clearly
must be recommended breakpoint locations.
The difference in *.gimple dumps on the 3 testcases is:
--- pr90197-1.c.005t.gimple_ 2019-04-25 10:55:55.087978371 +0200
+++ pr90197-1.c.005t.gimple 2019-04-25 10:56:08.845759542 +0200
@@ -7,23 +7,25 @@ test (unsigned int * dst, unsigned int b
# DEBUG BEGIN_STMT
i = 0;
goto <D.1912>;
<D.1911>:
# DEBUG BEGIN_STMT
_1 = (long unsigned int) i;
_2 = _1 * 4;
_3 = dst + _2;
*_3 = base;
+ # DEBUG BEGIN_STMT
i = i + 1;
base = base + 15;
<D.1912>:
+ # DEBUG BEGIN_STMT
if (i < count) goto <D.1911>; else goto <D.1913>;
<D.1913>:
}
}
main ()
{
int D.1919;
--- pr90197-2.c.005t.gimple_ 2019-04-25 10:55:55.192976701 +0200
+++ pr90197-2.c.005t.gimple 2019-04-25 10:56:08.925758270 +0200
@@ -10,20 +10,21 @@ test (unsigned int * dst, unsigned int b
<D.1911>:
# DEBUG BEGIN_STMT
base = base + 15;
i.0_1 = i;
i = i.0_1 + 1;
_2 = (long unsigned int) i.0_1;
_3 = _2 * 4;
_4 = dst + _3;
*_4 = base;
<D.1912>:
+ # DEBUG BEGIN_STMT
if (i < count) goto <D.1911>; else goto <D.1913>;
<D.1913>:
}
main ()
{
int D.1919;
{
--- pr90197-3.c.005t.gimple_ 2019-04-25 10:55:55.298975015 +0200
+++ pr90197-3.c.005t.gimple 2019-04-25 10:56:09.000757077 +0200
@@ -8,20 +8,21 @@ test (unsigned int * dst, unsigned int b
# DEBUG BEGIN_STMT
<D.1911>:
# DEBUG BEGIN_STMT
base = base + 15;
i.0_1 = i;
i = i.0_1 + 1;
_2 = (long unsigned int) i.0_1;
_3 = _2 * 4;
_4 = dst + _3;
*_4 = base;
+ # DEBUG BEGIN_STMT
if (i < count) goto <D.1911>; else goto <D.1912>;
<D.1912>:
}
main ()
{
int D.1918;
{
so it basically adds a DEBUG_BEGIN_STMT for the increment expression
in for (if not empty) and for the condition in all kinds of loop (both if it
is empty and if it is not).
The assembly changes are:
--- pr90197-1.s1 2019-04-25 11:09:54.749622921 +0200
+++ pr90197-1.s2 2019-04-25 11:09:46.569753036 +0200
@@ -13,6 +13,7 @@ test:
.loc 1 5 3
.LBB2:
.loc 1 5 8
+ .loc 1 5 19
.loc 1 5 3 is_stmt 0
testl %edx, %edx
jle .L1
@@ -27,11 +28,13 @@ test:
.loc 1 6 5 is_stmt 1 discriminator 3
.loc 1 6 12 is_stmt 0 discriminator 3
movl %esi, (%rdi)
- .loc 1 5 40 discriminator 3
+ .loc 1 5 30 is_stmt 1 discriminator 3
+ .loc 1 5 40 is_stmt 0 discriminator 3
addl $15, %esi
.LVL2:
+ .loc 1 5 19 is_stmt 1 discriminator 3
addq $4, %rdi
- .loc 1 5 3 discriminator 3
+ .loc 1 5 3 is_stmt 0 discriminator 3
cmpl %eax, %esi
jne .L3
.L1:
--- pr90197-2.s1 2019-04-25 11:09:54.791622253 +0200
+++ pr90197-2.s2 2019-04-25 11:09:46.611752367 +0200
@@ -12,7 +12,7 @@ test:
.LVL0:
.loc 1 5 3
.loc 1 6 3
- .loc 1 6 9 is_stmt 0
+ .loc 1 6 9
testl %edx, %edx
jle .L1
movl %edx, %eax
@@ -23,18 +23,18 @@ test:
.p2align 4,,10
.p2align 3
.L3:
- .loc 1 7 5 is_stmt 1
+ .loc 1 7 5
.loc 1 7 22 is_stmt 0
addl $15, %esi
.LVL2:
addq $4, %rdi
.loc 1 7 14
movl %esi, -4(%rdi)
- .loc 1 6 9
+ .loc 1 6 9 is_stmt 1
cmpl %eax, %esi
jne .L3
.L1:
- .loc 1 8 1
+ .loc 1 8 1 is_stmt 0
ret
.cfi_endproc
.LFE0:
--- pr90197-3.s1 2019-04-25 11:09:54.828621665 +0200
+++ pr90197-3.s2 2019-04-25 11:09:46.647751794 +0200
@@ -24,9 +24,10 @@ test:
.LVL2:
.loc 1 7 14 discriminator 1
movl %esi, (%rdi,%rax,4)
+ .loc 1 8 9 is_stmt 1 discriminator 1
addq $1, %rax
.LVL3:
- .loc 1 8 3 discriminator 1
+ .loc 1 8 3 is_stmt 0 discriminator 1
cmpl %eax, %edx
jg .L2
.loc 1 9 1
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I'll work on a C++ FE version of this next (needed as well).
2019-04-25 Jakub Jelinek <ja...@redhat.com>
PR debug/90197
* c-tree.h (c_finish_loop): Add 2 further location_t arguments.
* c-parser.c (c_parser_while_statement): Adjust c_finish_loop caller.
(c_parser_do_statement): Likewise.
(c_parser_for_statement): Likewise. Formatting fixes.
* c-typeck.c (c_finish_loop): Add COND_LOCUS and INCR_LOCUS arguments,
emit DEBUG_BEGIN_STMTs if needed.
--- gcc/c/c-tree.h.jj 2019-04-08 10:11:26.942246707 +0200
+++ gcc/c/c-tree.h 2019-04-24 13:35:02.313565354 +0200
@@ -694,7 +694,8 @@ extern int c_types_compatible_p (tree, t
extern tree c_begin_compound_stmt (bool);
extern tree c_end_compound_stmt (location_t, tree, bool);
extern void c_finish_if_stmt (location_t, tree, tree, tree);
-extern void c_finish_loop (location_t, tree, tree, tree, tree, tree, bool);
+extern void c_finish_loop (location_t, location_t, tree, location_t, tree,
+ tree, tree, tree, bool);
extern tree c_begin_stmt_expr (void);
extern tree c_finish_stmt_expr (location_t, tree);
extern tree c_process_expr_stmt (location_t, tree);
--- gcc/c/c-parser.c.jj 2019-03-14 23:44:26.876575932 +0100
+++ gcc/c/c-parser.c 2019-04-24 13:43:05.136794313 +0200
@@ -6001,7 +6001,8 @@ c_parser_while_statement (c_parser *pars
location_t loc_after_labels;
bool open_brace = c_parser_next_token_is (parser, CPP_OPEN_BRACE);
body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
- c_finish_loop (loc, cond, NULL, body, c_break_label, c_cont_label, true);
+ c_finish_loop (loc, loc, cond, UNKNOWN_LOCATION, NULL, body,
+ c_break_label, c_cont_label, true);
add_stmt (c_end_compound_stmt (loc, block, flag_isoc99));
c_parser_maybe_reclassify_token (parser);
@@ -6046,6 +6047,7 @@ c_parser_do_statement (c_parser *parser,
c_break_label = save_break;
new_cont = c_cont_label;
c_cont_label = save_cont;
+ location_t cond_loc = c_parser_peek_token (parser)->location;
cond = c_parser_paren_condition (parser);
if (ivdep && cond != error_mark_node)
cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
@@ -6059,7 +6061,8 @@ c_parser_do_statement (c_parser *parser,
build_int_cst (integer_type_node, unroll));
if (!c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>"))
c_parser_skip_to_end_of_block_or_statement (parser);
- c_finish_loop (loc, cond, NULL, body, new_break, new_cont, false);
+ c_finish_loop (loc, cond_loc, cond, UNKNOWN_LOCATION, NULL, body,
+ new_break, new_cont, false);
add_stmt (c_end_compound_stmt (loc, block, flag_isoc99));
}
@@ -6132,7 +6135,9 @@ c_parser_for_statement (c_parser *parser
/* Silence the bogus uninitialized warning. */
tree collection_expression = NULL;
location_t loc = c_parser_peek_token (parser)->location;
- location_t for_loc = c_parser_peek_token (parser)->location;
+ location_t for_loc = loc;
+ location_t cond_loc = UNKNOWN_LOCATION;
+ location_t incr_loc = UNKNOWN_LOCATION;
bool is_foreach_statement = false;
gcc_assert (c_parser_next_token_is_keyword (parser, RID_FOR));
token_indent_info for_tinfo
@@ -6166,7 +6171,8 @@ c_parser_for_statement (c_parser *parser
c_parser_consume_token (parser);
is_foreach_statement = true;
if (check_for_loop_decls (for_loc, true) == NULL_TREE)
- c_parser_error (parser, "multiple iterating variables in fast
enumeration");
+ c_parser_error (parser, "multiple iterating variables in "
+ "fast enumeration");
}
else
check_for_loop_decls (for_loc, flag_isoc99);
@@ -6196,7 +6202,8 @@ c_parser_for_statement (c_parser *parser
c_parser_consume_token (parser);
is_foreach_statement = true;
if (check_for_loop_decls (for_loc, true) == NULL_TREE)
- c_parser_error (parser, "multiple iterating variables in
fast enumeration");
+ c_parser_error (parser, "multiple iterating variables in "
+ "fast enumeration");
}
else
check_for_loop_decls (for_loc, flag_isoc99);
@@ -6218,15 +6225,18 @@ c_parser_for_statement (c_parser *parser
c_parser_consume_token (parser);
is_foreach_statement = true;
if (! lvalue_p (init_expression))
- c_parser_error (parser, "invalid iterating variable in fast
enumeration");
- object_expression = c_fully_fold (init_expression, false, NULL);
+ c_parser_error (parser, "invalid iterating variable in "
+ "fast enumeration");
+ object_expression
+ = c_fully_fold (init_expression, false, NULL);
}
else
{
ce = convert_lvalue_to_rvalue (loc, ce, true, false);
init_expression = ce.value;
c_finish_expr_stmt (loc, init_expression);
- c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected
%<;%>");
+ c_parser_skip_until_found (parser, CPP_SEMICOLON,
+ "expected %<;%>");
}
}
}
@@ -6235,18 +6245,19 @@ c_parser_for_statement (c_parser *parser
gcc_assert (!parser->objc_could_be_foreach_context);
if (!is_foreach_statement)
{
+ cond_loc = c_parser_peek_token (parser)->location;
if (c_parser_next_token_is (parser, CPP_SEMICOLON))
{
if (ivdep)
{
- c_parser_error (parser, "missing loop condition in loop with "
- "%<GCC ivdep%> pragma");
+ c_parser_error (parser, "missing loop condition in loop "
+ "with %<GCC ivdep%> pragma");
cond = error_mark_node;
}
else if (unroll)
{
- c_parser_error (parser, "missing loop condition in loop with "
- "%<GCC unroll%> pragma");
+ c_parser_error (parser, "missing loop condition in loop "
+ "with %<GCC unroll%> pragma");
cond = error_mark_node;
}
else
@@ -6275,11 +6286,13 @@ c_parser_for_statement (c_parser *parser
/* Parse the increment expression (the third expression in a
for-statement). In the case of a foreach-statement, this is
the expression that follows the 'in'. */
+ loc = incr_loc = c_parser_peek_token (parser)->location;
if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
{
if (is_foreach_statement)
{
- c_parser_error (parser, "missing collection in fast enumeration");
+ c_parser_error (parser,
+ "missing collection in fast enumeration");
collection_expression = error_mark_node;
}
else
@@ -6288,8 +6301,8 @@ c_parser_for_statement (c_parser *parser
else
{
if (is_foreach_statement)
- collection_expression = c_fully_fold (c_parser_expression
(parser).value,
- false, NULL);
+ collection_expression
+ = c_fully_fold (c_parser_expression (parser).value, false, NULL);
else
{
struct c_expr ce = c_parser_expression (parser);
@@ -6312,10 +6325,14 @@ c_parser_for_statement (c_parser *parser
body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
if (is_foreach_statement)
- objc_finish_foreach_loop (loc, object_expression, collection_expression,
body, c_break_label, c_cont_label);
+ objc_finish_foreach_loop (for_loc, object_expression,
+ collection_expression, body, c_break_label,
+ c_cont_label);
else
- c_finish_loop (loc, cond, incr, body, c_break_label, c_cont_label, true);
- add_stmt (c_end_compound_stmt (loc, block, flag_isoc99 || c_dialect_objc
()));
+ c_finish_loop (for_loc, cond_loc, cond, incr_loc, incr, body,
+ c_break_label, c_cont_label, true);
+ add_stmt (c_end_compound_stmt (for_loc, block,
+ flag_isoc99 || c_dialect_objc ()));
c_parser_maybe_reclassify_token (parser);
token_indent_info next_tinfo
--- gcc/c/c-typeck.c.jj 2019-04-19 13:54:36.543027196 +0200
+++ gcc/c/c-typeck.c 2019-04-24 14:52:26.412862798 +0200
@@ -10858,11 +10858,14 @@ c_finish_if_stmt (location_t if_locus, t
the beginning of the loop. COND is the loop condition. COND_IS_FIRST
is false for DO loops. INCR is the FOR increment expression. BODY is
the statement controlled by the loop. BLAB is the break label. CLAB is
- the continue label. Everything is allowed to be NULL. */
+ the continue label. Everything is allowed to be NULL.
+ COND_LOCUS is the location of the loop condition, INCR_LOCUS is the
+ location of the FOR increment expression. */
void
-c_finish_loop (location_t start_locus, tree cond, tree incr, tree body,
- tree blab, tree clab, bool cond_is_first)
+c_finish_loop (location_t start_locus, location_t cond_locus, tree cond,
+ location_t incr_locus, tree incr, tree body, tree blab,
+ tree clab, bool cond_is_first)
{
tree entry = NULL, exit = NULL, t;
@@ -10904,12 +10907,8 @@ c_finish_loop (location_t start_locus, t
}
t = build_and_jump (&blab);
- if (cond_is_first)
- exit = fold_build3_loc (start_locus,
- COND_EXPR, void_type_node, cond, exit, t);
- else
- exit = fold_build3_loc (input_location,
- COND_EXPR, void_type_node, cond, exit, t);
+ exit = fold_build3_loc (cond_is_first ? start_locus : input_location,
+ COND_EXPR, void_type_node, cond, exit, t);
}
else
{
@@ -10930,9 +10929,23 @@ c_finish_loop (location_t start_locus, t
if (clab)
add_stmt (build1 (LABEL_EXPR, void_type_node, clab));
if (incr)
- add_stmt (incr);
+ {
+ if (MAY_HAVE_DEBUG_MARKER_STMTS && incr_locus != UNKNOWN_LOCATION)
+ {
+ t = build0 (DEBUG_BEGIN_STMT, void_type_node);
+ SET_EXPR_LOCATION (t, incr_locus);
+ add_stmt (t);
+ }
+ add_stmt (incr);
+ }
if (entry)
add_stmt (entry);
+ if (MAY_HAVE_DEBUG_MARKER_STMTS && cond_locus != UNKNOWN_LOCATION)
+ {
+ t = build0 (DEBUG_BEGIN_STMT, void_type_node);
+ SET_EXPR_LOCATION (t, cond_locus);
+ add_stmt (t);
+ }
if (exit)
add_stmt (exit);
if (blab)
Jakub
__attribute__((noipa))
void
test (unsigned int *dst, unsigned int base, int count)
{
for (int i = 0; i < count; ++i, base += 15)
dst[i] = base;
}
int
main (void)
{
unsigned int dst[100];
test (dst, 0x4000, 100);
}
__attribute__((noipa))
void
test (unsigned int *dst, unsigned int base, int count)
{
int i = 0;
while (i < count)
dst[i++] = (base += 15);
}
int
main (void)
{
unsigned int dst[100];
test (dst, 0x4000, 100);
}
__attribute__((noipa))
void
test (unsigned int *dst, unsigned int base, int count)
{
int i = 0;
do
dst[i++] = (base += 15);
while (i < count);
}
int
main (void)
{
unsigned int dst[100];
test (dst, 0x4000, 100);
}
