[patch] improve sloc assignment on bind_expr entry/exit code

[Olivier Hainque <hainque at adacore dot com>]

Hello,

For blocks requiring it, the gimplifier generates stack pointer
save/restore operations on entry/exit, per:

 gimplify_bind_expr (...)

  if (gimplify_ctxp->save_stack)
    {
      gimple stack_restore;

      /* Save stack on entry and restore it on exit.  Add a try_finally
	 block to achieve this.  */
      build_stack_save_restore (&stack_save, &stack_restore);

      gimplify_seq_add_stmt (&cleanup, stack_restore);
    }

  /* Add clobbers for all variables that go out of scope.  */
  ...

There is no specific location assigned to these entry/exit statements
so they eventually inherits slocs coming from preceding statements.

This is problematic for tools relying on debug info to infer
which statements were executed out of execution traces (allowing
coverage analysis without code instrumentation).

An example of problematic scenario is provided below.

The attached patch is a proposal to improve this by propagating
start and end of block locations from the block structure to the
few gimple statements we generate. It adds an "end_locus" to the
block structure for this purpose, which the Ada front-end knows
how to fill already.

I verified that it does inserts proper .loc directives before the
entry/exit code on the example. The patch also bootstraps and regtests
fine for languages=all,ada on x86_64-pc-linux-gnu.

OK to commit ?

Thanks in advance for your feedback,

With Kind Regards,

Olivier

--

2014-06-11  Olivier Hainque  <hainque@adacore.com>

	* tree-core.h (tree_block): Add an "end_locus" field, allowing
	memorization of the end of block source location.
	* tree.h (BLOCK_SOURCE_END_LOCATION): New accessor.
	* gimplify.c (gimplify_bind_expr): Propagate the block start and
	end source location info we have on the block entry/exit code we
	generate.

--

Here is an Ada example to illustrate:

-- p.adb
     1	procedure P (Choice : Integer; N : in out Integer) is
     2	begin
     3	   if Choice > 0 then
     4	      declare
     5	         S : String (1 .. N * 2);
     6	         pragma Volatile (S);
     7	      begin
     8		 S := (others => 'B');
     9	      end;
    10	   else
    11	      declare
    12		 S : String (1 .. N );
    13		 pragma Volatile (S);
    14	      begin
    15		 S := (others => '1');
    16	      end;
    17	   end if;
    18	end;

The two if/else blocks allocate a local string of dynamic size and call for
stack save/restore operations. A very recent mainline on x86_64-linux produces
the following code (gcc -c -g -save-temps -fverbose-asm p.adb):

     .loc 1 3 0
     cmpl    $0, -84(%rbp)   #, choice
     jle     .L2             <-------- branch to the else block if choice <= 0
     [...]
     [code for the if block here]
     ...
     .loc 1 8 0 discriminator 3
     addl    $1, %eax        #, D.3124
     jmp     .L5     #
     ...
   .L2:                      <------- branch lands here
   .LBB4:
     movq    %rsp, %rax      #, tmp136         <--- stack save on entry of the
     movq    %rax, %rdi      # tmp136, D.3125  <--- else block.
     .loc 1 12 0 is_stmt 1
     movl    -88(%rbp), %ecx # n, D.3129


The stack save code has no specific sloc assigned, so inherits
what happens to be current at this point, here the .loc 1 8 corresponding
to the code last emitted for the string assignment on line 8.

This is inaccurate because the stack save code really has nothing to do with
the statement on line 8. And this is problematic when infering coverage
information from execution traces, as it incorrectly appears as if part of
line 8 was executed as soon as we reach here.

The proposed change ends up inserting a .loc 1 11 0 just before the first move
insn, curing the problem, as well as a .loc 1 4, a .loc 1 9 and a .loc 1 16
before the other start and end of blocks for similar reasons.

Attachment: bind_expr_locus.diff
Description: Binary data