Reviewers: Søren Gjesse, Description: Fix issue 289: check external source strings validity in Runtime_DebugGetLoadedScripts
Please review this at http://codereview.chromium.org/56002 Affected files: M src/runtime.cc M test/cctest/test-debug.cc Index: src/runtime.cc diff --git a/src/runtime.cc b/src/runtime.cc index 9d4383e55f04e286d73ecfc46b00478d5cbe50de..50ce62c04daf2d9258f2752d4077b51e4affd35e 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -6509,6 +6509,19 @@ static Object* Runtime_DebugEvaluateGlobal(Arguments args) { } +static bool IsExternalStringValid(Object* str) { + if (!str->IsString() || !StringShape(String::cast(str)).IsExternal()) { + return true; + } + if (StringShape(String::cast(str)).IsAsciiRepresentation()) { + return ExternalAsciiString::cast(str)->resource() != 0; + } else if (StringShape(String::cast(str)).IsTwoByteRepresentation()) { + return ExternalTwoByteString::cast(str)->resource() != 0; + } else { + return true; + } +} + // Helper function used by Runtime_DebugGetLoadedScripts below. static int DebugGetLoadedScripts(FixedArray* instances, int instances_size) { NoHandleAllocation ha; @@ -6520,7 +6533,7 @@ static int DebugGetLoadedScripts(FixedArray* instances, int instances_size) { while (iterator.has_next()) { HeapObject* obj = iterator.next(); ASSERT(obj != NULL); - if (obj->IsScript()) { + if (obj->IsScript() && IsExternalStringValid(Script::cast(obj)->source())) { if (instances != NULL && count < instances_size) { instances->set(count, obj); } Index: test/cctest/test-debug.cc diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 30a086a3a787ccb4641c30ca363d99eea1e77107..0410f46dc7f7fd694f0de20735fa56a7b8ad6a1c 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -3980,3 +3980,42 @@ TEST(DebuggerAgentProtocolOverflowHeader) { delete client; delete server; } + + +// Test for issue http://code.google.com/p/v8/issues/detail?id=289. +// Make sure that DebugGetLoadedScripts doesn't return scripts +// with disposed external source. +class EmptyExternalStringResource : public v8::String::ExternalStringResource { + public: + EmptyExternalStringResource() { empty_[0] = 0; } + virtual ~EmptyExternalStringResource() {}; + virtual size_t length() const { return empty_.length(); } + virtual const uint16_t* data() const { return empty_.start(); } + private: + ::v8::internal::EmbeddedVector<uint16_t,1> empty_; +}; + + +TEST(DebugGetLoadedScripts) { + v8::HandleScope scope; + DebugLocalContext env; + EmptyExternalStringResource source_ext_str; + v8::Local<v8::String> source = v8::String::NewExternal(&source_ext_str); + v8::Handle<v8::Script> evil_script = v8::Script::Compile(source); + Handle<i::ExternalTwoByteString> i_source( + i::ExternalTwoByteString::cast(*v8::Utils::OpenHandle(*source))); + // This situation can happen if source was an external string disposed + // by its owner. + i_source->set_resource(0); + + bool allow_natives_syntax = i::FLAG_allow_natives_syntax; + i::FLAG_allow_natives_syntax = true; + v8::Local<v8::Value> v_scripts = CompileRun( + "var scripts = %DebugGetLoadedScripts();" + "for (var i = 0; i < scripts.length; ++i) {" + " scripts[i].line_ends;" + "}" + ); + // Must not crash while accessing line_ends. + i::FLAG_allow_natives_syntax = allow_natives_syntax; +} --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
