llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.01k stars 11.95k forks source link

wrong coverage for the function `alloca()` call statement #44940

Open llvmbot opened 4 years ago

llvmbot commented 4 years ago
Bugzilla Link 45595
Version trunk
OS Linux
Reporter LLVM Bugzilla Contributor
CC @dwblaikie,@efriedma-quic,@vedantk,@yuanfang-chen,@ZequanWu

Extended Description

$ clang -v
clang version 9.0.1 
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-pc-linux-gnu/9.3.0
Found candidate GCC installation: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.3.0
Found candidate GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0
Found candidate GCC installation: /usr/lib64/gcc/x86_64-pc-linux-gnu/9.3.0
Selected GCC installation: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.3.0
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64

$ cat small.c
#include <setjmp.h>
#include <stdlib.h>

jmp_buf buf;

void sub (void)
{
  longjmp (buf, 1);
}

int main ()
{
  if (setjmp (buf)) {
    exit (0);
  }

  int *q = (int *) alloca (10 * sizeof (int));

  for (int i = 0; i < 10; i++)
    q[i] = 0;

  while (1)
    sub ();
}
$ clang -O0 -fcoverage-mapping -fprofile-instr-generate=small.profraw small.c; ./a.out; llvm-profdata merge small.profraw -o small.profdata; llvm-cov show a.out -instr-profile=small.profdata small.c > small.c.lcov; cat small.c.lcov
    1|       |#include <setjmp.h>
    2|       |#include <stdlib.h>
    3|       |
    4|       |jmp_buf buf;
    5|       |
    6|       |void sub (void)
    7|      1|{
    8|      1|  longjmp (buf, 1);
    9|      1|}
   10|       |
   11|       |int main ()
   12|      1|{
   13|      1|  if (setjmp (buf)) {
   14|      1|    exit (0);
   15|      1|  }
   16|      0|
   17|      0|  int *q = (int *) alloca (10 * sizeof (int));
   18|      0|
   19|     10|  for (int i = 0; i < 10; i++)
   20|     10|    q[i] = 0;
   21|      0|
   22|      1|  while (1)
   23|      1|    sub ();
   24|      0|}

*****

Line 17 is wrongly marked as not executed. The correct coverage should be 1|.

vedantk commented 3 years ago

Zequan (cc'd) recently landed a series of patches to hide line execution counts for whitespace/comment-only lines.

In general clang's FE-based coverage implementation doesn't behave well when dealing with longjmp (see https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#drawbacks-and-limitations).

In this specific case, clang terminates the current coverage region when it sees a noreturn call (see https://reviews.llvm.org/D36250). Usually this is fine, but in this case (adding -Xclang -dump-coverage-mapping):

It might be possible to fix this by reworking D36250 (to find some other way to make coverage reporting in switches cleaner), but I don't think it's easy to fix reporting issues with longjmp in general.

dwblaikie commented 3 years ago

Looks like a bunch of issues here - there are several blank lines with 0 counts instead of the absence of a count. (perhaps that's just llvm's profdata's way of doing things? Seems problematic/misleading. Especially 0 between rows with 1 without any control flow.

But, yes, also the missing coverage on 17 seems problematic (again, either the absence of a count, if the code has been optimized away maybe, or an accurate count of 1, would be good).