[C PATCH] Fix up C loop construct debug info without -gno-statement-frontiers (PR debug/90197)

[Jeff Law]

On 4/25/19 3:13 AM, Jakub Jelinek wrote:
> 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  <jakub@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.
OK.
jeff