[Libreoffice-commits] core.git: vcl/source

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Mon Jul 29 06:43:02 UTC 2019


 vcl/source/control/field.cxx |  107 +++++++++++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 39 deletions(-)

New commits:
commit f53fcf9cfcc0bef415f9d2d95132ccd8bbe96061
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Jul 29 07:50:06 2019 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Jul 29 08:42:21 2019 +0200

    Avoid -fsanitize=float-cast-overflow
    
    ...when converting sal_Int64 9223372036854775807 (i.e., 0xFFFFFFFFFFFFFFF) to
    double 9.22337e+18 (i.e., by rounding up to 2^63, represented as
    0x43e0000000000000) and then back to sal_Int64 (for which it is now too large).
    Started to happen during UITest_writer_dialogs:
    
    > vcl/source/control/field.cxx:1116:35: runtime error: 9.22337e+18 is outside the range of representable values of type 'long'
    >  #0 in MetricField::ConvertValue(long, unsigned short, MapUnit, FieldUnit) at vcl/source/control/field.cxx:1116:35
    >  #1 in svx::sidebar::PosSizePropertyPanel::SetPosSizeMinMax() at svx/source/sidebar/possize/PosSizePropertyPanel.cxx:1058:47
    >  #2 in svx::sidebar::PosSizePropertyPanel::MetricState(SfxItemState, SfxPoolItem const*) at svx/source/sidebar/possize/PosSizePropertyPanel.cxx:893:5
    >  #3 in svx::sidebar::PosSizePropertyPanel::NotifyItemUpdate(unsigned short, SfxItemState, SfxPoolItem const*, bool) at svx/source/sidebar/possize/PosSizePropertyPanel.cxx:735:13
    >  #4 in sfx2::sidebar::ControllerItem::StateChanged(unsigned short, SfxItemState, SfxPoolItem const*) at sfx2/source/sidebar/ControllerItem.cxx:67:26
    >  #5 in SfxStateCache::SetState_Impl(SfxItemState, SfxPoolItem const*, bool) at sfx2/source/control/statcach.cxx:412:24
    >  #6 in SfxStateCache::SetState(SfxItemState, SfxPoolItem const*, bool) at sfx2/source/control/statcach.cxx:326:5
    >  #7 in SfxBindings::Update(unsigned short) at sfx2/source/control/bindings.cxx:352:25
    >  #8 in svx::sidebar::PosSizePropertyPanel::PosSizePropertyPanel(vcl::Window*, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, SfxBindings*, com::sun::star::uno::Reference<com::sun::star::ui::XSidebar> const&) at svx/source/sidebar/possize/PosSizePropertyPanel.cxx:114:17
    [...]
    
    Change-Id: I23a39181a241129f3c6c83e9934405509bd98292
    Reviewed-on: https://gerrit.libreoffice.org/76512
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/vcl/source/control/field.cxx b/vcl/source/control/field.cxx
index 3239d419ac9e..5b343b5a83f4 100644
--- a/vcl/source/control/field.cxx
+++ b/vcl/source/control/field.cxx
@@ -1103,12 +1103,77 @@ sal_Int64 MetricField::ConvertValue( sal_Int64 nValue, sal_Int64 mnBaseValue, sa
     return nLong;
 }
 
+namespace {
+
+bool checkConversionUnits(MapUnit eInUnit, FieldUnit eOutUnit)
+{
+    return eOutUnit != FieldUnit::PERCENT
+        && eOutUnit != FieldUnit::CUSTOM
+        && eOutUnit != FieldUnit::NONE
+        && eInUnit != MapUnit::MapPixel
+        && eInUnit != MapUnit::MapSysFont
+        && eInUnit != MapUnit::MapAppFont
+        && eInUnit != MapUnit::MapRelative;
+}
+
+double convertValue( double nValue, long nDigits, FieldUnit eInUnit, FieldUnit eOutUnit )
+{
+    if ( nDigits < 0 )
+    {
+        while ( nDigits )
+        {
+            nValue += 5;
+            nValue /= 10;
+            nDigits++;
+        }
+    }
+    else
+    {
+        nValue *= ImplPower10(nDigits);
+    }
+
+    if ( eInUnit != eOutUnit )
+    {
+        sal_Int64 nDiv  = aImplFactor[sal_uInt16(eInUnit)][sal_uInt16(eOutUnit)];
+        sal_Int64 nMult = aImplFactor[sal_uInt16(eOutUnit)][sal_uInt16(eInUnit)];
+
+        SAL_WARN_IF( nMult <= 0, "vcl", "illegal *" );
+        SAL_WARN_IF( nDiv  <= 0, "vcl", "illegal /" );
+
+        if ( nMult != 1 && nMult > 0)
+            nValue *= nMult;
+        if ( nDiv != 1 && nDiv > 0 )
+        {
+            nValue += (nValue < 0) ? (-nDiv/2) : (nDiv/2);
+            nValue /= nDiv;
+        }
+    }
+    return nValue;
+}
+
+}
+
 sal_Int64 MetricField::ConvertValue( sal_Int64 nValue, sal_uInt16 nDigits,
                                      MapUnit eInUnit, FieldUnit eOutUnit )
 {
+    if ( !checkConversionUnits(eInUnit, eOutUnit) )
+    {
+        OSL_FAIL( "invalid parameters" );
+        return nValue;
+    }
+
+    long nDecDigits = nDigits;
+    FieldUnit eFieldUnit = ImplMap2FieldUnit( eInUnit, nDecDigits );
+
+    // Avoid sal_Int64 <-> double conversion issues if possible:
+    if (eFieldUnit == eOutUnit && nDigits == 0)
+    {
+        return nValue;
+    }
+
     return static_cast<sal_Int64>(
         nonValueDoubleToValueDouble(
-            ConvertDoubleValue( nValue, nDigits, eInUnit, eOutUnit ) ) );
+            convertValue( nValue, nDecDigits, eFieldUnit, eOutUnit ) ) );
 }
 
 double MetricField::ConvertDoubleValue( double nValue, sal_Int64 mnBaseValue, sal_uInt16 nDecDigits,
@@ -1168,13 +1233,7 @@ double MetricField::ConvertDoubleValue( double nValue, sal_Int64 mnBaseValue, sa
 double MetricField::ConvertDoubleValue( double nValue, sal_uInt16 nDigits,
                                         MapUnit eInUnit, FieldUnit eOutUnit )
 {
-    if ( eOutUnit == FieldUnit::PERCENT ||
-         eOutUnit == FieldUnit::CUSTOM ||
-         eOutUnit == FieldUnit::NONE ||
-         eInUnit == MapUnit::MapPixel ||
-         eInUnit == MapUnit::MapSysFont ||
-         eInUnit == MapUnit::MapAppFont ||
-         eInUnit == MapUnit::MapRelative )
+    if ( !checkConversionUnits(eInUnit, eOutUnit) )
     {
         OSL_FAIL( "invalid parameters" );
         return nValue;
@@ -1183,37 +1242,7 @@ double MetricField::ConvertDoubleValue( double nValue, sal_uInt16 nDigits,
     long nDecDigits = nDigits;
     FieldUnit eFieldUnit = ImplMap2FieldUnit( eInUnit, nDecDigits );
 
-    if ( nDecDigits < 0 )
-    {
-        while ( nDecDigits )
-        {
-            nValue += 5;
-            nValue /= 10;
-            nDecDigits++;
-        }
-    }
-    else
-    {
-        nValue *= ImplPower10(nDecDigits);
-    }
-
-    if ( eFieldUnit != eOutUnit )
-    {
-        sal_Int64 nDiv  = aImplFactor[sal_uInt16(eFieldUnit)][sal_uInt16(eOutUnit)];
-        sal_Int64 nMult = aImplFactor[sal_uInt16(eOutUnit)][sal_uInt16(eFieldUnit)];
-
-        SAL_WARN_IF( nMult <= 0, "vcl", "illegal *" );
-        SAL_WARN_IF( nDiv  <= 0, "vcl", "illegal /" );
-
-        if ( nMult != 1 && nMult > 0)
-            nValue *= nMult;
-        if ( nDiv != 1 && nDiv > 0 )
-        {
-            nValue += (nValue < 0) ? (-nDiv/2) : (nDiv/2);
-            nValue /= nDiv;
-        }
-    }
-    return nValue;
+    return convertValue(nValue, nDecDigits, eFieldUnit, eOutUnit);
 }
 
 double MetricField::ConvertDoubleValue( double nValue, sal_uInt16 nDigits,


More information about the Libreoffice-commits mailing list