[webkit-reviews] review granted: [Bug 112380] Add runtime check for improper register allocations in DFG : [Attachment 193190] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 112380: Add runtime check for improper register allocations in DFG
https://bugs.webkit.org/show_bug.cgi?id=112380

Attachment 193190: Patch
https://bugs.webkit.org/attachment.cgi?id=193190&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=193190&action=review


r=me

> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:728
> +	       unsigned temp;
> +	       temp = offset1;
> +	       offset1 = offset2;
> +	       offset2 = temp;

Please use std::swap.

> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:770
> +    Vector<RegisterAllocationOffset, 10>
m_unsafeRegisterAllocationForBranchingOver;

I think it would be a little clearer just to call this, and related functions,
something like "registerAlloationOffsets". There's nothing inherently unsafe
about the data we're tracking -- we just happen to use the data to discover
unsafe actions.

> Source/WTF/wtf/Platform.h:856
> +#if !defined(ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION) && ENABLE(DFG_JIT)
> +#define ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION 0
> +#endif

Let's turn this on by default in debug builds, so it can help us catch bugs.