Code comments are marked as uncovered

Support
1

Description

The codecov report is showing lines of code that contain only comments as uncovered.

Repository

CI/CD

AWS CodeBuild

Uploader

Bash uploader

Commit SHAs

A couple examples:

https://codecov.io/gh/awslabs/s2n/compare/07b5da57467209b750a3a7a1b320c39ebe4c66d7...6e4c7fc662280f4454968de5f0fb1ccbb3e6eb99/diff#D3-78

Codecov YAML

https://github.com/awslabs/s2n/blob/master/.codecov.yml

2

Hi @WesleyRosenblum, I’m taking a look at this commit that you supplied here and I see that for the last build labeled s2nfuzzerOpenSSL111Coverage:242aa65f-baaa-41a9-91e8-4fce13758dcb, that you are getting some metrics back on uncovered lines.

/codebuild/output/src889068026/src/github.com/awslabs/s2n/tls/extensions/s2n_client_key_share.c:
   54|       |const s2n_extension_type s2n_client_key_share_extension = {
   55|       |    .iana_value = TLS_EXTENSION_KEY_SHARE,
   56|       |    .is_response = false,
   57|       |    .send = s2n_client_key_share_send,
   58|       |    .recv = s2n_client_key_share_recv,
   59|       |    .should_send = s2n_extension_send_if_tls13_connection,
   60|       |    .if_missing = s2n_extension_noop_if_missing,
   61|       |};
   62|       |
   63|       |static int s2n_generate_preferred_key_shares(struct s2n_connection *conn, struct s2n_stuffer *out)
   64|      0|{
   65|      0|    notnull_check(conn);
   66|      0|    uint8_t preferred_key_shares = conn->preferred_key_shares;
   67|      0|    const struct s2n_ecc_named_curve *server_negotiated_curve = NULL;
   68|      0|    struct s2n_ecc_evp_params *ecc_evp_params = NULL;
   69|      0|
   70|      0|    const struct s2n_ecc_preferences *ecc_pref = NULL;
   71|      0|    GUARD(s2n_connection_get_ecc_preferences(conn, &ecc_pref));
   72|      0|    notnull_check(ecc_pref);
   73|      0|
   74|      0|    /* If lsb is set, skip keyshare generation for all curve */
   75|      0|    if (S2N_IS_KEY_SHARE_LIST_EMPTY(preferred_key_shares)) {
   76|      0|        return S2N_SUCCESS;
   77|      0|    }
   78|      0|
   79|      0|    if (!conn->preferred_key_shares) {
   80|      0|        /* Default behavior is to generate keyshares for all curves.
   81|      0|        * The bitmap to generate keyshares for all curve is 111111110 (254),
   82|      0|        * i.e. all bit values set except lsb which is RESERVED for empty keyshares */
   83|      0|        preferred_key_shares = S2N_ALL_KEY_SHARES_REQUESTED;
   84|      0|    }
...
  164|       |static int s2n_client_key_share_recv(struct s2n_connection *conn, struct s2n_stuffer *extension)
  165|  23.9M|{
  166|  23.9M|    notnull_check(conn);
  167|  23.9M|    notnull_check(extension);
  168|  23.9M|
  169|  23.9M|    if (!s2n_is_tls13_enabled() || conn->actual_protocol_version < S2N_TLS13) {
  170|  26.5k|        return S2N_SUCCESS;
  171|  26.5k|    }
  172|  23.9M|
  173|  23.9M|    const struct s2n_ecc_preferences *ecc_pref = NULL;
  174|  23.9M|    GUARD(s2n_connection_get_ecc_preferences(conn, &ecc_pref));
  175|  23.9M|    notnull_check(ecc_pref);
  176|  23.9M|
  177|  23.9M|    uint16_t key_shares_size;
  178|  23.9M|    GUARD(s2n_stuffer_read_uint16(extension, &key_shares_size));
  179|  23.9M|    S2N_ERROR_IF(s2n_stuffer_data_available(extension) < key_shares_size, S2N_ERR_BAD_MESSAGE);
  180|  23.3M|
  181|  23.3M|    const struct s2n_ecc_named_curve *supported_curve;
  182|  17.7M|    struct s2n_blob point_blob;
  183|  17.7M|    uint16_t named_group, share_size;
  184|  17.7M|    uint32_t supported_curve_index;
  185|  17.7M|
  186|  17.7M|    /* Whether a match was found */

The other two reports are harder to read, but I believe that this might be the reason you are seeing coverage on these lines when we merge reports together. Would you be able to run this without the third report?

3

Are you saying that if multiple builds produce coverage over the same lines, Codecov has difficulty merging the reports together? Or is there something wrong with the third build? Our CI requires running all of these builds to properly test our code, is there something we are doing to wrong that is producing coverage on commented lines?

4

Hi @WesleyRosenblum, to be clear, I’m saying that Codecov is merging all 3 reports together. Because the commented lines have a coverage line associated with it (0), we are using that to denote that there is no coverage for that line.

I’m not exactly sure how you are creating the third coverage report, but it is not excluding comments. I would look there to see why that might be happening.

5

Thanks @tom. That build is generating coverage using:
clang -fprofile-instr-generate -fcoverage-mapping
llvm-profdata merge -sparse default.profraw -o default.profdata
as described here: Source-based Code Coverage — Clang 16.0.0git documentation

Apparently that mechanism does mark “unexecuted” code comments and whitespace as have 0 coverage, and I haven’t found a way around it. Have you seen any other users of codecov that are using this method of generating coverage find a way to work around this?

6

Actually I see this reported as a bug to LLVM: 45757 – line coverage shows coverage of comments

1 Like
7

Hi @WesleyRosenblum, I hadn’t heard of the problem until you mentioned it to me. I’m glad you found a bug report for it. I’ll keep an eye on it to see if it gets resolved, but thanks for following up here.