Re: [PATCH] Wrong coverage attribution.

["Seongbae Park (박성배, 朴成培)" <seongbae.park@gmail.com>]

On 7/31/07, Seongbae Park (박성배, 朴成培) <seongbae.park@gmail.com> wrote:
> On 7/30/07, Nathan Sidwell <nathan@codesourcery.com> wrote:
> > Seongbae Park (???, ???) wrote:
> > > *ping*
> >
> > >> Attached patch fixes the problem shown in the testcase
> > >> - this testcase fails in the current mainline
> > >> because of GOTO moved to a different block during eh lowering.
> >
> > Sorry, I thought I'd emailed you about this, but I find no record of that.
> > Could you explain in more detail what the problem is and why your patch is correct?
>
> Sure.
>
> Given the new testcase included in the patch:
>
>      26   if (i == 1) {
>      27     f.method();
>      28     f.method();
>      29   } else {
>      30     f.method();
>      31     printf("unreachable\n");
>      32     return 2;        /* count(#####) */
>      33   }
>      34   f.method();
>      35   return 0;
>      36 }
>
> we get the following tree at the end of lowering (and before tree-eh):
>
>   [gcov-4.C : 35] try
>     {
>       [gcov-4.C : 26] if (i == 1) goto <D2786>; else goto <D2787>;
>       <D2786>:;
>       [gcov-4.C : 27] method (&f);
>       [gcov-4.C : 28] method (&f);
>       goto <D2788>;
>       <D2787>:;
>       [gcov-4.C : 30] method (&f);
>       [gcov-4.C : 31] __builtin_puts (&"unreachable"[0]);
>       [gcov-4.C : 32] D.2776 = 2;
>       [gcov-4.C : 32] goto <D2783>;
>       <D2788>:;
>       [gcov-4.C : 34] method (&f);
>       [gcov-4.C : 35] D.2776 = 0;
>       [gcov-4.C : 35] goto <D2783>;
>     }
>   finally
>     {
>       [gcov-4.C : 35] __comp_dtor  (&f);
>     }
>   <D2783>:;
>   return D.2776;
>
> After tree-eh (with the patch, without patch,
> GOTO D2783 in block D2789 would have the line no 32):
>
>   [gcov-4.C : 26] if (i == 1) goto <D2786>; else goto <D2787>;
>   <D2786>:;
>   [gcov-4.C : 27] method (&f);
>   [gcov-4.C : 28] method (&f);
>   goto <D2788>;
>   <D2787>:;
>   [gcov-4.C : 30] method (&f);
>   [gcov-4.C : 31] __builtin_puts (&"unreachable"[0]);
>   [gcov-4.C : 32] D.2776 = 2;
>   goto <D2789>;
>   <D2788>:;
>   [gcov-4.C : 34] method (&f);
>   [gcov-4.C : 35] D.2776 = 0;
>   goto <D2789>;
>   <D2789>:;
>   [gcov-4.C : 35] __comp_dtor  (&f);
>   goto <D2783>;
>   <D2783>:;
>   return D.2776;

I just realized that I haven't explained things in enough detail:

> lower_try_finally_onedest() is called only when try-finally
> has only one destination for the finally.
> During lower_try_finally_onedest(), among all collected goto's
> to the common destination of finally,
> we arbitrarily pick the first GOTO and append it to the finally block.

By the first goto, I meant

       [gcov-4.C : 32] goto <D2783>;

in block D2787 before tree-eh. This GOTO and all other
GOTOs that go to D2783 within try region are removed by
do_goto_redirection() call within lower_try_finally_onedest(),
and then the first one among removed GOTOs is append
to the end of the finally block.

> Obviously,  the original GOTO for line 32 doesn't belong to the finally block,
> hence the fix.

Without the fix, because the problematic GOTO is now moved to
the end of the finally block, it gets executed always,
unlike the original GOTO. This gives us the wrong coverage result
because the line that GOTO belongs to shouldn't be shown as executed.
-- 
#pragma ident "Seongbae Park, compiler, http://seongbae.blogspot.com"