[MERGE #2684 @curtisman] Avoid calling CompileScriptException::ProcessError in the catch handler, where the stack is not unwounded.

Merge pull request #2684 from curtisman:parseerr

If we are hitting a stack overflow, we should unwind the stack before running more code (like ProcessError)
Also clean up ErrHandler, which really does nothing but throw ParseException.  Change it to directly throw ParseException.

Clean up some debug only ifdef and disable background parsing code in release build, which is not enabled by default.

Fixes VSO: 11101148

Author: curtisman

lib/Common/CommonDefines.h

@@ -201,7 +201,6 @@
 #define ENABLE_PROFILE_INFO 1
 
 #define ENABLE_BACKGROUND_JOB_PROCESSOR 1
-#define ENABLE_BACKGROUND_PARSING 1
 #define ENABLE_COPYONACCESS_ARRAY 1
 #ifndef DYNAMIC_INTERPRETER_THUNK
 #if defined(_M_IX86_OR_ARM32) || defined(_M_X64_OR_ARM64)
@@ -210,6 +209,12 @@
 #define DYNAMIC_INTERPRETER_THUNK 0
 #endif
 #endif
+
+// Don't enable background parser in release build.
+#if ENABLE_DEBUG_CONFIG_OPTIONS
+#define ENABLE_BACKGROUND_PARSING 1
+#endif
+
 #endif
 
 #if ENABLE_NATIVE_CODEGEN

lib/Common/Memory/HeapInfo.cpp

@@ -1037,6 +1037,8 @@ HeapInfo::Rescan(RescanFlags flags)
     return scannedPageCount;
 }
 
+#ifdef DUMP_FRAGMENTATION_STATS
+
 template <ObjectInfoBits TBucketType, class TBlockAttributes>
 void DumpBucket(uint bucketIndex, typename SmallHeapBlockType<TBucketType, TBlockAttributes>::BucketType& bucket)
 {
@@ -1048,7 +1050,6 @@ void DumpBucket(uint bucketIndex, typename SmallHeapBlockType<TBucketType, TBloc
     Output::Print(_u("%d,%d,%d,%d,%d,%d,%d\n"), stats.totalBlockCount, stats.finalizeBlockCount, stats.emptyBlockCount, stats.objectCount, stats.finalizeCount, stats.objectByteCount, stats.totalByteCount);
 }
 
-#ifdef DUMP_FRAGMENTATION_STATS
 void
 HeapInfo::DumpFragmentationStats()
 {

lib/Parser/BackgroundParser.cpp

@@ -8,6 +8,7 @@
     "Cannot use this member of BackgroundParser from thread other than the creating context's current thread")
 
 #if ENABLE_NATIVE_CODEGEN
+#if ENABLE_BACKGROUND_PARSING
 BackgroundParser::BackgroundParser(Js::ScriptContext *scriptContext)
     :   JsUtil::WaitableJobManager(scriptContext->GetThreadContext()->GetJobProcessor()),
         scriptContext(scriptContext),
@@ -292,3 +293,4 @@ void BackgroundParseItem::AddRegExpNode(ParseNode *const pnode, ArenaAllocator *
     regExpNodes->Append(pnode);
 }
 #endif
+#endif

lib/Parser/Hash.cpp

@@ -26,11 +26,11 @@ const HashTbl::ReservedWordInfo HashTbl::s_reservedWordInfo[tkID] =
 #include "keywords.h"
 };
 
-HashTbl * HashTbl::Create(uint cidHash, ErrHandler * perr)
+HashTbl * HashTbl::Create(uint cidHash)
 {
     HashTbl * phtbl;
 
-    if (nullptr == (phtbl = HeapNewNoThrow(HashTbl,perr)))
+    if (nullptr == (phtbl = HeapNewNoThrow(HashTbl)))
         return nullptr;
     if (!phtbl->Init(cidHash))
     {
@@ -301,12 +301,12 @@ IdentPtr HashTbl::PidHashNameLenWithHash(_In_reads_(cch) CharType const * prgch,
         FAILED(ULongToLong(Len, &cb)))
     {
         cb = 0;
-        m_perr->Throw(ERRnoMemory);
+        OutOfMemory();
     }
 
 
     if (nullptr == (pid = (IdentPtr)m_noReleaseAllocator.Alloc(cb)))
-        m_perr->Throw(ERRnoMemory);
+        OutOfMemory();
 
     /* Insert the identifier into the hash list */
     *ppid = pid;
@@ -466,3 +466,8 @@ tokens HashTbl::TkFromNameLen(_In_reads_(cch) LPCOLESTR prgch, uint32 cch, bool
 }
 
 #pragma warning(pop)
+
+__declspec(noreturn) void HashTbl::OutOfMemory()
+{
+    throw ParseExceptionObject(ERRnoMemory);
+}

lib/Parser/Hash.h

@@ -324,7 +324,7 @@ struct Ident
 class HashTbl
 {
 public:
-    static HashTbl * Create(uint cidHash, ErrHandler * perr);
+    static HashTbl * Create(uint cidHash);
 
     void Release(void)
     {
@@ -394,14 +394,12 @@ class HashTbl
     Ident ** m_prgpidName;        // hash table for names
 
     uint32 m_luMask;                // hash mask
-    uint32 m_luCount;              // count of the number of entires in the hash table
-    ErrHandler * m_perr;        // error handler to use
+    uint32 m_luCount;              // count of the number of entires in the hash table    
     IdentPtr m_rpid[tkLimKwd];
 
-    HashTbl(ErrHandler * perr)
+    HashTbl()
     {
         m_prgpidName = nullptr;
-        m_perr = perr;
         memset(&m_rpid, 0, sizeof(m_rpid));
     }
     ~HashTbl(void) {}
@@ -481,6 +479,7 @@ class HashTbl
     static const KWD * KwdOfTok(tokens tk)
     { return (unsigned int)tk < tkLimKwd ? g_mptkkwd + tk : nullptr; }
 
+    static __declspec(noreturn) void OutOfMemory();
 #if PROFILE_DICTIONARY
     DictionaryStats *stats;
 #endif

lib/Parser/Parse.cpp

@@ -172,8 +172,7 @@ void Parser::OutOfMemory()
 
 void Parser::Error(HRESULT hr)
 {
-    Assert(FAILED(hr));
-    m_err.Throw(hr);
+    throw ParseExceptionObject(hr);
 }
 
 void Parser::Error(HRESULT hr, ParseNodePtr pnode)
@@ -247,7 +246,6 @@ HRESULT Parser::ValidateSyntax(LPCUTF8 pszSrc, size_t encodedCharCount, bool isG
     HRESULT hr;
     SmartFPUControl smartFpuControl;
 
-    DebugOnly( m_err.fInited = TRUE; )
     BOOL fDeferSave = m_deferringAST;
     try
     {
@@ -321,10 +319,13 @@ HRESULT Parser::ValidateSyntax(LPCUTF8 pszSrc, size_t encodedCharCount, bool isG
     catch(ParseExceptionObject& e)
     {
         m_deferringAST = fDeferSave;
-        m_err.m_hr = e.GetError();
-        hr = pse->ProcessError( m_pscan,  m_err.m_hr, /* pnodeBase */ NULL);
+        hr = e.GetError();
     }
 
+    if (nullptr != pse && FAILED(hr))
+    {
+        hr = pse->ProcessError(m_pscan, hr, /* pnodeBase */ NULL);
+    }
     return hr;
 }
 
@@ -360,8 +361,6 @@ HRESULT Parser::ParseSourceInternal(
     HRESULT hr;
     SmartFPUControl smartFpuControl;
 
-    DebugOnly( m_err.fInited = TRUE; )
-
     try
     {
         this->PrepareScanner(fromExternal);
@@ -405,8 +404,12 @@ HRESULT Parser::ParseSourceInternal(
     }
     catch(ParseExceptionObject& e)
     {
-        m_err.m_hr = e.GetError();
-        hr = pse->ProcessError( m_pscan, m_err.m_hr, pnodeBase);
+        hr = e.GetError();
+    }
+
+    if (FAILED(hr))
+    {
+        hr = pse->ProcessError(m_pscan, hr, pnodeBase);
     }
 
     if (this->m_hasParallelJob)
@@ -11246,12 +11249,12 @@ void Parser::PrepareScanner(bool fromExternal)
     // heap allocation for the colorizer interface.
 
     // create the hash table and init PID members
-    if (nullptr == (m_phtbl = HashTbl::Create(HASH_TABLE_SIZE, &m_err)))
+    if (nullptr == (m_phtbl = HashTbl::Create(HASH_TABLE_SIZE)))
         Error(ERRnoMemory);
     InitPids();
 
     // create the scanner
-    if (nullptr == (m_pscan = Scanner_t::Create(this, m_phtbl, &m_token, &m_err, m_scriptContext)))
+    if (nullptr == (m_pscan = Scanner_t::Create(this, m_phtbl, &m_token, m_scriptContext)))
         Error(ERRnoMemory);
 
     if (fromExternal)
@@ -11298,7 +11301,6 @@ void Parser::AddBackgroundRegExpNode(ParseNodePtr const pnode)
 
     currBackgroundParseItem->AddRegExpNode(pnode, &m_nodeAllocator);
 }
-#endif
 
 HRESULT Parser::ParseFunctionInBackground(ParseNodePtr pnodeFnc, ParseContext *parseContext, bool topLevelDeferred, CompileScriptException *pse)
 {
@@ -11309,7 +11311,6 @@ HRESULT Parser::ParseFunctionInBackground(ParseNodePtr pnodeFnc, ParseContext *p
     uint nextFunctionId = pnodeFnc->sxFnc.functionId + 1;
 
     this->RestoreContext(parseContext);
-    DebugOnly( m_err.fInited = TRUE; )
     m_nextFunctionId = &nextFunctionId;
     m_deferringAST = topLevelDeferred;
     m_inDeferredNestedFunc = false;
@@ -11383,8 +11384,12 @@ HRESULT Parser::ParseFunctionInBackground(ParseNodePtr pnodeFnc, ParseContext *p
     }
     catch(ParseExceptionObject& e)
     {
-        m_err.m_hr = e.GetError();
-        hr = pse->ProcessError( m_pscan, m_err.m_hr, nullptr);
+        hr = e.GetError();
+    }
+
+    if (FAILED(hr))
+    {
+        hr = pse->ProcessError(m_pscan, hr, nullptr);
     }
 
     if (IsStrictMode())
@@ -11404,6 +11409,8 @@ HRESULT Parser::ParseFunctionInBackground(ParseNodePtr pnodeFnc, ParseContext *p
     return hr;
 }
 
+#endif
+
 HRESULT Parser::ParseSourceWithOffset(__out ParseNodePtr* parseTree, LPCUTF8 pSrc, size_t offset, size_t cbLength, charcount_t cchOffset,
         bool isCesu8, ULONG grfscr, CompileScriptException *pse, Js::LocalFunctionId * nextFunctionId, ULONG lineNumber, SourceContextInfo * sourceContextInfo,
         Js::ParseableFunctionInfo* functionInfo)

lib/Parser/Parse.h

@@ -209,7 +209,6 @@ class Parser
 protected:
     Js::ScriptContext* m_scriptContext;
     HashTbl *   m_phtbl;
-    ErrHandler  m_err;
 
     static const uint HASH_TABLE_SIZE = 256;
 
@@ -310,12 +309,14 @@ class Parser
     void PrepareScanner(bool fromExternal);
     void PrepareForBackgroundParse();
     void AddFastScannedRegExpNode(ParseNodePtr const pnode);
+#if ENABLE_BACKGROUND_PARSING
     void AddBackgroundRegExpNode(ParseNodePtr const pnode);
     void AddBackgroundParseItem(BackgroundParseItem *const item);
     void FinishBackgroundRegExpNodes();
     void FinishBackgroundPidRefs(BackgroundParseItem *const item, bool isOtherParser);
     void WaitForBackgroundJobs(BackgroundParser *bgp, CompileScriptException *pse);
     HRESULT ParseFunctionInBackground(ParseNodePtr pnodeFunc, ParseContext *parseContext, bool topLevelDeferred, CompileScriptException *pse);
+#endif
 
     void CheckPidIsValid(IdentPtr pid, bool autoArgumentsObject = false);
     void AddVarDeclToBlock(ParseNode *pnode);

lib/Parser/Scan.cpp

@@ -124,16 +124,14 @@ IdentPtr Token::CreateIdentifier(HashTbl * hashTbl)
 }
 
 template <typename EncodingPolicy>
-Scanner<EncodingPolicy>::Scanner(Parser* parser, HashTbl *phtbl, Token *ptoken, ErrHandler *perr, Js::ScriptContext* scriptContext)
+Scanner<EncodingPolicy>::Scanner(Parser* parser, HashTbl *phtbl, Token *ptoken, Js::ScriptContext* scriptContext)
 {
     AssertMem(phtbl);
     AssertMem(ptoken);
-    AssertMem(perr);
     m_parser = parser;
     m_phtbl = phtbl;
     m_ptoken = ptoken;
     m_cMinLineMultiUnits = 0;
-    m_perr = perr;
     m_fHadEol = FALSE;
 
     m_doubleQuoteOnLastTkStrCon = FALSE;

lib/Parser/Scan.h

@@ -364,9 +364,9 @@ class Scanner : public IScanner, public EncodingPolicy
     typedef typename EncodingPolicy::EncodedCharPtr EncodedCharPtr;
 
 public:
-    static Scanner * Create(Parser* parser, HashTbl *phtbl, Token *ptoken, ErrHandler *perr, Js::ScriptContext *scriptContext)
+    static Scanner * Create(Parser* parser, HashTbl *phtbl, Token *ptoken, Js::ScriptContext *scriptContext)
     {
-        return HeapNewNoThrow(Scanner, parser, phtbl, ptoken, perr, scriptContext);
+        return HeapNewNoThrow(Scanner, parser, phtbl, ptoken, scriptContext);
     }
     void Release(void)
     {
@@ -677,7 +677,6 @@ class Scanner : public IScanner, public EncodingPolicy
     EncodedCharPtr m_pchPrevLine;      // beginning of previous line
     size_t m_cMinTokMultiUnits;        // number of multi-unit characters previous to m_pchMinTok
     size_t m_cMinLineMultiUnits;       // number of multi-unit characters previous to m_pchMinLine
-    ErrHandler *m_perr;                // error handler to use
     uint16 m_fStringTemplateDepth;     // we should treat } as string template middle starting character (depth instead of flag)
     BOOL m_fHadEol;
     BOOL m_fIsModuleCode : 1;
@@ -711,7 +710,7 @@ class Scanner : public IScanner, public EncodingPolicy
     tokens m_tkPrevious;
     size_t m_iecpLimTokPrevious;
 
-    Scanner(Parser* parser, HashTbl *phtbl, Token *ptoken, ErrHandler *perr, Js::ScriptContext *scriptContext);
+    Scanner(Parser* parser, HashTbl *phtbl, Token *ptoken, Js::ScriptContext *scriptContext);
     ~Scanner(void);
 
     template <bool forcePid>
@@ -729,11 +728,9 @@ class Scanner : public IScanner, public EncodingPolicy
 
     __declspec(noreturn) void Error(HRESULT hr)
     {
-        Assert(FAILED(hr));
         m_pchMinTok = m_currentCharacter;
         m_cMinTokMultiUnits = this->m_cMultiUnits;
-        AssertMem(m_perr);
-        m_perr->Throw(hr);
+        throw ParseExceptionObject(hr);
     }
 
     const EncodedCharPtr PchBase(void)

lib/Parser/cmperr.cpp

@@ -4,14 +4,8 @@
 //-------------------------------------------------------------------------------------------------------
 #include "ParserPch.h"
 
-#if DEBUG
-#include <stdarg.h>
-#endif //DEBUG
-
-void ErrHandler::Throw(HRESULT hr)
+ParseExceptionObject::ParseExceptionObject(HRESULT hr) : m_hr(hr)
 {
-    Assert(fInited);
     Assert(FAILED(hr));
-    m_hr = hr;
-    throw ParseExceptionObject(hr);
 }
+

lib/Parser/cmperr.h

@@ -17,28 +17,8 @@ enum
 class ParseExceptionObject
 {
 public:
-    ParseExceptionObject(HRESULT hr) : m_hr(hr) {}
+    ParseExceptionObject(HRESULT hr);
     HRESULT GetError() { return m_hr; }
 private:
     HRESULT m_hr;
 };
-
-typedef void (*ErrorCallback)(void *data, HRESULT hr);
-
-class ErrHandler
-{
-public:
-    HRESULT m_hr;
-
-    void *m_data;
-    ErrorCallback m_callback;
-
-    __declspec(noreturn) void Throw(HRESULT hr);
-
-#if DEBUG
-    BOOL fInited;
-    ErrHandler()
-    { fInited = FALSE; }
-#endif //DEBUG
-};
-

lib/Parser/screrror.cpp

@@ -245,9 +245,6 @@ void CompileScriptException::Free()
 
 HRESULT  CompileScriptException::ProcessError(IScanner * pScan, HRESULT hr, ParseNode * pnodeBase)
 {
-    if (nullptr == this)
-        return hr;
-
     // fill in the ScriptException structure
     Clear();
     ei.scode = GetScode(MapHr(hr));

lib/Parser/screrror.h

@@ -7,7 +7,6 @@
 /***************************************************************************
 Exception blocks
 ***************************************************************************/
-class ErrHandler;
 struct ParseNode;
 class COleScript;
 interface IScanner;