From 181523f28adf51de198e1772a35c54ec4819e6c9 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Wed, 25 Sep 2019 00:19:06 +0000 Subject: [PATCH] resolve several issues found by cppcheck (#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. --- src/debug/daccess/nidump.cpp | 15 +++++---------- src/dlls/dbgshim/dbgshim.cpp | 6 +++--- src/ilasm/assembler.cpp | 2 +- src/jit/flowgraph.cpp | 4 ++-- src/pal/src/debug/debug.cpp | 2 +- src/pal/src/libunwind/src/arm/Gex_tables.c | 5 ++--- src/pal/src/synchmgr/synchmanager.cpp | 3 ++- 7 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/debug/daccess/nidump.cpp b/src/debug/daccess/nidump.cpp index 0d5b1c655660..40216423a269 100644 --- a/src/debug/daccess/nidump.cpp +++ b/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/dlls/dbgshim/dbgshim.cpp b/src/dlls/dbgshim/dbgshim.cpp index c3f321a5ba87..4b9739f6633f 100644 --- a/src/dlls/dbgshim/dbgshim.cpp +++ b/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/ilasm/assembler.cpp b/src/ilasm/assembler.cpp index f790ddb19634..9859f1724621 100644 --- a/src/ilasm/assembler.cpp +++ b/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/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index 40f04c7e7579..c82c8a185df1 100644 --- a/src/jit/flowgraph.cpp +++ b/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/pal/src/debug/debug.cpp b/src/pal/src/debug/debug.cpp index abbfe08e752d..86709bc692ea 100644 --- a/src/pal/src/debug/debug.cpp +++ b/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/pal/src/libunwind/src/arm/Gex_tables.c b/src/pal/src/libunwind/src/arm/Gex_tables.c index d6573a65e0c7..e79903cd87d3 100644 --- a/src/pal/src/libunwind/src/arm/Gex_tables.c +++ b/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/pal/src/synchmgr/synchmanager.cpp b/src/pal/src/synchmgr/synchmanager.cpp index dbb2d52481b4..b5fb98b23d61 100644 --- a/src/pal/src/synchmgr/synchmanager.cpp +++ b/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",