From 169174824f57fcf4cef4db68abc21d736599a48a Mon Sep 17 00:00:00 2001 From: Yves Date: Sun, 3 Aug 2014 17:29:49 +0000 Subject: [PATCH] Exact stack rooting for ScriptInterface::ToString. I had to change a few other functions to take JS::MutableHandleValue because JS::Stringify takes a JS::MutableHandleValue as input parameter. That seems a bit strange because it should not change that value. I assume it has historical reasons. Refs #2415 Refs #2462 This was SVN commit r15605. --- source/network/NetClient.cpp | 9 ++++++--- source/network/NetMessageSim.cpp | 18 ++++++++++++------ source/ps/CConsole.cpp | 2 +- source/scriptinterface/ScriptInterface.cpp | 7 +++---- source/scriptinterface/ScriptInterface.h | 3 ++- .../tests/test_ScriptInterface.h | 2 +- .../simulation2/components/CCmpAIManager.cpp | 8 ++++---- .../components/CCmpCommandQueue.cpp | 2 +- .../simulation2/scripting/ScriptComponent.cpp | 4 ++-- .../serialization/BinarySerializer.h | 2 +- .../serialization/DebugSerializer.cpp | 2 +- .../serialization/DebugSerializer.h | 2 +- .../simulation2/serialization/ISerializer.cpp | 2 +- source/simulation2/serialization/ISerializer.h | 8 +++++--- .../tests/test_CmpTemplateManager.h | 8 ++++---- source/simulation2/tests/test_Serializer.h | 10 +++++----- 16 files changed, 50 insertions(+), 39 deletions(-) diff --git a/source/network/NetClient.cpp b/source/network/NetClient.cpp index 59984317e8..71b32a7f1a 100644 --- a/source/network/NetClient.cpp +++ b/source/network/NetClient.cpp @@ -185,13 +185,16 @@ void CNetClient::PushGuiMessage(const CScriptValRooted& message) std::wstring CNetClient::TestReadGuiMessages() { + JSContext* cx = GetScriptInterface().GetContext(); + JSAutoRequest rq(cx); + std::wstring r; while (true) { - CScriptValRooted msg = GuiPoll(); - if (msg.undefined()) + JS::RootedValue msg(cx, GuiPoll().get()); + if (msg.isUndefined()) break; - r += GetScriptInterface().ToString(msg.get()) + L"\n"; + r += GetScriptInterface().ToString(&msg) + L"\n"; } return r; } diff --git a/source/network/NetMessageSim.cpp b/source/network/NetMessageSim.cpp index c21fdfffa6..77ccc148ba 100644 --- a/source/network/NetMessageSim.cpp +++ b/source/network/NetMessageSim.cpp @@ -133,7 +133,7 @@ u8* CSimulationMessage::Serialize(u8* pBuffer) const serializer.NumberU32_Unbounded("client", m_Client); serializer.NumberI32_Unbounded("player", m_Player); serializer.NumberU32_Unbounded("turn", m_Turn); - serializer.ScriptVal("command", tmpData); + serializer.ScriptVal("command", &tmpData); return serializer.GetBuffer(); } @@ -168,13 +168,16 @@ size_t CSimulationMessage::GetSerializedLength() const serializer.NumberU32_Unbounded("client", m_Client); serializer.NumberI32_Unbounded("player", m_Player); serializer.NumberU32_Unbounded("turn", m_Turn); - serializer.ScriptVal("command", tmpData); + serializer.ScriptVal("command", &tmpData); return CNetMessage::GetSerializedLength() + serializer.GetLength(); } CStr CSimulationMessage::ToString() const { - std::string source = utf8_from_wstring(m_ScriptInterface->ToString(m_Data.get())); + JSContext* cx = m_ScriptInterface->GetContext(); + JSAutoRequest rq(cx); + JS::RootedValue tmpData(cx, m_Data.get()); // TODO: Check if this temporary root can be removed after SpiderMonkey 31 upgrade + std::string source = utf8_from_wstring(m_ScriptInterface->ToString(&tmpData)); std::stringstream stream; stream << "CSimulationMessage { m_Client: " << m_Client << ", m_Player: " << m_Player << ", m_Turn: " << m_Turn << ", m_Data: " << source << " }"; @@ -202,7 +205,7 @@ u8* CGameSetupMessage::Serialize(u8* pBuffer) const u8* pos = CNetMessage::Serialize(pBuffer); CBufferBinarySerializer serializer(m_ScriptInterface, pos); - serializer.ScriptVal("command", tmpData); + serializer.ScriptVal("command", &tmpData); return serializer.GetBuffer(); } @@ -228,13 +231,16 @@ size_t CGameSetupMessage::GetSerializedLength() const JS::RootedValue tmpData(cx, m_Data.get()); // TODO: Check if this temporary root can be removed after SpiderMonkey 31 upgrade CLengthBinarySerializer serializer(m_ScriptInterface); - serializer.ScriptVal("command", tmpData); + serializer.ScriptVal("command", &tmpData); return CNetMessage::GetSerializedLength() + serializer.GetLength(); } CStr CGameSetupMessage::ToString() const { - std::string source = utf8_from_wstring(m_ScriptInterface.ToString(m_Data.get())); + JSContext* cx = m_ScriptInterface.GetContext(); + JSAutoRequest rq(cx); + JS::RootedValue tmpData(cx, m_Data.get()); // TODO: Check if this temporary root can be removed after SpiderMonkey 31 upgrade + std::string source = utf8_from_wstring(m_ScriptInterface.ToString(&tmpData)); std::stringstream stream; stream << "CGameSetupMessage { m_Data: " << source << " }"; diff --git a/source/ps/CConsole.cpp b/source/ps/CConsole.cpp index 7721681e7c..0fa0a6b3b4 100644 --- a/source/ps/CConsole.cpp +++ b/source/ps/CConsole.cpp @@ -611,7 +611,7 @@ void CConsole::ProcessBuffer(const wchar_t* szLine) JS::RootedValue rval(cx); pScriptInterface->Eval(szLine, &rval); if (!rval.isUndefined()) - InsertMessageRaw(pScriptInterface->ToString(rval)); + InsertMessageRaw(pScriptInterface->ToString(&rval)); } void CConsole::LoadHistory() diff --git a/source/scriptinterface/ScriptInterface.cpp b/source/scriptinterface/ScriptInterface.cpp index f9464c6cb2..3cc3cf4a0c 100644 --- a/source/scriptinterface/ScriptInterface.cpp +++ b/source/scriptinterface/ScriptInterface.cpp @@ -1319,7 +1319,7 @@ std::string ScriptInterface::StringifyJSON(JS::MutableHandleValue obj, bool inde } -std::wstring ScriptInterface::ToString(jsval obj, bool pretty) +std::wstring ScriptInterface::ToString(JS::MutableHandleValue obj, bool pretty) { JSAutoRequest rq(m->m_cx); @@ -1335,7 +1335,7 @@ std::wstring ScriptInterface::ToString(jsval obj, bool pretty) // Temporary disable the error reporter, so we don't print complaints about cyclic values JSErrorReporter er = JS_SetErrorReporter(m->m_cx, NULL); - JSBool ok = JS_Stringify(m->m_cx, &obj, NULL, JS::NumberValue(2), &StringifierW::callback, &str); + JSBool ok = JS_Stringify(m->m_cx, obj.address(), NULL, JS::NumberValue(2), &StringifierW::callback, &str); // Restore error reporter JS_SetErrorReporter(m->m_cx, er); @@ -1351,8 +1351,7 @@ std::wstring ScriptInterface::ToString(jsval obj, bool pretty) // so fall back to obj.toSource() std::wstring source = L"(error)"; - JS::RootedValue tmpObj(m->m_cx, obj); // TODO: pass Handle as argument already - CallFunction(tmpObj, "toSource", source); + CallFunction(obj, "toSource", source); return source; } diff --git a/source/scriptinterface/ScriptInterface.h b/source/scriptinterface/ScriptInterface.h index 26f5059004..eeb8223740 100644 --- a/source/scriptinterface/ScriptInterface.h +++ b/source/scriptinterface/ScriptInterface.h @@ -265,7 +265,8 @@ public: template bool Eval(const CHAR* code, JS::MutableHandleValue out); template bool Eval(const CHAR* code, T& out); - std::wstring ToString(jsval obj, bool pretty = false); + // We have to use a mutable handle because JS_Stringify requires that for unknown reasons. + std::wstring ToString(JS::MutableHandleValue obj, bool pretty = false); /** * Parse a UTF-8-encoded JSON string. Returns the unmodified value on error and prints an error message. diff --git a/source/scriptinterface/tests/test_ScriptInterface.h b/source/scriptinterface/tests/test_ScriptInterface.h index 2ea90b4d52..c0eb420517 100644 --- a/source/scriptinterface/tests/test_ScriptInterface.h +++ b/source/scriptinterface/tests/test_ScriptInterface.h @@ -252,6 +252,6 @@ public: TS_ASSERT_STR_EQUALS(stringified, "{\n \"x\": 1,\n \"z\": [\n 2,\n \"3\xE2\x98\xBA\xEF\xBF\xBD\"\n ],\n \"y\": true\n}"); script.ParseJSON(stringified, &val); - TS_ASSERT_WSTR_EQUALS(script.ToString(val.get()), L"({x:1, z:[2, \"3\\u263A\\uFFFD\"], y:true})"); + TS_ASSERT_WSTR_EQUALS(script.ToString(&val), L"({x:1, z:[2, \"3\\u263A\\uFFFD\"], y:true})"); } }; diff --git a/source/simulation2/components/CCmpAIManager.cpp b/source/simulation2/components/CCmpAIManager.cpp index 178dad1d18..115ac0e893 100644 --- a/source/simulation2/components/CCmpAIManager.cpp +++ b/source/simulation2/components/CCmpAIManager.cpp @@ -608,7 +608,7 @@ public: JS::RootedValue tmpSharedAIObj(cx, m_SharedAIObj.get()); // TODO: Check if this temporary root can be removed after SpiderMonkey 31 upgrade if (!m_ScriptInterface->CallFunction(tmpSharedAIObj, "Serialize", &sharedData)) LOGERROR(L"AI shared script Serialize call failed"); - serializer.ScriptVal("sharedData", sharedData); + serializer.ScriptVal("sharedData", &sharedData); } for (size_t i = 0; i < m_Players.size(); ++i) { @@ -621,7 +621,7 @@ public: { JS::RootedValue val(cx); m_ScriptInterface->ReadStructuredClone(m_Players[i]->m_Commands[j], &val); - serializer.ScriptVal("command", val); + serializer.ScriptVal("command", &val); } JS::RootedValue tmpPlayerObj(cx, m_Players[i]->m_Obj.get()); // TODO: Check if this temporary root can be removed after SpiderMonkey 31 upgrade @@ -631,11 +631,11 @@ public: JS::RootedValue scriptData(cx); if (!m_ScriptInterface->CallFunction(tmpPlayerObj, "Serialize", &scriptData)) LOGERROR(L"AI script Serialize call failed"); - serializer.ScriptVal("data", scriptData); + serializer.ScriptVal("data", &scriptData); } else { - serializer.ScriptVal("data", tmpPlayerObj); + serializer.ScriptVal("data", &tmpPlayerObj); } } } diff --git a/source/simulation2/components/CCmpCommandQueue.cpp b/source/simulation2/components/CCmpCommandQueue.cpp index 8f8ecee2fb..3b3b0c62e9 100644 --- a/source/simulation2/components/CCmpCommandQueue.cpp +++ b/source/simulation2/components/CCmpCommandQueue.cpp @@ -59,7 +59,7 @@ public: { tmpRoot.set(m_LocalQueue[i].data.get()); serialize.NumberI32_Unbounded("player", m_LocalQueue[i].player); - serialize.ScriptVal("data", tmpRoot); + serialize.ScriptVal("data", &tmpRoot); } } diff --git a/source/simulation2/scripting/ScriptComponent.cpp b/source/simulation2/scripting/ScriptComponent.cpp index 5562ab4013..f3ada5bd4d 100644 --- a/source/simulation2/scripting/ScriptComponent.cpp +++ b/source/simulation2/scripting/ScriptComponent.cpp @@ -93,11 +93,11 @@ void CComponentTypeScript::Serialize(ISerializer& serialize) JS::RootedValue val(cx); if (!m_ScriptInterface.CallFunction(tmpInstance, "Serialize", &val)) LOGERROR(L"Script Serialize call failed"); - serialize.ScriptVal("object", val); + serialize.ScriptVal("object", &val); } else { - serialize.ScriptVal("object", tmpInstance); + serialize.ScriptVal("object", &tmpInstance); } } diff --git a/source/simulation2/serialization/BinarySerializer.h b/source/simulation2/serialization/BinarySerializer.h index 9cf2ab0f6c..70724af71c 100644 --- a/source/simulation2/serialization/BinarySerializer.h +++ b/source/simulation2/serialization/BinarySerializer.h @@ -185,7 +185,7 @@ protected: m_Impl.Put(name, (u8*)value.data(), value.length()); } - virtual void PutScriptVal(const char* UNUSED(name), JS::HandleValue value) + virtual void PutScriptVal(const char* UNUSED(name), JS::MutableHandleValue value) { m_ScriptImpl->HandleScriptVal(value); } diff --git a/source/simulation2/serialization/DebugSerializer.cpp b/source/simulation2/serialization/DebugSerializer.cpp index 53fa058a5d..07cdd76c2f 100644 --- a/source/simulation2/serialization/DebugSerializer.cpp +++ b/source/simulation2/serialization/DebugSerializer.cpp @@ -147,7 +147,7 @@ void CDebugSerializer::PutString(const char* name, const std::string& value) m_Stream << INDENT << name << ": " << "\"" << escaped << "\"\n"; } -void CDebugSerializer::PutScriptVal(const char* name, JS::HandleValue value) +void CDebugSerializer::PutScriptVal(const char* name, JS::MutableHandleValue value) { std::wstring source = m_ScriptInterface.ToString(value, true); diff --git a/source/simulation2/serialization/DebugSerializer.h b/source/simulation2/serialization/DebugSerializer.h index 2f2638a061..6783f2529f 100644 --- a/source/simulation2/serialization/DebugSerializer.h +++ b/source/simulation2/serialization/DebugSerializer.h @@ -54,7 +54,7 @@ protected: virtual void PutNumber(const char* name, fixed value); virtual void PutBool(const char* name, bool value); virtual void PutString(const char* name, const std::string& value); - virtual void PutScriptVal(const char* name, JS::HandleValue value); + virtual void PutScriptVal(const char* name, JS::MutableHandleValue value); virtual void PutRaw(const char* name, const u8* data, size_t len); private: diff --git a/source/simulation2/serialization/ISerializer.cpp b/source/simulation2/serialization/ISerializer.cpp index 24140cad75..92b9124e88 100644 --- a/source/simulation2/serialization/ISerializer.cpp +++ b/source/simulation2/serialization/ISerializer.cpp @@ -92,7 +92,7 @@ void ISerializer::String(const char* name, const std::wstring& value, uint32_t m PutString(name, str); } -void ISerializer::ScriptVal(const char* name, JS::HandleValue value) +void ISerializer::ScriptVal(const char* name, JS::MutableHandleValue value) { PutScriptVal(name, value); } diff --git a/source/simulation2/serialization/ISerializer.h b/source/simulation2/serialization/ISerializer.h index 81d11f151a..4ab47bb0b7 100644 --- a/source/simulation2/serialization/ISerializer.h +++ b/source/simulation2/serialization/ISerializer.h @@ -216,10 +216,11 @@ public: void String(const char* name, const std::wstring& value, uint32_t minlength, uint32_t maxlength); /** - * Serialize a jsval. + * Serialize a JS::MutableHandleValue. * The value must not contain any unserializable values (like functions). + * NOTE: We have to use a mutable handle because JS_Stringify requires that for unknown reasons. */ - void ScriptVal(const char* name, JS::HandleValue value); + void ScriptVal(const char* name, JS::MutableHandleValue value); /** * Serialize a stream of bytes. @@ -255,7 +256,8 @@ protected: virtual void PutNumber(const char* name, fixed value) = 0; virtual void PutBool(const char* name, bool value) = 0; virtual void PutString(const char* name, const std::string& value) = 0; - virtual void PutScriptVal(const char* name, JS::HandleValue value) = 0; + // We have to use a mutable handle because JS_Stringify requires that for unknown reasons. + virtual void PutScriptVal(const char* name, JS::MutableHandleValue value) = 0; virtual void PutRaw(const char* name, const u8* data, size_t len) = 0; }; diff --git a/source/simulation2/tests/test_CmpTemplateManager.h b/source/simulation2/tests/test_CmpTemplateManager.h index 2cdffe6941..4db94f5d18 100644 --- a/source/simulation2/tests/test_CmpTemplateManager.h +++ b/source/simulation2/tests/test_CmpTemplateManager.h @@ -135,19 +135,19 @@ public: const CParamNode* inherit1 = tempMan->LoadTemplate(ent2, "inherit1", -1); JS::RootedValue val(cx); ScriptInterface::ToJSVal(cx, &val, inherit1); - TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({Test1A:{'@a':\"a1\", '@b':\"b1\", '@c':\"c1\", d:\"d1\", e:\"e1\", f:\"f1\"}})"); + TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(&val), L"({Test1A:{'@a':\"a1\", '@b':\"b1\", '@c':\"c1\", d:\"d1\", e:\"e1\", f:\"f1\"}})"); const CParamNode* inherit2 = tempMan->LoadTemplate(ent2, "inherit2", -1); ScriptInterface::ToJSVal(cx, &val, inherit2); - TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({'@parent':\"inherit1\", Test1A:{'@a':\"a2\", '@b':\"b1\", '@c':\"c1\", d:\"d2\", e:\"e1\", f:\"f1\", g:\"g2\"}})"); + TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(&val), L"({'@parent':\"inherit1\", Test1A:{'@a':\"a2\", '@b':\"b1\", '@c':\"c1\", d:\"d2\", e:\"e1\", f:\"f1\", g:\"g2\"}})"); const CParamNode* actor = tempMan->LoadTemplate(ent2, "actor|example1", -1); ScriptInterface::ToJSVal(cx, &val, &actor->GetChild("VisualActor")); - TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({Actor:\"example1\", ActorOnly:(void 0), SilhouetteDisplay:\"false\", SilhouetteOccluder:\"false\", VisibleInAtlasOnly:\"false\"})"); + TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(&val), L"({Actor:\"example1\", ActorOnly:(void 0), SilhouetteDisplay:\"false\", SilhouetteOccluder:\"false\", VisibleInAtlasOnly:\"false\"})"); const CParamNode* foundation = tempMan->LoadTemplate(ent2, "foundation|actor|example1", -1); ScriptInterface::ToJSVal(cx, &val, &foundation->GetChild("VisualActor")); - TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({Actor:\"example1\", ActorOnly:(void 0), Foundation:(void 0), SilhouetteDisplay:\"false\", SilhouetteOccluder:\"false\", VisibleInAtlasOnly:\"false\"})"); + TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(&val), L"({Actor:\"example1\", ActorOnly:(void 0), Foundation:(void 0), SilhouetteDisplay:\"false\", SilhouetteOccluder:\"false\", VisibleInAtlasOnly:\"false\"})"); } void test_LoadTemplate_errors() diff --git a/source/simulation2/tests/test_Serializer.h b/source/simulation2/tests/test_Serializer.h index e8d8a724a3..6885916587 100644 --- a/source/simulation2/tests/test_Serializer.h +++ b/source/simulation2/tests/test_Serializer.h @@ -276,7 +276,7 @@ public: { std::stringstream stream; CDebugSerializer serialize(script, stream); - serialize.ScriptVal("script", obj); + serialize.ScriptVal("script", &obj); TS_ASSERT_STR_EQUALS(stream.str(), "script: {\n" " \"x\": 123,\n" @@ -297,7 +297,7 @@ public: std::stringstream stream; CStdSerializer serialize(script, stream); - serialize.ScriptVal("script", obj); + serialize.ScriptVal("script", &obj); TS_ASSERT_STREAM(stream, 119, "\x03" // SCRIPT_TYPE_OBJECT @@ -352,7 +352,7 @@ public: std::stringstream stream; CStdSerializer serialize(script, stream); - serialize.ScriptVal("script", obj); + serialize.ScriptVal("script", &obj); if (expstream) { @@ -584,7 +584,7 @@ public: TestLogger logger; TS_ASSERT(script.Eval("([1, 2, function () { }])", &obj)); - TS_ASSERT_THROWS(serialize.ScriptVal("script", obj), PSERROR_Serialize_InvalidScriptValue); + TS_ASSERT_THROWS(serialize.ScriptVal("script", &obj), PSERROR_Serialize_InvalidScriptValue); } void test_script_splice() @@ -619,7 +619,7 @@ public: std::stringstream stream; CStdSerializer serialize(script, stream); - serialize.ScriptVal("script", obj); + serialize.ScriptVal("script", &obj); CStdDeserializer deserialize(script, stream);