Summary: | DFG should be able to set watchpoints on global variables | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||||||||||||||||||||||||||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | ggaren, gustavo, milian.wolff, philn, rakuco, webkit.review.bot, xan.lopez | ||||||||||||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||||||||||||||||||
Bug Depends on: | 88976 | ||||||||||||||||||||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Filip Pizlo
2012-06-08 16:42:55 PDT
Created attachment 146663 [details]
work in progress
Created attachment 146684 [details]
more things
Created attachment 146687 [details]
did some more stuff
Messed around with the interface between global variable creation, global variable access, and watch pointing. It's starting to make sense.
Still more code to write, though.
Created attachment 146716 [details]
even more
I just have to link together a few more things and it should start working.
Created attachment 146721 [details]
more code
Still more stuff to do but it's getting there.
Created attachment 146747 [details]
it might be done
Now I need to compile it. And test it.
Created attachment 146749 [details]
omg it compiles!
Created attachment 146758 [details]
it does things
It looks like I can set a watchpoint on a global variable. Note I said "set", not "fire". I'm guessing it'll crash in hilarious ways if I actually try to fire a watchpoint.
(In reply to comment #8) > Created an attachment (id=146758) [details] > it does things > > It looks like I can set a watchpoint on a global variable. Note I said "set", not "fire". I'm guessing it'll crash in hilarious ways if I actually try to fire a watchpoint. Actually, it can fire the watchpoints as well. Fantastic! Still have to do way more tests though. Created attachment 146996 [details]
the patch
Comment on attachment 146996 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=146996&action=review r=me, please remember to ARM before landing. > Source/JavaScriptCore/assembler/RepatchBuffer.h:125 > + Boo. > Source/JavaScriptCore/assembler/X86Assembler.h:1737 > + // FIXME: need to know the size of the last watchpoint as well. No longer true. > Source/JavaScriptCore/assembler/X86Assembler.h:1739 > + while (UNLIKELY(static_cast<int>(result.m_offset) < m_indexOfTailOfLastWatchpoint)) { > + nop(); I think the ARMv7 code will also need to nop() pad in front of jumps, because of this case: before jump relaxation: watch_point big jump label: after jump relaxation: watch_point small jump label: [now inside the watchpoint] > Source/JavaScriptCore/assembler/X86Assembler.h:1823 > + int8_t *ptr = reinterpret_cast<int8_t*>(instructionStart); > + int8_t *dstPtr = reinterpret_cast<int8_t*>(to); > + intptr_t distance = (intptr_t)(dstPtr - (ptr + 5)); Move the "*", please. Created attachment 147154 [details]
rebased patch
Putting up for EWS.
Attachment 147154 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:583: The parameter name "constantMode" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:583: The parameter name "functionMode" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/bytecode/Watchpoint.h:52: The parameter name "linkBuffer" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 10 in 62 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 147154 [details] rebased patch Attachment 147154 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12945666 Comment on attachment 147154 [details] rebased patch Attachment 147154 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12939819 Created attachment 147193 [details]
the patch
Fixed Mac build issues. I think.
Attachment 147193 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:583: The parameter name "constantMode" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:583: The parameter name "functionMode" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 9 in 60 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 147193 [details] the patch Attachment 147193 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12944699 Comment on attachment 147193 [details] the patch Attachment 147193 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12951098 Comment on attachment 147193 [details] the patch Attachment 147193 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12948609 Comment on attachment 147193 [details] the patch Attachment 147193 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12941797 Created attachment 147207 [details]
backup
Still working on build systems, ports, crashes, etc.
Created attachment 147235 [details]
real patch
Still doing more things but it's starting to work.
Unlike the previous patch, this one actually includes all of the files I've added!
Created attachment 147238 [details]
ummmm please apply?
Rebased the patch in the hopes that the bots can apply it.
Attachment 147238 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:583: The parameter name "constantMode" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:583: The parameter name "functionMode" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/bytecode/Watchpoint.h:52: The parameter name "linkBuffer" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 10 in 68 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 147238 [details] ummmm please apply? Attachment 147238 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12941861 Created attachment 147245 [details]
attempting to fix Win
Attachment 147245 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 7 in 68 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 147245 [details] attempting to fix Win Attachment 147245 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12939939 Created attachment 147252 [details]
attempt to fix the compiler's failed attempts to be smart
Attachment 147252 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 7 in 68 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 147252 [details] attempt to fix the compiler's failed attempts to be smart Attachment 147252 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12941882 Created attachment 147255 [details]
fix link errors on Windows
Attachment 147255 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 7 in 69 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Landed in http://trac.webkit.org/changeset/120172 This commit breaks compilation on platforms where the ENALBE_JIT and hence ENABLE_ASSEMBLER is false (e.g. QNX). The reason is that MacroAssembler::Label is used in Watchpoint.h, but the former is not defined on such platforms (see dummy definition at the end of MacroAssembler in MacroAssembler.h). Should I create a new bug report for this? Created attachment 147393 [details]
more platform spoonfeeding
Attachment 147393 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:103: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:120: The parameter name "watchpointSet" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/SymbolTable.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/JavaScriptCore/runtime/SymbolTable.h:69: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Source/WTF/wtf/SegmentedVector.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/WTF/wtf/Assertions.h:386: UNREACHABLE_FOR_PLATFORM is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:805: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/assembler/X86Assembler.h:962: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 8 in 72 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Relanded in http://trac.webkit.org/changeset/120244 |