Last Comment Bug 726590 - Uninitialised value use in nsEventListenerManager::HandleEventInternal
: Uninitialised value use in nsEventListenerManager::HandleEventInternal
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Julian Seward [:jseward]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-13 07:33 PST by Julian Seward [:jseward]
Modified: 2012-04-05 10:54 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
trivial fix (806 bytes, patch)
2012-03-23 04:38 PDT, Julian Seward [:jseward]
bugs: review+
Details | Diff | Splinter Review

Description Julian Seward [:jseward] 2012-02-13 07:33:51 PST
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] 2012-03-22 13:43:23 PDT
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] 2012-03-22 14:15:27 PDT
nsEventStatus is in general used as a inout parameter, and should be initialized to some value.
Comment 3 Julian Seward [:jseward] 2012-03-23 04:38:13 PDT
Created attachment 608663 [details] [diff] [review]
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] 2012-04-04 11:28:55 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a268ac68fb58

Note You need to log in before you can comment on or make changes to this bug.