Modified: trunk/Source/_javascript_Core/ChangeLog (264159 => 264160)
--- trunk/Source/_javascript_Core/ChangeLog 2020-07-09 08:23:15 UTC (rev 264159)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-07-09 08:23:36 UTC (rev 264160)
@@ -1,3 +1,25 @@
+2020-07-09 Alexey Shvayka <[email protected]>
+
+ ErrorInstance::finishCreation() puts "message" twice, with different attributes
+ https://bugs.webkit.org/show_bug.cgi?id=214089
+
+ Reviewed by Yusuke Suzuki.
+
+ This change refactors appendSourceToError() to return new message, making it almost pure
+ and eliminating extra JSString -> String -> JSString conversion and {get,put}Direct() calls
+ from ErrorInstance::finishCreation(). Also, moves null message check as early as possible.
+
+ Removed putDirect() call didn't pass PropertyAttribute::DontEnum. An implementation detail,
+ that is about to change in https://webkit.org/b/142933, prevented "message" property from
+ beind redefined with PropertyAttribute::None.
+
+ No behavior change. Advances provided microbenchmark by 5%.
+
+ * runtime/ErrorInstance.cpp:
+ (JSC::appendSourceToErrorMessage):
+ (JSC::ErrorInstance::finishCreation):
+ (JSC::appendSourceToError): Deleted.
+
2020-07-08 Yusuke Suzuki <[email protected]>
[JSC] B3 PureCSE should ignore values which are moved to new BasicBlock
Modified: trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp (264159 => 264160)
--- trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp 2020-07-09 08:23:15 UTC (rev 264159)
+++ trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp 2020-07-09 08:23:36 UTC (rev 264160)
@@ -48,7 +48,7 @@
return create(globalObject, vm, structure, messageString, appender, type, useCurrentFrame);
}
-static void appendSourceToError(JSGlobalObject* globalObject, CallFrame* callFrame, ErrorInstance* exception, BytecodeIndex bytecodeIndex)
+static String appendSourceToErrorMessage(CallFrame* callFrame, ErrorInstance* exception, BytecodeIndex bytecodeIndex, const String& message)
{
ErrorInstance::SourceAppender appender = exception->sourceAppender();
exception->clearSourceAppender();
@@ -55,8 +55,8 @@
RuntimeType type = exception->runtimeTypeForCause();
exception->clearRuntimeTypeForCause();
- if (!callFrame->codeBlock()->hasExpressionInfo())
- return;
+ if (!callFrame->codeBlock()->hasExpressionInfo() || message.isNull())
+ return message;
int startOffset = 0;
int endOffset = 0;
@@ -78,35 +78,26 @@
StringView sourceString = codeBlock->source().provider()->source();
if (!expressionStop || expressionStart > static_cast<int>(sourceString.length()))
- return;
+ return message;
- VM& vm = globalObject->vm();
- JSValue jsMessage = exception->getDirect(vm, vm.propertyNames->message);
- if (!jsMessage || !jsMessage.isString())
- return;
-
- String message = asString(jsMessage)->value(globalObject);
if (expressionStart < expressionStop)
- message = appender(message, codeBlock->source().provider()->getRange(expressionStart, expressionStop).toString(), type, ErrorInstance::FoundExactSource);
- else {
- // No range information, so give a few characters of context.
- int dataLength = sourceString.length();
- int start = expressionStart;
- int stop = expressionStart;
- // Get up to 20 characters of context to the left and right of the divot, clamping to the line.
- // Then strip whitespace.
- while (start > 0 && (expressionStart - start < 20) && sourceString[start - 1] != '\n')
- start--;
- while (start < (expressionStart - 1) && isStrWhiteSpace(sourceString[start]))
- start++;
- while (stop < dataLength && (stop - expressionStart < 20) && sourceString[stop] != '\n')
- stop++;
- while (stop > expressionStart && isStrWhiteSpace(sourceString[stop - 1]))
- stop--;
- message = appender(message, codeBlock->source().provider()->getRange(start, stop).toString(), type, ErrorInstance::FoundApproximateSource);
- }
- exception->putDirect(vm, vm.propertyNames->message, jsString(vm, message));
+ return appender(message, codeBlock->source().provider()->getRange(expressionStart, expressionStop).toString(), type, ErrorInstance::FoundExactSource);
+ // No range information, so give a few characters of context.
+ int dataLength = sourceString.length();
+ int start = expressionStart;
+ int stop = expressionStart;
+ // Get up to 20 characters of context to the left and right of the divot, clamping to the line.
+ // Then strip whitespace.
+ while (start > 0 && (expressionStart - start < 20) && sourceString[start - 1] != '\n')
+ start--;
+ while (start < (expressionStart - 1) && isStrWhiteSpace(sourceString[start]))
+ start++;
+ while (stop < dataLength && (stop - expressionStart < 20) && sourceString[stop] != '\n')
+ stop++;
+ while (stop > expressionStart && isStrWhiteSpace(sourceString[stop - 1]))
+ stop--;
+ return appender(message, codeBlock->source().provider()->getRange(start, stop).toString(), type, ErrorInstance::FoundApproximateSource);
}
void ErrorInstance::finishCreation(VM& vm, JSGlobalObject* globalObject, const String& message, SourceAppender appender, RuntimeType type, bool useCurrentFrame)
@@ -117,9 +108,6 @@
m_sourceAppender = appender;
m_runtimeTypeForCause = type;
- if (!message.isNull())
- putDirect(vm, vm.propertyNames->message, jsString(vm, message), static_cast<unsigned>(PropertyAttribute::DontEnum));
-
std::unique_ptr<Vector<StackFrame>> stackTrace = getStackTrace(globalObject, vm, this, useCurrentFrame);
{
auto locker = holdLock(cellLock());
@@ -127,13 +115,17 @@
}
vm.heap.writeBarrier(this);
+ String messageWithSource = message;
if (m_stackTrace && !m_stackTrace->isEmpty() && hasSourceAppender()) {
BytecodeIndex bytecodeIndex;
CallFrame* callFrame;
getBytecodeIndex(vm, vm.topCallFrame, m_stackTrace.get(), callFrame, bytecodeIndex);
if (callFrame && callFrame->codeBlock() && !callFrame->callee().isWasm())
- appendSourceToError(globalObject, callFrame, this, bytecodeIndex);
+ messageWithSource = appendSourceToErrorMessage(callFrame, this, bytecodeIndex, message);
}
+
+ if (!messageWithSource.isNull())
+ putDirect(vm, vm.propertyNames->message, jsString(vm, messageWithSource), static_cast<unsigned>(PropertyAttribute::DontEnum));
}
// Based on ErrorPrototype's errorProtoFuncToString(), but is modified to
