Support nested #pragma GCC suppress_coverage

The warning on nested suppressions was intended to be helpful, but
there are just more good reasons to nest suppression than problems we
would catch by warnings.

An obvious example is custom asserts as macros, which could
legitimately be used in code suppressed for other reasons. There's
also recursive macro expansion or other preprocessing that effectively
expands this:

    REQUIRE (pred1 && pred2, "msg");

into this:

    #pragma GCC suppress_coverage begin
      #pragma GCC suppress_coverage begin
        ...
      #pragma GCC suppress_coverage end
    #pragma GCC suppress_coverage end

which I think should not trigger a warning.

There is a tradeoff. We lose the ability to warn unbalanced begin/end
in the middle somewhere, and coverage silently will be suppressed for
every subsequent line.

    #pragma GCC suppress_coverage begin
      // everything suppressed from here
      ...
      #pragma GCC suppress_coverage begin
        ...
      #pragma GCC suppress_coverage end
    // not closed here as intended, coverage still suppressed

Thankfully those cases should be rare and easy to trace as it's not
hard to see from where the rest of the file is suppressed.

gcc/c-family/ChangeLog:

	* c-pragma.cc (handle_pragma_suppress_coverage): Don't warn on
	nested begin.
	* c-pragma.h (suppress_coverage_begin): Change return type to
	void.

gcc/ChangeLog:

	* profile.cc (suppress_coverage_begin): Always push new range,
	change return type.
	(suppress_coverage_end): Search for last unclosed item.

gcc/testsuite/ChangeLog:

	* gcc.dg/pragma-suppress-coverage.c: Add test case for nested
	suppressions.
	* gcc.misc-tests/gcov-39.c: Likewise.
---
 gcc/c-family/c-pragma.cc                      |  7 ++--
 gcc/c-family/c-pragma.h                       |  2 +-
 gcc/profile.cc                                | 33 ++++++++++---------
 .../gcc.dg/pragma-suppress-coverage.c         | 18 ++++++++--
 gcc/testsuite/gcc.misc-tests/gcov-39.c        | 14 ++++++++
 5 files changed, 50 insertions(+), 24 deletions(-)

On 9/2/26 21:08, Jørgen Kvalsvik wrote:
> The warning on nested suppressions was intended to be helpful, but
> there are just more good reasons to nest suppression than problems we
> would catch by warnings.
> 
> An obvious example is custom asserts as macros, which could
> legitimately be used in code suppressed for other reasons. There's
> also recursive macro expansion or other preprocessing that effectively
> expands this:
> 
>      REQUIRE (pred1 && pred2, "msg");
> 
> into this:
> 
>      #pragma GCC suppress_coverage begin
>        #pragma GCC suppress_coverage begin
>          ...
>        #pragma GCC suppress_coverage end
>      #pragma GCC suppress_coverage end
> 
> which I think should not trigger a warning.
> 
> There is a tradeoff. We lose the ability to warn unbalanced begin/end
> in the middle somewhere, and coverage silently will be suppressed for
> every subsequent line.
> 
>      #pragma GCC suppress_coverage begin
>        // everything suppressed from here
>        ...
>        #pragma GCC suppress_coverage begin
>          ...
>        #pragma GCC suppress_coverage end
>      // not closed here as intended, coverage still suppressed
> 
> Thankfully those cases should be rare and easy to trace as it's not
> hard to see from where the rest of the file is suppressed.
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-pragma.cc (handle_pragma_suppress_coverage): Don't warn on
> 	nested begin.
> 	* c-pragma.h (suppress_coverage_begin): Change return type to
> 	void.
> 
> gcc/ChangeLog:
> 
> 	* profile.cc (suppress_coverage_begin): Always push new range,
> 	change return type.
> 	(suppress_coverage_end): Search for last unclosed item.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/pragma-suppress-coverage.c: Add test case for nested
> 	suppressions.
> 	* gcc.misc-tests/gcov-39.c: Likewise.
> ---
>   gcc/c-family/c-pragma.cc                      |  7 ++--
>   gcc/c-family/c-pragma.h                       |  2 +-
>   gcc/profile.cc                                | 33 ++++++++++---------
>   .../gcc.dg/pragma-suppress-coverage.c         | 18 ++++++++--
>   gcc/testsuite/gcc.misc-tests/gcov-39.c        | 14 ++++++++
>   5 files changed, 50 insertions(+), 24 deletions(-)
> 
> diff --git a/gcc/c-family/c-pragma.cc b/gcc/c-family/c-pragma.cc
> index d0d4ff69fd7..38fd14ad808 100644
> --- a/gcc/c-family/c-pragma.cc
> +++ b/gcc/c-family/c-pragma.cc
> @@ -1069,11 +1069,8 @@ handle_pragma_suppress_coverage (cpp_reader*)
>   	GCC_BAD ("no matching begin for %<#pragma GCC suppress_coverage end%>");
>       }
>     else
> -    {
> -      if (!suppress_coverage_begin (input_location))
> -	GCC_BAD ("%<#pragma GCC suppress_coverage begin%> "
> -		 "was already in effect, ignored");
> -    }
> +    suppress_coverage_begin (input_location);
> +
>     if (pragma_lex (&x, &loc) != CPP_EOF)
>       GCC_BAD_AT (loc, "junk at end of %<#pragma GCC suppress_coverage%>");
>   }
> diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h
> index 5d28cb45c66..5428e476ba9 100644
> --- a/gcc/c-family/c-pragma.h
> +++ b/gcc/c-family/c-pragma.h
> @@ -304,7 +304,7 @@ extern void c_pp_lookup_pragma (unsigned int, const char **, const char **);
>   extern GTY(()) tree pragma_extern_prefix;
>   
>   /* For recording #pragma GCC suppress_coverage locations.  */
> -extern bool suppress_coverage_begin (location_t);
> +extern void suppress_coverage_begin (location_t);
>   extern bool suppress_coverage_end (location_t);
>   
>   #endif /* GCC_C_PRAGMA_H */
> diff --git a/gcc/profile.cc b/gcc/profile.cc
> index 52704e93866..d6fd22eaade 100644
> --- a/gcc/profile.cc
> +++ b/gcc/profile.cc
> @@ -1247,32 +1247,35 @@ any_block_coverage_suppressed_p ()
>      UNKNOWN_LOCATION.  */
>   static vec<source_range> suppress_coverage_ranges;
>   
> -/* Try to add LOC as the beginning of a new range.  If a range was started
> -   already, this is a no-op.  Returns true if a new range was created.  */
> -bool
> +/* Try to add LOC as the beginning of a new range.  A new range is always
> +   created, even if the previous one was left open.  */
> +void
>   suppress_coverage_begin (location_t loc)
>   {
> -  if (!suppress_coverage_ranges.is_empty ()
> -      && suppress_coverage_ranges.last ().m_finish == UNKNOWN_LOCATION)
> -    return false;
> -
>     loc = get_pure_location (expansion_point_location (loc));
>     source_range range = source_range::from_locations (loc, UNKNOWN_LOCATION);
>     suppress_coverage_ranges.safe_push (range);
> -  return true;
>   }
>   
> -/* Try to close the last range created by suppress_coverage_begin at LOC.  If
> -   the range has been closed already (or not opened), this is a no-op.  Returns
> -   true if a range was closed.  */
> +/* Try to close the last open range created by suppress_coverage_begin at LOC.
> +   If all ranges have been closed already (or never opened), this is a no-op.
> +   Returns true if a range was closed.  */
>   bool
>   suppress_coverage_end (location_t loc)
>   {
> -  if (suppress_coverage_ranges.is_empty ()
> -      || suppress_coverage_ranges.last ().m_finish != UNKNOWN_LOCATION)
> -      return false;
> +  if (suppress_coverage_ranges.is_empty ())
> +    return false;
> +
> +  source_range *current = nullptr;
> +  for (int i = suppress_coverage_ranges.length () - 1; !current && i >= 0; i--)
> +    if (suppress_coverage_ranges[i].m_finish == UNKNOWN_LOCATION)
> +      current = &suppress_coverage_ranges[i];
> +
> +  if (!current)
> +    return false;
> +
>     loc = get_pure_location (expansion_point_location (loc));
> -  suppress_coverage_ranges.last ().m_finish = loc;
> +  current->m_finish = loc;
>     return true;
>   }
>   
> diff --git a/gcc/testsuite/gcc.dg/pragma-suppress-coverage.c b/gcc/testsuite/gcc.dg/pragma-suppress-coverage.c
> index 2a3bc42e883..6f8de76a254 100644
> --- a/gcc/testsuite/gcc.dg/pragma-suppress-coverage.c
> +++ b/gcc/testsuite/gcc.dg/pragma-suppress-coverage.c
> @@ -24,9 +24,21 @@
>                                        ^~~~
>      { dg-end-multiline-output "" }  */
>   
> -#pragma GCC suppress_coverage begin
> -/* { dg-warning "'#pragma GCC suppress_coverage begin' was already in effect, ignored" "" { target *-*-* } .-1 }
> +/* Pushing multiple suppresions is fine, this works like a stack.  It should
> +   not trigger a warning because this could easily happen with recursive
> +   expansions, for example when some assert-like macro with internal suppression
> +   is used in a suppressed context.  */
> +#define suppress_push() _Pragma("GCC suppress_coverage begin")
> +#define suppress_pop() _Pragma("GCC suppress_coverage end")
> +
> +suppress_push()
> +suppress_pop()
> +
> +#pragma GCC suppress_coverage end
> +
> +#pragma GCC suppress_coverage end
> +/* { dg-warning "no matching begin for '#pragma GCC suppress_coverage end'" "" { target *-*-* } .-1 }
>      { dg-begin-multiline-output "" }
> - #pragma GCC suppress_coverage begin
> + #pragma GCC suppress_coverage end
>            ^~~
>      { dg-end-multiline-output "" }  */
> diff --git a/gcc/testsuite/gcc.misc-tests/gcov-39.c b/gcc/testsuite/gcc.misc-tests/gcov-39.c
> index 1847db8e89c..c1c9b35c88a 100644
> --- a/gcc/testsuite/gcc.misc-tests/gcov-39.c
> +++ b/gcc/testsuite/gcc.misc-tests/gcov-39.c
> @@ -226,6 +226,19 @@ pathcov004d (int a, int b, int c, int d)
>       }
>   }
>   
> +/* BEGIN paths
> +   summary: 1/1 */
> +int
> +nested001a (int a) {
> +/* END */
> +#pragma GCC suppress_coverage begin
> +    REQUIRE (a >= 0);
> +    int suppressed_twice = a + a;
> +    ENSURE (suppressed_twice > a);
> +#pragma GCC suppress_coverage end
> +    return suppressed_twice;
> +}
> +
>   int
>   main ()
>   {
> @@ -238,6 +251,7 @@ main ()
>     suppressed_in_loop3 (10);
>     pathcov004c (0, 0, 0, 0);
>     pathcov004d (0, 1, 0, 0);
> +  nested001a (1);
>   }
>   
>   /* { dg-final { run-gcov prime-paths { --prime-paths-lines=both gcov-39.c } } } */

Ping.