940825 - Use of uninitialized value in nsDocumentViewer::PermitUnload()

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[Nicholas Nethercote [inactive]]

Valgrind says:

>  Conditional jump or move depends on uninitialised value(s)
>     at 0x92671D2: nsDocumentViewer::PermitUnload(bool, bool*) (nsDocumentViewer.cpp:1105)
>     by 0x9564FB2: nsDocShell::InternalLoad(nsIURI*, nsIURI*, nsISupports*, unsigned int, char16_t const*, char const*, nsAString_internal const&, nsIInputStream*, nsIInputStream*, unsigned int, nsISHEntry*, bool, nsAString_internal const&, nsIDocShell**, nsIRequest**) (nsDocShell.cpp:9404)
>     by 0x955F8E7: nsDocShell::LoadURI(nsIURI*, nsIDocShellLoadInfo*, unsigned int, bool) (nsDocShell.cpp:1544)
>     by 0x95779B9: nsWindowWatcher::OpenWindowInternal(nsIDOMWindow*, char const*, char const*, char const*, bool, bool, bool, nsIArray*, nsIDOMWindow**) (nsWindowWatcher.cpp:907)
>     by 0x9578B1C: nsWindowWatcher::OpenWindow2(nsIDOMWindow*, char const*, char const*, char const*, bool, bool, bool, nsISupports*, nsIDOMWindow**) (nsWindowWatcher.cpp:416)
>     by 0x8C52FDD: nsGlobalWindow::OpenInternal(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, bool, bool, bool, bool, bool, nsIArray*, nsISupports*, nsIPrincipal*, JSContext*, nsIDOMWindow**) (nsGlobalWindow.cpp:11165)
>     by 0x8C52902: nsGlobalWindow::OpenInternal(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, bool, bool, bool, bool, bool, nsIArray*, nsISupports*, nsIPrincipal*, JSContext*, nsIDOMWindow**) (nsGlobalWindow.cpp:11048)
>     by 0x8C53C13: nsGlobalWindow::OpenJS(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, nsIDOMWindow**) (nsGlobalWindow.cpp:7222)
>     by 0x8C4739F: nsGlobalWindow::Open(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) (nsGlobalWindow.cpp:7189)
>     by 0x8A30D58: mozilla::dom::WindowBinding::open(JSContext*, JS::Handle<JSObject*>, nsGlobalWindow*, JSJitMethodCallArgs const&) (WindowBinding.cpp:1199)
>     by 0x8A1AEDD: mozilla::dom::WindowBinding::genericMethod(JSContext*, unsigned int, JS::Value*) (WindowBinding.cpp:8658)
>     by 0x9C5B5AB: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:220)
>     by 0x9C548B9: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2502)
>     by 0x9C5A293: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:420)
>     by 0x9C5B545: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:482)
>   Uninitialised value was created by a stack allocation
>     at 0x9266F53: nsDocumentViewer::PermitUnload(bool, bool*) (nsDocumentViewer.cpp:1049)

It's easy to understand:  utils->AreDialogsEnabled() can fail but this isn't
checked for.

roc said this isn't s-s.

Comment 1

[Dongie Agnir [:dagnir]]

Hello,

I'm interested in taking this on.  I'm new to the Mozilla codebase.  I'm assuming we just want to check to the return value of util->AreDialogsEnabled()?

Comment 2

[Nicholas Nethercote [inactive]]

I suspect just initializing dialogsWereEnabled to false would suffice, but I'm not familiar with this part of the code.

Comment 3

[Dongie Agnir [:dagnir]]

Attachment: ensure dialogsWereEnabled is initialized

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 8335083 [details] [diff] [review]
ensure dialogsWereEnabled is initialized

Thanks, Dongie!  When you upload a patch you usually need to ask someone to review it.  Knowing who to ask can be tricky, but for this patch I've asked roc to do it.

BTW, the patch's form suggests that you've set up Mercurial nicely, which is great!

Comment 5

[Josh Matthews [:jdm]]

I went to lunch after this bug was filed, and returned after it was fixed and approved. Good job everyone! Dongie, if you could attach a version with r=roc instead of r=reviewers, we could commit that to the tree :)

Comment 6

[Dongie Agnir [:dagnir]]

Attachment: ensure dialogsWereEnabled is initialized - updated reviewer

Comment 7

[Dongie Agnir [:dagnir]]

Thanks everyone!  I've updated the patch per Josh's instructions.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d9749cee00f

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5d9749cee00f