keeleysam / tenfourfox

Automatically exported from code.google.com/p/tenfourfox
0 stars 0 forks source link

MacroAssemblerPPC.h asserts on certain JavaScript conformance tests #211

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Duplicate of http://code.google.com/p/aurorafox/issues/detail?id=34

Starting program: /Volumes/BruceDeuce/src/mozilla-20b/obj-ff-dbg/dist/bin/js -f 
js/src/tests/shell.js -f js/src/tests/js1_5/Regress/regress-280769-1.js
[...]
BUGNUMBER: 280769
STATUS: Do not crash on overflow of 64K boundary of [] offset in regexp search 
string 
Assertion failure: src != addressTempRegister, at 
/Volumes/BruceDeuce/src/mozilla-20b/js/src/assembler/assembler/MacroAssemblerPPC
.h:170

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x004f3168 in JSC::MacroAssemblerPPC::add32 () at Vector.h:326
326             JS_ASSERT(!entered);

Note that even in 20, the JIT tests still pass (on slow systems there is one 
timeout, but the test actually does pass).

Original issue reported on code.google.com by classi...@floodgap.com on 7 Mar 2013 at 2:30

GoogleCodeExporter commented 9 years ago
(gdb) bt
#0  0x004f3168 in JSC::MacroAssemblerPPC::add32 () at Vector.h:326
#1  0x0044c8d8 in JSC::MacroAssemblerPPC::add32 (this=0xbfffccb8, imm={m_value 
= -200002}, dest=r12) at MacroAssemblerPPC.h:170
#2  0x0044cb64 in JSC::MacroAssemblerPPC::load32WithUnalignedHalfWords 
(this=0xbfffccb8, address={base = r3, index = r4, scale = TimesTwo, offset = 
0}, dest=r7) at MacroAssemblerPPC.h:571
#3  0x0044e394 in 
JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::generatePatternChara
cterOnce (this=0x7, opIndex=1) at 
/Volumes/BruceDeuce/src/mozilla-20b/js/src/yarr/YarrJIT.cpp:898

Original comment by classi...@floodgap.com on 7 Mar 2013 at 2:38

GoogleCodeExporter commented 9 years ago
This is our problem:

JSC::MacroAssemblerPPC::add32 (this=0xbfffccb8, imm={m_value = -200002}, 
dest=r12)

How did that happen? We need to look at 
MacroAssemblerPPC::load32WithUnalignedHalfWords.

Original comment by classi...@floodgap.com on 7 Mar 2013 at 2:40

GoogleCodeExporter commented 9 years ago
Actually, a better solution is to account for the case in add32().

Original comment by classi...@floodgap.com on 7 Mar 2013 at 2:45

GoogleCodeExporter commented 9 years ago
% hg diff js/src/assembler/assembler/MacroAssemblerPPC.h
diff --git a/js/src/assembler/assembler/MacroAssemblerPPC.h 
b/js/src/assembler/assembler/MacroAssemblerPPC.h
--- a/js/src/assembler/assembler/MacroAssemblerPPC.h
+++ b/js/src/assembler/assembler/MacroAssemblerPPC.h
@@ -161,18 +161,20 @@ js::JaegerSpew(js::JSpew_Insns, ISPFX "=
         add32(imm, dest, dest);
     }

     void add32(TrustedImm32 imm, RegisterID src, RegisterID dest)
     {
 js::JaegerSpew(js::JSpew_Insns, ISPFX "== add32(TrustedImm32 imm, RegisterID src, RegisterID dest) ==\n");
         if (PPC_IMM_OK_S(imm) && src != tempRegister) {
             m_assembler.addi(dest, src, int16_t(imm.m_value & 0xffff));
+        } else if (src == addressTempRegister) { // TenFourFox issue 211
+            move(imm, tempRegister);
+            m_assembler.add(dest, src, tempRegister);
         } else {
-            ASSERT(src != addressTempRegister);
             move(imm, addressTempRegister);
             m_assembler.add(dest, src, addressTempRegister);
         }
     }

     void add32(TrustedImm32 imm, Address address)
     {
 js::JaegerSpew(js::JSpew_Insns, ISPFX "== add32(TrustedImm32 imm, Address address) ==\n");

Ben, tell me if you foresee any problems with this. The test passes, natch.

Original comment by classi...@floodgap.com on 7 Mar 2013 at 3:21

GoogleCodeExporter commented 9 years ago
Since 17 is theoretically susceptible to this bug, we should backport it there 
too (especially since it would just behave badly instead of asserting).

Original comment by classi...@floodgap.com on 7 Mar 2013 at 4:56

GoogleCodeExporter commented 9 years ago
Landed on all. Ben, feel free to comment if you think this was the wrong fix, 
but all tests pass.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 3:56