[webkit-reviews] review granted: [Bug 88692] DFG should be able to set watchpoints on global variables : [Attachment 146996] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 88692: DFG should be able to set watchpoints on global variables
https://bugs.webkit.org/show_bug.cgi?id=88692

Attachment 146996: the patch
https://bugs.webkit.org/attachment.cgi?id=146996&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
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.