Move validity check to svl::Items, to avoid invalid WhichRangesContainer

This uncovered lots of pre-existing invalid ranges, e.g. introduced
in commit 6dbfbebad37fd84208e4c336f0864d26019db153, or in much older
commit 46952138c938730afcc3607e1a524bb590b0e30e.

Also this makes the static in svl::Items to be array of WhichPair,
to avoid questionable reinterpret_cast in WhichRangesContainer ctor.

Change-Id: I86030b2a2ac0a6d98870f8f7f5cc83e071c6597c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119003
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

13 files changed, 47 insertions, 59 deletions

diff --git a/cui/source/tabpages/align.cxx b/cui/source/tabpages/align.cxx
index b178630ab61e..4c9b43454c74 100644
--- a/cui/source/tabpages/align.cxx
+++ b/cui/source/tabpages/align.cxx
@@ -41,15 +41,15 @@ namespace svx {
 
 const WhichRangesContainer AlignmentTabPage::s_pRanges(
     svl::Items<
-    SID_ATTR_ALIGN_HOR_JUSTIFY, SID_ATTR_ALIGN_VER_JUSTIFY,
     SID_ATTR_ALIGN_STACKED, SID_ATTR_ALIGN_LINEBREAK,
     SID_ATTR_ALIGN_INDENT, SID_ATTR_ALIGN_INDENT,
     SID_ATTR_ALIGN_DEGREES, SID_ATTR_ALIGN_DEGREES,
     SID_ATTR_ALIGN_LOCKPOS, SID_ATTR_ALIGN_LOCKPOS,
     SID_ATTR_ALIGN_HYPHENATION, SID_ATTR_ALIGN_HYPHENATION,
-    SID_ATTR_ALIGN_ASIANVERTICAL, SID_ATTR_ALIGN_ASIANVERTICAL,
     SID_ATTR_FRAMEDIRECTION, SID_ATTR_FRAMEDIRECTION,
-    SID_ATTR_ALIGN_SHRINKTOFIT, SID_ATTR_ALIGN_SHRINKTOFIT>::value);
+    SID_ATTR_ALIGN_ASIANVERTICAL, SID_ATTR_ALIGN_ASIANVERTICAL,
+    SID_ATTR_ALIGN_SHRINKTOFIT, SID_ATTR_ALIGN_SHRINKTOFIT,
+    SID_ATTR_ALIGN_HOR_JUSTIFY, SID_ATTR_ALIGN_VER_JUSTIFY>::value);
 
 
 namespace {
diff --git a/cui/source/tabpages/numfmt.cxx b/cui/source/tabpages/numfmt.cxx
index 6f889f747517..f1560168b16a 100644
--- a/cui/source/tabpages/numfmt.cxx
+++ b/cui/source/tabpages/numfmt.cxx
@@ -57,8 +57,8 @@ using ::com::sun::star::uno::UNO_QUERY;
 const WhichRangesContainer SvxNumberFormatTabPage::pRanges(
     svl::Items<
     SID_ATTR_NUMBERFORMAT_VALUE, SID_ATTR_NUMBERFORMAT_INFO,
-    SID_ATTR_NUMBERFORMAT_NOLANGUAGE, SID_ATTR_NUMBERFORMAT_NOLANGUAGE,
     SID_ATTR_NUMBERFORMAT_ONE_AREA, SID_ATTR_NUMBERFORMAT_ONE_AREA,
+    SID_ATTR_NUMBERFORMAT_NOLANGUAGE, SID_ATTR_NUMBERFORMAT_NOLANGUAGE,
     SID_ATTR_NUMBERFORMAT_SOURCE, SID_ATTR_NUMBERFORMAT_SOURCE>::value);
 
 /*************************************************************************
diff --git a/cui/source/tabpages/swpossizetabpage.cxx b/cui/source/tabpages/swpossizetabpage.cxx
index ac9a0da25866..fc0e3d90ef92 100644
--- a/cui/source/tabpages/swpossizetabpage.cxx
+++ b/cui/source/tabpages/swpossizetabpage.cxx
@@ -723,9 +723,6 @@ WhichRangesContainer SvxSwPosSizeTabPage::GetRanges()
 {
     static const WhichRangesContainer ranges(svl::Items<
         SID_ATTR_TRANSFORM_POS_X, SID_ATTR_TRANSFORM_POS_Y,
-        SID_ATTR_TRANSFORM_PROTECT_POS, SID_ATTR_TRANSFORM_PROTECT_POS,
-        SID_ATTR_TRANSFORM_INTERN, SID_ATTR_TRANSFORM_INTERN,
-        SID_ATTR_TRANSFORM_ANCHOR, SID_ATTR_TRANSFORM_VERT_ORIENT,
         SID_ATTR_TRANSFORM_WIDTH, SID_ATTR_TRANSFORM_SIZE_POINT,
         SID_ATTR_TRANSFORM_PROTECT_POS, SID_ATTR_TRANSFORM_INTERN,
         SID_ATTR_TRANSFORM_AUTOWIDTH, SID_ATTR_TRANSFORM_VERT_ORIENT,
diff --git a/cui/source/tabpages/tpshadow.cxx b/cui/source/tabpages/tpshadow.cxx
index 8e0c86eaea59..3169b723e37d 100644
--- a/cui/source/tabpages/tpshadow.cxx
+++ b/cui/source/tabpages/tpshadow.cxx
@@ -43,9 +43,10 @@ using namespace com::sun::star;
 
 const WhichRangesContainer SvxShadowTabPage::pShadowRanges(svl::Items<
     SDRATTR_SHADOWCOLOR, SDRATTR_SHADOWTRANSPARENCE,
-    SDRATTR_SHADOWBLUR, SID_ATTR_FILL_SHADOW,
-    SID_ATTR_FILL_SHADOW, SID_ATTR_SHADOW_TRANSPARENCE,
-    SID_ATTR_SHADOW_BLUR, SID_ATTR_SHADOW_YDISTANCE
+    SDRATTR_SHADOWBLUR, SDRATTR_SHADOWBLUR,
+    SID_ATTR_FILL_SHADOW, SID_ATTR_FILL_SHADOW,
+    SID_ATTR_SHADOW_BLUR, SID_ATTR_SHADOW_BLUR,
+    SID_ATTR_SHADOW_TRANSPARENCE, SID_ATTR_SHADOW_YDISTANCE
 >::value);
 
 SvxShadowTabPage::SvxShadowTabPage(weld::Container* pPage, weld::DialogController* pController, const SfxItemSet& rInAttrs)
diff --git a/cui/source/tabpages/tptrans.cxx b/cui/source/tabpages/tptrans.cxx
index 594a41ecf704..c50d6728c544 100644
--- a/cui/source/tabpages/tptrans.cxx
+++ b/cui/source/tabpages/tptrans.cxx
@@ -35,8 +35,8 @@ using namespace com::sun::star;
 
 const WhichRangesContainer SvxTransparenceTabPage::pTransparenceRanges(svl::Items<
     XATTR_FILLTRANSPARENCE, XATTR_FILLTRANSPARENCE,
-    SDRATTR_SHADOWTRANSPARENCE, SDRATTR_SHADOWTRANSPARENCE,
-    XATTR_FILLFLOATTRANSPARENCE, XATTR_FILLFLOATTRANSPARENCE
+    XATTR_FILLFLOATTRANSPARENCE, XATTR_FILLFLOATTRANSPARENCE,
+    SDRATTR_SHADOWTRANSPARENCE, SDRATTR_SHADOWTRANSPARENCE
 >::value);
 
 /*************************************************************************
diff --git a/cui/source/tabpages/transfrm.cxx b/cui/source/tabpages/transfrm.cxx
index 6737aba40f67..eec4b1e87566 100644
--- a/cui/source/tabpages/transfrm.cxx
+++ b/cui/source/tabpages/transfrm.cxx
@@ -45,12 +45,10 @@
 
 const WhichRangesContainer SvxPositionSizeTabPage::pPosSizeRanges(svl::Items<
     SID_ATTR_TRANSFORM_POS_X, SID_ATTR_TRANSFORM_POS_Y,
-    SID_ATTR_TRANSFORM_PROTECT_POS, SID_ATTR_TRANSFORM_PROTECT_POS,
-    SID_ATTR_TRANSFORM_INTERN, SID_ATTR_TRANSFORM_INTERN,
-    SID_ATTR_TRANSFORM_ANCHOR, SID_ATTR_TRANSFORM_VERT_ORIENT,
     SID_ATTR_TRANSFORM_WIDTH, SID_ATTR_TRANSFORM_SIZE_POINT,
     SID_ATTR_TRANSFORM_PROTECT_POS, SID_ATTR_TRANSFORM_INTERN,
-    SID_ATTR_TRANSFORM_AUTOWIDTH, SID_ATTR_TRANSFORM_AUTOHEIGHT
+    SID_ATTR_TRANSFORM_AUTOWIDTH, SID_ATTR_TRANSFORM_AUTOHEIGHT,
+    SID_ATTR_TRANSFORM_ANCHOR, SID_ATTR_TRANSFORM_VERT_ORIENT
 >::value);
 
 const WhichRangesContainer SvxAngleTabPage::pAngleRanges(svl::Items<
@@ -60,8 +58,8 @@ const WhichRangesContainer SvxAngleTabPage::pAngleRanges(svl::Items<
 
 const WhichRangesContainer SvxSlantTabPage::pSlantRanges(svl::Items<
     SDRATTR_CORNER_RADIUS, SDRATTR_CORNER_RADIUS,
-    SID_ATTR_TRANSFORM_SHEAR, SID_ATTR_TRANSFORM_SHEAR_VERTICAL,
-    SID_ATTR_TRANSFORM_INTERN, SID_ATTR_TRANSFORM_INTERN
+    SID_ATTR_TRANSFORM_INTERN, SID_ATTR_TRANSFORM_INTERN,
+    SID_ATTR_TRANSFORM_SHEAR, SID_ATTR_TRANSFORM_SHEAR_VERTICAL
 >::value);
 
 /*************************************************************************
diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx
index 6c1116fb009c..4ca99faa13e8 100644
--- a/include/svl/itemset.hxx
+++ b/include/svl/itemset.hxx
@@ -55,41 +55,36 @@ constexpr bool validRanges() {
         && validRanges<WID3, WIDs...>();
 }
 
-inline constexpr bool validRanges(const sal_uInt16* pRanges)
-{
-    for (auto p = pRanges; p && *p; p += 2)
-    {
-        if (!validRange(p[0], p[1]))
-            return false;
-        // ranges must be sorted
-        if (p[2] != 0 && !validGap(p[1], p[2]))
-            return false;
-    }
-    return true;
-}
-
-// The calculations in rangeSize and rangesSize cannot overflow, assuming
+// The calculations in rangeSize cannot overflow, assuming
 // std::size_t is no smaller than sal_uInt16:
-
 constexpr std::size_t rangeSize(sal_uInt16 wid1, sal_uInt16 wid2) {
     assert(validRange(wid1, wid2));
     return wid2 - wid1 + 1;
 }
 
-template<sal_uInt16 WID1, sal_uInt16 WID2> constexpr std::size_t rangesSize()
-{ return rangeSize(WID1, WID2); }
-
-template<sal_uInt16 WID1, sal_uInt16 WID2, sal_uInt16 WID3, sal_uInt16... WIDs>
-constexpr std::size_t rangesSize()
-{ return rangeSize(WID1, WID2) + rangesSize<WID3, WIDs...>(); }
-
 }
 
 template<sal_uInt16... WIDs> struct Items
 {
+    using Array = std::array<WhichPair, sizeof...(WIDs) / 2>;
+    template <sal_uInt16 WID1, sal_uInt16 WID2, sal_uInt16... Rest>
+    static constexpr void fill(typename Array::iterator it)
+    {
+        it->first = WID1;
+        it->second = WID2;
+        if constexpr (sizeof...(Rest) > 0)
+            fill<Rest...>(++it);
+    }
+    static constexpr Array make()
+    {
+        assert(svl::detail::validRanges<WIDs...>());
+        Array a{};
+        fill<WIDs...>(a.begin());
+        return a;
+    }
     // This is passed to WhichRangesContainer so we can avoid needing to malloc()
     // for compile-time data.
-    static constexpr std::array<sal_uInt16, sizeof...(WIDs)> value = { { WIDs... } };
+    static constexpr Array value = make();
 };
 
 }
@@ -144,11 +139,8 @@ public:
         : SfxItemSet(rPool, WhichRangesContainer(nWhichStart, nWhichEnd)) {}
 
     template<sal_uInt16... WIDs>
-    SfxItemSet(
-        typename std::enable_if<
-            svl::detail::validRanges<WIDs...>(), SfxItemPool &>::type pool,
-        svl::Items<WIDs...>)
-        : SfxItemSet(pool, WhichRangesContainer(svl::Items<WIDs...>::value), svl::detail::rangesSize<WIDs...>()) {}
+    SfxItemSet(SfxItemPool& pool, svl::Items<WIDs...>)
+        : SfxItemSet(pool, WhichRangesContainer(svl::Items<WIDs...>::value)) {}
 
     SfxItemSet( SfxItemPool&, const sal_uInt16* nWhichPairTable );
 
diff --git a/include/svl/whichranges.hxx b/include/svl/whichranges.hxx
index b1e87bba38b4..44f4b15fddbe 100644
--- a/include/svl/whichranges.hxx
+++ b/include/svl/whichranges.hxx
@@ -31,7 +31,7 @@ struct SVL_DLLPUBLIC WhichRangesContainer
       * at a global const literal */
     bool m_bOwnRanges = false;
 
-    WhichRangesContainer() {}
+    WhichRangesContainer() = default;
 
     WhichRangesContainer(std::unique_ptr<WhichPair[]> wids, sal_Int32 nSize)
         : m_pairs(wids.release())
@@ -40,9 +40,9 @@ struct SVL_DLLPUBLIC WhichRangesContainer
     {
     }
     template <std::size_t N>
-    WhichRangesContainer(const std::array<sal_uInt16, N>& ranges)
-        : m_pairs(reinterpret_cast<const WhichPair*>(ranges.data()))
-        , m_size(static_cast<sal_Int32>(N / 2))
+    WhichRangesContainer(const std::array<WhichPair, N>& ranges)
+        : m_pairs(ranges.data())
+        , m_size(N)
         , m_bOwnRanges(false)
     {
     }
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index b2fb14f5c73f..59c640bb6c5d 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -153,6 +153,7 @@ SfxItemSet::SfxItemSet( SfxItemPool& rPool, const sal_uInt16* pWhichPairTable )
     const auto& [nCnt, nCap] = svl::detail::CountRangesOld(pWhichPairTable);
     m_pItems.reset(new const SfxPoolItem* [nCap] {});
     m_pWhichRanges = WhichRangesContainer(reinterpret_cast<const WhichPair*>(pWhichPairTable), (nCnt-1)/2);
+    assert(svl::detail::validRanges2(m_pWhichRanges));
 }
 
 SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
diff --git a/svx/source/dialog/hdft.cxx b/svx/source/dialog/hdft.cxx
index 0fa7b35a4a3f..4fa388274755 100644
--- a/svx/source/dialog/hdft.cxx
+++ b/svx/source/dialog/hdft.cxx
@@ -55,14 +55,13 @@ const tools::Long DEF_DIST_WRITER = 500;    // 5mm (Writer)
 const tools::Long DEF_DIST_CALC = 250;      // 2.5mm (Calc)
 
 const WhichRangesContainer SvxHFPage::pRanges(svl::Items<
-    SID_ATTR_BRUSH,          SID_ATTR_BRUSH,
-
     // Support DrawingLayer FillStyles (no real call to below GetRanges()
     // detected, still do the complete transition)
     XATTR_FILL_FIRST,        XATTR_FILL_LAST,
 
-    SID_ATTR_BORDER_OUTER,   SID_ATTR_BORDER_OUTER,
+    SID_ATTR_BRUSH,          SID_ATTR_BRUSH,
     SID_ATTR_BORDER_INNER,   SID_ATTR_BORDER_INNER,
+    SID_ATTR_BORDER_OUTER,   SID_ATTR_BORDER_OUTER,
     SID_ATTR_BORDER_SHADOW,  SID_ATTR_BORDER_SHADOW,
     SID_ATTR_LRSPACE,        SID_ATTR_LRSPACE,
     SID_ATTR_ULSPACE,        SID_ATTR_ULSPACE,
@@ -72,8 +71,8 @@ const WhichRangesContainer SvxHFPage::pRanges(svl::Items<
     SID_ATTR_PAGE_ON,        SID_ATTR_PAGE_ON,
     SID_ATTR_PAGE_DYNAMIC,   SID_ATTR_PAGE_DYNAMIC,
     SID_ATTR_PAGE_SHARED,    SID_ATTR_PAGE_SHARED,
-    SID_ATTR_PAGE_SHARED_FIRST,    SID_ATTR_PAGE_SHARED_FIRST,
-    SID_ATTR_HDFT_DYNAMIC_SPACING, SID_ATTR_HDFT_DYNAMIC_SPACING
+    SID_ATTR_HDFT_DYNAMIC_SPACING, SID_ATTR_HDFT_DYNAMIC_SPACING,
+    SID_ATTR_PAGE_SHARED_FIRST,    SID_ATTR_PAGE_SHARED_FIRST
 >::value);
 
 namespace svx {
diff --git a/sw/source/ui/envelp/envfmt.cxx b/sw/source/ui/envelp/envfmt.cxx
index 62c98cc54a3a..ae653bfa30af 100644
--- a/sw/source/ui/envelp/envfmt.cxx
+++ b/sw/source/ui/envelp/envfmt.cxx
@@ -261,8 +261,8 @@ SfxItemSet *SwEnvFormatPage::GetCollItemSet(SwTextFormatColl const * pColl, bool
             RES_PARATR_TABSTOP, RES_PARATR_END-1,
             RES_LR_SPACE, RES_UL_SPACE,
             RES_BACKGROUND, RES_SHADOW,
-            SID_ATTR_TABSTOP_POS, SID_ATTR_TABSTOP_POS,
             SID_ATTR_TABSTOP_DEFAULTS, SID_ATTR_TABSTOP_DEFAULTS,
+            SID_ATTR_TABSTOP_POS, SID_ATTR_TABSTOP_POS,
             SID_ATTR_TABSTOP_OFFSET, SID_ATTR_TABSTOP_OFFSET,
             SID_ATTR_BORDER_INNER, SID_ATTR_BORDER_INNER
         >::value);
diff --git a/sw/source/ui/frmdlg/frmpage.cxx b/sw/source/ui/frmdlg/frmpage.cxx
index 987fd665e0d5..8569230bd78b 100644
--- a/sw/source/ui/frmdlg/frmpage.cxx
+++ b/sw/source/ui/frmdlg/frmpage.cxx
@@ -394,8 +394,8 @@ const WhichRangesContainer SwFramePage::aPageRg(svl::Items<
     RES_FOLLOW_TEXT_FLOW, RES_FOLLOW_TEXT_FLOW
 >::value);
 const WhichRangesContainer SwFrameAddPage::aAddPgRg(svl::Items<
-    RES_PROTECT,            RES_PROTECT,
     RES_PRINT,              RES_PRINT,
+    RES_PROTECT,            RES_PROTECT,
     FN_SET_FRM_NAME,        FN_SET_FRM_NAME,
     FN_SET_FRM_ALT_NAME,    FN_SET_FRM_ALT_NAME,
     FN_UNO_DESCRIPTION,     FN_UNO_DESCRIPTION
diff --git a/sw/source/ui/frmdlg/wrap.cxx b/sw/source/ui/frmdlg/wrap.cxx
index 61cb43dd23ae..cbd87585f19a 100644
--- a/sw/source/ui/frmdlg/wrap.cxx
+++ b/sw/source/ui/frmdlg/wrap.cxx
@@ -46,8 +46,8 @@ using namespace ::com::sun::star;
 
 const WhichRangesContainer SwWrapTabPage::m_aWrapPageRg(svl::Items<
     RES_LR_SPACE, RES_UL_SPACE,
-    RES_PROTECT, RES_SURROUND,
-    RES_PRINT, RES_PRINT
+    RES_PRINT, RES_PRINT,
+    RES_PROTECT, RES_SURROUND
 >::value);
 
 SwWrapDlg::SwWrapDlg(weld::Window* pParent, SfxItemSet& rSet, SwWrtShell* pWrtShell, bool bDrawMode)