[C++] False positives after upgrade to version 8.5.1.38104

Report a bug False-positive
#1

After upgrade to SonarQube 8.5.1.38104, suddenly a lot of extra bugs are reported. We checked about 20 and they all make no sense to us.
(see below)
The assert boils done to this code:
‘’’
void CmsResult::Verify (const CmsResultCode &resultCode, const char *file, const long &line, const QString &description)
{

if (resultCode.m_resultCode < 0)
{
throw CmsException (resultCode, file, line, description);
}
}’’’

Screenshot 2020-11-11 111323
Screenshot 2020-11-11 111323700×231 16.8 KB

which will throw an exception, so the offending code will never be reached. The code also works for release builds.
Thus, the issue seems to be a false positive. All issues we found resolve to these macros.
If you need more info, please let me know, the code behind CMSASSERT is a bit complicated.

#3

Hello Marco.

Welcome to the community!
I have 2 questions to try to solve your problem:

  • What was your previous SQ version?
  • Is the body of the function defined in MonitorGui.cpp (I guess not) or in a header file included in the that file?

Cheers,

#4

Hello Geoffray,

  • the previous version was 8.4.[something].
  • The body is indeed defined in CmsMonitor.cpp. The macro isn’t. The macro in itself is a piece of code with some checks in it. So: could it be that SQ thinks there is a possibility of a null pointer or is it a certainty?
#5

Hello Marco.

Thanks for the additional information.

You are a victim of a recent improvement we made in the C++ analyzer :smiley:
So at first sight, in your case, it does not look like an improvement but trust me, it is. We have quite a few rules based on symbolic execution (for example buffer overflow detection).
Small glitches in parsing your code in our analyzer could prevent them from running and detecting interesting issues.
Now, if these glitches are minor, these rules can run. So in the end, you will have more interesting issues reported.

In that case, the problematic issues you are reporting are new and they are indeed FPs.
This happens because there, the body of the function Verify is not accessible to our analyzer while analyzing MonitorGui.cpp. So, it cannot know that the CMSASSERT macro while always raise an exception when the condition is not met.
This is a current limitation in our analyzer as we currently analyze each cpp file one at a time.

I can suggest a work-around to fix this problem. Simply move the body of the function Verify into the CMSASSERT macro.

I hope it helps.

#6

Hello @lowdike64

Did you try the work-around I suggested?
Did it help?

Cheers

#7

Hello Geoffrey,
I am reluctant to do this, as the code for e.g. CMSASSERT is at the core of all our products and though not to be tampered with if not really necessary…

The code boils down like this (refactored for easier reading)

#define CMSASSERT(condition) \
  MyResult::Assert ((condition), __FILE__, __LINE__, (#condition));

calls:

void MyResult::Assert (const bool &resultCode,
                        const char *file,
                        const long &line,
                        const char *condition,
                        const QString &description)
{
  if (false == resultCode)
  {
    Verify (My_ASSERT_FAILED, file, line, QString ("Assert condition (%1) failed. More info: %2").arg (condition).arg (description));
  }
}

Where Verify boils down to:

void MyResult::Verify (const MyResultCode &resultCode, const char *file, const long &line, const QString &description)
{
  if (resultCode.m_resultCode < 0)
  {
    throw MyException (resultCode, file, line, description);
  }
}

So, all calls are in class MyResult, which in this case throws an Exception object of class MyException.

My question here is: where does the scope of SonarQube stop?

#8

Hello Marco.

Currently, our symbolic execution engine (that supports part of our rules) is limited to a single translation unit. In other words, it can see all the code in the c++ analyzed file (c++ files are analyzed one by one exactly as each c++ source file is compiled). Note that it includes all the code that is inside headers that are included in the analyzed source file.
This explains my suggestion: if the bodies of MyResult::Assert and MyResult::Verify are in MyResult.cpp, the symbolic execution cannot see the constraint imposed by the condition.
It only knows the prototypes of these functions from MyResult.h.
This can be fixed if the bodies of these functions are found in MyResult.h.

I hope it can help you to find a way through.

Cheers