c:S878 - False Positive Based on Description

Running SonarQube version 9.5.0.56709, SonarScanner 4.7.0.2747. The C code is compiled with GCC 4.8.5 - don’t know if that matters.

Rule 878 for C - “Comma operator should not be used” seems to report false positives in for statements which, according to the rules description, are to be excepted:

Exceptions

Use of comma operator is tolerated in initialization and increment expressions of for loops.

for(i = 0, j = 5; i < 6; i++, j++) { … }

However, these two examples each create an issue:
for(i = 0, idx = 0; i < LIMIT_VALUE; i++)
for (i = INIT_VALUE + 1, j = 1; i < INIT_VALUE + LEN; i++, j++)

I would understand why the first might not adhere to the exception (since idx is not incremented directly in the “for” loop increment expression), however this is a normal use case in a lot of contexts and the second example, which clearly follows the exception pattern, is also flagged and creates an issue.

I could not find any fixes related to this issue in the 9.6 release notes so did not try upgrading - please let me know if upgrading would resolve the issue or if this is truly a false positive.

Hello @Adrianh,

you are right both cases shouldn’t trigger.
I tried this example, and it didn’t raise any S878 violation:

#define LIMIT_VALUE 10
#define INIT_VALUE 0
void fun(int i, int idx, int j, int LEN) {

for (i = 0, idx = 0; i < LIMIT_VALUE; i++) {
//
}

for (i = INIT_VALUE + 1, j = 1; i < INIT_VALUE + LEN; i++, j++) {
//
}
}

This means either there is something in your code that isn’t reflected in my example or there is something different in your environment that leads to the false positive.

To investigate this issue I will need a .reproducer.

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 (for instance, a file that contains a 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.
  • Please share this file. If you think this file contains private information, let us know, and we’ll send you a private message that will allow you to send it privately.

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

Thanks,

Hello, @Abbas_Sabra

The generated reproducer contained a good chunk of proprietary code which I couldn’t share so I created a small example and managed to find that there’s a difference in our (rather old) chroot environment that does the build. In short, this only reproduces under CentOS7 + GCC 4.8.5, I was unable to reproduce in Ubuntu + gcc7/9. This is the only difference I could make out, the scanner, analyzer, plugin versions, build wrapper seem to be the same.

Since I’m in the process of upgrading the build environment (CentOS 7 is nearly EOL) this won’t be much of an issue on our side. I can provide the reproducer for the test if you are still interested.

@Adrianh, Yes if you can share the reproducer of the test it would be great. It might be a generic issue related to the eviroment that we can fix if you share the reproducer.

Here you go. If you need any more details about the environment, let me know.
sonar-cfamily-reproducer.zip (43.9 KB)

Hi @Adrianh ,
Indeed, we have a false positive when running the analysis on C89 code. Are you really limited to C89, or is it just that you don’t specify the language level?
If you can compile with -std=C99 the problem should disappear.
If you cannot, I created a ticket but we cannot guarantee when we’ll be able to address it.

1 Like

Hello, @Fred_Tingaud
We can work around this, we are currently upgrading tooling anyway so this shouldn’t be a problem for us. I can mark the existing issues as false positives until the transition is complete so no rush.

Thank you,
Adrian

1 Like

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.