446317 - NS_InvokeByIndex_P (xptcinvoke_gcc_x86_unix.cpp) screws up stack alignment; causes segfault in SSE2 instructions

Status: RESOLVED DUPLICATE of bug 410509

(Core :: XPConnect, defect)

Description

[Ed Catmur]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.0.1) Gecko/2008071903 (Gentoo) Firefox/3.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en; rv:1.9.0.1) Gecko/20080528 (Gentoo) Epiphany/2.22 Firefox/3.0 

Epiphany crashes inside libjpeg code when an image is displayed by a JS callback.

Reproducible: Always

Steps to Reproduce:
1. Open http://maps.google.co.uk/; wait to load
2. Click on collapser between sidebar and map
Actual Results:  
Crash in libjpeg code

Expected Results:  
Not crash.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb6a6a700 (LWP 19137)]
0xb5a086c8 in jpeg_make_d_derived_tbl (cinfo=0x9e74418, isDC=0, tblno=0, 
    pdtbl=0xa077b2c) at ./jdhuff.c:188
188	      huffsize[p++] = (char) l;

This line number is incorrect; it appears the debug information may be a little screwed up (libjpeg was compiled with -O3 optimisations).  The actual line number is:
201           huffcode[p++] = code;

The actual instruction:
(gdb) disass 0xb5a08600 0xb5a08700
Dump of assembler code from 0xb5a08600 to 0xb5a08700:
0xb5a08600 <jpeg_make_d_derived_tbl+240>:	mov    %edi,-0x520(%ebp)
0xb5a08606 <jpeg_make_d_derived_tbl+246>:	jle    0xb5a08619 <jpeg_make_d_derived_tbl+265>
0xb5a08608 <jpeg_make_d_derived_tbl+248>:	mov    0x8(%ebp),%edx
0xb5a0860b <jpeg_make_d_derived_tbl+251>:	mov    (%edx),%eax
0xb5a0860d <jpeg_make_d_derived_tbl+253>:	movl   $0x9,0x14(%eax)
0xb5a08614 <jpeg_make_d_derived_tbl+260>:	mov    %edx,(%esp)
0xb5a08617 <jpeg_make_d_derived_tbl+263>:	call   *(%eax)
0xb5a08619 <jpeg_make_d_derived_tbl+265>:	test   %esi,%esi
0xb5a0861b <jpeg_make_d_derived_tbl+267>:	je     0xb5a08843 <jpeg_make_d_derived_tbl+819>
0xb5a08621 <jpeg_make_d_derived_tbl+273>:	movzbl -0x581(%ebp),%eax
0xb5a08628 <jpeg_make_d_derived_tbl+280>:	dec    %esi
0xb5a08629 <jpeg_make_d_derived_tbl+281>:	movzbl -0x578(%ebp),%edi
0xb5a08630 <jpeg_make_d_derived_tbl+288>:	mov    %eax,-0x5b0(%ebp)
0xb5a08636 <jpeg_make_d_derived_tbl+294>:	mov    -0x59c(%ebp),%eax
0xb5a0863c <jpeg_make_d_derived_tbl+300>:	add    -0x51c(%ebp),%eax
0xb5a08642 <jpeg_make_d_derived_tbl+306>:	mov    -0x5b0(%ebp),%ecx
0xb5a08648 <jpeg_make_d_derived_tbl+312>:	mov    %eax,%edx
0xb5a0864a <jpeg_make_d_derived_tbl+314>:	neg    %edx
0xb5a0864c <jpeg_make_d_derived_tbl+316>:	and    $0xf,%edx
0xb5a0864f <jpeg_make_d_derived_tbl+319>:	cmp    %ecx,%edx
0xb5a08651 <jpeg_make_d_derived_tbl+321>:	cmovbe %edx,%ecx
0xb5a08654 <jpeg_make_d_derived_tbl+324>:	mov    -0x51c(%ebp),%edx
0xb5a0865a <jpeg_make_d_derived_tbl+330>:	mov    %ecx,-0x568(%ebp)
0xb5a08660 <jpeg_make_d_derived_tbl+336>:	mov    %edx,-0x57c(%ebp)
0xb5a08666 <jpeg_make_d_derived_tbl+342>:	xor    %edx,%edx
0xb5a08668 <jpeg_make_d_derived_tbl+344>:	test   %ecx,%ecx
0xb5a0866a <jpeg_make_d_derived_tbl+346>:	je     0xb5a08697 <jpeg_make_d_derived_tbl+391>
0xb5a0866c <jpeg_make_d_derived_tbl+348>:	lea    0x0(%esi,%eiz,1),%esi
0xb5a08670 <jpeg_make_d_derived_tbl+352>:	mov    %edi,%ecx
0xb5a08672 <jpeg_make_d_derived_tbl+354>:	dec    %esi
0xb5a08673 <jpeg_make_d_derived_tbl+355>:	inc    %edx
0xb5a08674 <jpeg_make_d_derived_tbl+356>:	mov    %cl,(%eax)
0xb5a08676 <jpeg_make_d_derived_tbl+358>:	inc    %eax
0xb5a08677 <jpeg_make_d_derived_tbl+359>:	incl   -0x57c(%ebp)
0xb5a0867d <jpeg_make_d_derived_tbl+365>:	cmp    %edx,-0x568(%ebp)
0xb5a08683 <jpeg_make_d_derived_tbl+371>:	ja     0xb5a08670 <jpeg_make_d_derived_tbl+352>
0xb5a08685 <jpeg_make_d_derived_tbl+373>:	mov    -0x5b0(%ebp),%eax
0xb5a0868b <jpeg_make_d_derived_tbl+379>:	cmp    %eax,-0x568(%ebp)
---Type <return> to continue, or q <return> to quit---
0xb5a08691 <jpeg_make_d_derived_tbl+385>:	je     0xb5a08823 <jpeg_make_d_derived_tbl+787>
0xb5a08697 <jpeg_make_d_derived_tbl+391>:	mov    -0x5b0(%ebp),%edx
0xb5a0869d <jpeg_make_d_derived_tbl+397>:	sub    -0x568(%ebp),%edx
0xb5a086a3 <jpeg_make_d_derived_tbl+403>:	mov    %edx,-0x564(%ebp)
0xb5a086a9 <jpeg_make_d_derived_tbl+409>:	shr    $0x4,%edx
0xb5a086ac <jpeg_make_d_derived_tbl+412>:	mov    %edx,-0x560(%ebp)
0xb5a086b2 <jpeg_make_d_derived_tbl+418>:	shl    $0x4,%edx
0xb5a086b5 <jpeg_make_d_derived_tbl+421>:	mov    %edx,-0x55c(%ebp)
0xb5a086bb <jpeg_make_d_derived_tbl+427>:	je     0xb5a0880b <jpeg_make_d_derived_tbl+763>
0xb5a086c1 <jpeg_make_d_derived_tbl+433>:	xorps  %xmm0,%xmm0
0xb5a086c4 <jpeg_make_d_derived_tbl+436>:	mov    %edi,%eax
0xb5a086c6 <jpeg_make_d_derived_tbl+438>:	mov    %edi,%ecx
0xb5a086c8 <jpeg_make_d_derived_tbl+440>:	movaps %xmm0,-0x558(%ebp)
0xb5a086cf <jpeg_make_d_derived_tbl+447>:	mov    %al,-0x558(%ebp)
0xb5a086d5 <jpeg_make_d_derived_tbl+453>:	mov    -0x558(%ebp),%edx
0xb5a086db <jpeg_make_d_derived_tbl+459>:	mov    %cl,%dh
0xb5a086dd <jpeg_make_d_derived_tbl+461>:	mov    %edx,-0x558(%ebp)
0xb5a086e3 <jpeg_make_d_derived_tbl+467>:	movzbl %al,%edx
0xb5a086e6 <jpeg_make_d_derived_tbl+470>:	mov    -0x558(%ebp),%eax
0xb5a086ec <jpeg_make_d_derived_tbl+476>:	mov    %edx,%ecx
0xb5a086ee <jpeg_make_d_derived_tbl+478>:	shl    $0x18,%edx
0xb5a086f1 <jpeg_make_d_derived_tbl+481>:	shl    $0x10,%ecx
0xb5a086f4 <jpeg_make_d_derived_tbl+484>:	mov    %edx,-0x5b8(%ebp)
0xb5a086fa <jpeg_make_d_derived_tbl+490>:	and    $0xff00ffff,%eax
0xb5a086ff <jpeg_make_d_derived_tbl+495>:	or     %ecx,%eax
End of assembler dump.

That is:
0xb5a086c8 <jpeg_make_d_derived_tbl+440>:	movaps %xmm0,-0x558(%ebp)

%ebp points to accessible memory:
(gdb) p $ebp
$42 = (void *) 0xbffbcb54

The seg happens because movaps is a SSE2 instruction, Move Aligned Packed Single-Precision Floating-Point Values[1], and requires that its memory operands be 16-byte aligned.

(gdb) p (int)$ebp % 16
$43 = -12

-0x558(%ebp) is in the local 
  unsigned int huffcode[257];
(gdb) p &huffcode
$44 = (unsigned int (*)[257]) 0xbffbc644
which is misaligned:
(gdb) p (int)&huffcode % 16
$45 = -12

and indeed esp itself is misaligned:
(gdb) p (int)$esp % 16
$46 = -12

Searching the stack:

(gdb) bt
#0  0xb5a086c8 in jpeg_make_d_derived_tbl (cinfo=0x9e74418, isDC=0, tblno=0, 
    pdtbl=0xa077b2c) at ./jdhuff.c:188
#1  0xb5a08c34 in start_pass_huff_decoder (cinfo=0x9e74418) at ./jdhuff.c:108
#2  0xb5a04646 in start_input_pass (cinfo=0x9e74418) at ./jdinput.c:258
#3  0xb5a0414b in jinit_master_decompress (cinfo=0x9e74418) at ./jdmaster.c:399
#4  0xb5a031a0 in jpeg_start_decompress (cinfo=0x9e74418) at ./jdapistd.c:42
#5  0xb5a4c4f2 in nsJPEGDecoder::ProcessData (this=0x9e74400, 
    data=0xa27fc48 "����", count=24088, writeCount=0xbffbcdb8)
    at nsJPEGDecoder.cpp:534
#6  0xb5a4c9fa in ReadDataOut (in=0x9e710c8, closure=0x9e74400, 
    fromRawSegment=0xa27fc48 "����", toOffset=0, count=24088, 
    writeCount=0xbffbcdb8) at nsJPEGDecoder.cpp:248
#7  0xb672e253 in nsStringInputStream::ReadSegments (this=0x9e710c8, 
    writer=0xb5a4c9e2 <ReadDataOut>, closure=0x9e74400, aCount=16, 
    result=0xbffbcdb8) at nsStringStream.cpp:276
#8  0xb5a4bc9c in nsJPEGDecoder::WriteFrom (this=0x9e74400, inStr=0x10, 
    count=24088, writeCount=0xbffbcdb8) at nsJPEGDecoder.cpp:266
#9  0xb5a41f55 in imgContainer::ReloadImages (this=0xa7d00d0)
    at imgContainer.cpp:1493
#10 0xb5a420d9 in imgContainer::RestoreDiscardedData (this=0xa7d00d0)
    at imgContainer.cpp:1292
#11 0xb5a42258 in imgContainer::GetCurrentFrameNoRef (this=0xa7d00d0)
    at imgContainer.cpp:168
#12 0xb5a423b8 in imgContainer::GetCurrentFrame (this=0xa7d00d0, 
    aCurrentFrame=0xbffbce6c) at imgContainer.cpp:184
#13 0xb5a469de in imgRequest::NotifyProxyListener (this=0x9bedd68, 
    proxy=0x8e50118) at imgRequest.cpp:219
#14 0xb5a4545b in imgLoader::LoadImage (this=0x92324c8, aURI=0xa1498a8, 
    aInitialDocumentURI=0x8c73f80, aReferrerURI=0x8c73f80, 
    aLoadGroup=0x9517ca8, aObserver=0x9b83228, aCX=0x93fc238, 
    aLoadFlags=<value optimized out>, cacheKey=0x0, aRequest=0x0, 
    _retval=0x9b83230) at imgLoader.cpp:550
#15 0xb60ad489 in nsContentUtils::LoadImage (aURI=0xa1498a8, 
    aLoadingDocument=0x93fc238, aLoadingPrincipal=0x8c63e18, 
    aReferrer=0x8c73f80, aObserver=0x9b83228, aLoadFlags=0, aRequest=0x9b83230)
    at nsContentUtils.cpp:2329
#16 0xb60db1f3 in nsImageLoadingContent::LoadImage (this=0x9b83228, 
    aNewURI=0xa1498a8, aForce=1, aNotify=1, aDocument=0x93fc238, aLoadFlags=0)
    at nsImageLoadingContent.cpp:587
#17 0xb60db3aa in nsImageLoadingContent::LoadImage (this=0x9b83228, 
    aNewURI=@0xbffbd408, aForce=1, aNotify=1) at nsImageLoadingContent.cpp:491
#18 0xb6135867 in nsHTMLImageElement::SetAttr (this=0x9b83210, aNameSpaceID=0, 
    aName=0x8c25070, aPrefix=0x0, aValue=@0xbffbd408, aNotify=1)
---Type <return> to continue, or q <return> to quit---
    at nsHTMLImageElement.cpp:502
#19 0xb611feb4 in nsGenericHTMLElement::SetAttr (this=0x9b83210, 
    aNameSpaceID=0, aName=0x8c25070, aValue=@0xbffbd408, aNotify=1)
    at nsGenericHTMLElement.h:206
#20 0xb611999a in nsGenericHTMLElement::SetAttrHelper (this=0x9b83210, 
    aAttr=0x8c25070, aValue=@0xbffbd408) at nsGenericHTMLElement.cpp:2300
#21 0xb613603b in nsHTMLImageElement::SetSrc (this=0x9b83210, 
    aValue=@0xbffbd408) at nsHTMLImageElement.cpp:216
#22 0xb674b68c in NS_InvokeByIndex_P ()
   from /usr/lib/xulrunner-1.9/libxpcom_core.so
#23 0xb64eb8dc in XPCWrappedNative::CallMethod (ccx=@0xbffbd394, 
    mode=XPCWrappedNative::CALL_SETTER) at xpcwrappednative.cpp:2393
#24 0xb64f28fb in XPC_WN_GetterSetter (cx=0x9536200, obj=0x9e240c0, argc=1, 
    argv=0x99d8458, vp=0xbffbd4d4) at xpcprivate.h:2257
#25 0xb6669725 in js_Invoke (cx=0x9536200, argc=1, vp=0x99d8450, flags=2)
    at jsinterp.c:1297
#26 0xb6669a80 in js_InternalInvoke (cx=0x9536200, obj=0x9e240c0, 
    fval=164825088, flags=0, argc=1, argv=0xbffbd8f4, rval=0xbffbd8f4)
    at jsinterp.c:1369
#27 0xb6669b87 in js_InternalGetOrSet (cx=0x9536200, obj=0x9e240c0, 
    id=148701604, fval=164825088, mode=JSACC_WRITE, argc=1, argv=0xbffbd8f4, 
    rval=0xbffbd8f4) at jsinterp.c:1427
#28 0xb6671022 in js_SetPropertyHelper (cx=0x9536200, obj=0x9e240c0, 
    id=148701604, vp=0xbffbd8f4, entryp=0xbffbd8dc) at jsobj.c:3846
#29 0xb665d8dc in js_Interpret (cx=0x9536200) at jsinterp.c:4511
#30 0xb6669774 in js_Invoke (cx=0x9536200, argc=0, vp=0x99d8438, flags=0)
    at jsinterp.c:1313
#31 0xb6658642 in fun_call (cx=0x9536200, argc=0, vp=0x99d8424) at jsfun.c:1589
#32 0xb665d9ed in js_Interpret (cx=0x9536200) at jsinterp.c:4835
#33 0xb6669774 in js_Invoke (cx=0x9536200, argc=1, vp=0x99d8418, flags=0)
    at jsinterp.c:1313
#34 0xb6669a80 in js_InternalInvoke (cx=0x9536200, obj=0x8f4ad00, 
    fval=168228864, flags=0, argc=1, argv=0x9f1ea68, rval=0xbffbdf00)
    at jsinterp.c:1369
#35 0xb66354fc in JS_CallFunctionValue (cx=0x9536200, obj=0x8f4ad00, 
    fval=168228864, argc=1, argv=0x9f1ea68, rval=0xbffbdf00) at jsapi.c:5054
#36 0xb61dfd1d in nsJSContext::CallEventHandler (this=0x948c8c0, 
    aTarget=0x8c7dd30, aScope=0x8f4ad00, aHandler=0xa06f800, aargv=0x9e305cc, 
    arv=0xbffbdffc) at nsJSEnvironment.cpp:1962
#37 0xb61f501c in nsGlobalWindow::RunTimeout (this=0x8c7dd30, 
    aTimeout=0x9d009d8) at nsGlobalWindow.cpp:7894
#38 0xb61f5458 in nsGlobalWindow::TimerCallback (aTimer=0x9b677f8, 
    aClosure=0x9d009d8) at nsGlobalWindow.cpp:8225
---Type <return> to continue, or q <return> to quit---
#39 0xb6741c39 in nsTimerImpl::Fire (this=0x9b677f8) at nsTimerImpl.cpp:400
#40 0xb6741ce3 in nsTimerEvent::Run (this=0xae6005a0) at nsTimerImpl.cpp:490
#41 0xb673f75c in nsThread::ProcessNextEvent (this=0x8c22c80, mayWait=0, 
    result=0xbffbe108) at nsThread.cpp:510
#42 0xb670ffa7 in NS_ProcessPendingEvents_P (thread=0x8c22c80, timeout=20)
    at nsThreadUtils.cpp:180
#43 0xb5bde2d4 in nsBaseAppShell::NativeEventCallback (this=0x9481400)
    at nsBaseAppShell.cpp:121
#44 0xb5bc9bdc in nsAppShell::EventProcessorCallback (source=0x94ea6f0, 
    condition=G_IO_IN, data=0x9481400) at nsAppShell.cpp:69
#45 0xb767002d in g_io_unix_dispatch (source=0x94ea748, 
    callback=0xb5bc9bb0 <nsAppShell::EventProcessorCallback(_GIOChannel*, GIOCondition, void*)>, user_data=0x9481400) at giounix.c:162
#46 0xb762fd60 in g_main_dispatch (context=0x8bbe5c8) at gmain.c:2068
#47 0xb7631bfd in g_main_context_iterate (context=0x8bbe5c8, block=1, 
    dispatch=1, self=0x8b98068) at gmain.c:2620
#48 0xb7632112 in IA__g_main_loop_run (loop=0x9270860) at gmain.c:2924
#49 0xb7a992a9 in IA__gtk_main () at gtkmain.c:1169
#50 0x08084d4a in main (argc=1, argv=0xbffbf4e4) at ephy-main.c:742

we find that esp for outer stack frames is aligned to 16 bytes:

(gdb) frame 23
#23 0xb64eb8dc in XPCWrappedNative::CallMethod (ccx=@0xbffbd394, 
    mode=XPCWrappedNative::CALL_SETTER) at xpcwrappednative.cpp:2393
2393	                                        dispatchParams);
Current language:  auto; currently c++
(gdb) p (int)$esp % 16
$47 = 0

The culprit is NS_InvokeByIndex_P:
(gdb) frame 22
#22 0xb674b68c in NS_InvokeByIndex_P ()
   from /usr/lib/xulrunner-1.9/libxpcom_core.so
(gdb) p (int)$esp % 16
$48 = -12

I'm not too hot on alignment requirements for calling conventions, but if I understand [2] correctly, on x86-Linux the initial stack frame in a thread should be 16-byte aligned and subsequent stack frames will preserve that alignment, but gcc will not generate code to realign stack frames after they have been screwed up.

The commenter on bug 195736 comment 5 says:[3]
> Also note that gcc 3.2.2 tries hard to maintain stack alignment but xptcinvoke
> messes it up and there's no way at the moment to fix that. It may be a prblem
> down the road.

Note: this has only exhibited recently, after compiling libjpeg with gcc 4.3.1; perhaps earlier versions were less agressive about using SSE2 instructions when optimising.

1. http://www.ews.uiuc.edu/~cjiang/reference/vc181.htm
2. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10395
3. https://bugzilla.mozilla.org/show_bug.cgi?id=195736#c5

Comment 1

[Ed Catmur]

Attachment: xptcinvoke-align.patch

This seems to work; it prevents the above crash and doesn't appear to break anything else.