127269 2014-01-20 00:06:10 -0800 A few utility functions for setting/clearing the op_debug hasBreakpointFlag. 2014-01-20 15:46:01 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 122836 1 mark.lam mark.lam fpizlo ggaren mhahnenberg msaboff oliver oldest_to_newest 969741 0 mark.lam 2014-01-20 00:06:10 -0800 The bytecode level breakpoint debugger will have to set and clear breakpoints by setting/clearing the hasBreakpointFlag operand in the op_debug bytecode of a CodeBlock. In addition, as an optimization, there’s also a need to for a utility function clear all breakpoints in a CodeBlock that may have been set. This is useful when a debugger detaches, and breakpoints need to be cleared in bulk. This patch will provide these utility functions in anticipation of upcoming support for bytecode level breakpoints. These functions are not currently in use. 969745 1 221628 mark.lam 2014-01-20 00:23:33 -0800 Created attachment 221628 the patch. 969771 2 mark.lam 2014-01-20 01:59:36 -0800 FYI, the efl-wk2 bot failure is due to an internal compiler error on the bot. Hence, it is not due to the patch. 969882 3 221628 ggaren 2014-01-20 09:26:56 -0800 Comment on attachment 221628 the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=221628&action=review > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:406 > +void UnlinkedCodeBlock::clearAllBreakpointsIn(RefCountedArray<Instruction>& instructions) > +{ > + LineColumnInfoList& list = opDebugLineColumnInfoList(); > + for (LineColumnInfoList::iterator it = list.begin(); it != list.end(); ++it) { > + static const int hasBreakpointFlagOffset = 2; > + unsigned offset = it->instructionOffset; > + ASSERT(m_vm->interpreter->getOpcodeID(instructions[offset].u.opcode) == op_debug); > + instructions[offset + hasBreakpointFlagOffset] = false; > + } > +} This doesn't look right. The unlinked CodeBlock should never contain breakpoints, or metadata about whether they're set. The unlinked CodeBlock is cached and shared among webpages. Putting breakpoint metadata in it would cause debugging in one webpage to set breakpoints in another, and it would cause breakpoints to persist across page loads. 969888 4 mark.lam 2014-01-20 09:33:47 -0800 (In reply to comment #3) > (From update of attachment 221628 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221628&action=review > > > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:406 > > +void UnlinkedCodeBlock::clearAllBreakpointsIn(RefCountedArray<Instruction>& instructions) > > +{ > > + LineColumnInfoList& list = opDebugLineColumnInfoList(); > > + for (LineColumnInfoList::iterator it = list.begin(); it != list.end(); ++it) { > > + static const int hasBreakpointFlagOffset = 2; > > + unsigned offset = it->instructionOffset; > > + ASSERT(m_vm->interpreter->getOpcodeID(instructions[offset].u.opcode) == op_debug); > > + instructions[offset + hasBreakpointFlagOffset] = false; > > + } > > +} > > This doesn't look right. The unlinked CodeBlock should never contain breakpoints, or metadata about whether they're set. The unlinked CodeBlock is cached and shared among webpages. Putting breakpoint metadata in it would cause debugging in one webpage to set breakpoints in another, and it would cause breakpoints to persist across page loads. I’m not touching the UnlinkedCodeBlock bytecode here. The bytecode instructions array is provided by the CodeBlock and passed in. What the UnlinkedCodeBlock has is a map of where all the op_debug bytecodes are so that we can get to them quickly. I opted to have the UnlinkedCodeBlock provided this utility function that the CodeBlock can use to clear its breakpoints because I wanted to keep that list private. The alternative is to make opDebugLineColumnInfoList() public and have CodeBlock walk it to do the breakpoint clearing. 970050 5 mark.lam 2014-01-20 15:46:01 -0800 We have a change of plans. Instead of going with bytecode level breakpoints, we're going with CodeBlock level breakpoints. This patch is no longer relevant. 221628 2014-01-20 00:23:33 -0800 2014-01-20 09:26:56 -0800 the patch. bug-127269.patch text/plain 5936 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTYyMzA1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMwIEBA CisyMDE0LTAxLTE5ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBB IGZldyB1dGlsaXR5IGZ1bmN0aW9ucyBmb3Igc2V0dGluZy9jbGVhcmluZyB0aGUgb3BfZGVidWcg aGFzQnJlYWtwb2ludEZsYWcuCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8xMjcyNjk+ CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQSBieXRl Y29kZSBsZXZlbCBicmVha3BvaW50IGRlYnVnZ2VyIHdpbGwgaGF2ZSB0byBlbmFibGUgLyBkaXNh YmxlCisgICAgICAgIGJyZWFrcG9pbnRzIGJ5IHNldHRpbmcgLyBjbGVhcmluZyB0aGUgaGFzQnJl YWtwb2ludEZsYWcgb3BlcmFuZCBpbiB0aGUKKyAgICAgICAgb3BfZGVidWcgYnl0ZWNvZGVzIG9m IGEgQ29kZUJsb2NrLiBJbiBhZGRpdGlvbiwgYXMgYW4gb3B0aW1pemF0aW9uLAorICAgICAgICB0 aGVyZSdzIGFsc28gYSBuZWVkIGZvciBhIHV0aWxpdHkgZnVuY3Rpb24gdG8gY2xlYXIgYWxsIGJy ZWFrcG9pbnRzIGluCisgICAgICAgIGEgQ29kZUJsb2NrIHRoYXQgbWF5IGhhdmUgYmVlbiBzZXQu IFRoaXMgaXMgdXNlZnVsIHdoZW4gYSBkZWJ1Z2dlcgorICAgICAgICBkZXRhY2hlcywgYW5kIGJy ZWFrcG9pbnRzIG5lZWQgdG8gYmUgY2xlYXJlZCBpbiBidWxrLgorCisgICAgICAgIFRoaXMgcGF0 Y2ggYWRkcyB0aGVzZSB1dGlsaXR5IGZ1bmN0aW9ucyBmb3IgdGhlc2UgcHVycG9zZXMgaW4KKyAg ICAgICAgYW50aWNpcGF0aW9uIG9mIHVwY29taW5nIHN1cHBvcnQgZm9yIGJ5dGVjb2RlIGxldmVs IGJyZWFrcG9pbnRzLiBUaGVzZQorICAgICAgICBmdW5jdGlvbnMgYXJlIG5vdCBjdXJyZW50bHkg aW4gdXNlIHlldC4KKworICAgICAgICAqIGJ5dGVjb2RlL0NvZGVCbG9jay5jcHA6CisgICAgICAg IChKU0M6OkNvZGVCbG9jazo6Q29kZUJsb2NrKToKKyAgICAgICAgKEpTQzo6Q29kZUJsb2NrOjpz ZXRCcmVha3BvaW50QXQpOgorICAgICAgICAoSlNDOjpDb2RlQmxvY2s6OmNsZWFyQWxsQnJlYWtw b2ludHMpOgorICAgICAgICAqIGJ5dGVjb2RlL0NvZGVCbG9jay5oOgorICAgICAgICAqIGJ5dGVj b2RlL1VubGlua2VkQ29kZUJsb2NrLmNwcDoKKyAgICAgICAgKEpTQzo6VW5saW5rZWRDb2RlQmxv Y2s6OmNsZWFyQWxsQnJlYWtwb2ludHNJbik6CisgICAgICAgICogYnl0ZWNvZGUvVW5saW5rZWRD b2RlQmxvY2suaDoKKwogMjAxNC0wMS0xOCAgQW5kcmVhcyBLbGluZyAgPGFrbGluZ0BhcHBsZS5j b20+CiAKICAgICAgICAgQ29kZUJsb2NrOiBTaXplIG1fY2FsbExpbmtJbmZvcyBhbmQgbV9ieVZh bEluZm9zIHRvIGZpdCBlYXJsaWVyLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVj b2RlL0NvZGVCbG9jay5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2J5 dGVjb2RlL0NvZGVCbG9jay5jcHAJKHJldmlzaW9uIDE2MjMwMikKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9ieXRlY29kZS9Db2RlQmxvY2suY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNDc2LDYg KzE0NzYsNyBAQCBDb2RlQmxvY2s6OkNvZGVCbG9jayhDb3B5UGFyc2VkQmxvY2tUYWcsCiAgICAg LCBtX2FjdGl2YXRpb25SZWdpc3RlcihvdGhlci5tX2FjdGl2YXRpb25SZWdpc3RlcikKICAgICAs IG1faXNTdHJpY3RNb2RlKG90aGVyLm1faXNTdHJpY3RNb2RlKQogICAgICwgbV9uZWVkc0FjdGl2 YXRpb24ob3RoZXIubV9uZWVkc0FjdGl2YXRpb24pCisgICAgLCBtX251bUJyZWFrcG9pbnRzU2V0 KDApCiAgICAgLCBtX3NvdXJjZShvdGhlci5tX3NvdXJjZSkKICAgICAsIG1fc291cmNlT2Zmc2V0 KG90aGVyLm1fc291cmNlT2Zmc2V0KQogICAgICwgbV9maXJzdExpbmVDb2x1bW5PZmZzZXQob3Ro ZXIubV9maXJzdExpbmVDb2x1bW5PZmZzZXQpCkBAIC0xNTI5LDYgKzE1MzAsNyBAQCBDb2RlQmxv Y2s6OkNvZGVCbG9jayhTY3JpcHRFeGVjdXRhYmxlKiBvCiAgICAgLCBtX2FjdGl2YXRpb25SZWdp c3Rlcih1bmxpbmtlZENvZGVCbG9jay0+YWN0aXZhdGlvblJlZ2lzdGVyKCkpCiAgICAgLCBtX2lz U3RyaWN0TW9kZSh1bmxpbmtlZENvZGVCbG9jay0+aXNTdHJpY3RNb2RlKCkpCiAgICAgLCBtX25l ZWRzQWN0aXZhdGlvbih1bmxpbmtlZENvZGVCbG9jay0+bmVlZHNGdWxsU2NvcGVDaGFpbigpICYm IHVubGlua2VkQ29kZUJsb2NrLT5jb2RlVHlwZSgpID09IEZ1bmN0aW9uQ29kZSkKKyAgICAsIG1f bnVtQnJlYWtwb2ludHNTZXQoMCkKICAgICAsIG1fc291cmNlKHNvdXJjZVByb3ZpZGVyKQogICAg ICwgbV9zb3VyY2VPZmZzZXQoc291cmNlT2Zmc2V0KQogICAgICwgbV9maXJzdExpbmVDb2x1bW5P ZmZzZXQoZmlyc3RMaW5lQ29sdW1uT2Zmc2V0KQpAQCAtMzQxOSw2ICszNDIxLDI0IEBAIFN0cmlu ZyBDb2RlQmxvY2s6Om5hbWVGb3JSZWdpc3RlcihWaXJ0dWEKICAgICByZXR1cm4gIiI7CiB9CiAK K3ZvaWQgQ29kZUJsb2NrOjpzZXRCcmVha3BvaW50QXQodW5zaWduZWQgb2Zmc2V0LCBib29sIGVu YWJsZU9yTm90KQoreworICAgIHN0YXRpYyBjb25zdCBpbnQgaGFzQnJlYWtwb2ludEZsYWdPZmZz ZXQgPSAyOyAKKyAgICBBU1NFUlQoZ2xvYmFsT2JqZWN0KCktPnZtKCkuaW50ZXJwcmV0ZXItPmdl dE9wY29kZUlEKGluc3RydWN0aW9ucygpW29mZnNldF0udS5vcGNvZGUpID09IG9wX2RlYnVnKTsK KyAgICBBU1NFUlQoaW5zdHJ1Y3Rpb25zKClbb2Zmc2V0ICsgaGFzQnJlYWtwb2ludEZsYWdPZmZz ZXRdLnUub3BlcmFuZCAhPSBlbmFibGVPck5vdCk7CisgICAgaW5zdHJ1Y3Rpb25zKClbb2Zmc2V0 ICsgaGFzQnJlYWtwb2ludEZsYWdPZmZzZXRdLnUub3BlcmFuZCA9IGVuYWJsZU9yTm90OworICAg IG1fbnVtQnJlYWtwb2ludHNTZXQgKz0gKGVuYWJsZU9yTm90ID8gMSA6IC0xKTsKKyAgICBBU1NF UlQobV9udW1CcmVha3BvaW50c1NldCA+PSAwKTsKK30KKwordm9pZCBDb2RlQmxvY2s6OmNsZWFy QWxsQnJlYWtwb2ludHMoKQoreworICAgIGlmICghbV9udW1CcmVha3BvaW50c1NldCkKKyAgICAg ICAgcmV0dXJuOworICAgIHVubGlua2VkQ29kZUJsb2NrKCktPmNsZWFyQWxsQnJlYWtwb2ludHNJ bihpbnN0cnVjdGlvbnMoKSk7CisgICAgbV9udW1CcmVha3BvaW50c1NldCA9IDA7Cit9CisKIG5h bWVzcGFjZSB7CiAKIHN0cnVjdCBWZXJpZnlDYXB0dXJlZERlZiB7CkluZGV4OiBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvYnl0ZWNvZGUvQ29kZUJsb2NrLmgKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0ph dmFTY3JpcHRDb3JlL2J5dGVjb2RlL0NvZGVCbG9jay5oCShyZXZpc2lvbiAxNjIzMDIpCisrKyBT b3VyY2UvSmF2YVNjcmlwdENvcmUvYnl0ZWNvZGUvQ29kZUJsb2NrLmgJKHdvcmtpbmcgY29weSkK QEAgLTg3MCw2ICs4NzAsOSBAQCBwdWJsaWM6CiAKICAgICB1bnNpZ25lZCBmcmFtZVJlZ2lzdGVy Q291bnQoKTsKIAorICAgIHZvaWQgc2V0QnJlYWtwb2ludEF0KHVuc2lnbmVkIG9mZnNldCwgYm9v bCBlbmFibGVPck5vdCk7CisgICAgdm9pZCBjbGVhckFsbEJyZWFrcG9pbnRzKCk7CisKICAgICAv LyBGSVhNRTogTWFrZSB0aGVzZSByZW1haW5pbmcgbWVtYmVycyBwcml2YXRlLgogCiAgICAgaW50 IG1fbnVtQ2FsbGVlUmVnaXN0ZXJzOwpAQCAtMTAxOSw2ICsxMDIyLDcgQEAgcHJpdmF0ZToKICAg ICBib29sIG1fbmVlZHNBY3RpdmF0aW9uOwogICAgIGJvb2wgbV9tYXlCZUV4ZWN1dGluZzsKICAg ICB1aW50OF90IG1fdmlzaXRBZ2dyZWdhdGVIYXNCZWVuQ2FsbGVkOworICAgIGludCBtX251bUJy ZWFrcG9pbnRzU2V0OwogCiAgICAgUmVmUHRyPFNvdXJjZVByb3ZpZGVyPiBtX3NvdXJjZTsKICAg ICB1bnNpZ25lZCBtX3NvdXJjZU9mZnNldDsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9i eXRlY29kZS9VbmxpbmtlZENvZGVCbG9jay5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFT Y3JpcHRDb3JlL2J5dGVjb2RlL1VubGlua2VkQ29kZUJsb2NrLmNwcAkocmV2aXNpb24gMTYyMzAy KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL1VubGlua2VkQ29kZUJsb2NrLmNw cAkod29ya2luZyBjb3B5KQpAQCAtMzk0LDYgKzM5NCwxNyBAQCB1bnNpZ25lZCBVbmxpbmtlZENv ZGVCbG9jazo6b3BEZWJ1Z0J5dGVjCiAgICAgcmV0dXJuIG9mZnNldDsKIH0KIAordm9pZCBVbmxp bmtlZENvZGVCbG9jazo6Y2xlYXJBbGxCcmVha3BvaW50c0luKFJlZkNvdW50ZWRBcnJheTxJbnN0 cnVjdGlvbj4mIGluc3RydWN0aW9ucykKK3sKKyAgICBMaW5lQ29sdW1uSW5mb0xpc3QmIGxpc3Qg PSBvcERlYnVnTGluZUNvbHVtbkluZm9MaXN0KCk7CisgICAgZm9yIChMaW5lQ29sdW1uSW5mb0xp c3Q6Oml0ZXJhdG9yIGl0ID0gbGlzdC5iZWdpbigpOyBpdCAhPSBsaXN0LmVuZCgpOyArK2l0KSB7 CisgICAgICAgIHN0YXRpYyBjb25zdCBpbnQgaGFzQnJlYWtwb2ludEZsYWdPZmZzZXQgPSAyOyAK KyAgICAgICAgdW5zaWduZWQgb2Zmc2V0ID0gaXQtPmluc3RydWN0aW9uT2Zmc2V0OworICAgICAg ICBBU1NFUlQobV92bS0+aW50ZXJwcmV0ZXItPmdldE9wY29kZUlEKGluc3RydWN0aW9uc1tvZmZz ZXRdLnUub3Bjb2RlKSA9PSBvcF9kZWJ1Zyk7CisgICAgICAgIGluc3RydWN0aW9uc1tvZmZzZXQg KyBoYXNCcmVha3BvaW50RmxhZ09mZnNldF0gPSBmYWxzZTsKKyAgICB9Cit9CisKIHN0YXRpYyBi b29sIGNvbXBhcmVMaW5lQ29sdW1uSW5mbyhjb25zdCBMaW5lQ29sdW1uSW5mbyYgZmlyc3QsIGNv bnN0IExpbmVDb2x1bW5JbmZvJiBzZWNvbmQpCiB7CiAgICAgcmV0dXJuIGZpcnN0IDwgc2Vjb25k OwpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL1VubGlua2VkQ29kZUJsb2Nr LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL1VubGlua2Vk Q29kZUJsb2NrLmgJKHJldmlzaW9uIDE2MjMwMikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9i eXRlY29kZS9VbmxpbmtlZENvZGVCbG9jay5oCSh3b3JraW5nIGNvcHkpCkBAIC00NDYsNiArNDQ2 LDcgQEAgcHVibGljOgogICAgIHZvaWQgZXhwcmVzc2lvblJhbmdlRm9yQnl0ZWNvZGVPZmZzZXQo dW5zaWduZWQgYnl0ZWNvZGVPZmZzZXQsIGludCYgZGl2b3QsCiAgICAgICAgIGludCYgc3RhcnRP ZmZzZXQsIGludCYgZW5kT2Zmc2V0LCB1bnNpZ25lZCYgbGluZSwgdW5zaWduZWQmIGNvbHVtbik7 CiAgICAgdW5zaWduZWQgb3BEZWJ1Z0J5dGVjb2RlT2Zmc2V0Rm9yTGluZUFuZENvbHVtbih1bnNp Z25lZCYgbGluZSwgdW5zaWduZWQmIGNvbHVtbik7CisgICAgdm9pZCBjbGVhckFsbEJyZWFrcG9p bnRzSW4oUmVmQ291bnRlZEFycmF5PEluc3RydWN0aW9uPiYpOwogCiAgICAgdm9pZCByZWNvcmRQ YXJzZShDb2RlRmVhdHVyZXMgZmVhdHVyZXMsIGJvb2wgaGFzQ2FwdHVyZWRWYXJpYWJsZXMsIHVu c2lnbmVkIGZpcnN0TGluZSwgdW5zaWduZWQgbGluZUNvdW50LCB1bnNpZ25lZCBlbmRDb2x1bW4p CiAgICAgewo=