Issue 116872 - drawinglayer: BPixelRasterToBitmapEx causes false valgrind positive in rtl_crc32
Summary: drawinglayer: BPixelRasterToBitmapEx causes false valgrind positive in rtl_crc32
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: code (show other issues)
Version: DEV300m99
Hardware: All All
: P3 Trivial (vote)
Target Milestone: ---
Assignee: Armin Le Grand
QA Contact: issues@graphics
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-08 12:04 UTC by Stephan Bergmann
Modified: 2017-05-20 10:30 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
potential fix (2.00 KB, text/plain)
2011-02-21 07:45 UTC, Stephan Bergmann
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description Stephan Bergmann 2011-02-08 12:04:25 UTC
At least on DEV300_m99 based CWS sb140, unxlngi6 non-pro, executing
chart2/qa/unoapi under valgrind reports

Use of uninitialised value of size 4
   at 0x40618A2: rtl_crc32 (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_sal.so.3)
   by 0x5B582DE: Bitmap::GetChecksum() const (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libvclli.so)
   by 0x5B7192C: BitmapEx::GetChecksum() const (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libvclli.so)
   by 0x5A5B63F: ImpGraphic::ImplGetChecksum() const (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libvclli.so)
   by 0x5B9AF4B: Graphic::GetChecksum() const (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libvclli.so)
   by 0x5179A72: GraphicID::GraphicID(GraphicObject const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x5179CA5: GraphicCache::AddGraphicObject(GraphicObject const&, Graphic&,
ByteString const*, GraphicObject const*) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x5185D5F: GraphicManager::ImplRegisterObj(GraphicObject const&, Graphic&,
ByteString const*, GraphicObject const*) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x517C307: GraphicObject::ImplSetGraphicManager(GraphicManager const*,
ByteString const*, GraphicObject const*) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x517D15D: GraphicObject::GraphicObject(Graphic const&, GraphicManager
const*) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x13024693:
drawinglayer::RenderBitmapPrimitive2D_GraphicManager(OutputDevice&, BitmapEx
const&, basegfx::B2DHomMatrix const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libdrawinglayerli.so)
   by 0x1302AD95:
drawinglayer::processor2d::VclProcessor2D::RenderBitmapPrimitive2D(drawinglayer::primitive2d::BitmapPrimitive2D
const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libdrawinglayerli.so)

This appears to be something of a false positive:

- The BitmapEx in question has been produced by BPixelRasterToBitmapEx
(drawinglayer/source/processor3d/zbufferprocessor3d.cxx).  For any fully
transparent pixels, the underlying Bitmap color data are uninitialized, while
the corresponding AlphaMask value is 255.  (This full transparency causes the
color data to be effectively ignored, so that most of the time it is irrelevant
that it is uninitialized.)

- The calculated BitmapEx::GetChecksum apparently depends on the uninitialized
data, but it is unclear to me whether that is really important (i.e., whether it
is important that two instances of BitmapEx that only differ in color data for
fully transparent pixel produce the same checksum).  @ka: Please clarify.

I see three possible solutions for this:

1  Fix BPixelRasterToBitmapEx to initialize all color data.  As a proof of
concept, the below patch would silence valgrind.

2  Insert a VALGRIND_MAKE_MEM_DEFINED client request (see
<http://valgrind.org/docs/manual/mc-manual.html#mc-manual.clientreqs>) into
BPixelRasterToBitmapEx to make valgrind think all the color data *is* initialized.

3  Introduce a suppression file (see
<http://valgrind.org/docs/manual/manual-core.html#manual-core.suppress>) and use
that whenever running valgrind.

The patch for (1):

diff --git a/drawinglayer/source/processor3d/zbufferprocessor3d.cxx
b/drawinglayer/source/processor3d/zbufferprocessor3d.cxx
--- a/drawinglayer/source/processor3d/zbufferprocessor3d.cxx
+++ b/drawinglayer/source/processor3d/zbufferprocessor3d.cxx
@@ -107,6 +107,7 @@
 									(sal_uInt8)(nBlue / nDivisor)));
 								pAlpha->SetPixel(y, x, BitmapColor(255 - (sal_uInt8)nOpacity));
 							}
+                            else pContent->SetPixel(y, x, BitmapColor(0, 0, 0));
 						}
 					}
 				}
@@ -125,6 +126,7 @@
 								pContent->SetPixel(y, x, BitmapColor(rPixel.getRed(),
rPixel.getGreen(), rPixel.getBlue()));
 								pAlpha->SetPixel(y, x, BitmapColor(255 - rPixel.getOpacity()));
 							}
+                            else pContent->SetPixel(y, x, BitmapColor(0, 0, 0));
 						}
 					}
 				}
Comment 1 Stephan Bergmann 2011-02-09 08:07:06 UTC
Another false positive in the same scenario (DEV300_m99 based CWS sb140,
unxlngi6 non-pro, chart2/qa/unoapi) that also goes away with the patch for (1) is

Syscall param pwrite64(buf) points to uninitialised byte(s)
   at 0x47A1C0B: pwrite64 (pwrite64.c:54)
   by 0x4056241: FileHandle_Impl::writeAt(long long, void const*, unsigned int,
unsigned long long*) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_sal.so.3)
   by 0x4056D79: FileHandle_Impl::writeFileAt(long long, void const*, unsigned
int, unsigned long long*) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_sal.so.3)
   by 0x405726F: osl_writeFile (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_sal.so.3)
   by 0xA40197C: osl::File::write(void const*, unsigned long long, unsigned long
long&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libucpfile1.so)
   by 0xA420976: fileaccess::ReconnectingFile::write(void const*, unsigned long
long, unsigned long long&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libucpfile1.so)
   by 0xA418A6B:
fileaccess::XStream_impl::writeBytes(com::sun::star::uno::Sequence<signed char>
const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libucpfile1.so)
   by 0x15A161C0: OWriteStream::writeBytes(com::sun::star::uno::Sequence<signed
char> const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libxstor.so)
   by 0x488CF47:
comphelper::OStorageHelper::CopyInputToOutput(com::sun::star::uno::Reference<com::sun::star::io::XInputStream>
const&, com::sun::star::uno::Reference<com::sun::star::io::XOutputStream>
const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libcomphelp4gcc3.so)
   by 0x4803B24:
comphelper::EmbeddedObjectContainer::InsertGraphicStream(com::sun::star::uno::Reference<com::sun::star::io::XInputStream>
const&, rtl::OUString const&, rtl::OUString const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libcomphelp4gcc3.so)
   by 0x51955AF: svt::EmbeddedObjectRef::GetGraphicStream(unsigned char) const
(in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x5195AF9: svt::EmbeddedObjectRef::GetReplacement(unsigned char) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
 Address 0x1621ee33 is 715 bytes inside a block of size 32,008 alloc'd
   at 0x4025F20: malloc (vg_replace_malloc.c:236)
   by 0x4085590: rtl_allocateMemory (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_sal.so.3)
   by 0x4A35DC4: cppu::reallocSeq(_sal_Sequence*, unsigned long, long) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_cppu.so.3)
   by 0x4A36151: cppu::idefaultConstructElements(_sal_Sequence**,
_typelib_TypeDescriptionReference*, long, long, long) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_cppu.so.3)
   by 0x4A38764: uno_type_sequence_construct (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/ure/lib/libuno_cppu.so.3)
   by 0x4854949: com::sun::star::uno::Sequence<signed char>::Sequence(long) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libcomphelp4gcc3.so)
   by 0x488CE8D:
comphelper::OStorageHelper::CopyInputToOutput(com::sun::star::uno::Reference<com::sun::star::io::XInputStream>
const&, com::sun::star::uno::Reference<com::sun::star::io::XOutputStream>
const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libcomphelp4gcc3.so)
   by 0x4803B24:
comphelper::EmbeddedObjectContainer::InsertGraphicStream(com::sun::star::uno::Reference<com::sun::star::io::XInputStream>
const&, rtl::OUString const&, rtl::OUString const&) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libcomphelp4gcc3.so)
   by 0x51955AF: svt::EmbeddedObjectRef::GetGraphicStream(unsigned char) const
(in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x5195AF9: svt::EmbeddedObjectRef::GetReplacement(unsigned char) (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x5195BB7: svt::EmbeddedObjectRef::GetGraphic(rtl::OUString*) const (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvtli.so)
   by 0x135363C8: SdrOle2Obj::GetGraphic() const (in
/cws/sb140_m99/DEV300/unxlngi6/installation/opt/openoffice.org/basis3.4/program/libsvxcoreli.so)
Comment 2 Stephan Bergmann 2011-02-21 07:45:43 UTC
Created attachment 75901 [details]
potential fix
Comment 3 Stephan Bergmann 2011-02-21 07:47:32 UTC
So, any objections against the attached i116872.patch (against DEV300_m100)?  It
pre-initializes neither Bitmap nor AlphaMask, and unconditionally sets all RGB
and alpha values, regardless of opacity.
Comment 4 Stephan Bergmann 2011-02-28 13:10:10 UTC
attached i116872.patch applied as <http://hg.services.openoffice.org/cws/sb140/rev/3b6084cd3349>
Comment 5 Stephan Bergmann 2011-04-18 11:24:56 UTC
@aw: please verify
Comment 6 Stephan Bergmann 2011-04-19 11:10:39 UTC
plus <http://hg.services.openoffice.org/cws/sb140/rev/74d7ca131142>
Comment 7 Armin Le Grand 2011-04-19 13:38:55 UTC
AW: Checked with requested changes, Okay.