uninitialised value use in FastConvertYUVToRGB32Row

RESOLVED FIXED in mozilla2.0b10

Status

()

RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: jseward, Assigned: derf)

Tracking

Trunk
mozilla2.0b10
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
M-C of 15 Jan 2011.
x64-linux release build, "--disable-jemalloc", "-g -O2".

TEST_PATH=content/media/test/test_playback.html

Produces the error shown below.  On peering at the assembly (this is
handwritten assembly):

0000000001547000 <FastConvertYUVToRGB32Row>:
 1547000:       48 8d 05 79 01 d3 00    lea    0xd30179(%rip),%rax
                # 2277180 <kCoefficientsRgbY>
 1547007:       eb 5e                   jmp    1547067
                <FastConvertYUVToRGB32Row+0x67>
0: (loop head)
 1547009:       4c 0f b6 16             movzbq (%rsi),%r10
 154700d:       48 83 c6 01             add    $0x1,%rsi
 1547011:       4c 0f b6 1a             movzbq (%rdx),%r11
 1547015:       48 83 c2 01             add    $0x1,%rdx
 1547019:       f3 42 0f 7e 84 d0 00    movq   0x800(%rax,%r10,8),%xmm0

what is uninitialised is the address expression "0x800(%rax,%r10,8)", so
either rax or r10 (or both) contain at least one bit which is undefined.


Thread 19:
Use of uninitialised value of size 8
   at 0x6592019: FastConvertYUVToRGB32Row (gfx/ycbcr/yuv_row_posix.cpp:69)
   by 0x6591195: mozilla::gfx::ConvertYCbCrToRGB32
                 (gfx/ycbcr/yuv_convert.cpp:113)
   by 0x65612C8: mozilla::layers::BasicPlanarYCbCrImage::SetData
                 (gfx/layers/basic/BasicImages.cpp:231)
   by 0x5D1DA62: VideoData::Create
                 (content/media/nsBuiltinDecoderReader.cpp:153)
   by 0x5D310E2: nsOggReader::DecodeTheora
                 (content/media/ogg/nsOggReader.cpp:568)
   by 0x5D314E0: nsOggReader::DecodeVideoFrame
                 (content/media/ogg/nsOggReader.cpp:622)
   by 0x5D1E578: nsBuiltinDecoderReader::DecodeVideoFrame()
                 (content/media/nsBuiltinDecoderReader.h:527)
   by 0x5D1E645: VideoData* nsBuiltinDecoderReader::DecodeToFirstData
                 (content/media/nsBuiltinDecoderReader.cpp:314)
   by 0x5D1D748: nsBuiltinDecoderReader::FindStartTime
                 (content/media/nsBuiltinDecoderReader.cpp:276)
   by 0x5D19D5F: nsBuiltinDecoderStateMachine::FindStartTime
                 (content/media/nsBuiltinDecoderStateMachine.cpp:1375)
   by 0x5D1C50D: nsBuiltinDecoderStateMachine::Run
                 (content/media/nsBuiltinDecoderStateMachine.cpp:938)
   by 0x64921DD: nsThread::ProcessNextEvent (xpcom/threads/nsThread.cpp:633)

 Uninitialised value was created by a heap allocation
   at 0x4C27878: malloc (vg_replace_malloc.c:236)
   by 0x5D2810D: oc_aligned_malloc (media/libtheora/lib/internal.c:103)
   by 0x5D29D7C: oc_state_init (media/libtheora/lib/state.c:586)
   by 0x5D234C8: th_decode_alloc (media/libtheora/lib/decode.c:374)
   by 0x5D2D6A4: nsTheoraState::Init()
                 (content/media/ogg/nsOggCodecState.cpp:190)
   by 0x5D30784: nsOggReader::ReadMetadata()
                 (content/media/ogg/nsOggReader.cpp:290)
   by 0x5D1A922: nsBuiltinDecoderStateMachine::LoadMetadata()
                 (content/media/nsBuiltinDecoderStateMachine.cpp:1451)
   by 0x5D2CBC8: nsOggDecoderStateMachine::LoadMetadata()
                 (content/media/ogg/nsOggDecoderStateMachine.cpp:51)
   by 0x5D1C4FB: nsBuiltinDecoderStateMachine::Run()
                 (content/media/nsBuiltinDecoderStateMachine.cpp:933)
   by 0x64921DD: nsThread::ProcessNextEvent(int, int*)
                 (xpcom/threads/nsThread.cpp:633)
   by 0x644F2F3: NS_ProcessNextEvent_P(nsIThread*, int)
                 (ff-opt/xpcom/build/nsThreadUtils.cpp:250)
   by 0x6492C84: nsThread::ThreadFunc(void*) (xpcom/threads/nsThread.cpp:278)
(Assignee)

Comment 1

8 years ago
Created attachment 504052 [details] [diff] [review]
Patch version 1.

This was actually a libtheora bug. I've just committed a patch upstream in r17780.  This was harmless (instead of clearing the desired reference frame, it actually cleared most of the current frame's buffer, and some of the padding to the side), but did lead to unpredictable output for streams that started without a keyframe.

For reference, the file that triggered this behavior was content/media/test/bug498380.ogv

I've verified on a local 64-bit Linux build that this fixes the problem.
Assignee: nobody → tterribe
Status: NEW → ASSIGNED
Attachment #504052 - Flags: review?(chris)
Attachment #504052 - Flags: review?(chris) → review+
(Assignee)

Updated

8 years ago
Attachment #504052 - Flags: approval2.0?
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 2

8 years ago
http://hg.mozilla.org/mozilla-central/rev/14f62a4633a6
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.