Using sonar scanner version: 3.0.3.778.
SonarCfamily version: 5.0 (build 9359)
We have a self defined assertion macro (TEST_ASSERT) defined in the header file assert.h. We use this TEST_ASSERT to detect null function pointer but sonarqube does not take this macro into consideration during analysis. For now we have closed the issue as false positive.
Is there any way to specify self defined macro definitions during analysis?
To be clear, we’re talking about C/C++ analysis? Also, what’s your version of SonarCFamily (found in the SonarQube UI in Administration > Marketplace)?
We only have C program files. So will this below property help?
sonar.cxx.forceIncludes=<path_to_header_file>
The header file is already included in the particular C file but the path to the header file is not defined from the root of the workspace. So I was thinking I can force include the header file again inside sonarproject.properties file. But since I closed the defect as false positive I am unable to verify whether this change is actually working or not.
If you’re really using SonarCFamily, then no cxx property will help. This is partly the reason I asked for your version of SonarCFamily; to see which analyzer you’re using.
We wrote a macro to check if a function pointer is NULL and this macro assert function is defined inside a header file.
RUNTIME_ASSERT( func != NULL );
But sonarqube analysis complains saying the function pointer value might be null and that condition is not being checked in the code. It is not considering the macro that is defined in our header file which already checks if the pointer is null.
It is difficult to exactly understand what happens with this level of information. If you are using the build wrapper, you should not have to manually tweak the configuration.
As requested by Ann, would need a small fully working example (with the definition of the macro and its usage) that reproduces the situation, to evaluate if this behaviour is expected or not…
It’s already better to read some code, but what you provided is not a full code that we could analyze on our side…However, I can already make a few remarks:
Your macro RUNTIME_ASSERT can obviously be deactivated, but you did not tell us if you tried to analyze the code where it was enabled or not?
Even if you analyze code where RUNTIME_ASSERT is not empty, how do you think we could have the information that the function runTimeAssert is not a normal function, and will not simply return to the calling code when it is done, leading to dereferencing a nullptr?
My doubt is that RUNTIME_ASSERT is checking if a function pointer is null or not. But sonarqube seems to not consider the macro and say that the pointer is not being checked for null value. Is there any work-around to make sonarqube understand that our self-defined macro is already checking that case.
This does not look like a complete file to me: You have a #else and a #endif that are not matching any #if…
Anyways, assuming that we are in the #ifbranch, not the #else (see my previous question on whether the macro is activated or not in the configuration used for analysis), your code is going to look like this after preprocessing:
bool testCondition( T_condition func )
{
if( func == 0) // We test the value
{
runTimeAssert( __FILE__, __LINE__, __VA_ARGS__"\n" ); // Maybe we log something
}
// We continue execution, no matter what the test said
while( 1 )
{
if( func() != 0 ) { /*...*/ } // We call the function, unconditionally
}
So, even if func is null, you are still going to call that function. What is missing for SonarQube to understand that this assertion will prevent the execution of the rest of the function is not something at the macro level, but something for the declaration of runTimeAssert function so that SonarQube knows this is a special function that will never return. If your code builds with gcc of clang, you can decorate this function with __attribute__((noreturn)) to indicate this special situation.
So, here is the action plan I advise to you:
Make sure you analyze the project in a mode where the macro is not empty, but contains the checking logic
Make sure the function called in case of failed assert in the macro is decorated with __attribute__((noreturn))
To show you an example, I analyzed a sample project on :sonarcloud: SonarCloud, here are the results:
A bit late to the discussion, but would it also be possible to mark a method to not return conditionally?
In our project we have a custom assert method implemented. So not a macro, but a method. If the condition provided is false, it will not return. Otherwise it will. Hence, we have the same issue as the original poster, where SonarQube is telling us that a pointer might still be null, although we already used our custom assert method to check that it isn’t.
This goes for both C++ as C#, so any advice on how to mark any methods as custom assert methods or conditionally not return would be greatly appreciated.
Just speaking for C++ here, I don’t know how it’s done in C#:
There is no such mechanism, and by itself, it would not be very helpful. Should we raise an issue or not? However, if your function is inline, and it calls two different functions, depending on a condition, one being noreturn, the other one being normal, I think it should work (I did not test though)
The function is not inline unfortunately. We could make it inline of course. Thanks for that suggestion.
I think it would be useful to have a means to mark a method as a custom assertion method, so that SonarQube does not give any warnings on things that cannot happen, because they were already checked by our custom assertion.