The current region mapping for do-while loops that contain statements
such as break or continue results in inaccurate line coverage reports
for the line following the loop.
This change handles terminating statements the same way that other loop
constructs do, correcting the region mapping for accurate reports. It
also fixes a fragile test relying on exact line numbers.
Fixes#139122
Relands #130976 with adjustments to test requirements.
Calls to __noreturn__ functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
__noreturn__ functions (or other constructs that result in region
termination) occur within [GNU statement expressions][1].
In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.
This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.
[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.htmlFixes#124296
Calls to __noreturn__ functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
__noreturn__ functions (or other constructs that result in region
termination) occur within [GNU statement expressions][1].
In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.
This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.
[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.htmlFixes#124296
To resolve the error, rename mcdc-error-nests.cpp ->
mcdc-nested-expr.cpp at first.
- `func_condop` A corner case that contains close decisions.
- `func_expect` Uses `__builtin_expect`. (#124565)
- `func_lnot` Contains logical not(s) `!` among MC/DC binary operators.
(#124563)
In preparation of making `-Wreturn-type` default to an error (as there
is virtually no situation where you’d *want* to fall off the end of a
function that is supposed to return a value), this patch fixes tests
that have relied on this being only a warning, of which there seem
to be 3 kinds:
1. Tests which for no apparent reason have a function that triggers the
warning.
I suspect that a lot of these were on accident (or from before the
warning was introduced), since a lot of people will open issues w/ their
problematic code in the `main` function (which is the one case where you
don’t need to return from a non-void function, after all...), which
someone will then copy, possibly into a namespace, possibly renaming it,
the end result of that being that you end up w/ something that
definitely is not `main` anymore, but which still is declared as
returning `int`, and which still has no return statement (another reason
why I think this might apply to a lot of these is because usually the
actual return type of such problematic functions is quite literally
`int`).
A lot of these are really old tests that don’t use `-verify`, which is
why no-one noticed or had to care about the extra warning that was
already being emitted by them until now.
2. Tests which test either `-Wreturn-type`, `[[noreturn]]`, or what
codegen and sanitisers do whenever you do fall off the end of a
function.
3. Tests where I struggle to figure out what is even being tested
(usually because they’re Objective-C tests, and I don’t know
Objective-C), whether falling off the end of a function matters in the
first place, and tests where actually spelling out an expression to
return would be rather cumbersome (e.g. matrix types currently don’t
support list initialisation, so I can’t write e.g. `return {}`).
For tests that fall into categories 2 and 3, I just added
`-Wno-error=return-type` to the `RUN` lines and called it a day. This
was especially necessary for the former since `-Wreturn-type` is an
analysis-based warning, meaning that it is currently impossible to test
for more than one occurrence of it in the same compilation if it
defaults to an error since the analysis pass is skipped for subsequent
functions as soon as an error is emitted.
I’ve also added `-Werror=return-type` to a few tests that I had already
updated as this patch was previously already making the warning an error
by default, but we’ve decided to split that into two patches instead.
Restructure some tests to split into `%.test` and Inputs/%.c*`.
Add test actions with `yaml2obj` for single byte coverage. `FileCheck`
lines are:
- Relax to accept both counter values and single values `1`. A few line
counting are greater than `1` due to `llvm-profdata merge`. They will be
fixed by #110972.
- Suppress matching with `--check-prefixes=CHECK,BRCOV`, since the
current implementation of single byte doesn't emit any branch coverages.
They will be integrated to `CHECK` finally.
- Some tests are not unified but use dedicated `CHECK` lines for single
byte, since old format is different (esp. "partial fold"). They will be
unified when `Inputs` will be regenerated.
Introduce llvm/test/tools/llvm-cov/Inputs/yaml.makefile for convenience.
`%-single.yaml` and `%-single.proftext` are generated by it. It could be
used to regenerate other files.
https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492
I missed that FalseCnt for each Case was used to calculate percentage in
the SwitchStmt. At the moment I resurrect them.
In `!HasDefaultCase`, the pair of Counters shall be `[CaseCountSum,
FalseCnt]`. (Reversal of before #112694)
I think it can be considered as the False count on SwitchStmt.
FalseCnt shall be folded (same as current impl) in the coming
SingleByteCoverage changes, since percentage would not make sense.
Currently both True/False counts were folded. It lost the information,
"It is True or False before folding." It prevented recalling branch
counts in merging template instantiations.
In `llvm-cov`, a folded branch is shown as:
- `[True: n, Folded]`
- `[Folded, False n]`
In the case If `n` is zero, a branch is reported as "uncovered". This is
distinguished from "folded" branch. When folded branches are merged,
`Folded` may be dissolved.
In the coverage map, either `Counter` is `Zero`. Currently both were
`Zero`.
Since "partial fold" has been introduced, either case in `switch` is
omitted as `Folded`.
Each `case:` in `switch` is reported as `[True: n, Folded]`, since
`False` count doesn't show meaningful value.
When `switch` doesn't have `default:`, `switch (Cond)` is reported as
`[Folded, False: n]`, since `True` count was just the sum of `case`(s).
`switch` with `default` can be considered as "the statement that doesn't
have any `False`(s)".
With `system-headers-coverage=false`, functions defined in system
headers were not instrumented but corresponding covmaps were emitted. It
caused wasting covmap and profraw.
This change improves:
- Reduce object size (due to reduced covmap)
- Reduce size of profraw (uninstrumented system headers occupied
counters)
- Smarter view of coverage report. Stubs of uninstrumented system
headers will be no longer seen.
By storing possible test vectors instead of combinations of conditions,
the restriction is dramatically relaxed.
This introduces two options to `cc1`:
* `-fmcdc-max-conditions=32767`
* `-fmcdc-max-test-vectors=2147483646`
This change makes coverage mapping, profraw, and profdata incompatible
with Clang-18.
- Bitmap semantics changed. It is incompatible with previous format.
- `BitmapIdx` in `Decision` points to the end of the bitmap.
- Bitmap is packed per function.
- `llvm-cov` can understand `profdata` generated by `llvm-profdata-18`.
RFC:
https://discourse.llvm.org/t/rfc-coverage-new-algorithm-and-file-format-for-mc-dc/76798
--
Change(s) since llvmorg-19-init-14288-g7ead2d8c7e91
- Update compiler-rt/test/profile/ContinuousSyncMode/image-with-mcdc.c
This broke the lit tests on Mac:
https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/1096/
> By storing possible test vectors instead of combinations of conditions,
> the restriction is dramatically relaxed.
>
> This introduces two options to `cc1`:
>
> * `-fmcdc-max-conditions=32767`
> * `-fmcdc-max-test-vectors=2147483646`
>
> This change makes coverage mapping, profraw, and profdata incompatible
> with Clang-18.
>
> - Bitmap semantics changed. It is incompatible with previous format.
> - `BitmapIdx` in `Decision` points to the end of the bitmap.
> - Bitmap is packed per function.
> - `llvm-cov` can understand `profdata` generated by `llvm-profdata-18`.
>
> RFC:
> https://discourse.llvm.org/t/rfc-coverage-new-algorithm-and-file-format-for-mc-dc/76798
This reverts commit 7ead2d8c7e9114b3f23666209a1654939987cb30.
By storing possible test vectors instead of combinations of conditions,
the restriction is dramatically relaxed.
This introduces two options to `cc1`:
* `-fmcdc-max-conditions=32767`
* `-fmcdc-max-test-vectors=2147483646`
This change makes coverage mapping, profraw, and profdata incompatible
with Clang-18.
- Bitmap semantics changed. It is incompatible with previous format.
- `BitmapIdx` in `Decision` points to the end of the bitmap.
- Bitmap is packed per function.
- `llvm-cov` can understand `profdata` generated by `llvm-profdata-18`.
RFC:
https://discourse.llvm.org/t/rfc-coverage-new-algorithm-and-file-format-for-mc-dc/76798
Introduced in #78041, originally reported as #79957 and fixed partially
in #80050.
`OpaqueValueExpr` used with `TemplateArgument::StructuralValue` has no
corresponding source expression.
A test case with subobject-referring NTTP added.
The problematic program is as follows:
```shell
#define pre_a 0
#define PRE(x) pre_##x
void f(void) {
PRE(a) && 0;
}
int main(void) { return 0; }
```
in which after token concatenation (`##`), there's another nested macro
`pre_a`.
Currently only the outer expansion region will be produced. ([compiler
explorer
link](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:___c,selection:(endColumn:29,endLineNumber:8,positionColumn:29,positionLineNumber:8,selectionStartColumn:29,selectionStartLineNumber:8,startColumn:29,startLineNumber:8),source:'%23define+pre_a+0%0A%23define+PRE(x)+pre_%23%23x%0A%0Avoid+f(void)+%7B%0A++++PRE(a)+%26%26+0%3B%0A%7D%0A%0Aint+main(void)+%7B+return+0%3B+%7D'),l:'5',n:'0',o:'C+source+%231',t:'0')),k:51.69491525423727,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((g:!((h:compiler,i:(compiler:cclang_assertions_trunk,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'0',intel:'0',libraryCode:'1',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:2,lang:___c,libs:!(),options:'-fprofile-instr-generate+-fcoverage-mapping+-fcoverage-mcdc+-Xclang+-dump-coverage-mapping+',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x86-64+clang+(assertions+trunk)+(Editor+%231)',t:'0')),k:34.5741843594503,l:'4',m:28.903654485049834,n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compilerName:'x86-64+clang+(trunk)',editorid:1,fontScale:14,fontUsePx:'0',j:2,wrap:'1'),l:'5',n:'0',o:'Output+of+x86-64+clang+(assertions+trunk)+(Compiler+%232)',t:'0')),header:(),l:'4',m:71.09634551495017,n:'0',o:'',s:0,t:'0')),k:48.30508474576271,l:'3',n:'0',o:'',t:'0')),l:'2',m:100,n:'0',o:'',t:'0')),version:4))
```text
f:
File 0, 4:14 -> 6:2 = #0
Decision,File 0, 5:5 -> 5:16 = M:0, C:2
Expansion,File 0, 5:5 -> 5:8 = #0 (Expanded file = 1)
File 0, 5:15 -> 5:16 = #1
Branch,File 0, 5:15 -> 5:16 = 0, 0 [2,0,0]
File 1, 2:16 -> 2:23 = #0
File 2, 1:15 -> 1:16 = #0
File 2, 1:15 -> 1:16 = #0
Branch,File 2, 1:15 -> 1:16 = 0, 0 [1,2,0]
```
The inner expansion region isn't produced because:
1. In the range-based for loop quoted below, each sloc is processed and
possibly emit a corresponding expansion region.
2. For our sloc in question, its direct parent returned by
`getIncludeOrExpansionLoc()` is a `<scratch space>`, because that's how
`##` is processed.
88b6186af3/clang/lib/CodeGen/CoverageMappingGen.cpp (L518-L520)
3. This `<scratch space>` cannot be found in the FileID mapping so
`ParentFileID` will be assigned an `std::nullopt`
88b6186af3/clang/lib/CodeGen/CoverageMappingGen.cpp (L521-L526)
4. As a result this iteration of for loop finishes early and no
expansion region is added for the sloc.
This problem gets worse with MC/DC: as the example shows, there's a
branch from File 2 but File 2 itself is missing. This will trigger
assertion failures.
The fix is more or less a workaround and takes a similar approach as
#89573.
~~Depends on #89573.~~ This includes #89573. Kudos to @chapuni!
This and #89573 together fix#87000: I tested locally, both the reduced
program and my original use case (fwiw, Linux kernel) can run
successfully.
---------
Co-authored-by: NAKAMURA Takumi <geek4civic@gmail.com>
`emitSourceRegions()` has bugs to emit malformed MC/DC coverage
mappings. They were detected in `llvm-cov` as the crash.
Detect inconsistencies earlier in `clang` with assertions.
* mcdc-scratch-space.c covers #87000.
- Introduce `LeafExprSet`,
- Suppress traversing LAnd and LOr expr under system headers.
- Handle LAnd and LOr as instrumented leaves to override
`!isInstrumentedCondition(C)`.
- Replace Loc with FileLoc if it is expanded with system headers.
Fixes#78920
`ArrayInitLoopExpr` AST node has two occurences of its as-written
initializing expression in its subexpressions through a non-unique
`OpaqueValueExpr`. It causes double-visiting of the initializing
expression if not handled explicitly, as discussed in #85837.
This avoids visiting `co_await` or `co_yield` operand 5 times (it is
repeated under transformed awaiter subexpression, and under
`await_ready`, `await_suspend`, and `await_resume` generated call
subexpressions).
This patch inserts 1-byte counters instead of an 8-byte counters into
llvm profiles for source-based code coverage. The origial idea was
proposed as block-cov for PGO, and this patch repurposes that idea for
coverage: https://groups.google.com/g/llvm-dev/c/r03Z6JoN7d4
The current 8-byte counters mechanism add counters to minimal regions,
and infer the counters in the remaining regions via adding or
subtracting counters. For example, it infers the counter in the if.else
region by subtracting the counters between if.entry and if.then regions
in an if statement. Whenever there is a control-flow merge, it adds the
counters from all the incoming regions. However, we are not going to be
able to infer counters by subtracting two execution counts when using
single-byte counters. Therefore, this patch conservatively inserts
additional counters for the cases where we need to add or subtract
counters.
RFC:
https://discourse.llvm.org/t/rfc-single-byte-counters-for-source-based-code-coverage/75685
Fix an issue that produces a wrong coverage mapping when using binary
conditional operators as show in the example below.
Before this patch:
1| 1|int binary_cond(int x) {
2| 1| x = x ?: 4;
3| 1| int y = 0;
4| 0| return x; <-- Not covered
5| 1|}
After this patch:
1| 1|int binary_cond(int x) {
2| 1| x = x ?: 4;
3| 1| int y = 0;
4| 1| return x; <-- Covered
5| 1|}
`if constexpr` and `if consteval` conditional statements code coverage
should behave more like a preprocesor `#if`-s than normal
ConditionalStmt. This PR should fix that.
---------
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
Clean-up of the algorithm that assigns MC/DC True/False control-flow
condition IDs when constructing an MC/DC decision region. This patch
creates a common API for setting/getting the condition IDs, making the
binary logical operator visitor functions much cleaner.
This patch also fixes issue
https://github.com/llvm/llvm-project/issues/77873 in which a record's
control flow map can be malformed due to an incorrect calculation of the
True/False condition IDs.
During the ISO C++ Committee meeting plenary session the C++23 Standard
has been voted as technical complete.
This updates the reference to c++2b to c++23 and updates the __cplusplus
macro.
Drive-by fixes c++1z -> c++17 and c++2a -> c++20 when seen.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D149553
This patch prefixes omp outlined helpers and reduction funcs
with the original function's name.
Reviewed By: jdoerfert
Differential Revision: https://reviews.llvm.org/D140722
This patch attempts to prefix omp outlined helpers and reduction funcs
with the original function's name.
Reviewed By: jdoerfert
Differential Revision: https://reviews.llvm.org/D140722
marked as 'not executed'.
In the current coverage mapping implementation, we terminate the current region
and start a zero region when we hit a nonreturn function. However, for logical OR,
the second operand is not executed if the first operand evaluates to true. If the
nonreturn function is called in the right side of logical OR and the left side of
logical OR is TRUE, we should not start a zero `GapRegionCounter`. This will also
apply to `VisitAbstractConditionalOperator`.
Fixes https://github.com/llvm/llvm-project/issues/59030
Reviewed By: zequanwu
Differential Revision: https://reviews.llvm.org/D144371
Since we decided to remove the support for `-fcoroutines-ts` in
clang/llvm17 and the clang16/llvm16 is branched. So we're going to
remove the `-fcoroutines-ts` option.
Originally, the following commit removed mapping coverage regions for system headers:
93205af066
It might be viable and useful to collect coverage from system headers in some systems.
This patch adds --system-headers-coverage option (disabled by default) to enable
collecting coverage from system headers.
Differential Revision: https://reviews.llvm.org/D143304
This is a reland of https://reviews.llvm.org/D122336.
Original patch caused a problem in collecting coverage in
Fuchsia because it was returning early without putting unused
function names into __llvm_prf_names section. This patch
fixes that issue.
The original commit message is as the following:
CoverageMappingModuleGen generates a coverage mapping record
even for unused functions with internal linkage, e.g.
static int foo() { return 100; }
Clang frontend eliminates such functions, but InstrProfiling pass
still emits runtime hook since there is a coverage record.
Fuchsia uses runtime counter relocation, and pulling in profile
runtime for unused functions causes a linker error:
undefined hidden symbol: __llvm_profile_counter_bias.
Since https://reviews.llvm.org/D98061, we do not hook profile
runtime for the binaries that none of its translation units
have been instrumented in Fuchsia. This patch extends that for
the instrumented binaries that consist of only unused functions.
Reviewed By: phosek
Differential Revision: https://reviews.llvm.org/D122336
Clang crashes when encountering an `if consteval` statement.
This is the minimum fix not to crash.
The fix is consistent with the current behavior of if constexpr,
which does generate coverage data for the discarded branches.
This is of course not correct and a better solution is
needed for both if constexpr and if consteval.
See https://github.com/llvm/llvm-project/issues/54419.
Fixes#57377
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D132723
I noticed these two tests emit a warning about a missing
unhandled_exception. That's irrelevant to what is being tested, but
is unnecessary noise.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D125535
