cpp:S3807 false positive - Parameter values should be appropriate

Hi - our project started using SonarQube (9.5 Enterprise Edition). We have a very old large code base, mixed C and // C++. We’re getting a large number of what we would consider false positives and we’re pretty sure it’s because of the nested #defines. Is anything we can do about this without restructuring the code (which we can’t do) or using //NOSONAR (which I don’t want to do)? We have hundreds of these.

Example:

typedef signed int RS;
#define	MAX_OP_NAME_LEN	40
#define RS_NO_ERR		0

#define RS_FAIL_BIT_LOCATION		31
#define RS_PASS(rstat)  ((((rstat) & 31) == 0) ? true:false)
#define RS_FAIL(rstat)  ((((rstat) & 31) == 0) ? false:true)
#define RS_MAKE(bFail, bLogged, zMod, zNum) RSEncode(bFail, bLogged, zMod, zNum)
#define RS_MAKE_FAIL(zMod, zNum) RS_MAKE (true, false, zMod, zNum)

#define NULL_POINTER_02_ERR			 (RS_MAKE_FAIL("XXX", "020"))
#define INPUT_STRING_TOO_LONG_05_ERR (RS_MAKE_FAIL("XXX", "099"))

typedef char SC_OP_NAME[SC_MAX_OP_NAME_LEN + 1];

RS RSEncode (bool bFail, bool bLogged, const char *pzMod, const char  *pzNum)
{
    // takes the arguments and creates a non-zero return value of type RS
    // with bit location 31 set to true.
}

static RET_STAT SetCfgMode(const char * const pzName)
{
    RET_STAT rStatus = RS_NO_ERR;
    char zThingName[SC_MAX_OP_NAME_LEN + 1];

    if (nullptr == pzName)
    {
        // This causes RET
        rStatus = NULL_POINTER_02_ERR;
    }
    if (RS_PASS(rStatus))
    {
        if (strlen(pzName) >= sizeof(zThingName))
        {
            // Log an error
            rStatus = INPUT_STRING_TOO_LONG_05_ERR;
        }
        else
        {
            strcpy(zThingName, pzName);
        }
    }
    return rStatus;
}

Screen shot of where SonarQube decides the pointer is null (this is a screenshot from Eclipse using SonarLint but the SonarQube results report the same thing):

Hi,

Your SonarQube version is past EOL, but you mention you’re also seeing this in Eclipse. What’s your SonarLint version?

 
Thx,
Ann

Hi - we have plans to upgrade but that’s out of my control. Hopefully it will be soon.
The SonarQube interface shows the same path as the screenshot but I can’t share that screenshot because it’s proprietary code. I had to scrub the code before I took a screenshot. I’m hoping that this isn’t the first time someone has had an issue with nested #defines.

Hi,

What I’m interested in is whether this is still a problem in the latest SonarLInt version in unconnected mode (i.e. with the latest rule implementation).

 
Ann

Hi again,

I’ve been reminded that C/C++ support is only available in Eclipse in connected mode, which means you can’t try the latest rule implementation in Eclipse. So I’ve browsed around in Jira a bit. Since I don’t see anything that looks relevant to me, I’m going to flag this for the language experts.

 
Ann

Hi Sarah

Thank you for sharing a suspect for a false positive. Normally, macro definitions should not be a big obstacle to the detection of these kinds of bugs. They might obscure the bug report, or even prevent it, but not lead to a false positive.

Unfortunately, your code snippet is missing the definition of RSEncode - it does not compile.
It is also missing the definitions for SC_MAX_OP_NAME_LEN and RET_STAT.
Depending on how I complete the definition of this function, the issue may be there and detected or not and not reported.

Let me follow the comment:

RS RSEncode (bool bFail, bool bLogged, const char *pzMod, const char  *pzNum)
{
    return 1 << 31; // Set 31-st bit to '1'
}

In this case, the code does contain a bug: RS_PASS(rStatus) would expand into ((((rstat) & 31) == 0) ? true:false) where rStatus is 1 << 31. Note that (1 << 31) & 31 is 0. The nullptr dereference bug is correctly reported, as you can see on Compiler Explorer.

If I define the function in a way that would prevent the bug:

RS RSEncode (bool bFail, bool bLogged, const char *pzMod, const char  *pzNum)
{
    return 31; // Make RS_PASS produce false
}

Our analyzer also does not report any nullptr dereference. See Compiler Explorer.

Having played with the code, I could not find any misbehavior of our analyzer, at least of its latest version deployed on Compiler Explorer, and 6.34 used by SonarQube 9.5.

If none of the options to complete your code snippet above work, do you have a more specific example?

1 Like

Thank you. I will work on the code example so that it better represents what our code is doing. It’s a bit difficult because I have to scrub the code.

I’ll be waiting for the example.
Note that you can generate a reproducer file (which includes your source code unabridged) and send it to me privately. The reproducer file makes it most convenient for me and my team to investigate, and we are used to dealing with source code that cannot be shared publicly.

Can you point me at directions for generating the reproducer file? Much appreciated.

To generate the reproducer file:

  • Search in the analysis log for the full path of the source file for which you want to create a reproducer - the file that contains the false-positive. You will have to use exactly this name (same case, / or \…)
  • Add the reproducer option to the scanner configuration:
    sonar.cfamily.reproducer=“Full path to the .cpp”
  • Re-run the scanner to generate a file named sonar-cfamily.reproducer in the project folder.
  • Share this file privately by replying to the private message I’ve sent you.

P.S. in case of an issue in a header file, you want to generate the reproducer of the source file that includes that header.