include/vcl/bitmapaccess.hxx | 35 ++++++++++++++++++++++++----------- vcl/source/gdi/bmpacc.cxx | 4 +++- 2 files changed, 27 insertions(+), 12 deletions(-)
New commits: commit 96fc7c66349ffaa9841f1abe0d152838aebb8317 Author: Caolán McNamara <caol...@redhat.com> AuthorDate: Fri Sep 10 16:12:02 2021 +0100 Commit: Caolán McNamara <caol...@redhat.com> CommitDate: Mon Sep 13 18:02:22 2021 +0200 crashtesting: threaded scaling crash on re-export of ooo24840-1.sxw to odt #13 0x00007f1cb843752a in o3tl::cow_wrapper<ImplBitmapPalette, o3tl::UnsafeRefCountingPolicy>::operator->() (this=0x5596086d5968) at include/o3tl/cow_wrapper.hxx:329 __PRETTY_FUNCTION__ = "BitmapColor& BitmapPalette::operator[](sal_uInt16)" #14 0x00007f1cb843752a in BitmapPalette::operator[](unsigned short) (this=0x5596086d5968, nIndex=nIndex@entry=0) at vcl/source/bitmap/bitmappalette.cxx:139 __PRETTY_FUNCTION__ = "BitmapColor& BitmapPalette::operator[](sal_uInt16)" #15 0x00007f1cb849f5f5 in BitmapInfoAccess::GetPaletteColor(unsigned short) const (nColor=0, this=0x5596085989f0) at include/vcl/BitmapInfoAccess.hxx:114 __PRETTY_FUNCTION__ = "const BitmapColor& BitmapInfoAccess::GetPaletteColor(sal_uInt16) const" the mpBuffer member of BitmapInfoAccess is BitmapBuffer* mpBuffer; not const BitmapBuffer* mpBuffer; so mpBuffer->maPalette.foo() calls non-const variants of foo(), (BitmapPalette::operator[](unsigned short) in this case), which is presumably non the expected outcome, as the copy-on-write mpImpl of BitmapPalette unsafely creates a new copy its internals on the first dereference of mpImpl in a non-const method. Change-Id: I1ebb3c67386a9028e5b8bab4b2d1cc5862700aa1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121917 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caol...@redhat.com> diff --git a/include/vcl/bitmapaccess.hxx b/include/vcl/bitmapaccess.hxx index 12304d96ff86..559b5c7a3c02 100644 --- a/include/vcl/bitmapaccess.hxx +++ b/include/vcl/bitmapaccess.hxx @@ -96,31 +96,39 @@ public: bool HasPalette() const { - assert(mpBuffer && "Access is not valid!"); + const BitmapBuffer* pBuffer = mpBuffer; + + assert(pBuffer && "Access is not valid!"); - return mpBuffer && !!mpBuffer->maPalette; + return pBuffer && !!pBuffer->maPalette; } const BitmapPalette& GetPalette() const { - assert(mpBuffer && "Access is not valid!"); + const BitmapBuffer* pBuffer = mpBuffer; - return mpBuffer->maPalette; + assert(pBuffer && "Access is not valid!"); + + return pBuffer->maPalette; } sal_uInt16 GetPaletteEntryCount() const { + const BitmapBuffer* pBuffer = mpBuffer; + assert(HasPalette() && "Bitmap has no palette!"); - return HasPalette() ? mpBuffer->maPalette.GetEntryCount() : 0; + return HasPalette() ? pBuffer->maPalette.GetEntryCount() : 0; } const BitmapColor& GetPaletteColor( sal_uInt16 nColor ) const { - assert(mpBuffer && "Access is not valid!"); + const BitmapBuffer* pBuffer = mpBuffer; + + assert(pBuffer && "Access is not valid!"); assert(HasPalette() && "Bitmap has no palette!"); - return mpBuffer->maPalette[nColor]; + return pBuffer->maPalette[nColor]; } const BitmapColor& GetBestPaletteColor(const BitmapColor& rBitmapColor) const @@ -130,11 +138,13 @@ public: sal_uInt16 GetBestPaletteIndex(const BitmapColor& rBitmapColor) const; - ColorMask& GetColorMask() const + const ColorMask& GetColorMask() const { - assert(mpBuffer && "Access is not valid!"); + const BitmapBuffer* pBuffer = mpBuffer; + + assert(pBuffer && "Access is not valid!"); - return mpBuffer->maColorMask; + return pBuffer->maColorMask; } private: @@ -207,7 +217,10 @@ public: BitmapColor GetColor(tools::Long nY, tools::Long nX) const { if (HasPalette()) - return mpBuffer->maPalette[GetPixelIndex(nY, nX)]; + { + const BitmapBuffer* pBuffer = mpBuffer; + return pBuffer->maPalette[GetPixelIndex(nY, nX)]; + } else return GetPixel(nY, nX); } diff --git a/vcl/source/gdi/bmpacc.cxx b/vcl/source/gdi/bmpacc.cxx index 18c5769678c5..9c418a374e33 100644 --- a/vcl/source/gdi/bmpacc.cxx +++ b/vcl/source/gdi/bmpacc.cxx @@ -79,7 +79,9 @@ BitmapInfoAccess::~BitmapInfoAccess() sal_uInt16 BitmapInfoAccess::GetBestPaletteIndex( const BitmapColor& rBitmapColor ) const { - return( HasPalette() ? mpBuffer->maPalette.GetBestIndex( rBitmapColor ) : 0 ); + const BitmapBuffer* pBuffer = mpBuffer; + + return( HasPalette() ? pBuffer->maPalette.GetBestIndex( rBitmapColor ) : 0 ); } BitmapReadAccess::BitmapReadAccess( Bitmap& rBitmap, BitmapAccessMode nMode ) :