Closed Bug 634594 Opened 13 years ago Closed 13 years ago

Fennec Qt [hardfp toolchain] crashes on startup, misaligned store

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: romaxa, Assigned: luke)

References

Details

(Keywords: regression, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

I found that regression happen in:
http://hg.mozilla.org/mozilla-central/rev/02be97f9ef0d

Bug 627954, part 2: ensure nsXPCConvert::VariantData2JS et al are in the correct compartment (r=mrbkap) 

with http://hg.mozilla.org/mozilla-central/rev/d518bc36d7b4 everything works fine.



#0  nsAString_internal (this=<value optimized out>, str=0x4111a500, len=28) at ../../../../dist/include/nsTSubstring.h:593
593	           mFlags(flags) {}
(gdb) bt
#0  nsAString_internal (this=<value optimized out>, str=0x4111a500, len=28) at ../../../../dist/include/nsTSubstring.h:593
#1  nsString (this=<value optimized out>, str=0x4111a500, len=28) at ../../../../dist/include/nsTString.h:398
#2  nsDependentString (this=<value optimized out>, str=0x4111a500, len=28) at ../../../../dist/include/nsTDependentString.h:82
#3  XPCReadableJSStringWrapper (this=<value optimized out>, str=0x4111a500, len=28) at mozilla-central/js/src/xpconnect/src/xpcprivate.h:1021
#4  XPCCallContext::NewStringWrapper (this=<value optimized out>, str=0x4111a500, len=28) at mozilla-central/js/src/xpconnect/src/xpccallcontext.cpp:460
#5  0x3b762c50 in XPCConvert::JSData2Native (ccx=..., d=0xaef6b960, s=18446462621296199376, type=..., useAllocator=1, iid=0xaef6bb40, pErr=0xaef6bbe8)
    at mozilla-central/js/src/xpconnect/src/xpcconvert.cpp:762
#6  0x3b7859f8 in ConvertIndependentParams (ccx=<value optimized out>, mode=<value optimized out>) at mozilla-central/js/src/xpconnect/src/xpcwrappednative.cpp:2941
#7  Call (ccx=<value optimized out>, mode=<value optimized out>) at mozilla-central/js/src/xpconnect/src/xpcwrappednative.cpp:2357
#8  XPCWrappedNative::CallMethod (ccx=<value optimized out>, mode=<value optimized out>) at mozilla-central/js/src/xpconnect/src/xpcwrappednative.cpp:2327
#9  0x3b78cf84 in XPC_WN_CallMethod (cx=0x40449300, argc=2, vp=<value optimized out>) at mozilla-central/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1613
#10 0x3c0831c8 in CallJSNative (cx=0x40449300, entryFrame=<value optimized out>, inlineCallCount=1, interpMode=<value optimized out>)
    at mozilla-central/js/src/jscntxtinlines.h:697
#11 js::Interpret (cx=0x40449300, entryFrame=<value optimized out>, inlineCallCount=1, interpMode=<value optimized out>) at mozilla-central/js/src/jsinterp.cpp:4758
#12 0x3bee2304 in js::RunScript (cx=0x40449300, script=<value optimized out>, fp=0x40a00040) at mozilla-central/js/src/jsinterp.cpp:640
#13 0x3bee3318 in js::Invoke (cx=0x40449300, argsRef=<value optimized out>, flags=<value optimized out>) at mozilla-central/js/src/jsinterp.cpp:720
#14 0x3bee3ce4 in js::ExternalInvoke (cx=0x40449300, thisv=..., fval=..., argc=2, argv=0xaef6c268, rval=0xaef6c330) at mozilla-central/js/src/jsinterp.cpp:841
#15 0x3be814c4 in JS_CallFunctionValue (cx=0x40449300, obj=<value optimized out>, fval=18446462629886199552, argc=2, argv=0xaef6c268, rval=0xaef6c330)
    at mozilla-central/js/src/jsapi.cpp:5055
#16 0x3b77e740 in nsXPCWrappedJSClass::CallMethod (this=<value optimized out>, wrapper=<value optimized out>, methodIndex=<value optimized out>, info=0x403295f0, nativeParams=0xaef6c488)
    at mozilla-central/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1672
#17 0x3b7786cc in nsXPCWrappedJS::CallMethod (this=0x4111ac00, methodIndex=3, info=0x403295f0, params=0xaef6c488)
    at mozilla-central/js/src/xpconnect/src/xpcwrappedjs.cpp:588
#18 0x3bd094fc in PrepareAndDispatch (self=<value optimized out>, methodIndex=<value optimized out>, args=0xaef6c54c)
    at mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:132
#19 0x3bd08b7c in SharedStub () from /nfsmozilla-central/obj-fn-qt-arm6/dist/bin/libxul.so
#20 0x3bced168 in nsComponentManagerImpl::CreateInstanceByContractID (this=0x3fc1f590, aContractID=0x3c1c1660 "@mozilla.org/addons/integration;1", aDelegate=0x0, aIID=..., aResult=0xaef6c5b8)
    at mozilla-central/xpcom/components/nsComponentManager.cpp:1315
#21 0x3bcede44 in nsComponentManagerImpl::GetServiceByContractID (this=0x3fc1f590, aContractID=<value optimized out>, aIID=<value optimized out>, result=<value optimized out>)
    at mozilla-central/xpcom/components/nsComponentManager.cpp:1676
#22 0x3bca53b0 in nsGetServiceByContractID::operator() (this=<value optimized out>, aIID=..., aInstancePtr=0x3c1bfed8) at nsComponentManagerUtils.cpp:278
#23 0x3bca4298 in nsCOMPtr_base::assign_from_gs_contractid (this=0xaef6c618, gs=..., iid=<value optimized out>) at nsCOMPtr.cpp:132
#24 0x3addbd00 in nsCOMPtr (this=0xaef6c87c) at ../../dist/include/nsCOMPtr.h:627
#25 nsXREDirProvider::DoStartup (this=0xaef6c87c) at mozilla-central/toolkit/xre/nsXREDirProvider.cpp:740
#26 0x3add77b0 in XRE_main (argc=<value optimized out>, argv=<value optimized out>, aAppData=<value optimized out>) at mozilla-central/toolkit/xre/nsAppRunner.cpp:3637
---Type <return> to continue, or q <return> to quit---
#27 0x00009528 in main (argc=8, argv=0xaef6cc04) at mozilla-central/mobile/app/nsBrowserApp.cpp:155

#12 0x3bee2304 in js::RunScript (cx=0x40449300, script=<value optimized out>, fp=0x40a00040) at mozilla-central/js/src/jsinterp.cpp:640
640	    return Interpret(cx, fp);
(gdb) p *cx
Cannot access memory at address 0x40449300


Here is compiler options:

c++ -o fennec  -frtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -gdwarf-2 -march=armv7-a -marm -fno-strict-aliasing -pthread -pipe  -DNDEBUG -DTRIMMED -gdwarf-2 -g -O2  nsBrowserApp.o   -lpthread  -Wl,-rpath='$ORIGIN'   -Wl,-rpath-link,/home/romaxa/mozdev/mozillahg/mozilla-central/obj-fn-qt-arm6/dist/bin -Wl,-rpath-link,/usr/local/lib  -L../../dist/bin -L../../dist/lib -Wl,--whole-archive -ljemalloc -Wl,--no-whole-archive -rdynamic -Wl,--version-script -Wl,/home/romaxa/mozdev/mozillahg/mozilla-central/build/unix/gnu-ld-scripts/jemalloc-standalone-linkage-version-script /home/romaxa/mozdev/mozillahg/mozilla-central/obj-fn-qt-arm6/dist/lib/libxpcomglue_s.a -L/home/romaxa/mozdev/mozillahg/mozilla-central/obj-fn-qt-arm6/dist/bin -lxpcom -lmozalloc -L/home/romaxa/mozdev/mozillahg/mozilla-central/obj-fn-qt-arm6/dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl  -lxul -ldl  -lQtNetwork -lQtOpenGL -lQtGui -lQtCore


mozconfig looks like this:
ac_add_options --enable-application=mobile
ac_add_options --disable-crashreporter
ac_add_options --enable-update-channel=nightly
ac_add_options --enable-update-packaging
ac_add_options --enable-tests
ac_add_options --enable-codesighs
ac_add_options --enable-cpp-rtti
ac_add_options --enable-chrome-format=flat
ac_add_options --enable-optimize=" -g -O2 "
ac_add_options --enable-default-toolkit=cairo-qt
export MOZ_DEBUG_SYMBOLS=1
ac_add_options --enable-debug-symbols="-gdwarf-2"
export MOZILLA_OFFICIAL=1
mk_add_options PROFILE_GEN_SCRIPT=@TOPSRCDIR@/build/profile_pageloader.pl
mk_add_options MOZ_MAKE_FLAGS="-j4"
mk_add_options MOZ_OBJDIR="obj-fn-qt-arm6"
ac_add_options --with-maemo-version=6
ac_add_options --disable-pedantic --with-arm-kuser
ac_add_options --disable-thumb2
CFLAGS="-gdwarf-2 -march=armv7-a -marm"
CXXFLAGS="-gdwarf-2 -march=armv7-a -marm"
ASFLAGS="-march=armv7-a -marm"
Does the crash reproduce on m-c tip?
yes.
I hear you'll be in the Mozilla MV office today; perhaps you can bring something I can gdb into?

Along those lines, does this repro on a non-optimized build?
What's weird about the reported address is that it seems that the memory used for the dependent string is bad but, looking at XPCCallContext::NewStringWrapper, the memory is either from within the XPCCallContext or fresh from operator new.  But maybe that's just the opt build scrambling things.
Seems it reproducible on non-debug,optimized build, but not on debug/logging + disabled optimization -O0.

Will try debug/logging build with -O1
Hah, we reduced it to an alignment fault!  In the patch I removed some members from XPCCallContext and that apparently shifted mStringWraperData, which is declared as an array of chars, but houses an array of structs and, since the previous member is a PRUint16, this causes a misaligned word store.
Attached patch align better (obsolete) — Splinter Review
Assignee: general → lw
Status: NEW → ASSIGNED
Attachment #512993 - Flags: review?(mrbkap)
Attached patch hg qref'dSplinter Review
Attachment #512993 - Attachment is obsolete: true
Attachment #512994 - Flags: review?(mrbkap)
Attachment #512993 - Flags: review?(mrbkap)
Comment on attachment 512994 [details] [diff] [review]
hg qref'd

Nice.
Attachment #512994 - Flags: review?(mrbkap) → review+
Actually I found why we have this problem reproducible on maemo6 but not on maemo5 and android... problem is that on maemo6 we have memalign warning level 0, which cause force crash on any memalign problem.

But this fix make lot of sense if this code in often use.
probably could be marked as perf bug for android.
Blocks: 627954
The corruption reported in comment 0 appears to be an artifact of gcc.
Group: core-security
Summary: Fennec Qt [hardfp toolchain] crashes on startup, memory corruption → Fennec Qt [hardfp toolchain] crashes on startup, misaligned store alignment
Summary: Fennec Qt [hardfp toolchain] crashes on startup, misaligned store alignment → Fennec Qt [hardfp toolchain] crashes on startup, misaligned store
Despite this being low-risk, I am not going to land on m-c until branch for FF4.0.  If anyone wants to do differently, just nominate and I'll land if approved.
luke, would this help perf on mobile?
I wouldn't expect there to be a measurable difference; if you are spending a lot of time in this area you are usually on a pretty slow path.
Comment on attachment 512994 [details] [diff] [review]
hg qref'd

Works fine with this patch, can we get it in?
Attachment #512994 - Flags: approval2.0?
I'm curious: why is it OK to remove the mInUse-clearing?  It looks like we still assert if it's set, but I don't see anywhere that clears it now.  Should it just already be cleared if we get this far, and not need the brute force from ~XPCCallContext?
(In reply to comment #17)
> I'm curious: why is it OK to remove the mInUse-clearing?

Still being cleared, just tucked away in StringWrapperEntry().
Comment on attachment 512994 [details] [diff] [review]
hg qref'd

a-, don't think the risk is worth the reward
Attachment #512994 - Flags: approval2.0? → approval2.0-
http://hg.mozilla.org/tracemonkey/rev/27e581481549
Whiteboard: fixed-in-tracemonkey
Blocks: 639162
Nominating for blocking-fennec, as we have a dupe of this bug that is already a blocker.
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0+
Comment on attachment 512994 [details] [diff] [review]
hg qref'd

Given this looks like our top crash on mobile at the moment, +'ing
Attachment #512994 - Flags: approval2.0- → approval2.0+
Keywords: checkin-needed
great, thank you guys, this was our hardblocker....
No longer blocks: 639162
http://hg.mozilla.org/mozilla-central/rev/fc05d053ed03
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: