resolve several issues found by cppcheck (dotnet/coreclr#26869)

* src/debug/daccess/nidump.cpp: remove dead code
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.

* src/dlls/dbgshim/dbgshim.cpp: resolve possible null pointer dereference
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.

* src/ilasm/assembler.cpp: resolve possible null pointer dereference
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.

* src/jit/flowgraph.cpp: resolve possible null pointer dereference
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.

* src/pal/src/debug/debug.cpp: resolve possible null pointer dereference
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.

* src/pal/src/libunwind/src/arm/Gex_tables.c: resolve possible null pointer 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.

* src/pal/src/synchmgr/synchmanager.cpp: resolve possible 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.

Commit migrated from https://github.com/dotnet/coreclr/commit/181523f28adf51de198e1772a35c54ec4819e6c9

diff --git a/src/coreclr/src/debug/daccess/nidump.cpp b/src/coreclr/src/debug/daccess/nidump.cpp
index 0d5b1c655660d501bb24bfbc8b8fe8ff1df485df..40216423a269e79c4b39a42a0a451db09c940acd 100644 (file)
--- a/src/coreclr/src/debug/daccess/nidump.cpp
+++ b/src/coreclr/src/debug/daccess/nidump.cpp
@@ -2838,16 +2838,11 @@ IMetaDataImport2 * NativeImageDumper::TypeToString(PTR_CCOR_SIGNATURE &sig,
                 {   
                     if (sizes[i] != 0 && lowerBounds[i] != 0)   
                     {   
-                        if (lowerBounds[i] == 0)    
-                            buf.AppendPrintf( W("%s"), sizes[i] );
-                        else    
-                        {   
-                            buf.AppendPrintf( W("%d ..."), lowerBounds[i] );
-                            if (sizes[i] != 0)  
-                                buf.AppendPrintf( W("%d"),
-                                                  lowerBounds[i] + sizes[i]
-                                                  + 1 );
-                        }   
+                        buf.AppendPrintf( W("%d ..."), lowerBounds[i] );
+                        if (sizes[i] != 0)  
+                            buf.AppendPrintf( W("%d"),
+                                              lowerBounds[i] + sizes[i]
+                                              + 1 );
                     }   
                     if (i < rank-1) 
                         buf.Append( W(",") );

diff --git a/src/coreclr/src/dlls/dbgshim/dbgshim.cpp b/src/coreclr/src/dlls/dbgshim/dbgshim.cpp
index c3f321a5ba872045300b6b5ce0edaf252e8be5b3..4b9739f6633febb858c0815d0885867d2af96608 100644 (file)
--- a/src/coreclr/src/dlls/dbgshim/dbgshim.cpp
+++ b/src/coreclr/src/dlls/dbgshim/dbgshim.cpp
@@ -1364,15 +1364,15 @@ CloseCLREnumeration(
 {
     PUBLIC_CONTRACT;
 
-    if ((pHandleArray + dwArrayLength) != (HANDLE*)pStringArray)
-        return E_INVALIDARG;
-
     // It's possible that EnumerateCLRs found nothing to enumerate, in which case
     // pointers and count are zeroed.  If a debugger calls this function in that
     // case, let's not try to delete [] on NULL.
     if (pHandleArray == NULL)
         return S_OK;
 
+    if ((pHandleArray + dwArrayLength) != (HANDLE*)pStringArray)
+        return E_INVALIDARG;
+
 #ifndef FEATURE_PAL
     for (DWORD i = 0; i < dwArrayLength; i++)
     {

diff --git a/src/coreclr/src/ilasm/assembler.cpp b/src/coreclr/src/ilasm/assembler.cpp
index f790ddb19634843b168dece317e7c70cb23f7a35..9859f1724621db2b3b9a8ec9d32a2e6a9eea815e 100644 (file)
--- a/src/coreclr/src/ilasm/assembler.cpp
+++ b/src/coreclr/src/ilasm/assembler.cpp
@@ -2327,12 +2327,12 @@ void Assembler::AddMethodImpl(mdToken tkImplementedTypeSpec, __in __nullterminat
     if(m_pCurClass)
     {
         MethodImplDescriptor*   pMID = new MethodImplDescriptor;
-        pMID->m_fNew = TRUE;
         if(pMID == NULL)
         {
             report->error("Failed to allocate MethodImpl Descriptor\n");
             return;
         }
+        pMID->m_fNew = TRUE;
         pMID->m_tkDefiningClass = m_pCurClass->m_cl;
         if(szImplementingName) //called from class scope, overriding method specified
         {

diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp
index 40f04c7e7579873d657532c23f1792b22ed8b019..c82c8a185df14c0bce5b01133a98bff23102c816 100644 (file)
--- a/src/coreclr/src/jit/flowgraph.cpp
+++ b/src/coreclr/src/jit/flowgraph.cpp
@@ -11002,12 +11002,12 @@ void Compiler::fgUnlinkRange(BasicBlock* bBeg, BasicBlock* bEnd)
 
 void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
 {
-    BasicBlock* bPrev = block->bbPrev;
-
     /* The block has to be either unreachable or empty */
 
     PREFIX_ASSUME(block != nullptr);
 
+    BasicBlock* bPrev = block->bbPrev;
+
     JITDUMP("fgRemoveBlock " FMT_BB "\n", block->bbNum);
 
     // If we've cached any mappings from switch blocks to SwitchDesc's (which contain only the

diff --git a/src/coreclr/src/pal/src/debug/debug.cpp b/src/coreclr/src/pal/src/debug/debug.cpp
index abbfe08e752dc09dfbe60d2de71eac823aca3b31..86709bc692eafb904933e1f81b3851545f2bf0ae 100644 (file)
--- a/src/coreclr/src/pal/src/debug/debug.cpp
+++ b/src/coreclr/src/pal/src/debug/debug.cpp
@@ -375,10 +375,10 @@ DebugBreakCommand()
 FAILED:
     if (command_string)
     {
+        fprintf (stderr, "Failed to execute command: '%s'\n", command_string);
         free(command_string);
     }
 
-    fprintf (stderr, "Failed to execute command: '%s'\n", command_string);
     return -1;
 #else // ENABLE_RUN_ON_DEBUG_BREAK
     return 0;

diff --git a/src/coreclr/src/pal/src/libunwind/src/arm/Gex_tables.c b/src/coreclr/src/pal/src/libunwind/src/arm/Gex_tables.c
index d6573a65e0c7d939010b6a56888c27a775c13507..e79903cd87d3fea6f2230f48e7359470582d3dcf 100644 (file)
--- a/src/coreclr/src/pal/src/libunwind/src/arm/Gex_tables.c
+++ b/src/coreclr/src/pal/src/libunwind/src/arm/Gex_tables.c
@@ -152,13 +152,12 @@ HIDDEN int
 arm_exidx_decode (const uint8_t *buf, uint8_t len, struct dwarf_cursor *c)
 {
 #define READ_OP() *buf++
+  assert(buf != NULL);
+  assert(len > 0);
   const uint8_t *end = buf + len;
   int ret;
   struct arm_exbuf_data edata;
 
-  assert(buf != NULL);
-  assert(len > 0);
-
   while (buf < end)
     {
       uint8_t op = READ_OP ();

diff --git a/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp b/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp
index dbb2d52481b47cc40c6a71ef0954f5b079256674..b5fb98b23d6122456b60b881d02075945e44f3b5 100644 (file)
--- a/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp
+++ b/src/coreclr/src/pal/src/synchmgr/synchmanager.cpp
@@ -2507,9 +2507,10 @@ namespace CorUnix
         BYTE * pbySrc, * pbyDst = rgSendBuf;
         WaitingThreadsListNode * pWLNode = SharedIDToTypePointer(WaitingThreadsListNode, shridWLNode);
 
+
+        _ASSERT_MSG(NULL != pWLNode, "Bad shared wait list node identifier (%p)\n", (VOID*)shridWLNode);
         _ASSERT_MSG(gPID != pWLNode->dwProcessId, "WakeUpRemoteThread called on local thread\n");
         _ASSERT_MSG(NULL != shridWLNode, "NULL shared identifier\n");
-        _ASSERT_MSG(NULL != pWLNode, "Bad shared wait list node identifier (%p)\n", (VOID*)shridWLNode);
         _ASSERT_MSG(MsgSize <= PIPE_BUF, "Message too long [MsgSize=%d PIPE_BUF=%d]\n", MsgSize, (int)PIPE_BUF);
 
         TRACE("Waking up remote thread {pid=%x, tid=%x} by sending cmd=%u and shridWLNode=%p over process pipe\n",