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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: romaxa, Assigned: luke)

Tracking

({regression})

Trunk
x86
Linux
regression
Points:
---

Firefox Tracking Flags

(fennec2.0+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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"
(Assignee)

Comment 1

7 years ago
Does the crash reproduce on m-c tip?
(Reporter)

Comment 2

7 years ago
yes.
(Assignee)

Comment 3

7 years ago
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?
(Assignee)

Comment 4

7 years ago
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.
(Reporter)

Comment 5

7 years ago
Seems it reproducible on non-debug,optimized build, but not on debug/logging + disabled optimization -O0.

Will try debug/logging build with -O1
(Assignee)

Comment 6

7 years ago
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.
(Assignee)

Comment 7

7 years ago
Created attachment 512993 [details] [diff] [review]
align better
Assignee: general → lw
Status: NEW → ASSIGNED
Attachment #512993 - Flags: review?(mrbkap)
(Assignee)

Comment 8

7 years ago
Created attachment 512994 [details] [diff] [review]
hg qref'd
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+
(Reporter)

Comment 10

7 years ago
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.
(Reporter)

Comment 11

7 years ago
probably could be marked as perf bug for android.
Blocks: 627954
(Assignee)

Comment 12

7 years ago
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
(Assignee)

Updated

7 years ago
Summary: Fennec Qt [hardfp toolchain] crashes on startup, misaligned store alignment → Fennec Qt [hardfp toolchain] crashes on startup, misaligned store
(Assignee)

Comment 13

7 years ago
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?
(Assignee)

Comment 15

7 years ago
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.
(Reporter)

Comment 16

7 years ago
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?
(Assignee)

Comment 18

7 years ago
(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-
(Assignee)

Comment 20

7 years ago
http://hg.mozilla.org/tracemonkey/rev/27e581481549
Whiteboard: fixed-in-tracemonkey

Updated

7 years ago
Blocks: 639162
Nominating for blocking-fennec, as we have a dupe of this bug that is already a blocker.
tracking-fennec: --- → ?

Updated

7 years ago
tracking-fennec: ? → 2.0+

Comment 22

7 years ago
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+

Updated

7 years ago
Keywords: checkin-needed
(Reporter)

Comment 23

7 years ago
great, thank you guys, this was our hardblocker....

Updated

7 years ago
No longer blocks: 639162
Duplicate of this bug: 639162
(Assignee)

Comment 25

7 years ago
http://hg.mozilla.org/mozilla-central/rev/fc05d053ed03
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 645467
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.