Support nested #pragma GCC suppress_coverage
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
Commit Message
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(-)
@@ -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%>");
}
@@ -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 */
@@ -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;
}
@@ -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 "" } */
@@ -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 } } } */