Ticket #9945 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Coverity - High Impact Outstanding issues in API files

Reported by: Michael Reuter Owned by: Martyn Gigg
Priority: critical Milestone: Release 3.3
Component: Framework Keywords: Maintenance
Cc: Blocked By:
Blocking: Tester: Anders Markvardsen

Description

There are three Coverity high impact outstanding issues in the API module.

Change History

comment:1 Changed 6 years ago by Martyn Gigg

  • Keywords Maintenance added

comment:2 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:3 Changed 6 years ago by Martyn Gigg

  • Status changed from assigned to inprogress

comment:4 Changed 6 years ago by Martyn Gigg

I believe the 3 issues are false positives and I have marked them as such in the system.

Coverity gave following trace for all 3 errors:

/// Called by the vector constructors to do the actual filling
 64  void SpectrumDetectorMapping::fillMapFromVector(const std::vector<specid_t>& spectrumNumbers,
 65                                                  const std::vector<detid_t>& detectorIDs,
 66                                                  const std::vector<detid_t>& ignoreDetIDs)
 67  {
 68    std::set<detid_t> ignoreIDs(ignoreDetIDs.begin(), ignoreDetIDs.end());
 69    const size_t nspec(spectrumNumbers.size());
   	1. Condition i < nspec, taking true branch
   	4. Condition i < nspec, taking true branch
   	7. Condition i < nspec, taking true branch
 70    for ( size_t i = 0; i < nspec; ++i )
 71    {
 72      auto id = detectorIDs[i];
   	2. Condition ignoreIDs.count(id) == 0, taking true branch
   	5. Condition ignoreIDs.count(id) == 0, taking true branch
   	8. Condition ignoreIDs.count(id) == 0, taking true branch
   	9. alloc_fn: Storage is returned from allocation function operator [].
   	10. noescape: Resource this->m_mapping[spectrumNumbers[i]] is not freed or pointed-to in insert.
   	
CID 1106377 (#1 of 1): Resource leak (RESOURCE_LEAK)
11. leaked_storage: Failing to save or free storage allocated by this->m_mapping[spectrumNumbers[i]] leaks it.
 73      if(ignoreIDs.count(id) == 0 ) m_mapping[spectrumNumbers[i]].insert(id);
   	3. Jumping back to the beginning of the loop
   	6. Jumping back to the beginning of the loop
 74    }
 75  }
 76

The way in which the memory is allocated by the [] operator seems to confuse the checks. I ran a test and allocated/deleted 1M objects and no memory was lost so I don't think this is a real leak.

comment:5 Changed 6 years ago by Martyn Gigg

  • Status changed from inprogress to verify
  • Resolution set to fixed

There are no code changes required as all were false positives. The coverity report on the API files can be viewed by first visiting ​here and clicking on my email address.

comment:6 Changed 6 years ago by Anders Markvardsen

  • Status changed from verify to verifying
  • Tester set to Anders Markvardsen

comment:7 Changed 6 years ago by Anders Markvardsen

  • Status changed from verifying to closed

Yes confusion appears to be related to [], and possibly confused with std::map insert method

comment:8 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​10787

Note: See TracTickets for help on using tickets.