From 1b46fb6747de5c7f0c499b18865401e34c1482f4 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Wed, 5 Oct 2016 16:04:50 -0700 Subject: [PATCH 1/2] Bug 1306121 - Add support for emulating V8 stack frame formatting to SpiderMonkey; r=fitzgen --- js/src/builtin/Error.js | 5 ++ js/src/jsapi-tests/testSavedStacks.cpp | 98 ++++++++++++++++++++++++++ js/src/jsapi.cpp | 12 ++++ js/src/jsapi.h | 22 +++++- js/src/vm/CommonPropertyNames.h | 1 + js/src/vm/ErrorObject.cpp | 20 ++++++ js/src/vm/Runtime.cpp | 4 +- js/src/vm/Runtime.h | 21 ++++++ js/src/vm/SavedStacks.cpp | 89 +++++++++++++++++------ 9 files changed, 250 insertions(+), 22 deletions(-) diff --git a/js/src/builtin/Error.js b/js/src/builtin/Error.js index 8dacf8b8c154..b0fd727eb22e 100644 --- a/js/src/builtin/Error.js +++ b/js/src/builtin/Error.js @@ -29,3 +29,8 @@ function ErrorToString() /* Step 11. */ return name + ": " + msg; } + +function ErrorToStringWithTrailingNewline() +{ + return FUN_APPLY(ErrorToString, this, []) + "\n"; +} diff --git a/js/src/jsapi-tests/testSavedStacks.cpp b/js/src/jsapi-tests/testSavedStacks.cpp index b59824fae90a..c329f27ce0e0 100644 --- a/js/src/jsapi-tests/testSavedStacks.cpp +++ b/js/src/jsapi-tests/testSavedStacks.cpp @@ -108,10 +108,108 @@ BEGIN_TEST(testSavedStacks_RangeBasedForLoops) } CHECK(rf == nullptr); + // Stack string + const char* SpiderMonkeyStack = "three@filename.js:4:14\n" + "two@filename.js:3:22\n" + "one@filename.js:2:20\n" + "@filename.js:1:11\n"; + const char* V8Stack = " at three (filename.js:4:14)\n" + " at two (filename.js:3:22)\n" + " at one (filename.js:2:20)\n" + " at filename.js:1:11"; + struct { + js::StackFormat format; + const char* expected; + } expectations[] = { + {js::StackFormat::Default, SpiderMonkeyStack}, + {js::StackFormat::SpiderMonkey, SpiderMonkeyStack}, + {js::StackFormat::V8, V8Stack} + }; + auto CheckStacks = [&]() { + for (auto& expectation : expectations) { + JS::RootedString str(cx); + CHECK(JS::BuildStackString(cx, savedFrame, &str, 0, expectation.format)); + JSLinearString* lin = str->ensureLinear(cx); + CHECK(lin); + CHECK(js::StringEqualsAscii(lin, expectation.expected)); + } + return true; + }; + + CHECK(CheckStacks()); + + js::SetStackFormat(cx, js::StackFormat::V8); + expectations[0].expected = V8Stack; + + CHECK(CheckStacks()); + return true; } END_TEST(testSavedStacks_RangeBasedForLoops) +BEGIN_TEST(testSavedStacks_ErrorStackSpiderMonkey) +{ + JS::RootedValue val(cx); + CHECK(evaluate("(function one() { \n" // 1 + " return (function two() { \n" // 2 + " return (function three() { \n" // 3 + " return new Error('foo'); \n" // 4 + " }()); \n" // 5 + " }()); \n" // 6 + "}()).stack \n", // 7 + "filename.js", + 1, + &val)); + + CHECK(val.isString()); + JS::RootedString stack(cx, val.toString()); + + // Stack string + const char* SpiderMonkeyStack = "three@filename.js:4:14\n" + "two@filename.js:3:22\n" + "one@filename.js:2:20\n" + "@filename.js:1:11\n"; + JSLinearString* lin = stack->ensureLinear(cx); + CHECK(lin); + CHECK(js::StringEqualsAscii(lin, SpiderMonkeyStack)); + + return true; +} +END_TEST(testSavedStacks_ErrorStackSpiderMonkey) + +BEGIN_TEST(testSavedStacks_ErrorStackV8) +{ + js::SetStackFormat(cx, js::StackFormat::V8); + + JS::RootedValue val(cx); + CHECK(evaluate("(function one() { \n" // 1 + " return (function two() { \n" // 2 + " return (function three() { \n" // 3 + " return new Error('foo'); \n" // 4 + " }()); \n" // 5 + " }()); \n" // 6 + "}()).stack \n", // 7 + "filename.js", + 1, + &val)); + + CHECK(val.isString()); + JS::RootedString stack(cx, val.toString()); + + // Stack string + const char* V8Stack = "Error: foo\n" + " at three (filename.js:4:14)\n" + " at two (filename.js:3:22)\n" + " at one (filename.js:2:20)\n" + " at filename.js:1:11"; + JSLinearString* lin = stack->ensureLinear(cx); + CHECK(lin); + CHECK(js::StringEqualsAscii(lin, V8Stack)); + + return true; +} +END_TEST(testSavedStacks_ErrorStackV8) + BEGIN_TEST(testSavedStacks_selfHostedFrames) { CHECK(js::DefineTestingFunctions(cx, global, false, false)); diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index f3bab09a7a7e..b00ab585520c 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -6773,3 +6773,15 @@ JS::GCThingTraceKind(void* thing) MOZ_ASSERT(thing); return static_cast(thing)->getTraceKind(); } + +JS_PUBLIC_API(void) +js::SetStackFormat(JSContext* cx, js::StackFormat format) +{ + cx->setStackFormat(format); +} + +JS_PUBLIC_API(js::StackFormat) +js::GetStackFormat(JSContext* cx) +{ + return cx->stackFormat(); +} diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 7cb9d7785032..899f639247d5 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -5863,6 +5863,25 @@ JS_DecodeScript(JSContext* cx, const void* data, uint32_t length); extern JS_PUBLIC_API(JSObject*) JS_DecodeInterpretedFunction(JSContext* cx, const void* data, uint32_t length); +namespace js { + +enum class StackFormat { SpiderMonkey, V8, Default }; + +/* + * Sets the format used for stringifying Error stacks. + * + * The default format is StackFormat::SpiderMonkey. Use StackFormat::V8 + * in order to emulate V8's stack formatting. StackFormat::Default can't be + * used here. + */ +extern JS_PUBLIC_API(void) +SetStackFormat(JSContext* cx, StackFormat format); + +extern JS_PUBLIC_API(StackFormat) +GetStackFormat(JSContext* cx); + +} + namespace JS { /* @@ -6274,7 +6293,8 @@ GetSavedFrameParent(JSContext* cx, HandleObject savedFrame, MutableHandleObject * each line. */ extern JS_PUBLIC_API(bool) -BuildStackString(JSContext* cx, HandleObject stack, MutableHandleString stringp, size_t indent = 0); +BuildStackString(JSContext* cx, HandleObject stack, MutableHandleString stringp, + size_t indent = 0, js::StackFormat stackFormat = js::StackFormat::Default); /** * Return true iff the given object is either a SavedFrame object or wrapper diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index 47f880183de6..de5ad1b2aa17 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -95,6 +95,7 @@ macro(enumerable, enumerable, "enumerable") \ macro(enumerate, enumerate, "enumerate") \ macro(era, era, "era") \ + macro(ErrorToStringWithTrailingNewline, ErrorToStringWithTrailingNewline, "ErrorToStringWithTrailingNewline") \ macro(escape, escape, "escape") \ macro(eval, eval, "eval") \ macro(exec, exec, "exec") \ diff --git a/js/src/vm/ErrorObject.cpp b/js/src/vm/ErrorObject.cpp index 125209843747..6c1bfff2639f 100644 --- a/js/src/vm/ErrorObject.cpp +++ b/js/src/vm/ErrorObject.cpp @@ -216,6 +216,26 @@ js::ErrorObject::getStack(JSContext* cx, unsigned argc, Value* vp) RootedString stackString(cx); if (!BuildStackString(cx, savedFrameObj, &stackString)) return false; + + if (cx->stackFormat() == js::StackFormat::V8) { + // When emulating V8 stack frames, we also need to prepend the + // stringified Error to the stack string. + HandlePropertyName name = cx->names().ErrorToStringWithTrailingNewline; + RootedValue val(cx); + if (!GlobalObject::getSelfHostedFunction(cx, cx->global(), name, name, 0, &val)) + return false; + + RootedValue rval(cx); + if (!js::Call(cx, val, args.thisv(), &rval)) + return false; + + if (!rval.isString()) + return false; + + RootedString stringified(cx, rval.toString()); + stackString = ConcatStrings(cx, stringified, stackString); + } + args.rval().setString(stackString); return true; } diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index 6c4505652f65..b7021c227530 100644 --- a/js/src/vm/Runtime.cpp +++ b/js/src/vm/Runtime.cpp @@ -244,7 +244,9 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime) debuggerMallocSizeOf(ReturnZeroSize), lastAnimationTime(0), performanceMonitoring(thisFromCtor()), - ionLazyLinkListSize_(0) + ionLazyLinkListSize_(0), + stackFormat_(parentRuntime ? js::StackFormat::Default + : js::StackFormat::SpiderMonkey) { setGCStoreBufferPtr(&gc.storeBuffer); diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index 734adeb9220d..5defcf6c3424 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -1265,6 +1265,27 @@ struct JSRuntime : public JS::shadow::Runtime, void ionLazyLinkListRemove(js::jit::IonBuilder* builder); void ionLazyLinkListAdd(js::jit::IonBuilder* builder); + + private: + /* The stack format for the current runtime. Only valid on non-child + * runtimes. */ + js::StackFormat stackFormat_; + + public: + js::StackFormat stackFormat() const { + const JSRuntime* rt = this; + while (rt->parentRuntime) { + MOZ_ASSERT(rt->stackFormat_ == js::StackFormat::Default); + rt = rt->parentRuntime; + } + MOZ_ASSERT(rt->stackFormat_ != js::StackFormat::Default); + return rt->stackFormat_; + } + void setStackFormat(js::StackFormat format) { + MOZ_ASSERT(!parentRuntime); + MOZ_ASSERT(format != js::StackFormat::Default); + stackFormat_ = format; + } }; namespace js { diff --git a/js/src/vm/SavedStacks.cpp b/js/src/vm/SavedStacks.cpp index 3cef981d7a73..f15f05aeb3d0 100644 --- a/js/src/vm/SavedStacks.cpp +++ b/js/src/vm/SavedStacks.cpp @@ -891,8 +891,52 @@ GetSavedFrameParent(JSContext* cx, HandleObject savedFrame, MutableHandleObject return SavedFrameResult::Ok; } +static bool +FormatSpiderMonkeyStackFrame(JSContext* cx, js::StringBuffer& sb, + HandleSavedFrame frame, size_t indent, + bool skippedAsync) +{ + RootedString asyncCause(cx, frame->getAsyncCause()); + if (!asyncCause && skippedAsync) + asyncCause.set(cx->names().Async); + + js::RootedAtom name(cx, frame->getFunctionDisplayName()); + return (!indent || sb.appendN(' ', indent)) + && (!asyncCause || (sb.append(asyncCause) && sb.append('*'))) + && (!name || sb.append(name)) + && sb.append('@') + && sb.append(frame->getSource()) + && sb.append(':') + && NumberValueToStringBuffer(cx, NumberValue(frame->getLine()), sb) + && sb.append(':') + && NumberValueToStringBuffer(cx, NumberValue(frame->getColumn()), sb) + && sb.append('\n'); +} + +static bool +FormatV8StackFrame(JSContext* cx, js::StringBuffer& sb, + HandleSavedFrame frame, size_t indent, bool lastFrame) +{ + js::RootedAtom name(cx, frame->getFunctionDisplayName()); + return sb.appendN(' ', indent + 4) + && sb.append('a') + && sb.append('t') + && sb.append(' ') + && (!name || (sb.append(name) && + sb.append(' ') && + sb.append('('))) + && sb.append(frame->getSource()) + && sb.append(':') + && NumberValueToStringBuffer(cx, NumberValue(frame->getLine()), sb) + && sb.append(':') + && NumberValueToStringBuffer(cx, NumberValue(frame->getColumn()), sb) + && (!name || sb.append(')')) + && (lastFrame || sb.append('\n')); +} + JS_PUBLIC_API(bool) -BuildStackString(JSContext* cx, HandleObject stack, MutableHandleString stringp, size_t indent) +BuildStackString(JSContext* cx, HandleObject stack, MutableHandleString stringp, + size_t indent, js::StackFormat format) { AssertHeapIsIdle(cx); CHECK_REQUEST(cx); @@ -900,6 +944,11 @@ BuildStackString(JSContext* cx, HandleObject stack, MutableHandleString stringp, js::StringBuffer sb(cx); + if (format == js::StackFormat::Default) { + format = cx->stackFormat(); + } + MOZ_ASSERT(format != js::StackFormat::Default); + // Enter a new block to constrain the scope of possibly entering the stack's // compartment. This ensures that when we finish the StringBuffer, we are // back in the cx's original compartment, and fulfill our contract with @@ -919,27 +968,27 @@ BuildStackString(JSContext* cx, HandleObject stack, MutableHandleString stringp, MOZ_ASSERT(SavedFrameSubsumedByCaller(cx, frame)); MOZ_ASSERT(!frame->isSelfHosted(cx)); - RootedString asyncCause(cx, frame->getAsyncCause()); - if (!asyncCause && skippedAsync) - asyncCause.set(cx->names().Async); - - js::RootedAtom name(cx, frame->getFunctionDisplayName()); - if ((indent && !sb.appendN(' ', indent)) - || (asyncCause && (!sb.append(asyncCause) || !sb.append('*'))) - || (name && !sb.append(name)) - || !sb.append('@') - || !sb.append(frame->getSource()) - || !sb.append(':') - || !NumberValueToStringBuffer(cx, NumberValue(frame->getLine()), sb) - || !sb.append(':') - || !NumberValueToStringBuffer(cx, NumberValue(frame->getColumn()), sb) - || !sb.append('\n')) - { - return false; + parent = frame->getParent(); + js::RootedSavedFrame nextFrame(cx, js::GetFirstSubsumedFrame(cx, parent, + SavedFrameSelfHosted::Exclude, skippedAsync)); + + switch (format) { + case js::StackFormat::SpiderMonkey: + if (!FormatSpiderMonkeyStackFrame(cx, sb, frame, indent, skippedAsync)) { + return false; + } + break; + case js::StackFormat::V8: + if (!FormatV8StackFrame(cx, sb, frame, indent, !nextFrame)) { + return false; + } + break; + case js::StackFormat::Default: + MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE("Unexpected value"); + break; } - parent = frame->getParent(); - frame = js::GetFirstSubsumedFrame(cx, parent, SavedFrameSelfHosted::Exclude, skippedAsync); + frame = nextFrame; } while (frame); } From d363e13707430722281aa2d479f0b2f98ed4e87d Mon Sep 17 00:00:00 2001 From: Myk Melez Date: Wed, 5 Oct 2016 17:12:52 -0700 Subject: [PATCH 2/2] update positron-spidernode; make submodule track master branch --- .gitmodules | 2 +- positron/configure.in | 2 +- positron/spidernode | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitmodules b/.gitmodules index 0bfbde34a10f..46b3bebac24a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,4 +4,4 @@ [submodule "positron/spidernode"] path = positron/spidernode url = https://github.com/mozilla/positron-spidernode.git - branch = existing-js-runtime + branch = master diff --git a/positron/configure.in b/positron/configure.in index 397cde1d9f30..8368c99a7c59 100644 --- a/positron/configure.in +++ b/positron/configure.in @@ -1,5 +1,5 @@ -configure_cmd="./configure --engine=spidermonkey --enable-static" +configure_cmd="./configure --engine=spidermonkey --enable-static --external-spidermonkey-has-nspr" # Determine whether or not to also generate a configuration for a debug build. # Regardless of the value of this arg, Node's configuration script will always diff --git a/positron/spidernode b/positron/spidernode index 8fca132f762f..d0c8adba373a 160000 --- a/positron/spidernode +++ b/positron/spidernode @@ -1 +1 @@ -Subproject commit 8fca132f762f19ccb7d656cb1c45bf9c9535c3db +Subproject commit d0c8adba373a9db11eaf553fb233d4eb929e853d