Revert "Use placement new to avoid one of the allocation calls..."

This reverts commit 503ab1ca9ae11978d9717557546c01ff598aaf88, plus follow-up
17915ab5202a4d7456e9bc031c3f6a72bc861844 "fix ubsan alloc-dealloc-mismatch".
It failed to properly destroy the object assembly, and caused e.g.
CppunitTest_svl_items to fail with

> ==850754==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x6060000024e0 in thread T0:
>   object passed to delete has wrong type:
>   size of the allocated type:   64 bytes;
>   size of the deallocated type: 56 bytes.
>  #0 in operator delete(void*, unsigned long) at /home/sbergman/github.com/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:164:3 (workdir/LinkTarget/Executable/cppunittester +0x330ae2)
>  #1 in SfxItemSet::~SfxItemSet() at svl/source/items/itemset.cxx:202:1 (instdir/program/libsvllo.so +0x110ccf6)
>  #2 in std::default_delete<SfxItemSet>::operator()(SfxItemSet*) const at /home/sbergman/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/unique_ptr.h:85:2 (instdir/program/libsvllo.so +0x1142a28)
>  #3 in std::_Sp_counted_deleter<SfxItemSet*, std::default_delete<SfxItemSet>, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() at /home/sbergman/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/shared_ptr_base.h:442:9 (instdir/program/libsvllo.so +0x12696e4)
>  #4 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() at /home/sbergman/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/shared_ptr_base.h:168:6 (instdir/program/libsvllo.so +0xe500b5)
[...]
> 0x6060000024e0 is located 0 bytes inside of 64-byte region [0x6060000024e0,0x606000002520)
> allocated by thread T0 here:
>  #0 in operator new(unsigned long) at /home/sbergman/github.com/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:95:3 (workdir/LinkTarget/Executable/cppunittester +0x32fe7d)
>  #1 in SfxItemSet::Clone(bool, SfxItemPool*) const at svl/source/items/itemset.cxx:1270:34 (instdir/program/libsvllo.so +0x1127854)
>  #2 in (anonymous namespace)::Node::setItemSet(SfxItemSet const&) at svl/source/items/stylepool.cxx:65:107 (instdir/program/libsvllo.so +0x1212179)
>  #3 in StylePoolImpl::insertItemSet(SfxItemSet const&, rtl::OUString const*) at svl/source/items/stylepool.cxx:417:19 (instdir/program/libsvllo.so +0x12103e1)
>  #4 in StylePool::insertItemSet(SfxItemSet const&, rtl::OUString const*) at svl/source/items/stylepool.cxx:456:17 (instdir/program/libsvllo.so +0x1212ffb)
[...]

in Clang ASan builds done with -fsized-deallocation.

Change-Id: I3ccba7e7d9712ecabf38a0149252d3cd70cdb446
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123446
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

1 files changed, 4 insertions, 60 deletions

diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 3b47719dbf04..04fab3c9306e 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -136,44 +136,6 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
     assert(svl::detail::validRanges2(m_pWhichRanges));
 }
 
-/** special constructor for Clone */
-SfxItemSet::SfxItemSet( const SfxItemSet& rASet, SfxPoolItem const ** ppItems )
-    : m_pPool( rASet.m_pPool )
-    , m_pParent( rASet.m_pParent )
-    , m_ppItems( ppItems )
-    , m_pWhichRanges( rASet.m_pWhichRanges )
-    , m_nCount( rASet.m_nCount )
-    , m_bItemsFixed(true)
-{
-    if (rASet.m_pWhichRanges.empty())
-        return;
-
-    auto nCnt = svl::detail::CountRanges(m_pWhichRanges);
-
-    // Copy attributes
-    SfxPoolItem const** ppDst = m_ppItems;
-    SfxPoolItem const** ppSrc = rASet.m_ppItems;
-    for( sal_uInt16 n = nCnt; n; --n, ++ppDst, ++ppSrc )
-        if ( nullptr == *ppSrc ||                 // Current Default?
-             IsInvalidItem(*ppSrc) ||       // DontCare?
-             IsStaticDefaultItem(*ppSrc) )  // Defaults that are not to be pooled?
-            // Just copy the pointer
-            *ppDst = *ppSrc;
-        else if (m_pPool->IsItemPoolable( **ppSrc ))
-        {
-            // Just copy the pointer and increase RefCount
-            *ppDst = *ppSrc;
-            (*ppDst)->AddRef();
-        }
-        else if ( !(*ppSrc)->Which() )
-            *ppDst = (*ppSrc)->Clone();
-        else
-            // !IsPoolable() => assign via Pool
-            *ppDst = &m_pPool->Put( **ppSrc );
-
-    assert(svl::detail::validRanges2(m_pWhichRanges));
-}
-
 SfxItemSet::SfxItemSet(SfxItemSet&& rASet) noexcept
     : m_pPool( rASet.m_pPool )
     , m_pParent( rASet.m_pParent )
@@ -1264,20 +1226,9 @@ bool SfxItemSet::Equals(const SfxItemSet &rCmp, bool bComparePool) const
 
 std::unique_ptr<SfxItemSet> SfxItemSet::Clone(bool bItems, SfxItemPool *pToPool ) const
 {
-    // Use placement new to avoid one of the allocation calls when constructing a new SfxItemSet.
-    // This is effectively a run-time equivalent of the SfxItemSetFixed template.
-    const int cnt = TotalCount();
-    char* p = static_cast<char*>(::operator new(sizeof(SfxItemSet) + (sizeof(const SfxPoolItem*) * cnt)));
-    SfxItemSet* pNewItemSet = reinterpret_cast<SfxItemSet*>(p);
-    const SfxPoolItem** ppNewItems = reinterpret_cast<const SfxPoolItem **>(p + sizeof(SfxItemSet));
     if (pToPool && pToPool != m_pPool)
     {
-        std::fill(ppNewItems, ppNewItems + cnt, nullptr);
-        std::unique_ptr<SfxItemSet> pNewSet(
-            new (pNewItemSet)
-                SfxItemSet(*pToPool,
-                    WhichRangesContainer(m_pWhichRanges),
-                    ppNewItems));
+        std::unique_ptr<SfxItemSet> pNewSet(new SfxItemSet(*pToPool, m_pWhichRanges));
         if ( bItems )
         {
             SfxWhichIter aIter(*pNewSet);
@@ -1292,17 +1243,10 @@ std::unique_ptr<SfxItemSet> SfxItemSet::Clone(bool bItems, SfxItemPool *pToPool
         }
         return pNewSet;
     }
-    else if (bItems)
-    {
-        return std::unique_ptr<SfxItemSet>(
-            new (pNewItemSet) SfxItemSet(*this, ppNewItems));
-    }
     else
-    {
-        std::fill(ppNewItems, ppNewItems + cnt, nullptr);
-        return std::unique_ptr<SfxItemSet>(
-            new (pNewItemSet) SfxItemSet(*m_pPool, WhichRangesContainer(m_pWhichRanges), ppNewItems));
-    }
+        return std::unique_ptr<SfxItemSet>(bItems
+                ? new SfxItemSet(*this)
+                : new SfxItemSet(*m_pPool, m_pWhichRanges));
 }
 
 SfxItemSet SfxItemSet::CloneAsValue(bool bItems, SfxItemPool *pToPool ) const