645938 - uninitialized memory read in nsContentList::Item

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

While starting up browser-chrome mochitests under valgrind (or maybe even past startup -- I was running them in toolkit/mozapps/extensions/), I saw:

==8280== Conditional jump or move depends on uninitialised value(s)
==8280==    at 0x5C94E95: nsContentList::GetNodeAt(unsigned int) (nsContentList.cpp:496)
==8280==    by 0x67FA4C4: nsIDOMNodeList_Item(JSContext*, unsigned int, jsval_layout*) (dom_quickstubs.cpp:6832)
==8280==    by 0x768E6AC: js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) (jscntxtinlines.h:701)
==8280==    by 0x741D6DF: js::RunScript(JSContext*, JSScript*, JSStackFrame*) (jsinterp.cpp:653)
==8280==    by 0x741DB98: js::Invoke(JSContext*, js::CallArgs const&, unsigned int) (jsinterp.cpp:740)
==8280==    by 0x741E414: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:863)
==8280==    by 0x7368B20: JS_CallFunctionValue (jsapi.cpp:5173)
==8280==    by 0x6072D31: nsXBLProtoImplAnonymousMethod::Execute(nsIContent*) (nsXBLProtoImplMethod.cpp:337)
==8280==    by 0x608DF73: nsBindingManager::ProcessAttachedQueue(unsigned int) (nsBindingManager.cpp:1019)
==8280==    by 0x599D09E: PresShell::InitialReflow(int, int) (nsPresShell.cpp:2737)
==8280==    by 0x60A3AC0: nsXULDocument::StartLayout() (nsXULDocument.cpp:2018)
==8280==    by 0x60A9494: nsXULDocument::DoneWalking() (nsXULDocument.cpp:3180)
==8280==    by 0x60B28BA: nsXULDocument::ResumeWalk() (nsXULDocument.cpp:3129)
==8280==    by 0x60B30E4: nsXULDocument::OnStreamComplete(nsIStreamLoader*, nsISupports*, unsigned int, unsigned int, unsigned char const*) (nsXULDocument.cpp:3589)
==8280==    by 0x56AD0DE: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsStreamLoader.cpp:125)
==8280==    by 0x565CA4B: nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsBaseChannel.cpp:727)
==8280==    by 0x5670EF3: nsInputStreamPump::OnStateStop() (nsInputStreamPump.cpp:578)
==8280==    by 0x56726BF: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:403)
==8280==    by 0x701FAEC: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:112)
==8280==    by 0x704CC5C: nsThread::ProcessNextEvent(int, int*) (nsThread.cpp:647)
==8280==    by 0x6FC7493: NS_ProcessNextEvent_P(nsIThread*, int) (nsThreadUtils.cpp:250)
==8280==    by 0x6E9BF4A: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:110)
==8280==    by 0x70C209B: MessageLoop::Run() (message_loop.cc:219)
==8280==    by 0x6CC73EF: nsBaseAppShell::Run() (nsBaseAppShell.cpp:192)
==8280==    by 0x698FB69: nsAppStartup::Run() (nsAppStartup.cpp:218)
==8280==    by 0x5638CB0: XRE_main (nsAppRunner.cpp:3768)
==8280==    by 0x400F6B: main (nsBrowserApp.cpp:158)

Line 496 is actually the first line of nsContentList::Item:
  if (mRootNode && aDoFlush && mFlushesNeeded) {

I think the problem here is that only one of the two constructors of nsContentList initializes mFlushesNeeded.  I haven't confirmed experimentally, although the fact that that bug was introduced last week, in http://hg.mozilla.org/mozilla-central/rev/77483e45592e , suggests it's likely.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Er, yes.  Fix coming up.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure to set mFlushesNeeded for all content lists.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 522576 [details] [diff] [review]
Make sure to set mFlushesNeeded for all content lists.

r=dbaron, although there might be value in a method to share code between the constructors or something...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/312fa00da430