Skip to content

Commit 978f5ff

Browse files
ppenzinppenzin
authored
Merge pull request #6581 from rhuanjl/printException
Use Exception metadata to print error if Error.toString fails
2 parents 51d75d0 + 4bea2b2 commit 978f5ff

8 files changed

+104
-12
lines changed

‎bin/ch/ChakraRtInterface.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#include "stdafx.h"
@@ -122,6 +123,7 @@ bool ChakraRTInterface::LoadChakraDll(ArgInfo* argInfo, HINSTANCE *outLibrary)
122123
m_jsApiHooks.pfJsrtGetArrayBufferStorage = (JsAPIHooks::JsrtGetArrayBufferStoragePtr)GetChakraCoreSymbol(library, "JsGetArrayBufferStorage");
123124
m_jsApiHooks.pfJsrtHasException = (JsAPIHooks::JsrtHasExceptionPtr)GetChakraCoreSymbol(library, "JsHasException");
124125
m_jsApiHooks.pfJsrtSetException = (JsAPIHooks::JsrtSetExceptionPtr)GetChakraCoreSymbol(library, "JsSetException");
126+
m_jsApiHooks.pfJsrtGetAndClearExceptionWithMetadata = (JsAPIHooks::JsrtGetAndClearExceptiopnWithMetadataPtr)GetChakraCoreSymbol(library, "JsGetAndClearExceptionWithMetadata");
125127
m_jsApiHooks.pfJsrtGetAndClearException = (JsAPIHooks::JsrtGetAndClearExceptiopnPtr)GetChakraCoreSymbol(library, "JsGetAndClearException");
126128
m_jsApiHooks.pfJsrtCreateError = (JsAPIHooks::JsrtCreateErrorPtr)GetChakraCoreSymbol(library, "JsCreateError");
127129
m_jsApiHooks.pfJsrtGetRuntime = (JsAPIHooks::JsrtGetRuntimePtr)GetChakraCoreSymbol(library, "JsGetRuntime");

‎bin/ch/ChakraRtInterface.h

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#pragma once
@@ -58,6 +59,7 @@ struct JsAPIHooks
5859
typedef JsErrorCode (WINAPI *JsrtCreateErrorPtr)(JsValueRef message, JsValueRef *error);
5960
typedef JsErrorCode (WINAPI *JsrtHasExceptionPtr)(bool *hasException);
6061
typedef JsErrorCode (WINAPI *JsrtSetExceptionPtr)(JsValueRef exception);
62+
typedef JsErrorCode (WINAPI *JsrtGetAndClearExceptiopnWithMetadataPtr)(JsValueRef* metadata);
6163
typedef JsErrorCode (WINAPI *JsrtGetAndClearExceptiopnPtr)(JsValueRef* exception);
6264
typedef JsErrorCode (WINAPI *JsrtGetRuntimePtr)(JsContextRef context, JsRuntimeHandle *runtime);
6365
typedef JsErrorCode (WINAPI *JsrtReleasePtr)(JsRef ref, unsigned int* count);
@@ -197,6 +199,7 @@ struct JsAPIHooks
197199
JsrtCreateErrorPtr pfJsrtCreateError;
198200
JsrtHasExceptionPtr pfJsrtHasException;
199201
JsrtSetExceptionPtr pfJsrtSetException;
202+
JsrtGetAndClearExceptiopnWithMetadataPtr pfJsrtGetAndClearExceptionWithMetadata;
200203
JsrtGetAndClearExceptiopnPtr pfJsrtGetAndClearException;
201204
JsrtGetRuntimePtr pfJsrtGetRuntime;
202205
JsrtReleasePtr pfJsrtRelease;
@@ -428,6 +431,7 @@ class ChakraRTInterface
428431
static JsErrorCode WINAPI JsHasException(bool *hasException) { return HOOK_JS_API(HasException(hasException)); }
429432
static JsErrorCode WINAPI JsSetException(JsValueRef exception) { return HOOK_JS_API(SetException(exception)); }
430433
static JsErrorCode WINAPI JsGetAndClearException(JsValueRef *exception) { return HOOK_JS_API(GetAndClearException(exception)); }
434+
static JsErrorCode WINAPI JsGetAndClearExceptionWithMetadata(JsValueRef * metadata) { return HOOK_JS_API(GetAndClearExceptionWithMetadata(metadata)); }
431435
static JsErrorCode WINAPI JsGetRuntime(JsContextRef context, JsRuntimeHandle *runtime) { return HOOK_JS_API(GetRuntime(context, runtime)); }
432436
static JsErrorCode WINAPI JsRelease(JsRef ref, unsigned int* count) { return HOOK_JS_API(Release(ref, count)); }
433437
static JsErrorCode WINAPI JsAddRef(JsRef ref, unsigned int* count) { return HOOK_JS_API(AddRef(ref, count)); }

‎bin/ch/WScriptJsrt.cpp

+63-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft Corporation and contributors. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#include "stdafx.h"
@@ -1766,10 +1767,21 @@ JsValueRef __stdcall WScriptJsrt::GetProxyPropertiesCallback(JsValueRef callee,
17661767
bool WScriptJsrt::PrintException(LPCSTR fileName, JsErrorCode jsErrorCode, JsValueRef exception)
17671768
{
17681769
LPCWSTR errorTypeString = ConvertErrorCodeToMessage(jsErrorCode);
1770+
JsValueRef metaData = JS_INVALID_REFERENCE;
17691771

17701772
if (exception == nullptr)
17711773
{
1772-
ChakraRTInterface::JsGetAndClearException(&exception);
1774+
if (ChakraRTInterface::JsGetAndClearExceptionWithMetadata(&metaData) == JsNoError)
1775+
{
1776+
JsPropertyIdRef exceptionId = JS_INVALID_REFERENCE;
1777+
IfJsrtErrorFail(CreatePropertyIdFromString("exception", &exceptionId), false);
1778+
IfJsrtErrorFail(ChakraRTInterface::JsGetProperty(metaData, exceptionId, &exception), false);
1779+
}
1780+
else
1781+
{
1782+
IfJsrtErrorFail(ChakraRTInterface::JsGetAndClearException(&exception), false);
1783+
}
1784+
17731785
}
17741786

17751787
if (HostConfigFlags::flags.MuteHostErrorMsgIsEnabled)
@@ -1783,7 +1795,56 @@ bool WScriptJsrt::PrintException(LPCSTR fileName, JsErrorCode jsErrorCode, JsVal
17831795
{
17841796
AutoString errorMessage;
17851797

1786-
IfJsrtErrorFail(errorMessage.Initialize(exception), false);
1798+
if (errorMessage.Initialize(exception) != JsNoError)
1799+
{
1800+
fwprintf(stderr, _u("ERROR attempting to coerce error to string, using alternate handler\n"));
1801+
bool hasException = false;
1802+
ChakraRTInterface::JsHasException(&hasException);
1803+
if (hasException)
1804+
{
1805+
JsValueRef throwAway = JS_INVALID_REFERENCE;
1806+
ChakraRTInterface::JsGetAndClearException(&throwAway);
1807+
}
1808+
JsPropertyIdRef messagePropertyId = JS_INVALID_REFERENCE;
1809+
IfJsrtErrorFail(CreatePropertyIdFromString("message", &messagePropertyId), false);
1810+
JsValueRef message = JS_INVALID_REFERENCE;
1811+
IfJsrtErrorFail(ChakraRTInterface::JsGetProperty(exception, messagePropertyId, &message), false);
1812+
IfJsrtErrorFail(errorMessage.Initialize(message), false);
1813+
1814+
if (jsErrorCode != JsErrorCode::JsErrorScriptCompile)
1815+
{
1816+
CHAR shortFileName[_MAX_PATH];
1817+
CHAR ext[_MAX_EXT];
1818+
_splitpath_s(fileName, nullptr, 0, nullptr, 0, shortFileName, _countof(shortFileName), ext, _countof(ext));
1819+
1820+
if (metaData != JS_INVALID_REFERENCE)
1821+
{
1822+
JsPropertyIdRef linePropertyId = JS_INVALID_REFERENCE;
1823+
JsValueRef lineProperty = JS_INVALID_REFERENCE;
1824+
1825+
JsPropertyIdRef columnPropertyId = JS_INVALID_REFERENCE;
1826+
JsValueRef columnProperty = JS_INVALID_REFERENCE;
1827+
1828+
int line;
1829+
int column;
1830+
1831+
IfJsrtErrorFail(CreatePropertyIdFromString("line", &linePropertyId), false);
1832+
IfJsrtErrorFail(ChakraRTInterface::JsGetProperty(metaData, linePropertyId, &lineProperty), false);
1833+
IfJsrtErrorFail(ChakraRTInterface::JsNumberToInt(lineProperty, &line), false);
1834+
1835+
IfJsrtErrorFail(CreatePropertyIdFromString("column", &columnPropertyId), false);
1836+
IfJsrtErrorFail(ChakraRTInterface::JsGetProperty(metaData, columnPropertyId, &columnProperty), false);
1837+
IfJsrtErrorFail(ChakraRTInterface::JsNumberToInt(columnProperty, &column), false);
1838+
fwprintf(stderr, _u("%ls\n at code (%S%S:%d:%d)\n"),
1839+
errorMessage.GetWideString(), shortFileName, ext, line + 1, column + 1);
1840+
}
1841+
else
1842+
{
1843+
fwprintf(stderr, _u("%ls\n\tat code (%S%S:\?\?:\?\?)\n"), errorMessage.GetWideString(), shortFileName, ext);
1844+
}
1845+
return true;
1846+
}
1847+
}
17871848

17881849
if (jsErrorCode == JsErrorCode::JsErrorScriptCompile)
17891850
{

‎bin/ch/ch.cpp

+11-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#include "stdafx.h"
@@ -398,6 +399,10 @@ HRESULT RunScript(const char* fileName, LPCSTR fileContents, size_t fileLength,
398399
IfJsErrorFailLogLabel(ChakraRTInterface::JsCreateString(fullPath,
399400
strlen(fullPath), &fname), ErrorRunFinalize);
400401

402+
// memory management for serialized script case - need to define these here
403+
SerializedCallbackInfo serializedCallbackInfo;
404+
serializedCallbackInfo.freeingHandled = true;
405+
401406
if (bufferValue != nullptr)
402407
{
403408
if (fileContents == nullptr)
@@ -414,7 +419,6 @@ HRESULT RunScript(const char* fileName, LPCSTR fileContents, size_t fileLength,
414419
else // fileContents != nullptr
415420
{
416421
// Memory management is a little more complex here
417-
SerializedCallbackInfo serializedCallbackInfo;
418422
serializedCallbackInfo.scriptBody = (void*)fileContents;
419423
serializedCallbackInfo.scriptBodyFinalizeCallback = fileContentsFinalizeCallback;
420424
serializedCallbackInfo.freeingHandled = false;
@@ -427,15 +431,6 @@ HRESULT RunScript(const char* fileName, LPCSTR fileContents, size_t fileLength,
427431
// Use source ptr as sourceContext
428432
fname,
429433
nullptr /*result*/);
430-
// Now that we're down here, we can free the fileContents if they weren't sent into
431-
// a GC-managed object.
432-
if (!serializedCallbackInfo.freeingHandled)
433-
{
434-
if (fileContentsFinalizeCallback != nullptr)
435-
{
436-
fileContentsFinalizeCallback((void*)fileContents);
437-
}
438-
}
439434
}
440435
}
441436
else if (parserStateCache != nullptr)
@@ -520,6 +515,12 @@ HRESULT RunScript(const char* fileName, LPCSTR fileContents, size_t fileLength,
520515
IfFailGo(messageQueue->ProcessAll(fileName));
521516
} while(!messageQueue->IsEmpty());
522517
}
518+
519+
// free the source for the serialized script case if it's not been handed to a managed object
520+
if (!serializedCallbackInfo.freeingHandled && fileContentsFinalizeCallback != nullptr)
521+
{
522+
fileContentsFinalizeCallback((void*)fileContents);
523+
}
523524
}
524525

525526
if(false)

‎bin/ch/stdafx.h

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#pragma once
@@ -197,6 +198,7 @@ class AutoString
197198

198199
JsErrorCode Initialize(JsValueRef value)
199200
{
201+
errorCode = JsNoError;
200202
JsValueRef strValue;
201203
JsValueType type;
202204
ChakraRTInterface::JsGetValueType(value, &type);

‎test/Error/exceptionInErrorToString.baseline

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
CALLED toString
2+
ERROR attempting to coerce error to string, using alternate handler
3+
'a' is not defined
4+
at code (exceptionInErrorToString.js:12:1)

‎test/Error/exceptionInErrorToString.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
4+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
5+
//-------------------------------------------------------------------------------------------------------
6+
7+
8+
// Print sensible output for crashing when Error.toString is overwritten
9+
// See https://github.com/chakra-core/ChakraCore/issues/6567 for more detail
10+
11+
ReferenceError.prototype.toString = function ( ) { print("CALLED toString"); return { toString () { return "hi"}};}
12+
a.b();

‎test/Error/rlexe.xml

+6
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,10 @@
179179
<files>errorPrototypetoString.js</files>
180180
</default>
181181
</test>
182+
<test>
183+
<default>
184+
<files>exceptionInErrorToString.js</files>
185+
<baseline>exceptionInErrorToString.baseline</baseline>
186+
</default>
187+
</test>
182188
</regress-exe>

0 commit comments

Comments
 (0)