Last Comment Bug 747870 - Unaligned access in XPCCallContext::XPCCallContext
: Unaligned access in XPCCallContext::XPCCallContext
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: Sun Linux
-- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
: Andrew Overholt [:overholt]
Mentors:
http://bugs.debian.org/cgi-bin/bugrep...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-23 03:19 PDT by Mike Hommey [:glandium]
Modified: 2012-04-25 07:47 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Properly align XPCLazyCallContext::mData (1.81 KB, patch)
2012-04-23 03:20 PDT, Mike Hommey [:glandium]
bobbyholley: review+
Details | Diff | Splinter Review

Description User image Mike Hommey [:glandium] 2012-04-23 03:19:41 PDT
I got a report about Iceweasel crashing at startup on sparc. The relevant part of the backtrace is here:
#0  XPCCallContext::XPCCallContext (this=0xffff566c, callerLanguage=XPCContext::LANG_JS, cx=0xf7924340, callBeginRequest=0, obj=0xf0c9d900, 
    flattenedJSObject=0xf0c9d900, wrapper=0x0, tearOff=0x0)
    at js/xpconnect/src/XPCCallContext.cpp:83
#1  0xf6deef30 in XPCLazyCallContext::GetXPCCallContext (this=0xffff5648)
    at js/xpconnect/src/xpcprivate.h:1335

The problem comes from using the placement new operator with an array of char, which is not correctly aligned for the XPCCallContext class.

Thanks to Jurij Smakov for tracking down the root cause.
Comment 1 User image Mike Hommey [:glandium] 2012-04-23 03:20:57 PDT
Created attachment 617439 [details] [diff] [review]
Properly align XPCLazyCallContext::mData
Comment 2 User image :Ms2ger (⌚ UTC+1/+2) 2012-04-23 04:30:29 PDT
Comment on attachment 617439 [details] [diff] [review]
Properly align XPCLazyCallContext::mData

Review of attachment 617439 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/xpcprivate.h
@@ +1315,1 @@
>                                             mCallBeginRequest == CALL_BEGINREQUEST,

Fix indentation somehow, please.
Comment 3 User image Bobby Holley (:bholley) (busy with Stylo) 2012-04-23 04:49:39 PDT
Comment on attachment 617439 [details] [diff] [review]
Properly align XPCLazyCallContext::mData

This won't cause any extra code to be run on XPCLazyCallContext instantiation, right? These things need to be super fast, at least until the new DOM bindings are done.

r=bholley with the indentation fixed.
Comment 4 User image Mike Hommey [:glandium] 2012-04-23 05:07:23 PDT
No, it's just a POD type with the right alignment.
Comment 5 User image Mike Hommey [:glandium] 2012-04-23 06:16:42 PDT
(In reply to Ms2ger from comment #2)
> Comment on attachment 617439 [details] [diff] [review]
> Properly align XPCLazyCallContext::mData
> 
> Review of attachment 617439 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/xpconnect/src/xpcprivate.h
> @@ +1315,1 @@
> >                                             mCallBeginRequest == CALL_BEGINREQUEST,
> 
> Fix indentation somehow, please.

As indenting would make that line you quote go overboard, what style would you prefer?
mCcxToDestroy = mCcx = new (mData.addr())
    XPCCallContext(mCallerLanguage, (...)

mCcxToDestroy = mCcx =
    new (mData.addr())
        XPCCallContext(mCallerLanguage, (...)

char *data = mData.addr();
mCcxToDestroy = mCcx =
    new (data) XPCCallContext(mCallerLanguage, (...)

other ?

(The latter case allowing the current indentation being kept)
Comment 6 User image Mike Hommey [:glandium] 2012-04-23 06:17:34 PDT
(In reply to Mike Hommey [:glandium] from comment #5)
> char *data = mData.addr();

XPCCallContext *data, even.
Comment 7 User image Bobby Holley (:bholley) (busy with Stylo) 2012-04-23 06:50:32 PDT
(In reply to Mike Hommey [:glandium] from comment #5)
> (The latter case allowing the current indentation being kept)

SGTM.

Note You need to log in before you can comment on or make changes to this bug.