Open
Bug 798989
Opened 12 years ago
Updated 2 years ago
Uninitialised value use in gfxUtils::GetYCbCrToRGBDestFormatAndSize (aSuggestedFormat)
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
NEW
People
(Reporter: kinetik, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: valgrind)
Conditional jump or move depends on uninitialised value(s)
at 0x6455231: gfxUtils::GetYCbCrToRGBDestFormatAndSize(layers::PlanarYCbCrImage::Data const&, gfxASurface::gfxImageFormat&, nsIntSize&) (gfxUtils.cpp:647)
by 0x6477F26: layers::PlanarYCbCrImage::GetAsSurface() (ImageContainer.cpp:483)
by 0x64788A5: layers::ImageContainer::LockCurrentAsSurface(nsIntSize*, layers::Image**) (ImageContainer.cpp:287)
by 0x647328B: layers::BasicImageLayer::GetAndPaintCurrentImage(gfxContext*, float, layers::Layer*) (ImageContainer.h:560)
by 0x64735C2: layers::BasicImageLayer::Paint(gfxContext*, layers::Layer*) (BasicImageLayer.cpp:73)
by 0x6473FB9: layers::BasicShadowableImageLayer::Paint(gfxContext*, layers::Layer*) (BasicImageLayer.cpp:248)
by 0x646B7B1: layers::BasicLayerManager::PaintSelfOrChildren(layers::PaintContext&, gfxContext*) (BasicLayerManager.cpp:815)
Conditional jump or move depends on uninitialised value(s)
at 0x6455236: gfxUtils::GetYCbCrToRGBDestFormatAndSize(layers::PlanarYCbCrImage::Data const&, gfxASurface::gfxImageFormat&, nsIntSize&) (gfxUtils.cpp:670)
by 0x6477F26: layers::PlanarYCbCrImage::GetAsSurface() (ImageContainer.cpp:483)
by 0x64788A5: layers::ImageContainer::LockCurrentAsSurface(nsIntSize*, layers::Image**) (ImageContainer.cpp:287)
by 0x647328B: layers::BasicImageLayer::GetAndPaintCurrentImage(gfxContext*, float, layers::Layer*) (ImageContainer.h:560)
by 0x64735C2: layers::BasicImageLayer::Paint(gfxContext*, layers::Layer*) (BasicImageLayer.cpp:73)
by 0x6473FB9: layers::BasicShadowableImageLayer::Paint(gfxContext*, layers::Layer*) (BasicImageLayer.cpp:248)
by 0x646B7B1: layers::BasicLayerManager::PaintSelfOrChildren(layers::PaintContext&, gfxContext*) (BasicLayerManager.cpp:815)
This is referring to mOffscreenSurface in PlanarYCbCrImage, which is initialised via SetOffscreenFormat. SetOffscreenFormat is only ever called from the subclass BasicPlanarYCbCrImage. This instance of PlanarYCbCrImage was created by ImageFactory::Create.
|
||
Comment 1•12 years ago
|
||
Sounds like ImageFactory::Create should initialize mOffscreenSurface
|
||
Comment 2•12 years ago
|
||
Hi, I think there are other variable(s) that may require initialization.
Title: Uninitialized value usage in mozilla/gfx/thebes/gfxUtils.cpp
Found by valgrind run. See bug 803816 about mozmill run of TB.
comm-central thunderbird.
Source version:
$ hg identify
1016cef82fd8+ tip
ishikawa@debian-vm:~/TB-NEW/TB-3HG/new-src$ cd mozilla
ishikawa@debian-vm:~/TB-NEW/TB-3HG/new-src/mozilla$ hg identify
a517f7ea5bef+ tip
There are a few cases quoted below and more cases of warnings that seem to
be caused uninitialized image data structure. (Caused by
allocation in CreateImage() [and possibly the lower level routines that are called under it.])
CASE I:
valgrind log
15887:==27214== Conditional jump or move depends on uninitialised value(s)
15888-==27214== at 0x606F9F1: gfxUtils::GetYCbCrToRGBDestFormatAndSize(mozilla::layers::PlanarYCbCrImage::Data const&, gfxASurface::gfxImageFormat&, nsIntSize&) (gfxUtils.cpp:647)
15889-==27214== by 0x473155B: ??? (in /TB-NEW/TB-3HG/objdir-tb3/mozilla/nsprpub/pr/src/libnspr4.so)
15890:==27214== Uninitialised value was created by a heap allocation
15891-==27214== at 0x40271C4: malloc (vg_replace_malloc.c:270)
15892-==27214== by 0x4041E10: moz_xmalloc (mozalloc.cpp:54)
15893-==27214== by 0x6098170: mozilla::layers::ImageFactory::CreateImage(mozilla::ImageFormat const*, unsigned int, nsIntSize const&, mozilla::layers::BufferRecycleBin*) (mozalloc.h:200)
15894-==27214== by 0x6097AE1: mozilla::layers::ImageContainer::CreateImage(mozilla::ImageFormat const*, unsigned int) (ImageContainer.cpp:147)
15895-==27214== by 0x55AADD7: VideoData::Create(nsVideoInfo&, mozilla::layers::ImageContainer*, long long, long long, long long, VideoData::YCbCrBuffer const&, bool, long long, nsIntRect) (nsBuiltinDecoderReader.cpp:209)
15896-==27214== by 0x55BD54B: nsOggReader::DecodeTheora(ogg_packet*, long long) (nsOggReader.cpp:651)
Observation:
mozilla/gfx/thebes/gfxUtils.cpp
632 gfxUtils::GetYCbCrToRGBDestFormatAndSize(const PlanarYCbCrImage::Data& aData,
633 gfxASurface::gfxImageFormat& aSuggestedFormat,
634 gfxIntSize& aSuggestedSize)
635 {
636 gfx::YUVType yuvtype =
637 gfx::TypeFromSize(aData.mYSize.width,
638 aData.mYSize.height,
639 aData.mCbCrSize.width,
640 aData.mCbCrSize.height);
641
642 // 'prescale' is true if the scaling is to be done as part of the
643 // YCbCr to RGB conversion rather than on the RGB data when rendered.
644 bool prescale = aSuggestedSize.width > 0 && aSuggestedSize.height > 0 &&
645 aSuggestedSize != aData.mPicSize;
646
647 if (aSuggestedFormat == gfxASurface::ImageFormatRGB16_565) {
ImageContainer.cpp
143 ImageContainer::CreateImage(const ImageFormat *aFormats,
144 uint32_t aNumFormats)
145 {
146 ReentrantMonitorAutoEnter mon(mReentrantMonitor);
147 return mImageFactory->CreateImage(aFormats, aNumFormats, mScaleHint, mRecycleBin);
148 }
149
CASE II
15911:==27214== Conditional jump or move depends on uninitialised value(s)
15912-==27214== at 0x606F9F6: gfxUtils::GetYCbCrToRGBDestFormatAndSize(mozilla::layers::PlanarYCbCrImage::Data const&, gfxASurface::gfxImageFormat&, nsIntSize&) (gfxUtils.cpp:670)
15913-==27214== by 0x473155B: ??? (in /TB-NEW/TB-3HG/objdir-tb3/mozilla/nsprpub/pr/src/libnspr4.so)
15914:==27214== Uninitialised value was created by a heap allocation
15915-==27214== at 0x40271C4: malloc (vg_replace_malloc.c:270)
15916-==27214== by 0x4041E10: moz_xmalloc (mozalloc.cpp:54)
15917-==27214== by 0x6098170: mozilla::layers::ImageFactory::CreateImage(mozilla::ImageFormat const*, unsigned int, nsIntSize const&, mozilla::layers::BufferRecycleBin*) (mozalloc.h:200)
15918-==27214== by 0x6097AE1: mozilla::layers::ImageContainer::CreateImage(mozilla::ImageFormat const*, unsigned int) (ImageContainer.cpp:147)
15919-==27214== by 0x55AADD7: VideoData::Create(nsVideoInfo&, mozilla::layers::ImageContainer*, long long, long long, long long, VideoData::YCbCrBuffer const&, bool, long long, nsIntRect) (nsBuiltinDecoderReader.cpp:209)
15920-==27214== by 0x55BD54B: nsOggReader::DecodeTheora(ogg_packet*, long long) (nsOggReader.cpp:651)
--
This is more difficult to figure out.
If aSuggestedFormat is not defined on line 670, it ought to
have raised a warning on line 647 already as in case I.
Am I missing something?
647 if (aSuggestedFormat == gfxASurface::ImageFormatRGB16_565) {
648 #if defined(HAVE_YCBCR_TO_RGB565)
649 if (prescale &&
650 !gfx::IsScaleYCbCrToRGB565Fast(aData.mPicX,
651 aData.mPicY,
652 aData.mPicSize.width,
653 aData.mPicSize.height,
654 aSuggestedSize.width,
655 aSuggestedSize.height,
656 yuvtype,
657 gfx::FILTER_BILINEAR) &&
658 gfx::IsConvertYCbCrToRGB565Fast(aData.mPicX,
659 aData.mPicY,
660 aData.mPicSize.width,
661 aData.mPicSize.height,
662 yuvtype)) {
663 prescale = false;
664 }
665 #else
666 // yuv2rgb16 function not available
667 aSuggestedFormat = gfxASurface::ImageFormatRGB24;
668 #endif
669 }
670 else if (aSuggestedFormat != gfxASurface::ImageFormatRGB24) {
CASE III:
15935:==27214== Conditional jump or move depends on uninitialised value(s)
15936-==27214== at 0x604D7B6: gfxImageSurface::ComputeStride(nsIntSize const&, gfxASurface::gfxImageFormat) (gfxImageSurface.cpp:157)
15937-==27214== by 0x604D8A0: gfxImageSurface::gfxImageSurface(nsIntSize const&, gfxASurface::gfxImageFormat, bool) (gfxImageSurface.h:100)
15938-==27214== by 0x609852E: mozilla::layers::PlanarYCbCrImage::GetAsSurface() (ImageContainer.cpp:491)
15939-==27214== by 0x6098A37: mozilla::layers::ImageContainer::LockCurrentAsSurface(nsIntSize*, mozilla::layers::Image**) (ImageContainer.cpp:287)
15940-==27214== by 0x27F: ???
15941:==27214== Uninitialised value was created by a heap allocation
15942-==27214== at 0x40271C4: malloc (vg_replace_malloc.c:270)
15943-==27214== by 0x4041E10: moz_xmalloc (mozalloc.cpp:54)
15944-==27214== by 0x6098170: mozilla::layers::ImageFactory::CreateImage(mozilla::ImageFormat const*, unsigned int, nsIntSize const&, mozilla::layers::BufferRecycleBin*) (mozalloc.h:200)
15945-==27214== by 0x6097AE1: mozilla::layers::ImageContainer::CreateImage(mozilla::ImageFormat const*, unsigned int) (ImageContainer.cpp:147)
15946-==27214== by 0x55AADD7: VideoData::Create(nsVideoInfo&, mozilla::layers::ImageContainer*, long long, long long, long long, VideoData::YCbCrBuffer const&, bool, long long, nsIntRect) (nsBuiltinDecoderReader.cpp:209)
15947-==27214== by 0x55BD54B: nsOggReader::DecodeTheora(ogg_packet*, long long) (nsOggReader.cpp:651)
The image data dynamically allocated is not properly initialized and
eventually, its uninitialized data is referenced in ComputeStride.
152 /*static*/ long
153 gfxImageSurface::ComputeStride(const gfxIntSize& aSize, gfxImageFormat aFormat)
154 {
155 long stride;
156
157 if (aFormat == ImageFormatARGB32)
158 stride = aSize.width * 4;
159 else if (aFormat == ImageFormatRGB24)
This is caused by the failure to initialize data allocated by the same
routine, CreateImage() in the case I above. So I am reporting this
problem in this same bugzilla entry.
ImageContainer.cpp
143 ImageContainer::CreateImage(const ImageFormat *aFormats,
144 uint32_t aNumFormats)
CASE IV
==27214== Conditional jump or move depends on uninitialised value(s)
==27214== at 0x604D7BB: gfxImageSurface::ComputeStride(nsIntSize const&, gfxASurface::gfxImageFormat) (gfxImageSurface.cpp:159)
==27214== by 0x604D8A0: gfxImageSurface::gfxImageSurface(nsIntSize const&, gfxASurface::gfxImageFormat, bool) (gfxImageSurface.h:100)
==27214== by 0x609852E: mozilla::layers::PlanarYCbCrImage::GetAsSurface() (ImageContainer.cpp:491)
==27214== by 0x6098A37: mozilla::layers::ImageContainer::LockCurrentAsSurface(nsIntSize*, mozilla::layers::Image**) (ImageContainer.cpp:287)
==27214== by 0x27F: ???
==27214== Uninitialised value was created by a heap allocation
==27214== at 0x40271C4: malloc (vg_replace_malloc.c:270)
==27214== by 0x4041E10: moz_xmalloc (mozalloc.cpp:54)
==27214== by 0x6098170: mozilla::layers::ImageFactory::CreateImage(mozilla::ImageFormat const*, unsigned int, nsIntSize const&, mozilla::layers::BufferRecycleBin*) (mozalloc.h:200)
==27214== by 0x6097AE1: mozilla::layers::ImageContainer::CreateImage(mozilla::ImageFormat const*, unsigned int) (ImageContainer.cpp:147)
==27214== by 0x55AADD7: VideoData::Create(nsVideoInfo&, mozilla::layers::ImageContainer*, long long, long long, long long, VideoData::YCbCrBuffer const&, bool, long long, nsIntRect) (nsBuiltinDecoderReader.cpp:209)
==27214== by 0x55BD54B: nsOggReader::DecodeTheora(ogg_packet*, long long) (nsOggReader.cpp:651)
==27214== by 0x55BF1D5: nsOggReader::DecodeVideoFrame(bool&, long long) (nsOggReader.cpp:696)
==27214== by 0x55AA149: nsBuiltinDecoderReader::DecodeVideoFrame() (nsBuiltinDecoderReader.h:523)
==27214== by 0x55AA455: nsBuiltinDecoderReader::FindStartTime(long long&) (nsBuiltinDecoderReader.h:513)
==27214== by 0x55A453A: nsBuiltinDecoderStateMachine::FindStartTime() (nsBuiltinDecoderStateMachine.cpp:2366)
==27214== by 0x55A659E: nsBuiltinDecoderStateMachine::DecodeMetadata() (nsBuiltinDecoderStateMachine.cpp:1755)
==27214== by 0x55AA0E4: nsBuiltinDecoderStateMachine::DecodeThreadRun() (nsBuiltinDecoderStateMachine.cpp:490)
==27214== by 0x426723D: clone (clone.S:130)
Note the line number in gfxImageSurface.cpp: 159. Case III has
157.
Can it be that there are cases where aFormat is defined, but
aSize is undefined?
CASE nnn:
There are many cases where the warning seems to be triggered by
non-initialized data created by CreateImage() routine (or
possibly further allocated by a lower routine that gets called below
it.) See the full log psted in bug 803816.
Right, ImageFactor::Create may be the better higher-level routine to initialize that, come to think of it, but I will leave the decision to people in the know.
It is nice to fix this since there are SO MANY warnings related to this problem in "make mozmill" run of TB.
TIA
|
||
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•