800146 - Out of range writes in js::XDRStaticBlockObject<(js::XDRMode)0>

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Julian Seward [:jseward]]

m-c from about 3 hours ago (109680:aa5e3b445810), segfaults on startup
when built -O2 but not -O (I think).

There are a bunch of invalid writes leading to heap trashing just
before it faults.

Invalid write of size 4
   at 0x30E9C9AE: bool js::XDRStaticBlockObject<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Han
dle<JSScript*>, js::StaticBlockObject**) (js/src/jsapi.h:1302)
   by 0x30E642DD: bool js::XDRScript<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScrip
t*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>) (js/src/jsscript.cpp:716)
   by 0x30E055A9: bool js::XDRInterpretedFunction<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::H
andle<JSScript*>, JS::MutableHandle<JSObject*>) (js/src/jsfun.cpp:412)
   by 0x30E641B5: bool js::XDRScript<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScrip
t*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>) (js/src/jsscript.cpp:690)
   by 0x30EEE8D9: js::XDRState<(js::XDRMode)0>::codeScript(JS::MutableHandle<JSScript*>) (js/src/vm/Xdr.cpp:147)
   by 0x30DD63AF: JS_EncodeScript (js/src/jsapi.cpp:7272)
   by 0x307D1D0D: WriteCachedScript(mozilla::scache::StartupCache*, nsACString_internal&, JSContext*, nsIPrincipal*, JSScript
*) (js/xpconnect/loader/mozJSLoaderUtils.cpp:47)
   by 0x307D0019: mozJSComponentLoader::GlobalForLocation(nsIFile*, nsIURI*, JSObject**, char**, JS::Value*) (js/xpconnect/lo
ader/mozJSComponentLoader.cpp:891)
   by 0x307D109B: mozJSComponentLoader::LoadModule(mozilla::FileLocation&) (js/xpconnect/loader/mozJSComponentLoader.cpp:480)
   by 0x30AD1067: nsComponentManagerImpl::KnownModule::Load() (xpcom/components/nsComponentManager.cpp:671)
   by 0x30AD1095: nsFactoryEntry::GetFactory() (xpcom/components/nsComponentManager.cpp:1662)
   by 0x30AD13E7: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) (xpcom/components/nsComponentManager.cpp:1002)
   by 0x30AD1CBB: nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) (xpcom/components/nsComponentManager.cpp:1398)
   by 0x30AAF151: CallGetService(char const*, nsID const&, void**) (ff-opt/xpcom/build/nsComponentManagerUtils.cpp:62)
   by 0x30AAF18D: nsGetServiceByContractID::operator()(nsID const&, void**) const (ff-opt/xpcom/build/nsComponentManagerUtils.cpp:246)
   by 0x30AAE88B: nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) (ff-opt/xpcom/build/nsCOMPtr.cpp:92)
   by 0x30ACFDF9: NS_CreateServicesFromCategory(char const*, nsISupports*, char const*) (ff-opt/xpcom/components/../../dist/include/nsCOMPtr.h:910)
   by 0x3015DDE3: nsXREDirProvider::DoStartup() (toolkit/xre/nsXREDirProvider.cpp:785)
   by 0x3015C7E9: XREMain::XRE_mainRun() (toolkit/xre/nsAppRunner.cpp:3686)
   by 0x3015CCEF: XREMain::XRE_main(int, char**, nsXREAppData const*) (toolkit/xre/nsAppRunner.cpp:3858)

 Address 0x2064eebc is 0 bytes after a block of size 4 alloc'd
   at 0x480687C: malloc (/home/sewardj/Vg38BRANCH/branch38droid2/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
   by 0x30DD6EFB: js::TempAllocPolicy::malloc_(unsigned int) (ff-opt/js/src/./../../dist/include/js/Utility.h:153)
   by 0x30DDA139: js::Vector<JSString*, 8u, js::TempAllocPolicy>::growStorageBy(unsigned int) (ff-opt/js/src/./../../dist/include/js/Vector.h:630)
   by 0x30E9CAD9: bool js::XDRStaticBlockObject<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, js::StaticBlockObject**) (ff-opt/js/src/./../../dist/include/js/Vector.h:688)
   by 0x30E642DD: bool js::XDRScript<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>) (js/src/jsscript.cpp:716)
   by 0x30E055A9: bool js::XDRInterpretedFunction<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JSObject*>) (js/src/jsfun.cpp:412)
   by 0x30E641B5: bool js::XDRScript<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>) (js/src/jsscript.cpp:690)
   by 0x30EEE8D9: js::XDRState<(js::XDRMode)0>::codeScript(JS::MutableHandle<JSScript*>) (js/src/vm/Xdr.cpp:147)
   by 0x30DD63AF: JS_EncodeScript (js/src/jsapi.cpp:7272)
   by 0x307D1D0D: WriteCachedScript(mozilla::scache::StartupCache*, nsACString_internal&, JSContext*, nsIPrincipal*, JSScript*) (js/xpconnect/loader/mozJSLoaderUtils.cpp:47)
   by 0x307D0019: mozJSComponentLoader::GlobalForLocation(nsIFile*, nsIURI*, JSObject**, char**, JS::Value*) (js/xpconnect/loader/mozJSComponentLoader.cpp:891)
   by 0x307D109B: mozJSComponentLoader::LoadModule(mozilla::FileLocation&) (js/xpconnect/loader/mozJSComponentLoader.cpp:480)
   by 0x30AD1067: nsComponentManagerImpl::KnownModule::Load() (xpcom/components/nsComponentManager.cpp:671)
   by 0x30AD1095: nsFactoryEntry::GetFactory() (xpcom/components/nsComponentManager.cpp:1662)
   by 0x30AD13E7: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) (xpcom/components/nsComponentManager.cpp:1002)
   by 0x30AD1CBB: nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) (xpcom/components/nsComponentManager.cpp:1398)
   by 0x30AAF151: CallGetService(char const*, nsID const&, void**) (ff-opt/xpcom/build/nsComponentManagerUtils.cpp:62)

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

cc'ing some JS folks - any idea what this may be?

Comment 2

[Luke Wagner [:luke]]

The backtrace points pretty clearly to this 'growBy' call
  http://mxr.mozilla.org/mozilla-central/source/js/src/vm/ScopeObject.cpp#767
specifically the makeRangeGCSafe call at the end of AutoVectorRooter::growBy:
  http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.h#1299

This seems pretty odd.  For one thing, AutoShapeVector has an inline capacity of 8 which means we wouldn't be seeing a malloc of size 4 (the first would be 8 * 2 * sizeof(T*)).  The presence of inlining and the relative isolation of 'shapes' makes me wonder whether you could try a full clobber build.  If that doesn't fix it, perhaps you could check out the memset()?  If you can get this under a debugger, I'd like to see the values of the fields of 'shapes' to see why it is malloc()ing so early.

Comment 3

[Julian Seward [:jseward]]

I thought this all a bit strange.  To re-check, I tried from-clean
builds of 109796:c0977caba1d6 modified only with the diagnostic patch
shown below, at both -O and -O2.  And it really does crash at -O2 but
not at -O.

diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -817,18 +817,21 @@ js::CloneStaticBlockObject(JSContext *cx
 
     clone->initEnclosingStaticScope(enclosingScope);
     clone->setStackDepth(srcBlock->stackDepth());
 
     /* Shape::Range is reverse order, so build a list in forward order. */
     AutoShapeVector shapes(cx);
     if (!shapes.growBy(srcBlock->slotCount()))
         return NULL;
-    for (Shape::Range r = srcBlock->lastProperty()->all(); !r.empty(); r.popFront())
+    for (Shape::Range r = srcBlock->lastProperty()->all(); !r.empty(); r.popFront()) {
+        if (r.front().shortid() >= shapes.length())
+            MOZ_CRASH();
         shapes[r.front().shortid()] = &r.front();
+    }
 
     for (Shape **p = shapes.begin(); p != shapes.end(); ++p) {
         RootedId id(cx, (*p)->propid());
         unsigned i = (*p)->shortid();
 
         bool redeclared;
         if (!StaticBlockObject::addVar(cx, clone, id, i, &redeclared)) {
             JS_ASSERT(!redeclared);

Comment 4

[Luke Wagner [:luke]]

That is very strange indeed.  Could you augment the diagnostic with:

diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -817,18 +817,27 @@ js::CloneStaticBlockObject(JSContext *cx
 
     clone->initEnclosingStaticScope(enclosingScope);
     clone->setStackDepth(srcBlock->stackDepth());
 
     /* Shape::Range is reverse order, so build a list in forward order. */
     AutoShapeVector shapes(cx);
     if (!shapes.growBy(srcBlock->slotCount()))
         return NULL;
-    for (Shape::Range r = srcBlock->lastProperty()->all(); !r.empty(); r.popFront())
+    for (Shape::Range r = srcBlock->lastProperty()->all(); !r.empty(); r.popFront()) {
+        if (unsigned(r.front().shortid()) >= shapes.length()) {
+            fprintf(stderr, "srcBlock->slotCount() = %u\n", unsigned(srcBlock->slotCount()));
+            fprintf(stderr, "shapes.length() = %u\n", unsigned(shapes.length()));
+            for (Shape::Range r2 = srcBlock->lastProperty()->all(); !r2.empty(); r2.popFront())
+                fprintf(stderr, "prop with shortid %d\n", int(r2.front().shortid()));
+            fflush(stderr);
+            MOZ_CRASH();
+        }
         shapes[r.front().shortid()] = &r.front();
+    }

and see what comes out?  Thanks!

Comment 5

[Julian Seward [:jseward]]

(In reply to Luke Wagner [:luke] from comment #4)
> That is very strange indeed.  Could you augment the diagnostic with:
> [...]
> and see what comes out?  Thanks!

It still segfaults, but nothing gets printed.  At least, nothing
appears in the 'adb logcat' and there's nothing to suggest that the
MOZ_CRASH got hit -- although I am not sure what I am looking for
there.

Comment 6

[Luke Wagner [:luke]]

Oh wait, iirc, fprintf(stderr) doesn't make its way to logcat (at least not 1 year ago when I wanted to get some info out).  I heard there was a special android equivalent you had to use, but I forget what it is.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Luke Wagner [:luke] from comment #6)
> Oh wait, iirc, fprintf(stderr) doesn't make its way to logcat (at least not
> 1 year ago when I wanted to get some info out).  I heard there was a special
> android equivalent you had to use, but I forget what it is.

CC'ing Mark Finkle. Mark, do you know what we could use?

Comment 8

[Mike Hommey [:glandium]]

Use __android_log_print (requires a #include <android/log.h>)

Comment 9

[Luke Wagner [:luke]]

IIRC, Julian told me on IRC that the problem when away when he updated his compiler and rebuilt.  Is that right Julian or am I confusing this with another bug?

Comment 10

[Julian Seward [:jseward]]

IIRC this went away when I transitioned from gcc-4.4.3 -O2 to gcc-4.6.2 -O2,
and I haven't seen it since.  So I'm inclined to think this is a compiler bug.

Comment 11

[Luke Wagner [:luke]]

Based on comment 10, closing.