688733 - nsWindow::mIMEComposing is used uninitialised

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Julian Seward [:jseward]]

Ga!  My first Fx/Android/Valgrind bug, if it's real.

All of this is in widget/src/android/nsWindow.{h,cpp}

Seems like nsWindow::mIMEComposing can be used uninitialised, in nsWindow::CancelIMEComposition():

1829:    // Cancel composition on Gecko side
1830:    if (mIMEComposing) {

The one and only constructor doesn't set it:

  nsWindow::nsWindow() :
      mIsVisible(PR_FALSE),
      mParent(nsnull),
      mFocus(nsnull)
  {
  }

The resulting complaint is below.

STR: start Fx on a Nexus S, go to news.bbc.co.uk, then press the menu button, 
then "more", then "quit".  Then it warns that I am about to close 2 tabs and
do I want to continue.  If I confirm, then said complaint appears after I confirm
(I think.)

---------------------------------------------------------------------------------

I/fennec(11021): ==11022== Conditional jump or move depends on uninitialised value(s)
I/fennec(11021): ==11022==    at 0x829AE66E: nsWindow::CancelIMEComposition() (widget/src/android/nsWindow.cpp:1830)
I/fennec(11021): ==11022==    by 0x82525FED: nsIMEStateManager::OnRemoveContent(nsPresContext*, nsIContent*) (content/events/src/nsIMEStateManager.cpp:113)
I/fennec(11021): ==11022==    by 0x825F1D23: nsFocusManager::ContentRemoved(nsIDocument*, nsIContent*) (dom/base/nsFocusManager.cpp:815)
I/fennec(11021): ==11022==    by 0x82516139: nsEventStateManager::ContentRemoved(nsIDocument*, nsIContent*) (content/events/src/nsEventStateManager.cpp:4572)
I/fennec(11021): ==11022==    by 0x82367135: PresShell::ContentRemoved(nsIDocument*, nsIContent*, nsIContent*, int, nsIContent*) (layout/base/nsPresShell.cpp:5052)
I/fennec(11021): ==11022==    by 0x824BBC4D: nsNodeUtils::ContentRemoved(nsINode*, nsIContent*, int, nsIContent*) (content/base/src/nsNodeUtils.cpp:196)
I/fennec(11021): ==11022==    by 0x824B3301: nsINode::doRemoveChildAt(unsigned int, int, nsIContent*, nsAttrAndChildArray&) (content/base/src/nsGenericElement.cpp:3683)
I/fennec(11021): ==11022==    by 0x824B336D: nsGenericElement::RemoveChildAt(unsigned int, int) (content/base/src/nsGenericElement.cpp:3654)
I/fennec(11021): ==11022==    by 0x82750AF9: nsXULElement::RemoveChildAt(unsigned int, int) (content/xul/content/src/nsXULElement.cpp:1004)
I/fennec(11021): ==11022==    by 0x824ACB1B: nsINode::RemoveChild(nsINode*) (content/base/src/nsGenericElement.cpp:527)
I/fennec(11021): ==11022==    by 0x828537D1: nsIDOMNode_RemoveChild(JSContext*, unsigned int, unsigned long long*) (ff-opt/js/src/xpconnect/src/dom_quickstubs.cpp:6859)
I/fennec(11021): ==11022==    by 0x82C91897: js::InvokeKernel(JSContext*, js::CallArgs const&, js::MaybeConstruct) (js/src/jscntxtinlines.h:300)
I/fennec(11021): ==11022==  Uninitialised value was created by a heap allocation
I/fennec(11021): ==11022==    at 0x8010331C: malloc (/home/sewardj/VgTRUNK/trunk-android/coregrind/m_replacemalloc/vg_replace_malloc.c:263)
I/fennec(11021): ==11022==    by 0x807008DB: moz_xmalloc (memory/mozalloc/mozalloc.cpp:103)
I/fennec(11021): ==11022==    by 0x829A795F: nsWindowConstructor(nsISupports*, nsID const&, void**) (ff-opt/widget/src/android/../../../dist/include/mozilla/mozalloc.h:229)
I/fennec(11021): ==11022==    by 0x82A7DD7B: mozilla::GenericFactory::CreateInstance(nsISupports*, nsID const&, void**) (ff-opt/xpcom/build/GenericFactory.cpp:48)
I/fennec(11021): ==11022==    by 0x82A9FA55: nsComponentManagerImpl::CreateInstance(nsID const&, nsISupports*, nsID const&, void**) (xpcom/components/nsComponentManager.cpp:1212)
I/fennec(11021): ==11022==    by 0x82A79135: CallCreateInstance(nsID const&, nsISupports*, nsID const&, void**) (ff-opt/xpcom/build/nsComponentManagerUtils.cpp:157)
I/fennec(11021): ==11022==    by 0x82A79153: nsCreateInstanceByCID::operator()(nsID const&, void**) const (ff-opt/xpcom/build/nsComponentManagerUtils.cpp:199)
I/fennec(11021): ==11022==    by 0x82A787E9: nsCOMPtr_base::assign_from_helper(nsCOMPtr_helper const&, nsID const&) (ff-opt/xpcom/build/nsCOMPtr.cpp:150)
I/fennec(11021): ==11022==    by 0x829B1D0D: nsBaseWidget::CreateChild(nsIntRect const&, nsEventStatus (*)(nsGUIEvent*), nsDeviceContext*, nsIAppShell*, nsIToolkit*, nsWidgetInitData*, int) (ff-opt/widget/src/xpwidgets/../../../dist/include/nsCOMPtr.h:720)
I/fennec(11021): ==11022==    by 0x825E2CCD: nsView::CreateWidgetForParent(nsIWidget*, nsWidgetInitData*, int, int) (view/src/nsView.cpp:782)
I/fennec(11021): ==11022==    by 0x825E2D2F: nsIView::CreateWidgetForParent(nsIWidget*, nsWidgetInitData*, int, int) (view/src/nsView.cpp:693)
I/fennec(11021): ==11022==    by 0x8235237B: DocumentViewerImpl::MakeWindow(nsSize const&, nsIView*) (layout/base/nsDocumentViewer.cpp:2342)
I/fennec(11021): ==11022==

Comment 1

[Julian Seward [:jseward]]

Ah, actually this happened earlier in the run too (I don't know where):

NS_IMETHODIMP
nsWindow::ResetInputState()
{
    //ALOGIME("IME: ResetInputState: s=%d", aState);

    // Cancel composition on Gecko side
    if (mIMEComposing) {    <------------ 1779


I/fennec(11021): ==11022== Conditional jump or move depends on uninitialised value(s)
I/fennec(11021): ==11022==    at 0x829AE7C2: nsWindow::ResetInputState() (widget/src/android/nsWindow.cpp:1779)
I/fennec(11021): ==11022==    by 0x825261D5: nsIMEStateManager::OnChangeFocus(nsPresContext*, nsIContent*, unsigned int) (content/events/src/nsIMEStateManager.cpp:190)
I/fennec(11021): ==11022==    by 0x825F1907: nsFocusManager::Blur(nsPIDOMWindow*, nsPIDOMWindow*, int, int) (dom/base/nsFocusManager.cpp:1514)
I/fennec(11021): ==11022==    by 0x825F2F83: nsFocusManager::SetFocusInner(nsIContent*, int, int, int) (dom/base/nsFocusManager.cpp:1231)
I/fennec(11021): ==11022==    by 0x825F31B1: nsFocusManager::SetFocus(nsIDOMElement*, unsigned int) (dom/base/nsFocusManager.cpp:462)
I/fennec(11021): ==11022==    by 0x8275052B: nsXULElement::Focus() (content/xul/content/src/nsXULElement.cpp:2060)
I/fennec(11021): ==11022==    by 0x8282C319: nsIDOMXULElement_Focus(JSContext*, unsigned int, unsigned long long*) (ff-opt/js/src/xpconnect/src/dom_quickstubs.cpp:27008)
I/fennec(11021): ==11022==    by 0x82C91897: js::InvokeKernel(JSContext*, js::CallArgs const&, js::MaybeConstruct) (js/src/jscntxtinlines.h:300)
I/fennec(11021): ==11022==    by 0x82C8CF71: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (js/src/jsinterp.cpp:4058)
I/fennec(11021): ==11022==    by 0x82C90FD7: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (js/src/jsinterp.cpp:614)
I/fennec(11021): ==11022==    by 0x82C919A3: js::InvokeKernel(JSContext*, js::CallArgs const&, js::MaybeConstruct) (js/src/jsinterp.cpp:678)
I/fennec(11021): ==11022==    by 0x82C91B6D: js::Invoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (js/src/jsinterp.h:168)

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 3

[Julian Seward [:jseward]]

(In reply to Brad Lassey [:blassey] from comment #2)
I verified that stops V complaining.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b4f96bf6052

Comment 5

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/8b4f96bf6052