[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.3                         |9.4

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 9.3.0 has been released, adjusting target milestone.

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[marxin at gcc dot gnu.org]

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
@Marek, @David: Can you please help me with that?

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[marxin at gcc dot gnu.org]

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
Note that C++ FE is correct:

$ g++ pr94029.c --coverage -c
void test_t2()=3:3(location_t:2147483629)-3:3(location_t:2147483634)
void test_t1()=2:1(location_t:2147483638)-2:1(location_t:2147483643)

while C does:

$ gcc pr94029.c --coverage -c
test_t2=3:3(location_t:2147483629)-2:1(location_t:246216)
test_t1=2:1(location_t:2147483638)-2:1(location_t:246216)

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48053
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48053&action=edit
gcc10-pr94029.patch

The problem is that C FE finish_function uses input_location as
cfun->function_end_locus, but input_location isn't really updated very often in
the C FE anymore, mostly in the line change hooks but that doesn't trigger here
because the macro arguments span across newline.
So, either like in the completely untested patch we can update input_location
somewhat more often, or we'd need to propagate the location_t of the closing }
from c_parser_compound_statement_nostart to the callers.

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[marxin at gcc dot gnu.org]

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

--- Comment #11 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #10)
> Created attachment 48053 [details]
> gcc10-pr94029.patch
> 
> The problem is that C FE finish_function uses input_location as
> cfun->function_end_locus, but input_location isn't really updated very often
> in the C FE anymore, mostly in the line change hooks but that doesn't
> trigger here because the macro arguments span across newline.
> So, either like in the completely untested patch we can update
> input_location somewhat more often, or we'd need to propagate the location_t
> of the closing }
> from c_parser_compound_statement_nostart to the callers.

Thank you very much Jakub. If it survives regression tests, I would install the
suggested patch.

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48055
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48055&action=edit
gcc10-pr94029-2.patch

Variant patch which doesn't change input_location but propagates the closing }
location to finish_function.

[Bug gcov-profile/94029] [9/10 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9def91e9f2a7051c9c146f16c1a10d1b25d33b47

commit r10-7281-g9def91e9f2a7051c9c146f16c1a10d1b25d33b47
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 19 22:56:20 2020 +0100

    c: Fix up cfun->function_end_locus from the C FE [PR94029]

    On the following testcase we ICE because while
          DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus
            = c_parser_peek_token (parser)->location;
    and similarly DECL_SOURCE_LOCATION (fndecl) is set from some token's
    location, the end is set as:
      /* Store the end of the function, so that we get good line number
         info for the epilogue.  */
      cfun->function_end_locus = input_location;
    and the thing is that input_location is only very rarely set in the C FE
    (the primary spot that changes it is the cb_line_change/fe_file_change).
    Which means, e.g. for pretty much all C functions that are on a single
line,
    function_start_locus column is > than function_end_locus column, and the
    testcase even has smaller line in function_end_locus because cb_line_change
    isn't performed while parsing multi-line arguments of a function-like
macro.

    Attached are two possible fixes to achieve what the C++ FE does, in
    particular that cfun->function_end_locus is the locus of the closing } of
    the function.  The first one updates input_location when we see a closing }
    of a compound statement (though any, not just the function body) and thus
    input_location in the finish_function call is what we need.
    The second instead propagates the location_t from the parsing of the
    outermost compound statement (the function body) to finish_function.
    The second one is this version.

    2020-03-19  Jakub Jelinek  <jakub@redhat.com>

            PR gcov-profile/94029
            * c-tree.h (finish_function): Add location_t argument defaulted to
            input_location.
            * c-parser.c (c_parser_compound_statement): Add endlocp argument
and
            set it to the locus of closing } if non-NULL.
            (c_parser_compound_statement_nostart): Return locus of closing }.
            (c_parser_parse_rtl_body): Likewise.
            (c_parser_declaration_or_fndef): Propagate locus of closing } to
            finish_function.
            * c-decl.c (finish_function): Add end_loc argument, use it instead
of
            input_location to set function_end_locus.

            * gcc.misc-tests/gcov-pr94029.c: New test.

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10 Regression] gcc crash |[9 Regression] gcc crash in
                   |in coverage.c:655 since     |coverage.c:655 since
                   |r9-4216-g390e529e2b98983d   |r9-4216-g390e529e2b98983d

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[sandra at gcc dot gnu.org]

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

sandra at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at gcc dot gnu.org

--- Comment #15 from sandra at gcc dot gnu.org ---
Seen on nios2-elf:

FAIL: gcc.misc-tests/gcov-pr94029.c gcov failed: gcov-pr94029.c.gcov does not
exist

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[bernd.edlinger at hotmail dot de]

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

--- Comment #16 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Sandra,

I am pretty sure it should exist,
can you check which git revision you are looking at?

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:5db9e89323cd0a0be16a94f2f984121531ea7772

commit r10-7343-g5db9e89323cd0a0be16a94f2f984121531ea7772
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Mar 23 19:44:58 2020 +0100

    c: Fix up cfun->function_end_locus on invalid function bodies [PR94239]

    Unfortunately the patch broke
    +FAIL: gcc.dg/pr20245-1.c (internal compiler error)
    +FAIL: gcc.dg/pr20245-1.c (test for excess errors)
    +FAIL: gcc.dg/pr28419.c (internal compiler error)
    +FAIL: gcc.dg/pr28419.c (test for excess errors)
    on some targets (and under valgrind on the rest of them).

    Those functions don't have the opening { and so c_parser_compound_statement
    returned error_mark_node before initializing *endlocp.
    So, either we can initialize it in that case too:
    --- gcc/c/c-parser.c    2020-03-20 22:09:39.659411721 +0100
    +++ gcc/c/c-parser.c    2020-03-21 09:36:44.455705261 +0100
    @@ -5611,6 +5611,8 @@ c_parser_compound_statement (c_parser *p
             if we have just prepared to enter a function body.  */
           stmt = c_begin_compound_stmt (true);
           c_end_compound_stmt (brace_loc, stmt, true);
    +      if (endlocp)
    +       *endlocp = brace_loc;
           return error_mark_node;
         }
       stmt = c_begin_compound_stmt (true);
    or perhaps simpler initialize it to the function_start_locus at the
    beginning and have those functions without { have function_start_locus ==
    function_end_locus like the __GIMPLE functions (where propagating the
    closing } seemed too difficult).

    2020-03-23  Jakub Jelinek  <jakub@redhat.com>

            PR gcov-profile/94029
            PR c/94239
            * c-parser.c (c_parser_declaration_or_fndef): Initialize endloc to
            the function_start_locus location.  Don't do that afterwards for
the
            __GIMPLE body parsing.

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[sandra at gcc dot gnu.org]

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

--- Comment #18 from sandra at gcc dot gnu.org ---
I'm seeing the missing gcov file on nios2-linux-gnu as well.  Git revision
6e00d8dcf32ace6588a1a4843dfcc0e8b9f9d00f.

I took another look at the testcase.  I haven't used gcov for about a gazillion
years, but...

It says "{ dg-do compile }".  Don't you have to run the testcase to collect the
data to run with gcov?  And copy the .gcda file from the target to the host?

Then, it is trying to run "gcov gcov-pr94029.c" instead of e.g. "nios2-elf-gcov
gcov-pr94029.c", and it's picking up some random gcov program installed on the
host system instead of the one for the target.

Maybe this testcase should just be restricted to native targets?

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[bernd.edlinger at hotmail dot de]

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

--- Comment #19 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Okay, forget my previous comment,
I overlooked that you say the .c.gcov is missing...

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:827e5af19a214323c70ee5879dc8f14f4f44ad8d

commit r9-8466-g827e5af19a214323c70ee5879dc8f14f4f44ad8d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 19 22:56:20 2020 +0100

    c: Fix up cfun->function_end_locus from the C FE [PR94029]

    On the following testcase we ICE because while
          DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus
            = c_parser_peek_token (parser)->location;
    and similarly DECL_SOURCE_LOCATION (fndecl) is set from some token's
    location, the end is set as:
      /* Store the end of the function, so that we get good line number
         info for the epilogue.  */
      cfun->function_end_locus = input_location;
    and the thing is that input_location is only very rarely set in the C FE
    (the primary spot that changes it is the cb_line_change/fe_file_change).
    Which means, e.g. for pretty much all C functions that are on a single
line,
    function_start_locus column is > than function_end_locus column, and the
    testcase even has smaller line in function_end_locus because cb_line_change
    isn't performed while parsing multi-line arguments of a function-like
macro.

    Attached are two possible fixes to achieve what the C++ FE does, in
    particular that cfun->function_end_locus is the locus of the closing } of
    the function.  The first one updates input_location when we see a closing }
    of a compound statement (though any, not just the function body) and thus
    input_location in the finish_function call is what we need.
    The second instead propagates the location_t from the parsing of the
    outermost compound statement (the function body) to finish_function.
    The second one is this version.

    2020-03-19  Jakub Jelinek  <jakub@redhat.com>

            PR gcov-profile/94029
            * c-tree.h (finish_function): Add location_t argument defaulted to
            input_location.
            * c-parser.c (c_parser_compound_statement): Add endlocp argument
and
            set it to the locus of closing } if non-NULL.
            (c_parser_compound_statement_nostart): Return locus of closing }.
            (c_parser_parse_rtl_body): Likewise.
            (c_parser_declaration_or_fndef): Propagate locus of closing } to
            finish_function.
            * c-decl.c (finish_function): Add end_loc argument, use it instead
of
            input_location to set function_end_locus.

            * gcc.misc-tests/gcov-pr94029.c: New test.

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:f83c2d2991a762f729741a409a1ac7871733ee40

commit r9-8467-gf83c2d2991a762f729741a409a1ac7871733ee40
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Mar 23 19:44:58 2020 +0100

    c: Fix up cfun->function_end_locus on invalid function bodies [PR94239]

    Unfortunately the patch broke
    +FAIL: gcc.dg/pr20245-1.c (internal compiler error)
    +FAIL: gcc.dg/pr20245-1.c (test for excess errors)
    +FAIL: gcc.dg/pr28419.c (internal compiler error)
    +FAIL: gcc.dg/pr28419.c (test for excess errors)
    on some targets (and under valgrind on the rest of them).

    Those functions don't have the opening { and so c_parser_compound_statement
    returned error_mark_node before initializing *endlocp.
    So, either we can initialize it in that case too:
    --- gcc/c/c-parser.c    2020-03-20 22:09:39.659411721 +0100
    +++ gcc/c/c-parser.c    2020-03-21 09:36:44.455705261 +0100
    @@ -5611,6 +5611,8 @@ c_parser_compound_statement (c_parser *p
             if we have just prepared to enter a function body.  */
           stmt = c_begin_compound_stmt (true);
           c_end_compound_stmt (brace_loc, stmt, true);
    +      if (endlocp)
    +       *endlocp = brace_loc;
           return error_mark_node;
         }
       stmt = c_begin_compound_stmt (true);
    or perhaps simpler initialize it to the function_start_locus at the
    beginning and have those functions without { have function_start_locus ==
    function_end_locus like the __GIMPLE functions (where propagating the
    closing } seemed too difficult).

    2020-03-23  Jakub Jelinek  <jakub@redhat.com>

            PR gcov-profile/94029
            PR c/94239
            * c-parser.c (c_parser_declaration_or_fndef): Initialize endloc to
            the function_start_locus location.  Don't do that afterwards for
the
            __GIMPLE body parsing.

[Bug gcov-profile/94029] [9 Regression] gcc crash in coverage.c:655 since r9-4216-g390e529e2b98983d

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.