A recent change caused in the number of Coverity defects. This created a huge spike for Firefox: New defects found: 4045 And Thunderbird: New defects found: 545

Andi, could you have a look when you go back from pto? Thanks

To be clear, this is not caused by a new version of coverity The run of August first was done with 8.5.0.1: http://relman-ci.mozilla.org/job/firefox-coverity/150/consoleFull We would have get it at the time. I think it is a change in the code (in a header, very likely).

After discussing with Mark Armitage, a contact from Coverity, we realised that this spike appeared due to the update from 7.7.0.4 to the 8.5.0. Bellow is a quote from his email: >>I see that your project gained 4072 new defects on the 9th of this month, >>and this was the first analysis done with version 8.5.0 - the previous >>analysis was with version 7.7.0.4. So it’s almost certainly something >>related to this change. The other interesting change is that in that same >>run we over 100,000 more LOC in your project than in the previous run. >>So either something changed in your build, or (more likely) we may have >>fixed some bugs that were preventing some of your code being analyzed >>previously - so the 4000 extra defects are because we are analyzing more code! I will continue to investigate this with his support. For example Coverity ID 1368264: >>CID 1368264 (#1 of 1): Misused comma operator (NO_EFFECT) >>extra_comma: Part this->mRefCnt of statement (this->mRefCnt) , false has no effect due to the comma. >> 12NS_IMPL_ADDREF(nsSystemAlertsService) >>The details about this error are: >>The left hand side of the comma will be evaluated and then the value discarded. >>In nsSystemAlertsService::​AddRef(): Comma operator has a left sub-expression with no side-effects (CWE-480) The current problem seems to be related with the macro definition for NS_IMPL_ADDREF >>#define NS_IMPL_ADDREF(_class) \ >>NS_IMETHODIMP_(MozExternalRefCountType) _class::AddRef(void) \ >>{ \ >> MOZ_ASSERT_TYPE_OK_FOR_REFCOUNTING(_class) \ >> MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt"); \ >> if (!mRefCnt.isThreadSafe) \ >> NS_ASSERT_OWNINGTHREAD(_class); \ >> nsrefcnt count = ++mRefCnt; \ >> NS_LOG_ADDREF(this, count, #_class, sizeof(*this)); \ >> return count; \ >>} >>#define NS_IMETHODIMP_(type) type I don’t see an error here, the only think that i can think of that Coverity is suspicious about the cast int32_t(mRefCnt), where the type of mRefCnt is nsAutoRefCnt and gets casted to int32_t by using the overloading of operator nsrefcnt: >>class nsAutoRefCnt >>{ >>public: >> nsAutoRefCnt() : mValue(0) {} >> explicit nsAutoRefCnt(nsrefcnt aValue) : mValue(aValue) {} >> >> // only support prefix increment/decrement >> nsrefcnt operator++() { return ++mValue; } >> nsrefcnt operator--() { return --mValue; } >> >> nsrefcnt operator=(nsrefcnt aValue) { return (mValue = aValue); } >> operator nsrefcnt() const { return mValue; } >> nsrefcnt get() const { return mValue; } >> >> static const bool isThreadSafe = false; >>private: >> nsrefcnt operator++(int) = delete; >> nsrefcnt operator--(int) = delete; >> nsrefcnt mValue; >>}; I'll update this bug when there are new findings.

After investigating this issue with Mark, our poc from Coverity the conclusion was that this is a bug in their system that will be addressed when they release 8.5 scanner.

This issue can now be closed since by upgrading to the latest coverity scan, around 3100 false positive issues were removed.