131747 2014-04-16 11:03:42 -0700 Crash in CodeBlock::setOptimizationThresholdBasedOnCompilationResult() when the debugger activates 2014-04-16 16:11:26 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam fpizlo ggaren mhahnenberg mmirman msaboff oliver webkit-bug-importer oldest_to_newest 1001226 0 mark.lam 2014-04-16 11:03:42 -0700 When the debugger is about to activate (e.g. enter stepping mode), it first waits for all DFG compilations to complete. However, when the DFG completes, if compilation is successful, it will install a new DFG codeBlock. The CodeBlock installation process is required to register codeBlocks with the debugger. Debugger::registerCodeBlock() will eventually call CodeBlock::addBreakpoint() and/or CodeBlock::setSteppingMode() which may jettison the DFG codeBlock that we’re trying to install. Thereafter, chaos ensues. 1001227 1 mark.lam 2014-04-16 11:04:42 -0700 <rdar://problem/16278811> 1001347 2 mark.lam 2014-04-16 14:57:45 -0700 Some notes about the fix I will post soon: 1. Debugger::registerCodeBlock() eventually calls: a. CodeBlock::addBreakpoint() b. CodeBlock::setSteppingMode(). These 2 functions can jettison the code block, and this poses a problem if that happens during installation of said code block. 2. operationOptimize() will check if the debugger is stepping mode, or if the CodeBlock has any pending breakpoints before allowing a code block to be DFG compiled. If the base code block already has breakpoints enabled in it, we'll never optimize that code block, and hence, we'll never get to the scenario where we'll jettison the DFG code block for the reason of it having active breakpoints. If the debugger is already in stepping mode, we'll never optimize that code block, and hence, we'll never get to the scenario where we'll jettison the DFG code block for the reason of the debugger being in stepping mode. Which leaves ... 3. What happens if a DFG compilation is already in progress in a compiler thread and the debugger switches to stepping mode. The debugger is supposed to wait for all compilations to complete before switching to stepping mode. However, currently, the debugger is setting the stepping mode flag before compilation completes. This is the root cause of this bug. 4. What happens if a DFG compilation is already in progress in a compiler thread and the debugger adds a new breakpoint to the function for that code block. The debugger is supposed to wait for all compilations to complete before it applies the new breakpoint to the code blocks. And the debugger does behave correctly here (in Debugger::toggleBreakpoint()). 1001359 3 229486 mark.lam 2014-04-16 15:20:13 -0700 Created attachment 229486 the patch 1001367 4 229486 fpizlo 2014-04-16 15:43:35 -0700 Comment on attachment 229486 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=229486&action=review > Source/JavaScriptCore/debugger/Debugger.cpp:250 > + // FIXME: We should never have to jettison a code block (due to pending breakpoints > + // or stepping mode) that is being registered. operationOptimize() should have > + // prevented the optimizing of such code blocks in the first place. Find a way to > + // express this with greater clarity in the code. Can you file a bugzilla bug for this and reference it here? 1001373 5 229490 mark.lam 2014-04-16 15:50:31 -0700 Created attachment 229490 patch 2: with bug for fixme. 1001382 6 mark.lam 2014-04-16 16:11:26 -0700 Landed in r167396: <http://trac.webkit.org/r167396>. 229486 2014-04-16 15:20:13 -0700 2014-04-16 15:50:31 -0700 the patch bug-131747.patch text/plain 2667 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY3MzkwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI2IEBA CisyMDE0LTA0LTE2ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBD cmFzaCBpbiBDb2RlQmxvY2s6OnNldE9wdGltaXphdGlvblRocmVzaG9sZEJhc2VkT25Db21waWxh dGlvblJlc3VsdCgpIHdoZW4gdGhlIGRlYnVnZ2VyIGFjdGl2YXRlcy4KKyAgICAgICAgPGh0dHBz Oi8vd2Via2l0Lm9yZy9iLzEzMTc0Nz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBXaGVuIHRoZSBkZWJ1Z2dlciBpcyBhYm91dCB0byBhY3RpdmF0ZSAo ZS5nLiBlbnRlciBzdGVwcGluZyBtb2RlKSwgaXQgZmlyc3QKKyAgICAgICAgd2FpdHMgZm9yIGFs bCBERkcgY29tcGlsYXRpb25zIHRvIGNvbXBsZXRlLiAgSG93ZXZlciwgd2hlbiB0aGUgREZHIGNv bXBsZXRlcywKKyAgICAgICAgaWYgY29tcGlsYXRpb24gaXMgc3VjY2Vzc2Z1bCwgaXQgd2lsbCBp bnN0YWxsIGEgbmV3IERGRyBjb2RlQmxvY2suICBUaGUKKyAgICAgICAgQ29kZUJsb2NrIGluc3Rh bGxhdGlvbiBwcm9jZXNzIGlzIHJlcXVpcmVkIHRvIHJlZ2lzdGVyIGNvZGVCbG9ja3Mgd2l0aCB0 aGUKKyAgICAgICAgZGVidWdnZXIuICBEZWJ1Z2dlcjo6cmVnaXN0ZXJDb2RlQmxvY2soKSB3aWxs IGV2ZW50dWFsbHkgY2FsbAorICAgICAgICBDb2RlQmxvY2s6OnNldFN0ZXBwaW5nTW9kZSgpIHdo aWNoIG1heSBqZXR0aXNvbiB0aGUgREZHIGNvZGVCbG9jayB0aGF0IHdlJ3JlCisgICAgICAgIHRy eWluZyB0byBpbnN0YWxsLiAgVGhlcmVhZnRlciwgY2hhb3MgZW5zdWVzLgorCisgICAgICAgIFRo aXMgamV0dGlzb24naW5nIG9ubHkgaGFwcGVucyBiZWNhdXNlIHRoZSBkZWJ1Z2dlciBjdXJyZW50 bHkgc2V0IGl0cworICAgICAgICBtX3N0ZXBwaW5nTW9kZSBmbGFnIGJlZm9yZSB3YWl0aW5nIGZv ciBjb21waWxhdGlvbiB0byBjb21wbGV0ZS4gIFRoZSBmaXggaXMKKyAgICAgICAgc2ltcGx5IHRv IHNldCB0aGF0IGZsYWcgb25seSBhZnRlciBjb21waWxhdGlvbiBpcyBjb21wbGV0ZS4KKworICAg ICAgICAqIGRlYnVnZ2VyL0RlYnVnZ2VyLmNwcDoKKyAgICAgICAgKEpTQzo6RGVidWdnZXI6OnNl dFN0ZXBwaW5nTW9kZSk6CisgICAgICAgIChKU0M6OkRlYnVnZ2VyOjpyZWdpc3RlckNvZGVCbG9j ayk6CisKIDIwMTQtMDQtMTYgIHBlYXZvQG91dGxvb2suY29tICA8cGVhdm9Ab3V0bG9vay5jb20+ CiAKICAgICAgICAgRml4IEpTQyBEZWJ1ZyBSZWdyZXNzaW9ucyBvbiBXaW5kb3dzCkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvZGVidWdnZXIvRGVidWdnZXIuY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZWJ1Z2dlci9EZWJ1Z2dlci5jcHAJKHJldmlzaW9uIDE2 NzI2MikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZWJ1Z2dlci9EZWJ1Z2dlci5jcHAJKHdv cmtpbmcgY29weSkKQEAgLTIzMiwxOCArMjMyLDIyIEBAIHByaXZhdGU6CiAKIHZvaWQgRGVidWdn ZXI6OnNldFN0ZXBwaW5nTW9kZShTdGVwcGluZ01vZGUgbW9kZSkKIHsKLSAgICBpZiAobW9kZSA9 PSBtX3N0ZXBwaW5nTW9kZSkKKyAgICBpZiAobW9kZSA9PSBtX3N0ZXBwaW5nTW9kZSB8fCAhbV92 bSkKICAgICAgICAgcmV0dXJuOwotICAgIG1fc3RlcHBpbmdNb2RlID0gbW9kZTsKIAotICAgIGlm ICghbV92bSkKLSAgICAgICAgcmV0dXJuOworICAgIG1fdm0tPndhaXRGb3JDb21waWxhdGlvbnNU b0NvbXBsZXRlKCk7CisKKyAgICBtX3N0ZXBwaW5nTW9kZSA9IG1vZGU7CiAgICAgU2V0U3RlcHBp bmdNb2RlRnVuY3RvciBmdW5jdG9yKHRoaXMsIG1vZGUpOwotICAgIGZvckVhY2hDb2RlQmxvY2so ZnVuY3Rvcik7CisgICAgbV92bS0+aGVhcC5mb3JFYWNoQ29kZUJsb2NrKGZ1bmN0b3IpOwogfQog CiB2b2lkIERlYnVnZ2VyOjpyZWdpc3RlckNvZGVCbG9jayhDb2RlQmxvY2sqIGNvZGVCbG9jaykK IHsKKyAgICAvLyBGSVhNRTogV2Ugc2hvdWxkIG5ldmVyIGhhdmUgdG8gamV0dGlzb24gYSBjb2Rl IGJsb2NrIChkdWUgdG8gcGVuZGluZyBicmVha3BvaW50cworICAgIC8vIG9yIHN0ZXBwaW5nIG1v ZGUpIHRoYXQgaXMgYmVpbmcgcmVnaXN0ZXJlZC4gb3BlcmF0aW9uT3B0aW1pemUoKSBzaG91bGQg aGF2ZQorICAgIC8vIHByZXZlbnRlZCB0aGUgb3B0aW1pemluZyBvZiBzdWNoIGNvZGUgYmxvY2tz IGluIHRoZSBmaXJzdCBwbGFjZS4gRmluZCBhIHdheSB0bworICAgIC8vIGV4cHJlc3MgdGhpcyB3 aXRoIGdyZWF0ZXIgY2xhcml0eSBpbiB0aGUgY29kZS4KICAgICBhcHBseUJyZWFrcG9pbnRzKGNv ZGVCbG9jayk7CiAgICAgaWYgKGlzU3RlcHBpbmcoKSkKICAgICAgICAgY29kZUJsb2NrLT5zZXRT dGVwcGluZ01vZGUoQ29kZUJsb2NrOjpTdGVwcGluZ01vZGVFbmFibGVkKTsK 229490 2014-04-16 15:50:31 -0700 2014-04-16 15:59:06 -0700 patch 2: with bug for fixme. bug-131747.patch text/plain 2701 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY3MzkxKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI2IEBA CisyMDE0LTA0LTE2ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBD cmFzaCBpbiBDb2RlQmxvY2s6OnNldE9wdGltaXphdGlvblRocmVzaG9sZEJhc2VkT25Db21waWxh dGlvblJlc3VsdCgpIHdoZW4gdGhlIGRlYnVnZ2VyIGFjdGl2YXRlcy4KKyAgICAgICAgPGh0dHBz Oi8vd2Via2l0Lm9yZy9iLzEzMTc0Nz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBXaGVuIHRoZSBkZWJ1Z2dlciBpcyBhYm91dCB0byBhY3RpdmF0ZSAo ZS5nLiBlbnRlciBzdGVwcGluZyBtb2RlKSwgaXQgZmlyc3QKKyAgICAgICAgd2FpdHMgZm9yIGFs bCBERkcgY29tcGlsYXRpb25zIHRvIGNvbXBsZXRlLiAgSG93ZXZlciwgd2hlbiB0aGUgREZHIGNv bXBsZXRlcywKKyAgICAgICAgaWYgY29tcGlsYXRpb24gaXMgc3VjY2Vzc2Z1bCwgaXQgd2lsbCBp bnN0YWxsIGEgbmV3IERGRyBjb2RlQmxvY2suICBUaGUKKyAgICAgICAgQ29kZUJsb2NrIGluc3Rh bGxhdGlvbiBwcm9jZXNzIGlzIHJlcXVpcmVkIHRvIHJlZ2lzdGVyIGNvZGVCbG9ja3Mgd2l0aCB0 aGUKKyAgICAgICAgZGVidWdnZXIuICBEZWJ1Z2dlcjo6cmVnaXN0ZXJDb2RlQmxvY2soKSB3aWxs IGV2ZW50dWFsbHkgY2FsbAorICAgICAgICBDb2RlQmxvY2s6OnNldFN0ZXBwaW5nTW9kZSgpIHdo aWNoIG1heSBqZXR0aXNvbiB0aGUgREZHIGNvZGVCbG9jayB0aGF0IHdlJ3JlCisgICAgICAgIHRy eWluZyB0byBpbnN0YWxsLiAgVGhlcmVhZnRlciwgY2hhb3MgZW5zdWVzLgorCisgICAgICAgIFRo aXMgamV0dGlzb24naW5nIG9ubHkgaGFwcGVucyBiZWNhdXNlIHRoZSBkZWJ1Z2dlciBjdXJyZW50 bHkgc2V0IGl0cworICAgICAgICBtX3N0ZXBwaW5nTW9kZSBmbGFnIGJlZm9yZSB3YWl0aW5nIGZv ciBjb21waWxhdGlvbiB0byBjb21wbGV0ZS4gIFRoZSBmaXggaXMKKyAgICAgICAgc2ltcGx5IHRv IHNldCB0aGF0IGZsYWcgb25seSBhZnRlciBjb21waWxhdGlvbiBpcyBjb21wbGV0ZS4KKworICAg ICAgICAqIGRlYnVnZ2VyL0RlYnVnZ2VyLmNwcDoKKyAgICAgICAgKEpTQzo6RGVidWdnZXI6OnNl dFN0ZXBwaW5nTW9kZSk6CisgICAgICAgIChKU0M6OkRlYnVnZ2VyOjpyZWdpc3RlckNvZGVCbG9j ayk6CisKIDIwMTQtMDQtMTYgIHBlYXZvQG91dGxvb2suY29tICA8cGVhdm9Ab3V0bG9vay5jb20+ CiAKICAgICAgICAgRml4IEpTQyBEZWJ1ZyBSZWdyZXNzaW9ucyBvbiBXaW5kb3dzCkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvZGVidWdnZXIvRGVidWdnZXIuY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZWJ1Z2dlci9EZWJ1Z2dlci5jcHAJKHJldmlzaW9uIDE2 NzM5MSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZWJ1Z2dlci9EZWJ1Z2dlci5jcHAJKHdv cmtpbmcgY29weSkKQEAgLTIzMiwxOCArMjMyLDIyIEBAIHByaXZhdGU6CiAKIHZvaWQgRGVidWdn ZXI6OnNldFN0ZXBwaW5nTW9kZShTdGVwcGluZ01vZGUgbW9kZSkKIHsKLSAgICBpZiAobW9kZSA9 PSBtX3N0ZXBwaW5nTW9kZSkKKyAgICBpZiAobW9kZSA9PSBtX3N0ZXBwaW5nTW9kZSB8fCAhbV92 bSkKICAgICAgICAgcmV0dXJuOwotICAgIG1fc3RlcHBpbmdNb2RlID0gbW9kZTsKIAotICAgIGlm ICghbV92bSkKLSAgICAgICAgcmV0dXJuOworICAgIG1fdm0tPndhaXRGb3JDb21waWxhdGlvbnNU b0NvbXBsZXRlKCk7CisKKyAgICBtX3N0ZXBwaW5nTW9kZSA9IG1vZGU7CiAgICAgU2V0U3RlcHBp bmdNb2RlRnVuY3RvciBmdW5jdG9yKHRoaXMsIG1vZGUpOwotICAgIGZvckVhY2hDb2RlQmxvY2so ZnVuY3Rvcik7CisgICAgbV92bS0+aGVhcC5mb3JFYWNoQ29kZUJsb2NrKGZ1bmN0b3IpOwogfQog CiB2b2lkIERlYnVnZ2VyOjpyZWdpc3RlckNvZGVCbG9jayhDb2RlQmxvY2sqIGNvZGVCbG9jaykK IHsKKyAgICAvLyBGSVhNRTogV2Ugc2hvdWxkIG5ldmVyIGhhdmUgdG8gamV0dGlzb24gYSBjb2Rl IGJsb2NrIChkdWUgdG8gcGVuZGluZyBicmVha3BvaW50cworICAgIC8vIG9yIHN0ZXBwaW5nIG1v ZGUpIHRoYXQgaXMgYmVpbmcgcmVnaXN0ZXJlZC4gb3BlcmF0aW9uT3B0aW1pemUoKSBzaG91bGQg aGF2ZQorICAgIC8vIHByZXZlbnRlZCB0aGUgb3B0aW1pemluZyBvZiBzdWNoIGNvZGUgYmxvY2tz IGluIHRoZSBmaXJzdCBwbGFjZS4gRmluZCBhIHdheSB0bworICAgIC8vIGV4cHJlc3MgdGhpcyB3 aXRoIGdyZWF0ZXIgY2xhcml0eSBpbiB0aGUgY29kZS4gU2VlIDxodHRwczovL3dlYmtpdC5vcmcv YjEzMTc3MT4uCiAgICAgYXBwbHlCcmVha3BvaW50cyhjb2RlQmxvY2spOwogICAgIGlmIChpc1N0 ZXBwaW5nKCkpCiAgICAgICAgIGNvZGVCbG9jay0+c2V0U3RlcHBpbmdNb2RlKENvZGVCbG9jazo6 U3RlcHBpbmdNb2RlRW5hYmxlZCk7Cg==