This is an archive of the discontinued LLVM Phabricator instance.

[Coverage] Prevent false instantiations detection in case of macro expansions.
ClosedPublic

Authored by ikudrin on Apr 5 2016, 3:17 AM.

Details

Summary

The root of the problem was that findMainViewFileID(File, Function) could return some ID for any given file,
even though that file was not the main for that function.
This patch ensures that the result of this function is conformed with the result of findMainViewFileID(Function).

Diff Detail

Event Timeline

ikudrin updated this revision to Diff 52670.Apr 5 2016, 3:17 AM
ikudrin retitled this revision from to [Coverage] Prevent false instantiations detection in case of macro expansions..
ikudrin updated this object.
ikudrin added reviewers: bogner, davidxl, vsk.
ikudrin added a subscriber: llvm-commits.

Use the following commands to reproduce the issue:

$ cat > sample.cpp << EOF
#include "sample.h"

void func1() {
  DO_SOMETHING();
}

void func2() {
  DO_SOMETHING();
}

int main() {
  for (int I = 0; I < 10; ++I)
    if (I < 7)
      func1();
    else
      func2();
  return 0;
}
EOF
$ cat > sample.h << EOF
#define DO_SOMETHING() \
  do { \
  } while(0)

/*
 * We need some lines here to show the issue.
 *
 *
 *
 */
EOF
$ clang++ -fprofile-instr-generate -fcoverage-mapping sample.cpp 
$ ./a.out
$ llvm-profdata merge -o default.profdata default.profraw
$ llvm-cov show a.out -instr-profile default.profdata -filename-equivalence sample.h 
       |    1|#define DO_SOMETHING() \
     10|    2|  do { \
     10|    3|  } while(0)
       |    4|
       |    5|/*
  ------------------
  | _Z5func1v:
  |      7|    3|  } while(0)
  |      7|    4|
  |      7|    5|/*
  ------------------
       |    6| * We need some lines here to show the issue.
       |    7| *
       |    8| *
       |    9| *
  ------------------
  | _Z5func2v:
  |      3|    7| *
  |      3|    8| *
  |      3|    9| *
  ------------------
       |   10| */
davidxl edited edge metadata.Apr 9 2016, 12:53 PM

Can you also add the motivation case as one of the test case under tests/tools/llvm-cov ?

Otherwise looks good.

llvm/trunk/lib/ProfileData/CoverageMapping.cpp
369

while you are at it, please add a brief comment for this method.

375

Another fix is to check

&& FilenameEquivalence[CR.ExpandedFileID])

...

but I think your refactored code looks better.

380

Add a comment for the function.

ikudrin updated this revision to Diff 53748.Apr 14 2016, 10:04 AM
ikudrin updated this object.
ikudrin edited edge metadata.
  • Added the motivation case to the tests for llvm-cov.
  • Added comments.
  • Updated findMainViewFileID(File, Function) according to Justin's comment.
davidxl accepted this revision.Apr 14 2016, 2:57 PM
davidxl edited edge metadata.

lgtm

This revision is now accepted and ready to land.Apr 14 2016, 2:57 PM
This revision was automatically updated to reflect the committed changes.