[Bug gcov-profile/93680] [GCOV] "do-while" structure in case statement leads to incorrect code coverage

["yinyuefengyi at gmail dot com" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93680

Xionghu Luo (luoxhu at gcc dot gnu.org) <yinyuefengyi at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yinyuefengyi at gmail dot com

--- Comment #4 from Xionghu Luo (luoxhu at gcc dot gnu.org) <yinyuefengyi at gmail dot com> ---
Check the difference of the two switch cases, both called split_edge to
generate empty latch bb after the loop:

profile.cc:
              /* Edge with goto locus might get wrong coverage info unless
                 it is the only edge out of BB.
                 Don't do that when the locuses match, so
                 if (blah) goto something;
                 is not computed twice.  */
              if (last
                  && gimple_has_location (last)
                  && !RESERVED_LOCATION_P (e->goto_locus)
                  && !single_succ_p (bb)
                  && (LOCATION_FILE (e->goto_locus)
                      != LOCATION_FILE (gimple_location (last))
                      || (LOCATION_LINE (e->goto_locus)
                          != LOCATION_LINE (gimple_location (last)))))
                {
                  basic_block new_bb = split_edge (e);
                  edge ne = single_succ_edge (new_bb);
                  ne->goto_locus = e->goto_locus;
                }

but the second case failed to find a edge from dest_prev to dest if edge_in
forms a self loop (edge_in->src == edge_in->dest)


  <bb 2> :
  p_6 = 0;
  q_7 = 0;
  switch (s_8(D)) <default: <L6> [INV], case 0: <L0> [INV], case 1: <L3> [INV]>

  <bb 3> :
  # n_1 = PHI <n_9(D)(2), n_14(4)>
  # p_3 = PHI <p_6(2), p_13(4)>
<L0>:
  p_13 = p_3 + 1;
  n_14 = n_1 + -1;
  if (n_14 != 0)
    goto <bb 4>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 4> :
  goto <bb 3>; [100.00%]

  <bb 5> :
  _15 = p_13;
  goto <bb 10>; [INV]

  <bb 6> :

  <bb 7> :
  # n_2 = PHI <n_9(D)(2), n_11(6)>
  # p_4 = PHI <p_6(2), p_10(6)>
<L3>:
  p_10 = p_4 + 1;
  n_11 = n_2 + -1;
  if (n_11 != 0)
    goto <bb 6>; [INV]
  else
    goto <bb 8>; [INV]

  <bb 8> :
  _12 = p_10;
  goto <bb 10>; [INV]


Note the two loops have different latch bb location.



So add the check like this for self loop to return loop bb itself as after_bb?

diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index a9fcc7fd050..6fa1d83d366 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -3009,7 +3009,7 @@ split_edge_bb_loc (edge edge_in)
   if (dest_prev)
     {
       edge e = find_edge (dest_prev, dest);
-      if (e && !(e->flags & EDGE_COMPLEX))
+      if ((e && !(e->flags & EDGE_COMPLEX)) || edge_in->src == edge_in->dest)
        return edge_in->src;
     }
   return dest_prev;

With the fix, small.c.069i.profile:

  <bb 2> :
  p_6 = 0;
  q_7 = 0;
  switch (s_8(D)) <default: <L6> [INV], case 0: <L0> [INV], case 1: <L3> [INV]>

  <bb 3> :
  # n_1 = PHI <n_9(D)(2), n_14(4)>
  # p_3 = PHI <p_6(2), p_13(4)>
<L0>:
  p_13 = p_3 + 1;
  n_14 = n_1 + -1;
  if (n_14 != 0)
    goto <bb 4>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 4> :
  goto <bb 3>; [100.00%]

  <bb 5> :
  _15 = p_13;
  goto <bb 10>; [INV]

  <bb 6> :
  # n_2 = PHI <n_9(D)(2), n_11(7)>
  # p_4 = PHI <p_6(2), p_10(7)>
<L3>:
  p_10 = p_4 + 1;
  n_11 = n_2 + -1;
  if (n_11 != 0)
    goto <bb 7>; [INV]
  else
    goto <bb 8>; [INV]

  <bb 7> :
  goto <bb 6>; [100.00%]

  <bb 8> :
  _12 = p_10;
  goto <bb 10>; [INV]



cat small.c.gcov:
        -:    0:Source:small.c
        -:    0:Graph:small.gcno
        -:    0:Data:small.gcda
        -:    0:Runs:1
        2:    1:int f(int s, int n)
        -:    2:{
        2:    3:  int p = 0;
        2:    4:  int q = 0;
        -:    5:
        2:    6:  switch (s)
        -:    7:    {
        5:    8:    case 0:
        5:    9:      do { p++; } while (--n);
        1:   10:      return p;
        -:   11:
        5:   12:    case 1:
        5:   13:      do { p++; } while (--n);
        1:   14:      return p;
        -:   15:    }
        -:   16:
    #####:   17:  return 0;
        -:   18:}
        -:   19:
        1:   20:int main() { f(0, 5); f(1, 5);}


Is this reasonable Fix? If so I could cook a patch and send it to maillist for
review...