979f598: Clang Static Analyzer Report #12961

issue kallewoof opened this issue on April 12, 2018
  1. kallewoof commented at 2:25 AM on April 12, 2018: member

    BRANCH: master COMMIT: 979f59850c72624137d25f80be4188c3ba6b5fa0 ISSUE COUNT: 16 [11 unconfirmed, 5 pending fix] EXCLUDED: nothing SUPERCEDES: #9573 ANALYZER BUILD: checker-279 (2016-11-14 15:34:09)

    This report includes upstream repositories (leveldb, secp256k1). We may not want to fix them, but we should at least know about them, in case they cause issues. I can provide detailed reports for any of these, on request (see example of such a report for the rpc/mining.cpp error below).

    DISCLAIMER: These results have not been thoroughly confirmed, and may be improbable or flat out invalid, but it's worth having a list of these somewhere.

    BITCOIN CORE [4 issues]

    LOGIC ERRORS [1 present, 1 confirmed]:

        result.pushKV("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1);
                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    

    DEAD STORE [2 present, 2 confirmed]:

                rv = write(fd, &ch, 1);
                ^    ~~~~~~~~~~~~~~~~~
    
            target = 0;
            ^        ~
    

    MEMORY ERROR [1 present, 0 confirmed]:

        replySent = true;
        ~~~~~~~~~~^~~~~~
    

    LEVELDB [4 issues]

    LOGIC ERRORS [1 present, 1 confirmed]:

      return (ecx & (1 << 20)) != 0;
              ~~~ ^
    

    DEAD STORE [3 present, 0 confirmed]:

        offset_in_block = 0;
        ^                 ~
    
                in_fragmented_record = false;
                ^                      ~~~~~
    
                in_fragmented_record = false;
                ^                      ~~~~~
    

    SECP256K1 [8 issues]

    LOGIC ERRORS [7 present, 0 confirmed]:

        r->n[0] += a->n[0];
                ^  ~~~~~~~
    
    • src/field_5x52_impl.h:390:13: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
        r->n[0] *= a;
        ~~~~~~~ ^
    
    • src/field_5x52_impl.h:406:13: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
        r->n[0] += a->n[0];
        ~~~~~~~ ^
    
    • src/util.h:24:5: Access to field 'fn' results in a dereference of a null pointer (loaded from variable 'cb')
        cb->fn(text, (void*)cb->data);
        ^~~~~~
    
        no |= (a->d[3] < SECP256K1_N_3); /* No need for a > check. */
               ~~~~~~~ ^
    
    • src/ecmult_impl.h:217:12: Access to field 'pre_g' results in a dereference of a null pointer (loaded from variable 'ctx')
        return ctx->pre_g != NULL;
               ^~~~~~~~~~
    
        return ctx->prec != NULL;
               ^~~~~~~~~
    

    DEAD STORE [1 present, 1 confirmed]

        bits = 0;
        ^      ~
    
  2. fanquake commented at 2:57 AM on April 12, 2018: member

    leveldb:

    logic errors

    https://github.com/bitcoin-core/leveldb/pull/16

    dead store

    From what I can tell these will be handled in https://github.com/bitcoin-core/leveldb/pull/17

    secp256k1:

    dead store

    https://github.com/bitcoin-core/secp256k1/pull/485

  3. kallewoof commented at 3:32 AM on April 12, 2018: member

    @fanquake Added "RESOLVED BY"s. The leveldb dead stores I could not verify as the branch currently fails to compile on my end (https://github.com/bitcoin-core/leveldb/pull/17#issuecomment-380666692).

  4. practicalswift cross-referenced this on Apr 12, 2018 from issue Fix Clang Static Analyzer warnings by practicalswift
  5. practicalswift commented at 6:33 AM on April 12, 2018: contributor

    Thanks for doing these! Could you add Clang version used to the excellent summary table at the top?

  6. kallewoof commented at 6:41 AM on April 12, 2018: member

    @practicalswift Done (it's old, but seems to be the latest version).

  7. practicalswift commented at 11:29 AM on April 12, 2018: contributor

    Anyone who would like to look at the reported Potential leak of memory pointed to by 'ev' in src/httpserver.cpp? The others have been addressed in #12963.

  8. kallewoof commented at 5:28 AM on April 13, 2018: member

    I'll make a path report public for the 'ev' thing later today. I suspect it's the analyzer getting confused over something, though.

  9. MarcoFalke commented at 2:46 PM on May 14, 2018: member

    What is the status of this?

  10. MarcoFalke referenced this in commit c5870ab689 on May 14, 2018
  11. practicalswift commented at 2:58 PM on May 14, 2018: contributor

    @kallewoof The PR #12963 has now been merged. What about closing this PR and opening a new for the May 2018 edition of the report? :-)

  12. kallewoof commented at 3:53 PM on May 14, 2018: member

    There is the 'ev' thing that I never got around to but closing for now. If someone wants to look into it I can provide details.

  13. kallewoof closed this on May 14, 2018

  14. fanquake cross-referenced this on May 18, 2018 from issue "resource leak " issue by fbthrift
  15. practicalswift commented at 7:47 AM on September 4, 2018: contributor

    @kallewoof Time for another report? :-)

  16. HashUnlimited cross-referenced this on Sep 7, 2018 from issue Fix Clang Static Analyzer warnings by HashUnlimited
  17. UdjinM6 referenced this in commit 531d8ceade on May 21, 2021
  18. UdjinM6 referenced this in commit 32e84f8ea4 on May 25, 2021
  19. bitcoin locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:55 UTC