[1.10>master] [MERGE #5272 @Penguinwizzard] Address issues picked up …

…by Dev15 code analysis

Merge pull request #5272 from Penguinwizzard:dev15_prefast_fixes

This is one of the things that was preventing us from moving our jenkins CI to dev15.

bin/External/catch.hpp

@@ -7374,7 +7374,7 @@ namespace Catch {
             return TestCaseInfo::None;
     }
     inline bool isReservedTag( std::string const& tag ) {
-        return parseSpecialTag( tag ) == TestCaseInfo::None && tag.size() > 0 && !isalnum( tag[0] );
+        return parseSpecialTag( tag ) == TestCaseInfo::None && tag.size() > 0 && !isalnum( (unsigned char)tag[0] );
     }
     inline void enforceNotReservedTag( std::string const& tag, SourceLineInfo const& _lineInfo ) {
         if( isReservedTag( tag ) ) {

bin/GCStress/StubExternalApi.cpp

@@ -9,7 +9,7 @@
 
 bool ConfigParserAPI::FillConsoleTitle(__ecount(cchBufferSize) LPWSTR buffer, size_t cchBufferSize, __in LPWSTR moduleName)
 {
-    swprintf_s(buffer, cchBufferSize, _u("Chakra GC: %d - %s"), GetCurrentProcessId(), moduleName);
+    swprintf_s(buffer, cchBufferSize, _u("Chakra GC: %lu - %s"), GetCurrentProcessId(), moduleName);
 
     return true;
 }

bin/GCStress/WeightedTable.h

@@ -13,14 +13,14 @@ template <class T>
 class WeightedTable
 {
 public:
-    WeightedTable() :
+    WeightedTable() noexcept :
         entries(nullptr), size(0)
     {
     }
 
     void AddWeightedEntry(T value, unsigned int weight)
     {
-        T * newEntries = static_cast<T *>(realloc(entries, (size + weight) * sizeof(T)));
+        T * newEntries = static_cast<T *>(realloc(entries, ((size_t)size + weight) * sizeof(T)));
         if (newEntries == nullptr)
         {
             // Should throw something better

bin/NativeTests/CodexTests.cpp

@@ -3,6 +3,10 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 #include <process.h>
 #include "Codex\Utf8Codex.h"
@@ -280,4 +284,4 @@ namespace CodexTest
 
         RunUtf8DecodeTestCase(testCases, utf8::DecodeUnitsIntoAndNullTerminateNoAdvance);
     }
-};
+};

bin/NativeTests/FileLoadHelpers.cpp

@@ -114,7 +114,7 @@ HRESULT FileLoadHelpers::LoadScriptFromFile(LPCSTR filename, LPCWSTR& contents,
         utf8::DecodeOptions decodeOptions = utf8::doAllowInvalidWCHARs;
 
         UINT cUtf16Chars = utf8::ByteIndexIntoCharacterIndex(pRawBytes, lengthBytes, decodeOptions);
-        contents = (LPCWSTR)HeapAlloc(GetProcessHeap(), 0, (cUtf16Chars + 1) * sizeof(WCHAR));
+        contents = (LPCWSTR)HeapAlloc(GetProcessHeap(), 0, (cUtf16Chars + (size_t)1) * sizeof(WCHAR));
         if (nullptr == contents)
         {
             fwprintf(stderr, _u("out of memory"));

bin/NativeTests/FunctionExecutionTest.cpp

@@ -3,6 +3,10 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 #include "FunctionExecutionTest.h"
 

bin/NativeTests/FunctionExecutionTest.h

@@ -6,6 +6,7 @@
 // This file contains stubs needed to make FunctionExecutionTest successfully compile and link as well
 // as a means to emulate behavior of objects that interact with FunctionExecutionStateMachine
 
+#include "..\..\lib\Common\Warnings.h"
 #include "..\..\lib\Common\Core\CommonMinMax.h"
 
 #define ENUM_CLASS_HELPERS(x, y)

bin/NativeTests/JsDiagApiTest.cpp

@@ -4,6 +4,10 @@
 //-------------------------------------------------------------------------------------------------------
 
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 #include <process.h>
 

bin/NativeTests/JsRTApiTest.cpp

@@ -3,9 +3,14 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 #include <array>
 #include <process.h>
+#include <suppress.h>
 
 #pragma warning(disable:4100) // unreferenced formal parameter
 #pragma warning(disable:6387) // suppressing preFAST which raises warning for passing null to the JsRT APIs
@@ -1243,7 +1248,8 @@ namespace JsRTApiTest
             size_t length;
             REQUIRE(JsStringToPointer(nameValue, &name, &length) == JsNoError);
 
-            CHECK(length == 1);
+            REQUIRE(length == 1);
+#pragma prefast(suppress:__WARNING_MAYBE_UNINIT_VAR, "The require on the previous line should ensure that name[0] is initialized")
             CHECK(name[0] == ('a' + index));
         }
     }

bin/NativeTests/MemoryPolicyTest.cpp

@@ -3,6 +3,10 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 
 #pragma warning(disable:6387) // suppressing preFAST which raises warning for passing null to the JsRT APIs

bin/NativeTests/NativeTests.cpp

@@ -9,6 +9,10 @@
 // conversion from 'int' to 'char', possible loss of data
 #pragma warning(disable:4242)
 #pragma warning(disable:4244)
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 #pragma warning(pop)
 

bin/NativeTests/ThreadServiceTest.cpp

@@ -4,6 +4,10 @@
 //-------------------------------------------------------------------------------------------------------
 
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 
 #pragma warning(disable:6387) // suppressing preFAST which raises warning for passing null to the JsRT APIs

bin/NativeTests/UnicodeTextTests.cpp

@@ -4,6 +4,10 @@
 //-------------------------------------------------------------------------------------------------------
 
 #include "stdafx.h"
+#pragma warning(disable:26434) // Function definition hides non-virtual function in base class
+#pragma warning(disable:26439) // Implicit noexcept
+#pragma warning(disable:26451) // Arithmetic overflow
+#pragma warning(disable:26495) // Uninitialized member variable
 #include "catch.hpp"
 
 namespace UnicodeTextTest

bin/ch/Debugger.cpp

@@ -306,7 +306,7 @@ bool Debugger::SetBaseline()
 #ifdef _WIN32
     LPSTR script = nullptr;
     FILE *file = nullptr;
-    int numChars = 0;
+    size_t numChars = 0;
     HRESULT hr = S_OK;
 
     if (_wfopen_s(&file, HostConfigFlags::flags.dbgbaseline, _u("rb")) != 0)
@@ -316,13 +316,13 @@ bool Debugger::SetBaseline()
 
     if(file != nullptr)
     {
-        int fileSize = _filelength(_fileno(file));
-        if (fileSize <= MAX_BASELINE_SIZE)
+        long fileSize = _filelength(_fileno(file));
+        if (0 <= fileSize && fileSize <= MAX_BASELINE_SIZE)
         {
-            script = new char[fileSize + 1];
+            script = new char[(size_t)fileSize + 1];
 
-            numChars = static_cast<int>(fread(script, sizeof(script[0]), fileSize, file));
-            if (numChars == fileSize)
+            numChars = fread(script, sizeof(script[0]), fileSize, file);
+            if (numChars == (size_t)fileSize)
             {
                 script[numChars] = '\0';
 

bin/ch/HostConfigFlags.cpp

@@ -96,6 +96,7 @@ void HostConfigFlags::RemoveArg(int& argc, _Inout_updates_to_(argc, argc) LPWSTR
     Assert(index >= 0 && index < argc);
     for (int i = index + 1; i < argc; ++i)
     {
+#pragma prefast(suppress:__WARNING_READ_OVERRUN, "Operation is safe but PREfast is difficult to convince")
         argv[i - 1] = argv[i];
     }
     --argc;

bin/ch/JITProcessManager.cpp

@@ -52,7 +52,7 @@ HRESULT JITProcessManager::CreateServerProcess(int argc, __in_ecount(argc) LPWST
     STARTUPINFOW si = { 0 };
 
     // overallocate constant cmd line (jshost -jitserver:<guid>)
-    size_t cmdLineSize = (MAX_PATH + argc) * sizeof(WCHAR);
+    size_t cmdLineSize = (MAX_PATH + (size_t)argc) * sizeof(WCHAR);
     for (int i = 0; i < argc; ++i)
     {
         // calculate space requirement for each arg

bin/ch/RuntimeThreadData.cpp

@@ -20,19 +20,21 @@ RuntimeThreadLocalData& GetRuntimeThreadLocalData()
     return threadLocalData;
 }
 
-RuntimeThreadData::RuntimeThreadData()
+RuntimeThreadData::RuntimeThreadData() :
+    hSemaphore(nullptr), 
+    hThread(nullptr),
+    sharedContent(nullptr),
+    receiveBroadcastCallbackFunc(nullptr),
+    runtime(nullptr),
+    context(nullptr),
+    parent(nullptr),
+    leaving(false)
 {
     this->hevntInitialScriptCompleted = CreateEvent(NULL, TRUE, FALSE, NULL);
     this->hevntReceivedBroadcast = CreateEvent(NULL, FALSE, FALSE, NULL);
     this->hevntShutdown = CreateEvent(NULL, TRUE, FALSE, NULL);
 
-    this->sharedContent = nullptr;
-    this->receiveBroadcastCallbackFunc = nullptr;
-
-    this->leaving = false;
-
     InitializeCriticalSection(&csReportQ);
-
 }
 
 RuntimeThreadData::~RuntimeThreadData()

bin/ch/WScriptJsrt.cpp

@@ -35,6 +35,7 @@
 #endif // FreeBSD or unix ?
 #endif // _WIN32 ?
 
+#pragma prefast(disable:26444, "This warning unfortunately raises false positives when auto is used for declaring the type of an iterator in a loop.")
 #ifdef HAS_ICU
 #define INTL_LIBRARY_TEXT "icu"
 #elif defined(_WIN32)
@@ -133,7 +134,7 @@ JsValueRef __stdcall WScriptJsrt::EchoCallback(JsValueRef callee, bool isConstru
                 }
                 charcount_t len;
                 LPWSTR ws = str.GetWideString(&len);
-                LPWSTR wsNoNull = new WCHAR[len + 1];
+                LPWSTR wsNoNull = new WCHAR[((size_t)len) + 1];
                 charcount_t newIndex = 0;
                 for (charcount_t j = 0; j < len; j++)
                 {

bin/ch/ch.cpp

@@ -145,7 +145,7 @@ void __stdcall PrintChakraCoreVersion()
                 // Doesn't matter if you are on 32 bit or 64 bit,
                 // DWORD is always 32 bits, so first two revision numbers
                 // come from dwFileVersionMS, last two come from dwFileVersionLS
-                wprintf(_u("%s version %u.%u.%u.%u\n"),
+                wprintf(_u("%s version %lu.%lu.%lu.%lu\n"),
                     chakraDllName,
                     (verInfo->dwFileVersionMS >> 16) & 0xffff,
                     (verInfo->dwFileVersionMS >> 0) & 0xffff,

bin/rl/rl.h

@@ -21,7 +21,22 @@
 #include "xmlreader.h"
 #include "rlfeint.h"
 
+// Note that some of these look pretty bad, and are; this is a test host, so
+// we're not as concerned here.
 #pragma warning(disable:4127) // expression is constant, e.g., while(TRUE)
+#pragma warning(disable:6001) // using uninitialized memory
+#pragma warning(disable:6011) // dereferencing null pointer, potentially
+#pragma warning(disable:6031) // ignoring return value from some system calls
+#pragma warning(disable:6054) // string may not be zero-terminated
+#pragma warning(disable:6271) // Extra parameter not used by format string
+#pragma warning(disable:6262) // Function using too much stack for analyzer to look at it
+#pragma warning(disable:6335) // leaking process information handle
+#pragma warning(disable:6386) // Potential buffer overrun
+#pragma warning(disable:6387) // Potential misadherance to specification of library functions
+#pragma warning(disable:26439) // implicit noexcept
+#pragma warning(disable:26451) // Arithmetic on smaller type before widening conversion
+#pragma warning(disable:26495) // uninitialized member
+#pragma warning(disable:28193) // ignoring value that must be examined
 
 #define LOCAL static
 typedef __int32 int32;

bin/rl/xmlreader.h

@@ -15,6 +15,8 @@
 namespace Xml
 {
 
+#pragma prefast(disable:26439) // implicit noexcept
+#pragma prefast(disable:26495) // uninitialized member variable
 
 // May want Unicode someday.
 

lib/Backend/Lower.cpp

@@ -10518,7 +10518,7 @@ Lowerer::LowerStLoopBodyCount(IR::Instr* instr)
     IR::MemRefOpnd *loopBodyCounterOpnd = IR::MemRefOpnd::New((BYTE*)(header) + Js::LoopHeader::GetOffsetOfProfiledLoopCounter(), TyUint32, this->m_func);
     instr->SetDst(loopBodyCounterOpnd);
     instr->ReplaceSrc1(instr->GetSrc1()->AsRegOpnd()->UseWithNewType(TyUint32, this->m_func));
-    IR::AutoReuseOpnd(loopBodyCounterOpnd, this->m_func);
+    IR::AutoReuseOpnd autoReuse(loopBodyCounterOpnd, this->m_func);
     m_lowererMD.ChangeToAssign(instr);
     return;
 }
@@ -21155,8 +21155,12 @@ Lowerer::GenerateArgOutForStackArgs(IR::Instr* callInstr, IR::Instr* stackArgsIn
 
 
 #if defined(_M_IX86)
-     Assert(false);
-#endif
+    // We get a compilation error on x86 due to assigning a negative to a uint
+    // TODO: don't even define this function on x86 - we Assert(false) anyway there.
+    // Alternatively, don't define when INT_ARG_REG_COUNT - 4 < 0
+    AssertOrFailFast(false);
+    return nullptr;
+#else
 
     Assert(stackArgsInstr->m_opcode == Js::OpCode::ArgOut_A_FromStackArgs);
     Assert(callInstr->m_opcode == Js::OpCode::CallIDynamic);
@@ -21224,14 +21228,7 @@ Lowerer::GenerateArgOutForStackArgs(IR::Instr* callInstr, IR::Instr* stackArgsIn
 
     // 4 to denote this is 4th register after this, callinfo & function object
     // INT_ARG_REG_COUNT is the number of parameters passed in int regs
-    uint current_reg_pass =
-#if defined(_M_IX86)
-        // We get a compilation error on x86 due to assiging a negative to a uint
-        // TODO: don't even define this function on x86 - we Assert(false) anyway there.
-        0;
-#else
-        INT_ARG_REG_COUNT - 4;
-#endif
+    uint current_reg_pass = INT_ARG_REG_COUNT - 4;
 
     do
     {
@@ -21277,6 +21274,7 @@ Lowerer::GenerateArgOutForStackArgs(IR::Instr* callInstr, IR::Instr* stackArgsIn
 
     /*return the length which will be used for callInfo generations & stack allocation*/
     return saveLenInstr->GetDst()->AsRegOpnd();
+#endif
 }
 
 void

lib/Common/Codex/Utf8Codex.cpp

@@ -17,6 +17,7 @@
 #pragma warning(push)
 
 #pragma warning(disable: 4127)  // constant expression for template parameter
+#pragma warning(disable: 26451) // size-conversion/arithmetic-operation ordering
 #endif
 
 namespace utf8