Bug 20577

I was too fast to blame Darin on this, sorry! r36005 is not working either.

Ok apperently my build script did a svn up before building so it was building r35006 instead of r35005. I fixed the script and indeed the problem is in r35006. CC'in Darin again. Sorry for the mess.

I can confirm this.

I'm assuming Cameron already knows this, but I'll mention it here for future reference - this is what's hit when loading GMail: ASSERTION FAILED: offset + length <= static_cast<unsigned>(s.size()) (/Users/matt/Code/WebKit/JavaScriptCore/kjs/JSString.cpp:135 KJS::JSString* KJS::jsSubstring(KJS::ExecState*, const KJS::UString&, unsigned int, unsigned int)) Thread 0 Crashed: 0 com.apple.JavaScriptCore 0x00485282 KJS::jsSubstring(KJS::ExecState*, KJS::UString const&, unsigned int, unsigned int) + 238 (JSString.cpp:135) 1 com.apple.JavaScriptCore 0x004858f3 KJS::stringProtoFuncSubstr(KJS::ExecState*, KJS::JSObject*, KJS::JSValue*, KJS::ArgList const&) + 571 (StringPrototype.cpp:578) 2 com.apple.JavaScriptCore 0x0050b1bb KJS::Machine::privateExecute(KJS::Machine::ExecutionFlag, KJS::ExecState*, KJS::RegisterFile*, KJS::Register*, KJS::ScopeChainNode*, KJS::CodeBlock*, KJS::JSValue**) + 30083 (Machine.cpp:2484) 3 com.apple.JavaScriptCore 0x0050d286 KJS::Machine::execute(KJS::FunctionBodyNode*, KJS::ExecState*, KJS::JSFunction*, KJS::JSObject*, KJS::ArgList const&, KJS::ScopeChainNode*, KJS::JSValue**) + 716 (Machine.cpp:857) 4 com.apple.JavaScriptCore 0x0046aa2b KJS::JSFunction::call(KJS::ExecState*, KJS::JSValue*, KJS::ArgList const&) + 139 (JSFunction.cpp:71) 5 com.apple.JavaScriptCore 0x0046aac7 KJS::call(KJS::ExecState*, KJS::JSValue*, KJS::CallType, KJS::CallData const&, KJS::JSValue*, KJS::ArgList const&) + 149 (CallData.cpp:39) 6 com.apple.JavaScriptCore 0x00479112 KJS::functionProtoFuncApply(KJS::ExecState*, KJS::JSObject*, KJS::JSValue*, KJS::ArgList const&) + 494 (FunctionPrototype.cpp:107) 7 com.apple.JavaScriptCore 0x0050b1bb KJS::Machine::privateExecute(KJS::Machine::ExecutionFlag, KJS::ExecState*, KJS::RegisterFile*, KJS::Register*, KJS::ScopeChainNode*, KJS::CodeBlock*, KJS::JSValue**) + 30083 (Machine.cpp:2484) [snip]

Here is a reduction from Gmail that works on the console: "GMAIL_IMP=bf-i%2Fd-0-0%2Ftl-v".substr(10)

The problem is basically that stringProtoFuncSubstr() was written to do its index calculations for the UString::substr() method, which, like the JS function substr, ignores lengths that are too long. Just plugging in jsSubstring doesn't quite work unless the index computations are changed to never overflow the length of the string. I'll assign this to myself, because Darin is probably with his family. There are similar replacements of UString::substr() with jsSubstring() throughout Darin's patch, so after this is fixed it might be a good idea to go through and check that all of them won't fall prey to the same problem in unusual situations.

Created attachment 23092 [details] Proposed patch

Comment on attachment 23092 [details] Proposed patch r=me

Landed in r36009. I'll make a separate bug to check the other cases where UString::substr() was removed.

(In reply to comment #9) > Landed in r36009. I'll make a separate bug to check the other cases where > UString::substr() was removed. Thanks. This was the perfect fix. For what it's worth, I did fix some call sites that used to rely on substr's range checking; I just missed this one. I hope I didn't miss others.