Uninitialised value use in nsInputStreamPump::AsyncRead

RESOLVED FIXED in mozilla13

Status

()

Core
XPCOM
P4
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jseward, Assigned: bz)

Tracking

Trunk
mozilla13
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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)
> 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.
Component: Networking: HTTP → XPCOM
QA Contact: networking.http → xpcom
Assignee: nobody → bzbarsky
Created attachment 593827 [details] [diff] [review]
Make sure to always set our out param in nsMultiplexInputStream::IsNonBlocking when returning success.
Attachment #593827 - Flags: review?(benjamin)
Julian, does that fix the bug?
Priority: -- → P4
Whiteboard: [need review]
(Reporter)

Comment 4

6 years ago
(In reply to Boris Zbarsky (:bz) from comment #3)
Yes, that fixes it.
Attachment #593827 - Flags: review?(benjamin) → review+
(Reporter)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/integration/mozilla-inbound/rev/7869ec49aba8
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [need review]
Target Milestone: --- → mozilla13

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/7869ec49aba8
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.