Looks like quick_exit + dlclose interaction is not well-defined and this change may cause calls into dlclose-d libraries.

Thanks for the review @vsk and @richkadel for the details about Rust coverage instrumentation.

Thanks for the detail about the edge counters @richkadel.

Tighten and reword test. Also fix lint suggestion.

In D101780#2752181, @richkadel wrote:

Do we need counters on edges if we're only interested in line segment coverage?

Edge counters exist so they can be operands in expressions, and since they don't have a code region, they would not be referenced directly by a code region in a report; but the expression might, and that's we we need them.

In D101780#2752061, @richkadel wrote:

@vsk - Also, I want to confirm that it is OK to have counters that are incremented (via instrprof.increment intrinsic) that are not associated with a code region.

@tmiasko Thanks for adding this support to llvm. We're already using your work in Android. Can you comment on the status of this support? For e.g., I see that the following name is not demangled:

In D101780#2744912, @vsk wrote:

Thanks for digging into this. Could you explain why it's necessary to use more coverage counters than there are regions? If there's no longer a reserved counter (counter #0), then are counters N >= Regions.size() somehow redundant (and if so, are they worth deduplicating in the Rust FE)?

Add release note

In D101878#2740133, @curdeius wrote:

LGTM but let's wait a day or two before landing, so that others can chime in.
Btw, do you have commit rights? If no, please provide "Name <email>" for commit attribution.

Remove debug print().

Leave two empty lines after filter_symlinks.

In D95987#2543866, @zequanwu wrote:

For the example you provided, I am not sure if it's a bug or not. Segment at 3:45 (count = 0), RegionEntry corresponds to the error path of the ? operator, which means error path is not executed. So, the 0 count looks reasonable. If we mark line 3 as executed regardless of which path is chosen, we may lost the information of path choices.

I just realized that this also applies to C++ ? operator. In the following case, if we always mark line 5 as executed, we don't know which branch is chosen (But when generating html report by llvm-cov, we will see that the not executed branch is marked with red color).

Thanks for the patch Zequan. This fixes the C++ test case that prompted this issue.

Added CHECK line for TCRETURNri

In D78033#1978612, @beanz wrote:

I think this has some unintended consequences. If your tool wants to use libLLVM and libClang you really need libClang to be linked against libLLVM, otherwise you're actually just hiding the problem.

Merged. Thanks for the review @MaskRay.

Cleanup based on review comments.

Use isOSDarwin() instead of isOSBinFormatMachO()

Treat any bitode with non-Darwin triple as GNU.
Test cleanup.

Use K_GNU for all ELF targets.

Simplify test.

Apply clang-format.

In D76461#1932516, @MaskRay wrote:

https://llvm.org/doxygen/classllvm_1_1object_1_1Archive.html#ab288378fa8bfa0678dd25e36b5198a87. I think this regresses behavior for those other hosts.

*BSD may need the __.SYMDEF format.

Have you considered passing an explicit --format= via something like ARFLAGS?

Handle bitcode inputs with BSD target triple.

Cleanup test.

In D65550#1752082, @vsk wrote:

Thanks Nick & Eli, '-fno-integrated-as' does the trick, as does changing the makefile. If the effect of this patch on the asm writer is intentional/correct, we can just push for a project change instead of fiddling with the compiler.

• Do not have a declaration as a prevailing symbol in llvm-lto2's arguments
• Fix comments in test

In D66814#1666487, @tejohnson wrote:

In D66814#1666457, @pirama wrote:

(note the summary description of the patch needs to be adjusted similarly, it still refers to the summary).

@tejohnson Did my reply about shared metadata clarify your question?

Check for OMPRT_Unknown instead of a default case.

Test -static, -static-openmp interaction. Added these only for iomp5 to avoid test-case explosion.

I'll update this review addressing @joerg's reply to cfe-commits:

Needs testing for the -static interaction?

In D67200#1660147, @srhines wrote:

Looks really nice. I am sure the NDK developers will be happy to see support for static OpenMP. Do you want to add the public NDK github issue link in the commit message?

Mention NDK issue https://github.com/android-ndk/ndk/issues/1028.

Does this regress from existing behavior for unused argument warnings? Currently, -static-libstdc++ -nostdlib issues an unused argument warning for -static-libstdc++, while AFAICT -static=c++stdlib -nostdlib doesn't.

Change parameter name.

Supported this flag for NetBSD and FreeBSD as well.

Update/clean-up comments in the test.

Fix typo reffers -> refers

Avoid extra variable

Thanks Peter. I'll wait for another day for further comments, and if none, will submit this tomorrow morning (pacific time).

In D65550#1610801, @efriedma wrote:

This only affects inline asm, it looks like, not general instructions? (I looked briefly, and it looks like instructions go through AArch64InstPrinter::printOperand instead.)

I don't think this would cause any issues off the top of my head, but you might want to run something like an Android build to confirm that.

Thanks for the workaround @peter.smith. I suggested it to the NDK developer as well. The immediate need for this change is gone, but I agree that this is useful for the various reasons you outlined.