723498 - Uninitialised value use in nsInputStreamPump::AsyncRead

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P4)

Description

[Julian Seward [:jseward]]

TEST_PATH=content/base/test/test_blobbuilder.htm

(DISPLAY=:1 TEST_PATH=content/base/test/test_blobbuilder.html make -C ff-opt mochitest-plain EXTRA_TEST_ARGS='--close-when-done --debugger=vTRUNK --debugger-args="--tool=memcheck --suppressions=/home/sewardj/MOZ/fglrx-supp.supp --suppressions=/home/sewardj/MOZ/moz-supp.supp --error-limit=no --stats=yes --smc-check=all-non-file --trace-children=yes --child-silent-after-fork=yes '--trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel' --track-origins=yes"') 2>&1 | tee spew2-memcheck-2a

gives the complaint below.  Looking at the start of 
nsInputStreamPump::AsyncRead(nsIStreamListener*, nsISupports*)
we have

    bool nonBlocking;
    nsresult rv = mStream->IsNonBlocking(&nonBlocking);
    ...
    if (NS_FAILED(rv)) return rv; // not taken
    if (nonBlocking)  // complaint is here

so perhaps IsNonBlocking can return a non-fail rv but still fail to
set &nonBlocking.



Conditional jump or move depends on uninitialised value(s)
   at 0x7F2A534: nsInputStreamPump::AsyncRead(nsIStreamListener*, nsISupports*) (nsInputStreamPump.cpp:318)
   by 0x7F219D2: nsBaseChannel::BeginPumpingData() (nsBaseChannel.cpp:262)
   by 0x7F21FD5: nsBaseChannel::AsyncOpen(nsIStreamListener*, nsISupports*) (nsBaseChannel.cpp:609)
   by 0x82BECFD: nsDOMFileReader::ReadFileContent(JSContext*, nsIDOMBlob*, nsAString_internal const&, nsDOMFileReader::eDataFormat) (nsDOMFileReader.cpp:462)
   by 0x885DB47: nsIDOMFileReader_ReadAsBinaryString(JSContext*, unsigned int, JS::Value*) (dom_quickstubs.cpp:21087)
   by 0x908D9D6: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:311)
   by 0x908762D: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2801)
   by 0x908DACC: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jsinterp.cpp:537)
   by 0x9022E1D: array_forEach(JSContext*, unsigned int, JS::Value*) (jsinterp.h:157)
   by 0x908D9D6: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:311)
   by 0x908762D: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2801)
   by 0x908DACC: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jsinterp.cpp:537)

 Uninitialised value was created by a stack allocation
   at 0x7F2A4D0: nsInputStreamPump::AsyncRead(nsIStreamListener*, nsISupports*) (nsInputStreamPump.cpp:303)

Comment 1

[Boris Zbarsky [:bzbarsky]]

> so perhaps IsNonBlocking can return a non-fail rv but still fail to
> set &nonBlocking.

It _can_, but if so it's totally broken.

Kyle says that the underlying stream here can be a multiplex stream, a file stream, a partial file stream.  Looking at those, a multiplex stream with 0 underlying streams will in fact do the broken thing for IsNonBlocking.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure to always set our out param in nsMultiplexInputStream::IsNonBlocking when returning success.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Julian, does that fix the bug?

Comment 4

[Julian Seward [:jseward]]

(In reply to Boris Zbarsky (:bz) from comment #3)
Yes, that fixes it.

Comment 5

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7869ec49aba8

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7869ec49aba8