726590 - Uninitialised value use in nsEventListenerManager::HandleEventInternal

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Julian Seward [:jseward]]

Not sure if Event Handling is the right component, but anyway ..

TEST_PATH=dom/workers/test

(DISPLAY=:3.0 make -C ff-opt mochitest-plain TEST_PATH=dom/workers/test EXTRA_TEST_ARGS='--close-when-done --debugger=/home/sewardj/VgTRUNK/merge/Inst/bin/valgrind --debugger-args="--smc-check=all-non-file --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no --stats=yes --trace-children=yes --child-silent-after-fork=yes '--trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep' --tool=memcheck --track-origins=yes --stats=yes"') 2>&1 | cat

produces the two complaints below.  If you look at where the uninitialised
value comes from, it's clear they are both caused by the same problem.


Conditional jump or move depends on uninitialised value(s)
   at 0x64F94C7: nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) (nsEventListenerManager.cpp:763)
   by 0x6514093: nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*, bool, nsCxPusher*) (nsEventListenerManager.h:169)
   by 0x6514FEE: nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsIDOMEventTarget>*) (nsEventDispatcher.cpp:681)
   by 0x6629475: NS_HandleScriptError(nsIScriptGlobalObject*, nsScriptErrorEvent*, nsEventStatus*) (nsJSEnvironment.cpp:294)
   by 0x65F3138: nsIScriptGlobalObject::HandleScriptError(nsScriptErrorEvent*, nsEventStatus*) (nsIScriptGlobalObject.h:163)
   by 0x66BB89F: (anonymous namespace)::ReportErrorRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (WorkerPrivate.cpp:1136)
   by 0x66B94DF: mozilla::dom::workers::WorkerRunnable::Run() (WorkerPrivate.cpp:1717)
   by 0x6D90E7D: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:657)
   by 0x6D58D59: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x6CB1B65: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:134)
   by 0x6DB9671: MessageLoop::Run() (message_loop.cc:208)
   by 0x6BDDBAF: nsBaseAppShell::Run() (nsBaseAppShell.cpp:189)
 Uninitialised value was created by a stack allocation
   at 0x66BB350: (anonymous namespace)::ReportErrorRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (WorkerPrivate.cpp:1030)


Conditional jump or move depends on uninitialised value(s)
   at 0x64F94C7: nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) (nsEventListenerManager.cpp:763)
   by 0x65141B3: nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*, bool, nsCxPusher*) (nsEventListenerManager.h:169)
   by 0x6514FEE: nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsIDOMEventTarget>*) (nsEventDispatcher.cpp:681)
   by 0x6629475: NS_HandleScriptError(nsIScriptGlobalObject*, nsScriptErrorEvent*, nsEventStatus*) (nsJSEnvironment.cpp:294)
   by 0x65F3138: nsIScriptGlobalObject::HandleScriptError(nsScriptErrorEvent*, nsEventStatus*) (nsIScriptGlobalObject.h:163)
   by 0x66BB89F: (anonymous namespace)::ReportErrorRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (WorkerPrivate.cpp:1136)
   by 0x66B94DF: mozilla::dom::workers::WorkerRunnable::Run() (WorkerPrivate.cpp:1717)
   by 0x6D90E7D: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:657)
   by 0x6D58D59: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x6CB1B65: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:134)
   by 0x6DB9671: MessageLoop::Run() (message_loop.cc:208)
   by 0x6BDDBAF: nsBaseAppShell::Run() (nsBaseAppShell.cpp:189)
 Uninitialised value was created by a stack allocation
   at 0x66BB350: (anonymous namespace)::ReportErrorRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (WorkerPrivate.cpp:1030)

Comment 1

[Julian Seward [:jseward]]

Here is what I believe to be the chain that leads from the stack
allocation to the uninitialised use.  The line numbers are marginally
different from those in the stack traces above due to debugging
printfs etc I added.


dom/workers/WorkerPrivate.cpp: static bool
                               ReportError(JSContext* aCx, ...)
creates the undefined value
1157    nsEventStatus status;
1158    if (NS_FAILED(sgo->HandleScriptError(&event, &status))) {
1159      NS_WARNING("Failed to dispatch main thread error event!");
1160      status = nsEventStatus_eIgnore;
1161    }

dom/base/nsIScriptGlobalObject.h
passes through unchanged as 2nd arg 
161  virtual nsresult HandleScriptError(nsScriptErrorEvent *aErrorEvent,
162                                    nsEventStatus *aEventStatus)
163    return NS_HandleScriptError(this, aErrorEvent, aEventStatus);
164  }

dom/base/nsJSEnvironment.cpp: bool NS_HandleScriptError(...)
passed onwards as 5th arg
307   nsEventDispatcher::Dispatch(win, presContext, aErrorEvent, nsnull,
308                               aStatus);

content/events/src/nsEventDispatcher.cpp
nsEventDispatcher::Dispatch
not sure how the undefined value is passed on (wrapped up in some
object, I think)
682     rv = topEtci->HandleEventTargetChain(postVisitor,
                                             NS_EVENT_FLAG_BUBBLE |
                                             NS_EVENT_FLAG_CAPTURE,
                                             aCallback,
                                             false,
                                             &pusher);

content/events/src/nsEventListenerManager.h
passed in as *aEventStatus
  void HandleEvent(nsPresContext* aPresContext,
                   nsEvent* aEvent, 
                   nsIDOMEvent** aDOMEvent,
                   nsIDOMEventTarget* aCurrentTarget,
                   PRUint32 aFlags,
                   nsEventStatus* aEventStatus,
                   nsCxPusher* aPusher)

170 HandleEventInternal(aPresContext, aEvent, aDOMEvent, aCurrentTarget,
171                     aFlags, aEventStatus, aPusher);

content/events/src/nsEventListenerManager.cpp
nsEventListenerManager::HandleEventInternal
766  if (*aEventStatus == nsEventStatus_eConsumeNoDefault) {
767    aEvent->flags |= NS_EVENT_FLAG_NO_DEFAULT;
768  }

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

nsEventStatus is in general used as a inout parameter, and should be initialized to some value.

Comment 3

[Julian Seward [:jseward]]

Attachment: trivial fix

Initialises status before the call, as per comment #2.  No idea
if nsEventStatus_eIgnore is the right value.

Comment 4

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a268ac68fb58

Comment 5

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/a268ac68fb58