On Tue, 9 Apr 2024, Jørgen Kvalsvik wrote:

> PR114601 shows that it is possible to reach the condition_uid lookup
> without having also created the fn->cond_uids, through
> compiler-generated conditionals. Consider all lookups on non-existing
> maps misses, which they are from the perspective of the source code, to
> avoid the NULL access.

OK.

>       PR gcov-profile/114601
> 
> gcc/ChangeLog:
> 
>       * tree-profile.cc (condition_uid): Guard fn->cond_uids access.
> 
> gcc/testsuite/ChangeLog:
> 
>       * gcc.misc-tests/gcov-pr114601.c: New test.
> ---
>  gcc/testsuite/gcc.misc-tests/gcov-pr114601.c | 11 +++++++++++
>  gcc/tree-profile.cc                          |  9 +++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr114601.c
> 
> diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr114601.c 
> b/gcc/testsuite/gcc.misc-tests/gcov-pr114601.c
> new file mode 100644
> index 00000000000..72248c8fd25
> --- /dev/null
> +++ b/gcc/testsuite/gcc.misc-tests/gcov-pr114601.c
> @@ -0,0 +1,11 @@
> +/* PR gcov-profile/114601 */
> +/* { dg-do compile } */
> +/* { dg-options "-fcondition-coverage -finstrument-functions-once" } */
> +
> +/* -finstrument-functions-once inserts a hidden conditional expression into
> +   this function which otherwise has none.  This caused a crash on looking up
> +   the condition as the cond->expr map is not created unless it necessary.  
> */
> +void
> +empty (void)
> +{
> +}
> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
> index b85111624fe..b87c121790c 100644
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -359,12 +359,17 @@ condition_index (unsigned flag)
>     min-max, etc., which leaves ghost identifiers in basic blocks that do not
>     end with a conditional jump.  They are not really meaningful for condition
>     coverage anymore, but since coverage is unreliable under optimization 
> anyway
> -   this is not a big problem.  */
> +   this is not a big problem.
> +
> +   The cond_uids map in FN cannot be expected to exist.  It will only be
> +   created if it is needed, and a function may have gconds even though there
> +   are none in source.  This can be seen in PR gcov-profile/114601, when
> +   -finstrument-functions-once is used and the function has no conditions.  
> */
>  unsigned
>  condition_uid (struct function *fn, basic_block b)
>  {
>      gimple *stmt = gsi_stmt (gsi_last_bb (b));
> -    if (!safe_is_a<gcond *> (stmt))
> +    if (!safe_is_a <gcond*> (stmt) || !fn->cond_uids)
>       return 0;
>  
>      unsigned *v = fn->cond_uids->get (as_a <gcond*> (stmt));
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to