[patch] Small improvements to coverage info (4/n)

[Eric Botcazou]

Hi,

the returns are a little problematic for coverage info because they are 
commonalized in GIMPLE, i.e. turned into gotos to a representative return.
This means that you need to clear the location of the representative return, 
see lower_gimple_return:

          /* Remove the line number from the representative return statement.
             It now fills in for many such returns.  Failure to remove this
             will result in incorrect results for coverage analysis.  */
          gimple_set_location (tmp_rs.stmt, UNKNOWN_LOCATION);

otherwise the coverage info is blatantly wrong.  But this in turn means that 
such representative returns are vulneable to inheriting random source location 
information in the debug info generated by the compiler.

I have attached an Ada testcase that demonstrates the problem at -O0:

        .loc 1 14 10 discriminator 2
        movl    $0, %r13d
.L12:
        movl    %r13d, %eax
        jmp     .L23

.L12 is what's left at the RTL level of a representative return in GIMPLE and 
it inherits the location directive, which gives wrong coverage info (that's 
not visible with gcov because the instrumentation done by -fprofile-arcs 
papers over the issue, but for example with callgrind).

The proposed fix is attached: it sets the current location to the function's 
end locus when expanding such a representative return to RTL.  That's a bit on 
the kludgy side, but doing something in GIMPLE, e.g. in lower_gimple_return, 
leads to various regressions in terms of quality of diagnostics.

Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline?


2019-07-10  Eric Botcazou  <ebotca...@adacore.com>

        * cfgexpand.c (expand_gimple_stmt_1) <GIMPLE_RETURN>: If the statement
        doesn't have a location, set the current location to the function's end.

-- 
Eric Botcazou
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 273294)
+++ cfgexpand.c	(working copy)
@@ -3712,6 +3712,12 @@ expand_gimple_stmt_1 (gimple *stmt)
       {
 	op0 = gimple_return_retval (as_a <greturn *> (stmt));
 
+	/* If a return doesn't have a location, it very likely represents
+	   multiple user returns so we cannot let it inherit the location
+	   of the last statement of the previous basic block in RTL.  */
+	if (!gimple_has_location (stmt))
+	  set_curr_insn_location (cfun->function_end_locus);
+
 	if (op0 && op0 != error_mark_node)
 	  {
 	    tree result = DECL_RESULT (current_function_decl);

pragma Unsuppress (All_Checks);
pragma Check_Float_Overflow;
   
package body Ops is
   
   function Both_Ok (A, B : Capsum) return Boolean is
   begin
      if A.X + A. Y <= A.Cap and then B.X + B.Y <= B.Cap then -- # test
	 N_Ok := N_Ok + 1; -- # ok
	 return True;      -- # ok
      else
	 N_Ko := N_Ko + 1; -- # ko
	 return False;     -- # ko
      end if;
   exception
      when Constraint_Error =>
	 N_Ov := N_Ov + 1; -- # ov
	 return False; -- # ov
   end;
end;

package Ops is
   
   type Constrained_Float is new Float range Float'Range;
   
   type Capsum is record
      X, Y, Cap : Constrained_Float;
   end record;
   
   Cs_Ok : constant Capsum := (X => 1.0, Y => 1.0, Cap => 3.0);
   Cs_Ko : constant Capsum := (X => 1.0, Y => 2.0, Cap => 1.0);
   Cs_Ov : constant Capsum := (X => Constrained_Float'Last,
			       Y => Constrained_Float'Last,
			       Cap => 3.0);
   
   N_Ok, N_Ko, N_Ov : Integer := 0;
   
   function Both_Ok (A, B : Capsum) return Boolean;
   
end;