False positive for shadowed variable

Clean Code Report False-positive / False-negative...
,
1

Hey there!

For several uses of this template function, sonarqube triggers issues “declaration shadows a local variable” ( cpp:S1117 ) that look like false positives to me.

Example usage where the shadowed variable is reported on the second line:

    std::string str( "abcdefg" );
    mynamespace::utils::shuffle( str );

function definition:

template < std::ranges::random_access_range R >
    requires std::permutable< std::ranges::iterator_t< R > >
std::ranges::borrowed_iterator_t< R > shuffle( R&& r, uint32_t seed = getRandomSeed() ) {
    return std::ranges::shuffle( r, std::mt19937( seed ) );
}

Interestingly it’s not reported for any of the usages in the methods unit tests (provided via sonar.tests and sonar.test.inclusions property).

Must-share information:

  • using versions:
    sonarqube 9.9.6
    SonarScanner 4.8.0.2856
    CFamily plugin version: 6.41.2.69583

  • how is SonarQube deployed: Docker

3

Hi @jsinge ,
Could you rename the str variable and scan again ?

4

Can do!

Meanwhile, here is another occurence (on third line):

        std::vector< size_t > indices( num );
        std::iota( indices.begin(), indices.end(), 0 );
        mynamespace::utils::shuffle( indices );
6

Hi @jsinge
Did you try to rename your shuffle function ? Maybe a suffle function already importer expect test files.

9

Hi @jsinge, and thanks for sharing the problem with us,

I looked into the example you provided, but I couldn’t reproduce the behavior you described on Compiler Explorer.

To help us reproduce, would you be willing to share a reproducer file with us?

To generate such a file, you can set the property sonar.cfamily.reproducer to the absolute path of the cpp file where the issue is reported. If the False Positive is reported in a header, you can set the property to the path of a cpp file that includes this header. When running the scanner with this property, a file named sonar-cfamily-reproducer.tar.xz will be generated which includes the necessary information to help us reproduce the problem on our side.

I can also start a private thread with you to share the file privately with us…

Best regards,
Michael

12

Yes, please start a private thread.

13

After renaming the variable, sonar didn’t flag it again, maybe because it’s already set to wontfix?

I created another branch and added two new functions:

// some other comment

std::string newCode() {

    std::string soup( "123456" );

    scils::utils::shuffle( soup );

    // this code looks completely different, doesn't it?

    return soup.substr( 0, 12 );

}

// and another one

std::string moreNewCode() {

    std::string str( "123456" );

    scils::utils::shuffle( str );

    // this code also looks completely different, doesn't it?

    return str.substr( 0, 12 );

}

In this analysis, shuffle( soup ) was flagged but not shuffle( str ). No idea what to conclude from that :sweat_smile:

14

Is there another way to create the reproducer file? The absolute name is hard to predict when running the scanner from our CI.

15

You can still see issues that were marked as “won’t fix” by adjusting the status filter on SQ.

With the information I have at the moment, there could be many possiblities. One reason could be that the issue was already set to “won’t fix” as you mentioned. Another, could be a configuration issue that causes the analyzer to resolve scils::utils::shuffle to a type (instead of a function), and this causes the FP to be reported only in contexts where this happens. It could also be something completely different…

Generating the reproducer should hopefully enable us to determine the cause quickly on our side, and then we can try to suggest a suitable solution…

A colleague just pointed out to me that the CFamily analyzer doesn’t support sonar.tests. If you wish to analyze test code, you would need to add it to sonar.sources.

One workaround would be to determine the absolute path in CI, and pass the property while invoking the scanner. For example, if your CI is running in bash (untested):

FILE=$(realpath src/path/to/file_with_fp.cpp)
sonar-scanner ... -Dsonar.cfamily.reproducer=$FILE

I hope this helps,

Best regards,
Michael

16

yes specifying the property as argument on the sonar-scanner call turned out to be feasible. reproducer file is on your way.

Note for windows users: Seems as if it’s mandatory to use native format with backslash folder separates.

I also tried a branch analysis on my local PC (on the CI it’s running a PR analysis). There, no issue is listed (also not with resolution wontfix or false positive).

17

Oh, I didn’t know that. Indeed the tests are currently excluded from the analysis in that project configuration.

18

Hi @jsinge, and thanks for sharing the reproducer with us!

I had a look, and it seems that you are compiling with Visual Studio 2022 version 17.10.1 (released May 29th, 2024). Unfortunately, we didn’t have complete support for all C++20 features in CFamily version 6.41 (released back in January 2023).

The FP you encountered is triggered because the new MSVC version comes with headers that make more extensive use of the C++20 features. For example, by relying more on the relaxation of the typename disambiguator. These usages are not understood by the older version of the CFamily analyzer. If you enable verbose logs when invoking the scanner, I would expect to see a lot of entries that look like this in the verbose logs:

C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include\type_traits:1394 expected a type
C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include\type_traits:1394 expected ';' after alias declaration
C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include\type_traits:1399 no template named '_Copy_cv'
C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include\type_traits:1404 use of undeclared identifier '_Copy_cv'
C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include\type_traits:1409 use of undeclared identifier '_Copy_cv'
C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include\type_traits:1420 missing 'typename' prior to dependent type name 'common_reference<_Types...>::type'
...

Unfortunately, these errors negatively impact the analyzer’s capability to reason about the code, resulting in the false positive you reported and, very likely, many false negatives. You can verify that this is the case by enabling verbose logs during the analysis, and checking if you have similar entries.

I can suggest the following options:

  1. If possible, update to SQ 10.6, which comes with a more recent version of the CFamily analyzer, which can better understand these C++20 features. Furthermore, the logs have been improved, so you can spot such problems without enabling verbose logs.
  2. If the project doesn’t actually use C++20, you can try building with C++17. This is probably irrelevant to your current project, as I can see you are using std::ranges in the example you shared.
  3. As a workaround, if all the above is impossible: If your project can be compiled with other compilers (e.g. clang or gcc), or if you can build with an older version of MSVC (before the STL headers started adopting these features), you can try to run the analysis using any of these compilers. However, it should be noted that C++20 support is limited in CFamily 6.41 (and, hence, in SQ 9.9).

I hope this helps, and thanks for taking the time to report this false positive,

Best regards,
Michael

1 Like
20

Very interesting, thanks for explaining!
It’s also good to know there may be many false negatives when using the LTA on a c++20 project.

There’s no chance that cfamily analyzer changes get backported to the LTA version?

21

Hi,

We backport fixes of serious bugs/vulnerabilities, but never features.

 
Ann

1 Like