Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 723860 - Uninitalised value use in nsMediaCacheStream::GetNextCachedDataInternal
: Uninitalised value use in nsMediaCacheStream::GetNextCachedDataInternal
: valgrind
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla16
Assigned To: Matthew Gregan [:kinetik]
: Maire Reavy [:mreavy]
Depends on:
  Show dependency treegraph
Reported: 2012-02-03 02:35 PST by Julian Seward [:jseward]
Modified: 2012-06-07 05:44 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v0 (2.06 KB, patch)
2012-06-05 22:59 PDT, Matthew Gregan [:kinetik]
cajbir.bugzilla: review+
Details | Diff | Splinter Review

Description Julian Seward [:jseward] 2012-02-03 02:35:36 PST
x86_64-linux, running on a 16 bit deep vnc server.

TEST_PATH= .. well, from the log output it seems like it occurs in
content/media/test/test_referer.html.  But I can't reproduce it
with that test path.  I can only reproduce with TEST_PATH=content/media/test


(DISPLAY=:1 TEST_PATH=content/media/test 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-3b

Complaint is:

Conditional jump or move depends on uninitialised value(s)
   at 0x86B08D4: nsMediaCacheStream::GetNextCachedDataInternal(long) (nsMediaCache.cpp:2041)
   by 0x86B09A3: nsMediaCacheStream::GetNextCachedData(long) (nsMediaCache.cpp:1995)
   by 0x86C686E: nsWaveReader::GetBuffered(nsTimeRanges*, long) (nsWaveReader.cpp:275)
   by 0x86B77B7: nsBuiltinDecoderStateMachine::GetBuffered(nsTimeRanges*) (nsBuiltinDecoderStateMachine.cpp:2184)
   by 0x86B78EF: nsBuiltinDecoderStateMachine::NotifyDataArrived(char const*, unsigned int, unsigned int) (nsBuiltinDecoderStateMachine.cpp:1231)
   by 0x86B363E: nsMediaChannelStream::CopySegmentToCache(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) (nsMediaStream.cpp:378)
   by 0x8CD0E92: nsInputStreamTee::WriteSegmentFun(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) (nsInputStreamTee.cpp:223)
   by 0x8CD654E: nsPipeInputStream::ReadSegments(unsigned int (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) (nsPipe3.cpp:799)
   by 0x86B419C: nsMediaChannelStream::OnDataAvailable(nsIRequest*, nsIInputStream*, unsigned int) (nsMediaStream.cpp:411)
   by 0x7F470F5: nsStreamListenerTee::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned int, unsigned int) (nsStreamListenerTee.cpp:122)
   by 0x7FB0E00: nsHttpChannel::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned int, unsigned int) (nsHttpChannel.cpp:4456)
   by 0x7F2AA3A: nsInputStreamPump::OnStateTransfer() (nsInputStreamPump.cpp:514)

 Uninitialised value was created by a heap allocation
   at 0x4C28223: malloc (vg_replace_malloc.c:263)
   by 0x67BFECB: moz_xmalloc (mozalloc.cpp:103)
   by 0x86C664F: nsWaveDecoder::CreateStateMachine() (mozalloc.h:229)
   by 0x86B71FF: nsBuiltinDecoder::Load(nsMediaStream*, nsIStreamListener**, nsMediaDecoder*) (nsBuiltinDecoder.cpp:204)
   by 0x845C408: nsHTMLMediaElement::InitializeDecoderForChannel(nsIChannel*, nsIStreamListener**) (nsHTMLMediaElement.cpp:2074)
   by 0x845C7E9: nsHTMLMediaElement::MediaLoadListener::OnStartRequest(nsIRequest*, nsISupports*) (nsHTMLMediaElement.cpp:348)
   by 0x7FB7D60: nsHttpChannel::CallOnStartRequest() (nsHttpChannel.cpp:764)
   by 0x7FB827E: nsHttpChannel::ContinueProcessNormal(unsigned int) (nsHttpChannel.cpp:1260)
   by 0x7FB842A: nsHttpChannel::ProcessNormal() (nsHttpChannel.cpp:1197)
   by 0x7FBB2C2: nsHttpChannel::ProcessResponse() (nsHttpChannel.cpp:1099)
   by 0x7FBBB64: nsHttpChannel::OnStartRequest(nsIRequest*, nsISupports*) (nsHttpChannel.cpp:4164)
   by 0x7F2A837: nsInputStreamPump::OnStateStart() (nsInputStreamPump.cpp:444)
Comment 1 Julian Seward [:jseward] 2012-03-22 04:25:31 PDT
This is hard to reproduce, but it's still around, and I get the
impression there's something ungood about it.

nsMediaCacheStream::GetNextCachedDataInternal(PRInt64 aOffset) it's
clear that it can be supplied with an uninitialised aOffset parameter
at some point.  Indeed, printing them whilst running all of
content/media/test produces small values, eg, 0, 28942, 285310.  But
at the point where aOffset is reported as uninitialised, aOffset has
the value 4209526967819254892 (== 0x3a6b3e79654b3c6c), which doesn't
sound like a valid offset.  This also removes any doubt that it might
be a false error from valgrind.

Following the trail of callers here, it seems that the value
comes from

nsWaveReader::GetBuffered(nsTimeRanges*, long) (nsWaveReader.cpp:274)

in this call

PRInt64 startOffset =

so I am wondering if mWavePCMOffset could be uninitialised at this
point.  (Well, obviously yes; the real question is, why?)

Looking around nsWaveReader.cpp, the only place mWavePCMOffset is
assigned to is in nsWaveReader::FindDataOffset().
Comment 2 Julian Seward [:jseward] 2012-03-22 05:31:14 PDT
A bit more digging suggests that the uninitialised mWavePCMOffset
is created by this call stack (hand edited for clarity):

Uninitialised value was created by a heap allocation
  at 0x4029B9A: malloc (vg_replace_malloc.c:263)
  by 0x403CF6B: moz_xmalloc (mozalloc.cpp:103)
  by .........  nsWaveReader::nsWaveReader(nsBuiltinDecoder*)
  by 0x69F5505: nsWaveDecoder::CreateStateMachine() (nsWaveDecoder.cpp:61)
  by 0x69E408F: nsBuiltinDecoder::Load(mozilla::MediaResource*, nsIStreamListener**, nsMediaDecoder*) (nsBuiltinDecoder.cpp:203)
  by 0x679C268: nsHTMLMediaElement::InitializeDecoderForChannel(nsIChannel*, nsIStreamListener**) (nsHTMLMediaElement.cpp:2078)
  by 0x679C649: nsHTMLMediaElement::MediaLoadListener::OnStartRequest(nsIRequest*, nsISupports*) (nsHTMLMediaElement.cpp:347)
  by 0x632BC70: nsHttpChannel::CallOnStartRequest() (nsHttpChannel.cpp:764)
  by 0x632C18E: nsHttpChannel::ContinueProcessNormal(unsigned int) (nsHttpChannel.cpp:1260)
  by 0x632C33A: nsHttpChannel::ProcessNormal() (nsHttpChannel.cpp:1197)
  by 0x632F1D2: nsHttpChannel::ProcessResponse() (nsHttpChannel.cpp:1099)
  by 0x632FA74: nsHttpChannel::OnStartRequest(nsIRequest*, nsISupports*) (nsHttpChannel.cpp:4186)

nsWaveReader::nsWaveReader doesn't appear to initialise anything.
Maybe one of the other methods (nsWaveReader::Init ?)  is supposed to
do it?
Comment 3 Matthew Gregan [:kinetik] 2012-05-28 19:10:24 PDT
It looks like GetBuffered is called before ReadMetdata (which initializes mWavePCMOffset).  The simple fix is to add a initialization check to GetBuffered and early exit.  nsGstreamerReader.cpp needs a similar fix, otherwise it looks like it'll try to use a NULL mPlayBin.  The other nsBuiltinDecoderReader subclasses already contain initialization checks:

  if (!mInfo.mHasVideo && !mInfo.mHasAudio) {
and those two are initialized to false by default and may only become true when ReadMetadata is called.

  if (!mContext || nestegg_tstamp_scale(mContext, &timecodeScale) == -1) {
mContext will only be non-NULL after ReadMetadata.

nsRawReader.cpp doesn't implement GetBuffered.

The pluggable decoder/reader pair from bug 714408 needs to be checked too.
Comment 4 Matthew Gregan [:kinetik] 2012-06-05 22:59:52 PDT
Created attachment 630458 [details] [diff] [review]
patch v0

nsMediaPluginReader.cpp checks mPlugin which is initialized during ReadMetadata, so that didn't need to be fixed.  This patch fixes the Wave and Gstreamer readers, and removes a lie from the Ogg reader comments.
Comment 5 Matthew Gregan [:kinetik] 2012-06-06 20:14:40 PDT
Comment 6 Ed Morley [:emorley] 2012-06-07 05:44:43 PDT

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