Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misreported branch coverage of C++ projects from gcov #334

Open
Bronek opened this issue Apr 16, 2024 · 0 comments
Open

Misreported branch coverage of C++ projects from gcov #334

Bronek opened this issue Apr 16, 2024 · 0 comments
Labels
Support For general support questions, e.g. onboarding or configuration Waiting for: Support

Comments

@Bronek
Copy link

Bronek commented Apr 16, 2024

Describe the bug

Branch coverage generated from .gcov files is under-reported for C++ projects with generic (template) classes/functions. When unit tests coverage data are being processed by the gcov tool, the results are grouped per instantiation of generic function. This results in the same block of lines being reported in .gcov file multiple times, each time for a different instantiation of a function, for example (from https://github.com/Bronek/market/blob/main/libs/market/book.hpp , MIT license)

------------------
        -:  238:
        -:  239:        template <side Side, typename ... Args>
      158:  240:        size_type emplace_back(Args&& ... a) {
     158*:  241:            ASSERT(freel != nullptr);
     156*:  242:            ASSERT(side_i[0] + side_i[1] + (size_type)(tail_i + 1) == size_i);
      150:  243:            size_type result = npos;
      150:  244:            auto& size = side_i[(size_t)Side];
      150:  245:            if (size < capacity) {
     115*:  246:                ASSERT(tail_i != npos);
        -:  247:                // Note: must post-decrement tail_i here. Will change to npos if it was 0
      113:  248:                const auto l = freel[tail_i--];
      113:  249:                common::emplace(&levels[l], std::forward<Args>(a) ...);
      113:  250:                sides[(size_t)Side * capacity + size] = l;
      113:  251:                result = size++; // Note: must post-increment side_i[Side] here
        -:  252:            }
      148:  253:            return result;
        -:  254:        }
------------------
_ZN6market4bookIN12_GLOBAL__N_110ConstLevelES2_E12emplace_backILNS_4sideE0EJiEEEhDpOT0_:
function _ZN6market4bookIN12_GLOBAL__N_110ConstLevelES2_E12emplace_backILNS_4sideE0EJiEEEhDpOT0_ called 2 returned 100% blocks executed 60%
        2:  240:        size_type emplace_back(Args&& ... a) {
       2*:  241:            ASSERT(freel != nullptr);
branch  0 taken 0 (fallthrough)
branch  1 taken 2
call    2 never executed
call    3 never executed
       2*:  242:            ASSERT(side_i[0] + side_i[1] + (size_type)(tail_i + 1) == size_i);
branch  0 taken 0 (fallthrough)
branch  1 taken 2
call    2 never executed
call    3 never executed
        2:  243:            size_type result = npos;
        2:  244:            auto& size = side_i[(size_t)Side];
        2:  245:            if (size < capacity) {
branch  0 taken 2 (fallthrough)
branch  1 taken 0
       2*:  246:                ASSERT(tail_i != npos);
branch  0 taken 0 (fallthrough)
branch  1 taken 2
call    2 never executed
call    3 never executed
        -:  247:                // Note: must post-decrement tail_i here. Will change to npos if it was 0
        2:  248:                const auto l = freel[tail_i--];
        2:  249:                common::emplace(&levels[l], std::forward<Args>(a) ...);
call    0 returned 2
call    1 returned 2
        2:  250:                sides[(size_t)Side * capacity + size] = l;
        2:  251:                result = size++; // Note: must post-increment side_i[Side] here
        -:  252:            }
        2:  253:            return result;
        -:  254:        }
------------------
_ZN6market4bookIN12_GLOBAL__N_110ConstLevelES2_E12emplace_backILNS_4sideE1EJiEEEhDpOT0_:
function _ZN6market4bookIN12_GLOBAL__N_110ConstLevelES2_E12emplace_backILNS_4sideE1EJiEEEhDpOT0_ called 14 returned 100% blocks executed 60%
       14:  240:        size_type emplace_back(Args&& ... a) {
      14*:  241:            ASSERT(freel != nullptr);
branch  0 taken 0 (fallthrough)
branch  1 taken 14
call    2 never executed
call    3 never executed
      14*:  242:            ASSERT(side_i[0] + side_i[1] + (size_type)(tail_i + 1) == size_i);
branch  0 taken 0 (fallthrough)
branch  1 taken 14
call    2 never executed
call    3 never executed
       14:  243:            size_type result = npos;
       14:  244:            auto& size = side_i[(size_t)Side];
       14:  245:            if (size < capacity) {
branch  0 taken 11 (fallthrough)
branch  1 taken 3
      11*:  246:                ASSERT(tail_i != npos);
branch  0 taken 0 (fallthrough)
branch  1 taken 11
call    2 never executed
call    3 never executed
        -:  247:                // Note: must post-decrement tail_i here. Will change to npos if it was 0
       11:  248:                const auto l = freel[tail_i--];
       11:  249:                common::emplace(&levels[l], std::forward<Args>(a) ...);
call    0 returned 11
call    1 returned 11
       11:  250:                sides[(size_t)Side * capacity + size] = l;
       11:  251:                result = size++; // Note: must post-increment side_i[Side] here
        -:  252:            }
       14:  253:            return result;
        -:  254:        }
------------------
_ZN6market4bookIN12_GLOBAL__N_15LevelES2_E12emplace_backILNS_4sideE1EJiiEEEhDpOT0_:
function _ZN6market4bookIN12_GLOBAL__N_15LevelES2_E12emplace_backILNS_4sideE1EJiiEEEhDpOT0_ called 65 returned 94% blocks executed 88%
       65:  240:        size_type emplace_back(Args&& ... a) {
       65:  241:            ASSERT(freel != nullptr);
branch  0 taken 1 (fallthrough)
branch  1 taken 64
call    2 returned 1
call    3 returned 0
       64:  242:            ASSERT(side_i[0] + side_i[1] + (size_type)(tail_i + 1) == size_i);
branch  0 taken 3 (fallthrough)
branch  1 taken 61
call    2 returned 3
call    3 returned 0
       61:  243:            size_type result = npos;
       61:  244:            auto& size = side_i[(size_t)Side];
       61:  245:            if (size < capacity) {
branch  0 taken 53 (fallthrough)
branch  1 taken 8
      53*:  246:                ASSERT(tail_i != npos);
branch  0 taken 0 (fallthrough)
branch  1 taken 53
call    2 never executed
call    3 never executed
        -:  247:                // Note: must post-decrement tail_i here. Will change to npos if it was 0
       53:  248:                const auto l = freel[tail_i--];
       53:  249:                common::emplace(&levels[l], std::forward<Args>(a) ...);
call    0 returned 53
call    1 returned 53
call    2 returned 53
       53:  250:                sides[(size_t)Side * capacity + size] = l;
       53:  251:                result = size++; // Note: must post-increment side_i[Side] here
        -:  252:            }
       61:  253:            return result;
        -:  254:        }
------------------

It appears that the gcov reporter https://github.com/codecov/worker/blob/main/services/report/languages/gcov.py is unable to handle such files correctly and only gathers branch coverage from the first instantiation of such generic functions, resulting in significant under-reporting of branch coverage inside the function.

Environment (please complete the following information):

To Reproduce

Steps to reproduce the behaviour:

  1. Build https://github.com/Bronek/market project with coverage enabled, e.g.
mkdir .build
cd .build
export CC=$(which gcc); export CXX=$(which g++)
cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS="-g -coverage  -fprofile-abs-path" -DCMAKE_CXX_FLAGS="-g -coverage  -fprofile-abs-path" ..
cmake --build .
  1. Verify .gcno files are generated: find tests/ -type f -name '*.gcno'
  2. Run unit tests: tests/tests
  3. Verify .gcda files are generated: find tests/ -type f -name '*.gcda'
  4. Generate .gcov files: gcov -pbc $( find tests/ -type f -name '*.gcno' )
  5. Submit the generated .gcov files to codecov

Expected behavior

Expected unit test coverage is 100%

Screenshots

Reported unit test coverage is 98.35%. Example line 245 in function emplace_back is missing a branch

Screenshot 2024-04-16 at 14 14 57

Both branches of this line are exercised in unit tests, e.g. https://github.com/Bronek/market/blob/main/tests/book.cpp#L408

Additional context

Note the example project provided here is very small (basically just one file book.hpp). Assuming you have a recent C++ compiler (e.g. gcc-12 in Debian 12 "bookworm") you should be able to build and run tests of this project very quickly. This is also why the project has 100% unit test coverage (confirmed with gcovr tool). In this case the misreporting is tiny 1.65%. However for a non-trivial project the misreporting can have much larger impact.

For example by enabling branch reporting in .codecov.yml in libfn/functional#63 I have found a drop by some 12 percent point. I do not know how much of this is actually missing coverage, and how much is the result of the bug being reported, but reading trough the .gcov files of this project I can see many cases where all the branches have been executed, and yet codecov is showing incomplete branch coverage. I have found similar impact of this bug in other non-trivial projects.

The additional annoyance is the fact that codecov/codecov-action runs gcov plugin by default (unless explicitly disabled, e.g. with plugin: noop), automatically generating .gcov files for submission, so it is sensible to assume that codecov would be able to process these files correctly.

@covecod covecod bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Apr 16, 2024
@thomasrockhu-codecov thomasrockhu-codecov added Waiting for: Support Support For general support questions, e.g. onboarding or configuration labels Apr 16, 2024
@covecod covecod bot moved this from Waiting for: Product Owner to Waiting for: Support in GitHub Issues with 👀 Apr 16, 2024
ximinez pushed a commit to XRPLF/rippled that referenced this issue Apr 18, 2024
* Amend `.codecov.yml` to disable coverage reporting of test sources
  and explicitly set most parameters
* Increase codecov upload retry time to 210s (from 35s)
* Upgrade gcovr adding support for more coverage formats (lcov, clover, jacoco)
* Upgrade github actions in coverage workflow
* Explicitly disable codecov plugins (also removing `gcov` coverage, which is not
  correctly handled by codecov codecov/feedback#334)
sophiax851 pushed a commit to sophiax851/rippled that referenced this issue Jun 12, 2024
* Amend `.codecov.yml` to disable coverage reporting of test sources
  and explicitly set most parameters
* Increase codecov upload retry time to 210s (from 35s)
* Upgrade gcovr adding support for more coverage formats (lcov, clover, jacoco)
* Upgrade github actions in coverage workflow
* Explicitly disable codecov plugins (also removing `gcov` coverage, which is not
  correctly handled by codecov codecov/feedback#334)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Support For general support questions, e.g. onboarding or configuration Waiting for: Support
Projects
Status: Waiting for: Support
Development

No branches or pull requests

2 participants