Bug 133340 – Arrayify neglects to inform the clobberizer that it might fire watchpoints

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

And the 32-bit LLInt gets array profiles all wrong, which is probably the reason why this only showed up in 32-bit debug, and it also makes testing this in general much harder.

[Filip Pizlo]

Created attachment 232172 [details]
the patch

[Mark Lam]

Comment on attachment 232172 [details]
the patch

r=me

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/169428

[Csaba Osztrogonác]

Comment on attachment 232172 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232172&action=review

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:1408
> -    loadp JSCell::m_structureID[t3], t2
> +    loadp t3, t2

It broke the ARM Thumb2 Linux build:
[  4%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o
/tmp/ccowkxrW.s: Assembler messages:
/tmp/ccowkxrW.s:3508: Error: cannot represent T32_OFFSET_IMM relocation in this object file format
make[2]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o] Error 1

And it broke the ARM Traditional Linux build:
[  4%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o
/tmp/ccJfKxfJ.s: Assembler messages:
/tmp/ccJfKxfJ.s:3449: Error: internal_relocation (type: OFFSET_IMM) not fixed up
make[2]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o] Error 1
make[1]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/all] Error 2

Didn't you mean move t3, t2 here as the other part of the patch?

[Filip Pizlo]

(In reply to comment #4)
> (From update of attachment 232172 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232172&action=review
> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:1408
> > -    loadp JSCell::m_structureID[t3], t2
> > +    loadp t3, t2
> 
> It broke the ARM Thumb2 Linux build:
> [  4%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o
> /tmp/ccowkxrW.s: Assembler messages:
> /tmp/ccowkxrW.s:3508: Error: cannot represent T32_OFFSET_IMM relocation in this object file format
> make[2]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o] Error 1
> 
> And it broke the ARM Traditional Linux build:
> [  4%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o
> /tmp/ccJfKxfJ.s: Assembler messages:
> /tmp/ccJfKxfJ.s:3449: Error: internal_relocation (type: OFFSET_IMM) not fixed up
> make[2]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o] Error 1
> make[1]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/all] Error 2
> 
> Didn't you mean move t3, t2 here as the other part of the patch?

Yes.

[Filip Pizlo]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 232172 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=232172&action=review
> > 
> > > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:1408
> > > -    loadp JSCell::m_structureID[t3], t2
> > > +    loadp t3, t2
> > 
> > It broke the ARM Thumb2 Linux build:
> > [  4%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o
> > /tmp/ccowkxrW.s: Assembler messages:
> > /tmp/ccowkxrW.s:3508: Error: cannot represent T32_OFFSET_IMM relocation in this object file format
> > make[2]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o] Error 1
> > 
> > And it broke the ARM Traditional Linux build:
> > [  4%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o
> > /tmp/ccJfKxfJ.s: Assembler messages:
> > /tmp/ccJfKxfJ.s:3449: Error: internal_relocation (type: OFFSET_IMM) not fixed up
> > make[2]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/llint/LowLevelInterpreter.cpp.o] Error 1
> > make[1]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/all] Error 2
> > 
> > Didn't you mean move t3, t2 here as the other part of the patch?
> 
> Yes.

Fixed in http://trac.webkit.org/changeset/169431