Date: prev next · Thread: first prev next last
2016 Archives by date, by thread · List index




Sent from my iPad, please excuse any misspellings 

On 12 Jan 2016, at 12:05, Chris Sherlock <chris.sherlock79@gmail.com> wrote:

Hi all,

Unfortunately that was the case - I definitely went down the wrong path with that merge.

I've reverted it now - the commit can be found on 
http://cgit.freedesktop.org/libreoffice/core/commit/?id=b4b0cc2a5eef42434444e51fda4a13fc48183aa0

I need to check that UBSan tool more regularly.

But I definitely have to put my hand up for causing these errors. Apologies for this, I will do 
my level best not to let this occur again.

When I'm back home I've been meaning to send a post to the list summarising how font handling 
works and some ideas and questions around the code.
Even better would be to make a wiki page :-)

rgds
jan i

Thanks all (especially Caolon and Stephan)!

Chris

On 12 Jan 2016, at 7:20 PM, Stephan Bergmann <sbergman@redhat.com> wrote:

On 01/11/2016 06:29 PM, Caolán McNamara wrote:
commit c93a43c0c9113bc1c787fe6b3c5e58a8bfd80be0
Author: Caolán McNamara <caolanm@redhat.com>
Date:   Mon Jan 11 17:08:39 2016 +0000

   initialize new member variables

   Change-Id: I3839bc134b337ccb7cfdb2ee70524e4721c8f83c

diff --git a/vcl/source/font/fontattributes.cxx b/vcl/source/font/fontattributes.cxx
index be3ab68..49060f0 100644
--- a/vcl/source/font/fontattributes.cxx
+++ b/vcl/source/font/fontattributes.cxx
@@ -93,7 +93,18 @@ bool FontAttributes::CompareDeviceIndependentFontAttributes(const 
FontAttributes
}

FontAttributes::FontAttributes()
-    : mnWidth ( 0 )
+    : meWeight(WEIGHT_DONTKNOW)
+    , meItalic(ITALIC_DONTKNOW)
+    , meFamily(FAMILY_DONTKNOW)
+    , mePitch(PITCH_DONTKNOW)
+    , meWidthType(WIDTH_DONTKNOW)
+    , mbSymbolFlag(false)
+    , mnQuality(0)
+    , mbOrientation(false)
+    , mbDevice(false)
+    , mbSubsettable(false)
+    , mbEmbeddable(false)
+    , mnWidth ( 0 )
    , mnOrientation( 0 )
    , mnAscent( 0 )
    , mnDescent( 0 )
[...]

Tools like UBSan or Valgrind's memcheck started to complain about reads of uninitialized members 
after a series of commits that merged font-related structs in VCL together.  What looked fishy 
is that those members had not been default-initialized in the original structs, the number of 
setter calls had not changed, and yet the code started to grow uninitialized reads of the 
members.  Without a clear understanding of what was going on, we were reluctant to blindly 
default-initialize those members now, as it might hide problems (at least from the eyes of UBSan 
and Valgrind) instead of fixing them.

Now, as I understand, Chris is going to revert those merges, as he came to the conclusion that 
they went in the wrong direction after all, and I assume that will obsolete the above 
potentially problematic commit anyway.
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.