Bug 71045

Summary: Crash in JSC::Structure::materializePropertyMap when viewing Garden-O-Matic
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, fpizlo, ggaren, mhahnenberg, oliver, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.7   
URL: http://build.webkit.org/TestFailures/garden-o-matic.html
Attachments:
Description Flags
the patch ggaren: review+, fpizlo: commit-queue-

Adam Roben (:aroben)
Reported 2011-10-27 11:58:05 PDT
To reproduce: 1. Go to http://build.webkit.org/TestFailures/garden-o-matic.html You'll crash! Here's the backtrace: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010a2fb587 JSC::Structure::materializePropertyMap(JSC::JSGlobalData&) + 343 1 com.apple.JavaScriptCore 0x000000010a3c0adb JSC::DFG::ByteCodeParser::parseBlock(unsigned int) + 12971 2 com.apple.JavaScriptCore 0x000000010a3bd5d3 JSC::DFG::ByteCodeParser::parseCodeBlock() + 1203 3 com.apple.JavaScriptCore 0x000000010a3c22bf JSC::DFG::ByteCodeParser::parse() + 79 4 com.apple.JavaScriptCore 0x000000010a3c2499 JSC::DFG::parse(JSC::DFG::Graph&, JSC::JSGlobalData*, JSC::CodeBlock*) + 41 5 com.apple.JavaScriptCore 0x000000010a3a70de JSC::DFG::compile(JSC::DFG::CompileMode, JSC::ExecState*, JSC::CodeBlock*, JSC::JITCode&, JSC::MacroAssemblerCodePtr*) + 750 6 com.apple.JavaScriptCore 0x000000010a3a6de7 JSC::DFG::tryCompileFunction(JSC::ExecState*, JSC::CodeBlock*, JSC::JITCode&, JSC::MacroAssemblerCodePtr&) + 23 7 com.apple.JavaScriptCore 0x000000010a3dbf31 JSC::FunctionExecutable::compileForCallInternal(JSC::ExecState*, JSC::ScopeChainNode*, JSC::JITCode::JITType) + 385 8 com.apple.JavaScriptCore 0x000000010a3efb7d cti_optimize_from_ret + 253 9 ??? 0x000037cc8c4ed70e 0 + 61351666833166 10 com.apple.JavaScriptCore 0x000000010a2e383d JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1421 11 com.apple.JavaScriptCore 0x000000010a2e32ad JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 45 12 com.apple.WebCore 0x000000010a63d6d1 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) + 1153 13 com.apple.WebCore 0x000000010a63d184 WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&) + 356 14 com.apple.WebCore 0x000000010a5f4bfd WebCore::EventTarget::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 141 15 com.apple.WebCore 0x000000010a83b28d WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, WebCore::ProgressEventAction) + 61 16 com.apple.WebCore 0x000000010a83b043 WebCore::XMLHttpRequest::callReadyStateChangeListener() + 339 17 com.apple.WebCore 0x000000010a83e847 WebCore::XMLHttpRequest::didFinishLoading(unsigned long, double) + 407 18 com.apple.WebCore 0x000000010ac5aafa WebCore::DocumentThreadableLoader::notifyFinished(WebCore::CachedResource*) + 426 19 com.apple.WebCore 0x000000010a661d37 WebCore::CachedResource::checkNotify() + 151 20 com.apple.WebCore 0x000000010ac0aa05 WebCore::CachedRawResource::data(WTF::PassRefPtr<WebCore::SharedBuffer>, bool) + 453 21 com.apple.WebCore 0x000000010a65e64a WebCore::CachedResourceRequest::didFinishLoading(WebCore::SubresourceLoader*, double) + 202 22 com.apple.WebCore 0x000000010a65e4f8 WebCore::SubresourceLoader::didFinishLoading(double) + 56
Attachments
the patch (6.12 KB, patch)
2011-10-27 13:57 PDT, Filip Pizlo 		ggaren: review+
fpizlo: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2011-10-27 11:58:54 PDT
<rdar://problem/10356412>
Adam Roben (:aroben)
Comment 2 2011-10-27 12:01:48 PDT
I'm currently using a Release build from r98588.
Filip Pizlo
Comment 3 2011-10-27 13:29:31 PDT
Yup, I can see it in ToT. Investigating...
Filip Pizlo
Comment 4 2011-10-27 13:40:32 PDT
This looks like continued sloppiness in JSC::Structure's management of the property table. It sometimes creates structures marked pinned (meaning that they have a property table) but then never creates the property table. I don't see a good testing strategy to repro this particular case, because the garden-o-matic code seems to only trigger the bug by happening to run code in such an order, and with such frequency, that tiered compilation ends up accidentally poking and prodding JSC::Structure at just the right times to get this to appear. On the other hand, if I add a simple assertion in a few places that says "if you're pinned you must have a property table", then I can get JavaScriptCore to crash while allocating a JSDOMWindowShell. I'm slowly prodding along now, making sure that all of the places that pin structures also force an allocation of the property table... Thereafter so long as nobody removes my assertions, any regressions should be detectable when you try to run any script.
Filip Pizlo
Comment 5 2011-10-27 13:57:14 PDT
Created attachment 112745 [details] the patch
Geoffrey Garen
Comment 6 2011-10-27 14:06:41 PDT
Comment on attachment 112745 [details] the patch r=me
Filip Pizlo
Comment 7 2011-10-27 15:07:41 PDT
Sadly, this is a slight slow-down on SunSpider. But it is quite small, and it doesn't show up in other benchmarks, so the fix is likely worth it. Benchmark report for SunSpider, V8, and Kraken. VMs tested: "TipOfTree" at /Volumes/Data/pizlo/tertiary/OpenSource/WebKitBuild/Release/jsc "FixStructPin" at /Volumes/Data/pizlo/secondary/OpenSource/WebKitBuild/Release/jsc Collected 30 samples per benchmark/VM, with 10 VM invocations per benchmark. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. TipOfTree FixStructPin SunSpider: 3d-cube 7.9432+-0.0276 7.9117+-0.0268 3d-morph 8.5105+-0.0784 ^ 8.4044+-0.0181 ^ definitely 1.0126x faster 3d-raytrace 8.2174+-0.0432 8.1745+-0.0613 access-binary-trees 1.7146+-0.0143 ^ 1.6905+-0.0038 ^ definitely 1.0143x faster access-fannkuch 7.7460+-0.0093 ? 7.7609+-0.0059 ? access-nbody 4.5315+-0.0072 ? 4.5358+-0.0143 ? access-nsieve 3.2009+-0.0146 3.1979+-0.0179 bitops-3bit-bits-in-byte 1.3244+-0.0089 1.3130+-0.0043 bitops-bits-in-byte 5.2687+-0.0205 ? 5.2824+-0.0112 ? bitops-bitwise-and 3.4350+-0.0401 ? 3.4352+-0.0363 ? bitops-nsieve-bits 5.6555+-0.0224 5.6529+-0.0216 controlflow-recursive 2.3330+-0.0037 ? 2.3410+-0.0106 ? crypto-aes 7.6497+-0.0420 7.6323+-0.0484 crypto-md5 2.8601+-0.0087 ? 2.8605+-0.0145 ? crypto-sha1 2.6354+-0.0067 2.6312+-0.0071 date-format-tofte 10.5632+-0.0460 ! 10.6605+-0.0460 ! definitely 1.0092x slower date-format-xparb 9.9720+-0.1549 ? 10.1731+-0.1275 ? might be 1.0202x slower math-cordic 7.5686+-0.1547 ? 7.7440+-0.1560 ? might be 1.0232x slower math-partial-sums 10.6938+-0.1189 10.5761+-0.0265 might be 1.0111x faster math-spectral-norm 2.8803+-0.0045 2.8773+-0.0042 regexp-dna 13.4070+-0.1127 13.3740+-0.1064 string-base64 4.4634+-0.0362 4.4469+-0.0125 string-fasta 7.1400+-0.0245 ? 7.1445+-0.0361 ? string-tagcloud 13.3047+-0.0816 13.1774+-0.0828 string-unpack-code 22.7366+-0.1591 ! 23.6213+-0.1312 ! definitely 1.0389x slower string-validate-input 5.6028+-0.0248 ! 5.6641+-0.0201 ! definitely 1.0109x slower <arithmetic> * 6.9753+-0.0162 ! 7.0109+-0.0168 ! definitely 1.0051x slower <geometric> 5.6425+-0.0108 ? 5.6500+-0.0114 ? <harmonic> 4.4692+-0.0079 4.4621+-0.0084 TipOfTree FixStructPin V8: crypto 81.1896+-0.1168 ? 81.3671+-0.1623 ? deltablue 199.3296+-1.0399 198.1822+-0.9440 earley-boyer 111.8989+-0.4461 ! 113.1431+-0.5304 ! definitely 1.0111x slower raytrace 69.7200+-0.1919 ? 69.9946+-0.2200 ? regexp 123.9281+-0.2393 ! 124.4909+-0.2948 ! definitely 1.0045x slower richards 145.2759+-0.4161 ? 145.8049+-0.3323 ? splay 125.6654+-0.2809 ^ 121.0977+-0.2940 ^ definitely 1.0377x faster <arithmetic> 122.4296+-0.2022 ^ 122.0115+-0.1650 ^ definitely 1.0034x faster <geometric> * 116.1735+-0.1607 115.8837+-0.1459 <harmonic> 110.1899+-0.1402 110.0394+-0.1449 TipOfTree FixStructPin Kraken: ai-astar 830.8233+-4.4071 ? 835.0350+-0.5160 ? audio-beat-detection 213.1762+-0.6769 ? 213.2899+-1.0087 ? audio-dft 263.5786+-2.6509 262.5557+-1.3202 audio-fft 133.2385+-0.3857 133.0737+-0.2631 audio-oscillator 291.6292+-0.6770 ? 291.8211+-0.7753 ? imaging-darkroom 456.8501+-10.0365 450.3742+-3.3700 might be 1.0144x faster imaging-desaturate 245.3833+-0.2843 245.2521+-0.0855 imaging-gaussian-blur 622.0908+-0.8585 621.7663+-0.4884 json-parse-financial 69.8525+-0.1532 ! 72.4779+-0.1186 ! definitely 1.0376x slower json-stringify-tinderbox 79.5813+-0.1407 79.2584+-0.1880 stanford-crypto-aes 153.7820+-1.0984 ^ 151.5146+-0.9335 ^ definitely 1.0150x faster stanford-crypto-ccm 116.4856+-0.6340 115.7287+-0.4401 stanford-crypto-pbkdf2 236.9247+-1.5438 234.8863+-0.8870 stanford-crypto-sha256-iterative 85.3753+-0.1279 ^ 85.0792+-0.1425 ^ definitely 1.0035x faster <arithmetic> * 271.3408+-0.6121 270.8652+-0.2501 <geometric> 206.3747+-0.3980 206.1808+-0.1924 <harmonic> 162.0133+-0.2066 ? 162.3857+-0.1775 ? TipOfTree FixStructPin All benchmarks: <arithmetic> 102.9178+-0.1943 102.7336+-0.0848 <geometric> 25.8670+-0.0388 ? 25.8691+-0.0339 ? <harmonic> 7.8760+-0.0137 7.8638+-0.0145 TipOfTree FixStructPin Geomean of preferred means: <scaled-result> 60.3568+-0.0888 ? 60.3736+-0.0661 ?
Filip Pizlo
Comment 8 2011-10-27 15:08:23 PDT
Comment on attachment 112745 [details] the patch Clearing commit flag because I'm still running a few more tests, and I'll land manually once (if) I'm satisfied.
Filip Pizlo
Comment 9 2011-10-27 15:19:41 PDT
Landed in http://trac.webkit.org/changeset/98647
Note You need to log in before you can comment on or make changes to this bug.