Bug#688086: Analysis

[Jurij Smakov]

I don't see iceape crashing immediately on startup, but it's fairly 
common for it to crash on any site which uses Javascript, so I'll 
assume that this is the bug you see, lacking other information. I 
tried iceweasel (10.0.7esr-2) and it crashes in exactly the same way.

Here's a backtrace:

#0  updateLastPath (label=..., linker=..., this=0xee1544c4) at 
/build/buildd-iceweasel_10.0.7esr-2-sparc-752pBb/iceweasel-10.0.7esr/js/src/methodjit/PolyIC.h:437
#1  SetPropCompiler::generateStub (this=0xffcd3ddc, initialShape=<optimized out>, shape=<optimized out>, adding=<optimized out>)    at /build/buildd-iceweasel_10.0.7esr-2-sparc-752pBb/iceweasel-10.0.7esr/js/src/methodjit/PolyIC.cpp:502
#2  0xf741d748 in SetPropCompiler::update (this=0xffcd3ddc) at /build/buildd-iceweasel_10.0.7esr-2-sparc-752pBb/iceweasel-10.0.7esr/js/src/methodjit/PolyIC.cpp:668
#3  0xf74124b4 in js::mjit::ic::SetProp (f=..., pic=0xee1544c4)    at /build/buildd-iceweasel_10.0.7esr-2-sparc-752pBb/iceweasel-10.0.7esr/js/src/methodjit/PolyIC.cpp:2058
#4  0xf746ce94 in JaegerStubVeneer () at /build/buildd-iceweasel_10.0.7esr-2-sparc-752pBb/iceweasel-10.0.7esr/js/src/methodjit/TrampolineSparc.s:164
#5  0xf746ce94 in JaegerStubVeneer () at /build/buildd-iceweasel_10.0.7esr-2-sparc-752pBb/iceweasel-10.0.7esr/js/src/methodjit/TrampolineSparc.s:164
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

The disassembled portion of updateLastPath where the crash happens:

   0xf741c764 <+2436>:  ldd  [ %fp + -1512 ], %g2
   0xf741c768 <+2440>:  ld  [ %g1 + 0x14 ], %g4
=> 0xf741c76c <+2444>:  std  %g2, [ %g1 + 0x30 ]

As far as I can tell, it translates to the following line of code in 
js/src/methodjit/PolyIC.h (struct PICInfo):

lastStubStart = JITCode(loc.executableAddress(), linker.size());

JITCode is a class which has two word-size members, m_start (pointer) 
and m_size (size_t). Compiler tries to store it as a double-word in 
one instruction, and this is only possible if the lastStubStart is 
8-bytes aligned. Offset of lastStubStart into struct PICInfo is 0x30 
(as can be seen above), so in order for it to be 8-bytes aligned, the 
whole PICInfo structure needs to be 8-bytes aligned. This is clearly 
not the case, this=0xee1544c4 passed to updateLastPath is PICInfo 
structure's address, and it's only 4-bytes aligned.

Trying to track down the place where PICInfo is allocated violating 
alignment requirements, I found the mjit::Compiler::finishThisUp 
(in js/src/methodjit/Compiler.cpp) which tries to construct some 
complex data structure by computing its size as dataSize, allocating a 
chunk of memory for it, then manually stuffing objects there, 
including PICInfo:

    [...]
    ic::PICInfo *jitPics = (ic::PICInfo *)cursor;
    jit->nPICs = pics.length();
    cursor += sizeof(ic::PICInfo) * jit->nPICs;
    for (size_t i = 0; i < jit->nPICs; i++) {
        new (&jitPics[i]) ic::PICInfo();
        [...]

Not surprisingly, this goes wrong at some point, and PICInfo structure 
occasionally gets placed at an insufficiently aligned address - I was 
able to confirm that by inserting an fprintf statement there to print 
out the adresses of jitPics[i] and corresponding lastStubStart object.

I don't have a solid proof that this is the cause of the problem, but 
it seems pretty likely, as any attempt of "manual" memory management 
like this increases the probability that alignment requirements get 
violated. I suggest notifying the iceweasel maintainer and reporting 
this upstream, because I don't really see a simple way to fix it. 

Best regards,
-- 
Jurij Smakov                                           jurij at wooyd.org
Key: http://www.wooyd.org/pgpkey/                      KeyID: C99E03CC