-
Notifications
You must be signed in to change notification settings - Fork 2.6k
resolve several issues found by cppcheck #26869
Conversation
found by cppcheck [src/debug/daccess/nidump.cpp:2839] -> [src/debug/daccess/nidump.cpp:2841]: (warning) Opposite inner 'if' condition leads to a dead code block.
found by cppcheck [src/dlls/dbgshim/dbgshim.cpp:1373] -> [src/dlls/dbgshim/dbgshim.cpp:1367]: (warning) Either the condition 'pHandleArray==NULL' is redundant or there is pointer arithmetic with NULL pointer.
found by cppcheck [src/ilasm/assembler.cpp:2331] -> [src/ilasm/assembler.cpp:2330]: (warning) Either the condition 'pMID==NULL' is redundant or there is possible null pointer dereference: pMID.
found by cppcheck [src/jit/flowgraph.cpp:11009] -> [src/jit/flowgraph.cpp:11005]: (warning) Either the condition 'block!=nullptr' is redundant or there is possible null pointer dereference: block.
found by cppcheck [src/pal/src/debug/debug.cpp:376] -> [src/pal/src/debug/debug.cpp:381]: (warning) Either the condition 'command_string' is redundant or there is possible null pointer dereference: command_string.
…nter dereference found by cppcheck [src/pal/src/libunwind/src/arm/Gex_tables.c:159] -> [src/pal/src/libunwind/src/arm/Gex_tables.c:155]: (warning) Either the condition 'buf!=NULL' is redundant or there is pointer arithmetic with NULL pointer.
…dereference found by cppcheck [src/pal/src/synchmgr/synchmanager.cpp:2512] -> [src/pal/src/synchmgr/synchmanager.cpp:2510]: (warning) Either the condition 'NULL!=pWLNode' is redundant or there is possible null pointer dereference: pWLNode.
janvorli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
|
|
||
| /* The block has to be either unreachable or empty */ | ||
|
|
||
| PREFIX_ASSUME(block != nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janvorli Does it make sense to use PREFIX_ASSUME for null checks? I think in debug it is the same behavior as a regular assert and in release it can't give any valuable hint for the compiler, can it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In release, it is expanded to __assume. In static analyzer builds, it expands to if (!(_condition)) __UNREACHABLE(); in all build configurations. So it seems it makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the release code for:
__assume(p != nullptr);
p->field;
and for
p->field;
would be the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if I understand - the __assume is a hint to the compiler. Of course the generated code would be the same. So the PREFIX_ASSUME is basically an extended kind of assert that static analysis tools can understand even in release build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jan.
found by cppcheck [src/arm/Gex_tables.c:159] -> [src/arm/Gex_tables.c:155]: (warning) Either the condition 'buf!=NULL' is redundant or there is pointer arithmetic with NULL pointer. Upstreamed from dotnet/coreclr#26869
found by cppcheck [src/arm/Gex_tables.c:159] -> [src/arm/Gex_tables.c:155]: (warning) Either the condition 'buf!=NULL' is redundant or there is pointer arithmetic with NULL pointer. Upstreamed from dotnet/coreclr#26869
found by cppcheck [src/arm/Gex_tables.c:159] -> [src/arm/Gex_tables.c:155]: (warning) Either the condition 'buf!=NULL' is redundant or there is pointer arithmetic with NULL pointer. Upstreamed from dotnet/coreclr#26869
No description provided.