fix(Any): Modifications to fix OSS Fuzz report (#4818)

Changes triggered by the OSS Fuzz report: https://issues.oss-fuzz.com/issues/42538385?pli=1

* fix(Any): set pHolder to nullptr in destruct()

* enh(Any): modernised source code.

* enh(Any): introduce allocation type (empty, local, external).

* chore(Any): Apply suggestions from code review.
This commit is contained in:
Matej Kenda 2025-01-08 17:01:27 +01:00 committed by GitHub
parent 336e0e802f
commit bd7be38d6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 90 additions and 43 deletions

View File

@ -18,9 +18,9 @@
#include "Poco/Exception.h" #include "Poco/Exception.h"
#include "Poco/MetaProgramming.h" #include "Poco/MetaProgramming.h"
#include "Poco/Bugcheck.h"
#include <algorithm> #include <algorithm>
#include <typeinfo> #include <typeinfo>
#include <cstring>
#include <cstddef> #include <cstddef>
@ -60,7 +60,7 @@ union Placeholder
/// it will be placement-new-allocated into the local buffer /// it will be placement-new-allocated into the local buffer
/// (i.e. there will be no heap-allocation). The local buffer size is one byte /// (i.e. there will be no heap-allocation). The local buffer size is one byte
/// larger - [POCO_SMALL_OBJECT_SIZE + 1], additional byte value indicating /// larger - [POCO_SMALL_OBJECT_SIZE + 1], additional byte value indicating
/// where the object was allocated (0 => heap, 1 => local). /// where the object was allocated. See enum Allocation.
/// ///
/// Important: for SOO builds, only same-type (or trivial both-empty no-op) /// Important: for SOO builds, only same-type (or trivial both-empty no-op)
/// swap operation is allowed. /// swap operation is allowed.
@ -78,9 +78,12 @@ public:
#ifndef POCO_NO_SOO #ifndef POCO_NO_SOO
Placeholder(): pHolder(0) Placeholder(): pHolder(nullptr)
{ {
std::memset(holder, 0, sizeof(holder)); // Forces to use optimised memset internally
// https://travisdowns.github.io/blog/2020/01/20/zero.html
std::fill(std::begin(holder), std::end(holder), static_cast<char>(0));
setAllocation(Allocation::POCO_ANY_EMPTY);
} }
~Placeholder() ~Placeholder()
@ -101,32 +104,31 @@ public:
bool isEmpty() const bool isEmpty() const
{ {
static char buf[sizeof(holder)] = {}; return holder[SizeV] == Allocation::POCO_ANY_EMPTY;
return 0 == std::memcmp(holder, buf, sizeof(holder));
} }
bool isLocal() const bool isLocal() const
{ {
return holder[SizeV] != 0; return holder[SizeV] == Allocation::POCO_ANY_LOCAL;
} }
template<typename T, typename V, template<typename T, typename V,
typename std::enable_if<TypeSizeLE<T, Placeholder::Size::value>::value>::type* = nullptr> typename std::enable_if_t<TypeSizeLE<T, Placeholder::Size::value>::value>* = nullptr>
PlaceholderT* assign(const V& value) PlaceholderT* assign(const V& value)
{ {
erase(); erase();
new (reinterpret_cast<PlaceholderT*>(holder)) T(value); new (reinterpret_cast<PlaceholderT*>(holder)) T(value);
setLocal(true); setAllocation(Allocation::POCO_ANY_LOCAL);
return reinterpret_cast<PlaceholderT*>(holder); return reinterpret_cast<PlaceholderT*>(holder);
} }
template<typename T, typename V, template<typename T, typename V,
typename std::enable_if<TypeSizeGT<T, Placeholder::Size::value>::value>::type* = nullptr> typename std::enable_if_t<TypeSizeGT<T, Placeholder::Size::value>::value>* = nullptr>
PlaceholderT* assign(const V& value) PlaceholderT* assign(const V& value)
{ {
erase(); erase();
pHolder = new T(value); pHolder = new T(value);
setLocal(false); setAllocation(Allocation::POCO_ANY_EXTERNAL);
return pHolder; return pHolder;
} }
@ -139,24 +141,54 @@ public:
} }
private: private:
typedef std::max_align_t AlignerType; using AlignerType = std::max_align_t;
static_assert(sizeof(AlignerType) <= SizeV + 1, "Aligner type is bigger than the actual storage, so SizeV should be made bigger otherwise you simply waste unused memory."); static_assert(
sizeof(AlignerType) < SizeV,
"Aligner type is bigger than the actual storage, so SizeV should be made bigger otherwise you simply waste unused memory."
);
void setLocal(bool local) const enum Allocation : unsigned char
{ {
holder[SizeV] = local ? 1 : 0; POCO_ANY_EMPTY = 0,
POCO_ANY_LOCAL = 1,
POCO_ANY_EXTERNAL = 2
};
void setAllocation(Allocation alloc) const
{
holder[SizeV] = alloc;
} }
void destruct(bool clear) void destruct(bool clear)
{ {
if (!isEmpty()) const auto allocation {holder[SizeV]};
switch (allocation)
{ {
if (!isLocal()) case Allocation::POCO_ANY_EMPTY:
delete pHolder; break;
else case Allocation::POCO_ANY_LOCAL:
reinterpret_cast<PlaceholderT*>(holder)->~PlaceholderT(); {
// Do not deallocate, just explicitly call destructor
if (clear) std::memset(holder, 0, sizeof(holder)); auto* h { reinterpret_cast<PlaceholderT*>(holder) };
h->~PlaceholderT();
}
break;
case Allocation::POCO_ANY_EXTERNAL:
{
auto* h { pHolder };
pHolder = nullptr;
delete h;
}
break;
default:
poco_bugcheck();
break;
}
setAllocation(Allocation::POCO_ANY_EMPTY);
if (clear)
{
// Force to use optimised memset internally
std::fill(std::begin(holder), std::end(holder), static_cast<char>(0));
} }
} }
@ -165,7 +197,7 @@ private:
#else // POCO_NO_SOO #else // POCO_NO_SOO
Placeholder(): pHolder(0) Placeholder(): pHolder(nullptr)
{ {
} }
@ -182,12 +214,12 @@ private:
void erase() void erase()
{ {
delete pHolder; delete pHolder;
pHolder = 0; pHolder = nullptr;
} }
bool isEmpty() const bool isEmpty() const
{ {
return 0 == pHolder; return nullptr == pHolder;
} }
bool isLocal() const bool isLocal() const
@ -225,10 +257,8 @@ class Any
{ {
public: public:
Any() Any() = default;
/// Creates an empty any type. /// Creates an empty any type.
{
}
template<typename ValueType> template<typename ValueType>
Any(const ValueType & value) Any(const ValueType & value)
@ -248,12 +278,19 @@ public:
construct(other); construct(other);
} }
~Any() Any(Any&& other)
/// Destructor. If Any is locally held, calls ValueHolder destructor; /// Move constructor, works with both empty and initialized Any values.
/// otherwise, deletes the placeholder from the heap.
{ {
construct(other);
} }
~Any() = default;
/// Destructor.
/// Small Object Optimization mode behavior:
/// Invokes the Placeholder destructor, which calls the
/// ValueHolder destructor explicitly (locally held Any), or
/// deletes the ValueHolder heap memory (heap-allocated Any).
Any& swap(Any& other) noexcept Any& swap(Any& other) noexcept
/// Swaps the content of the two Anys. /// Swaps the content of the two Anys.
/// ///
@ -298,11 +335,23 @@ public:
Any& operator = (const Any& rhs) Any& operator = (const Any& rhs)
/// Assignment operator for Any. /// Assignment operator for Any.
{ {
if ((this != &rhs) && !rhs.empty()) if (this != &rhs)
{
if (!rhs.empty())
construct(rhs); construct(rhs);
else if ((this != &rhs) && rhs.empty()) else
_valueHolder.erase(); _valueHolder.erase();
}
return *this;
}
Any& operator = (Any&& rhs)
/// Move operator for Any.
{
if (!rhs.empty())
construct(rhs);
else
_valueHolder.erase();
return *this; return *this;
} }
@ -347,21 +396,19 @@ private:
{ {
} }
virtual const std::type_info& type() const Holder & operator = (const Holder &) = delete;
const std::type_info& type() const override
{ {
return typeid(ValueType); return typeid(ValueType);
} }
virtual void clone(Placeholder<ValueHolder>* pPlaceholder) const void clone(Placeholder<ValueHolder>* pPlaceholder) const override
{ {
pPlaceholder->assign<Holder<ValueType>, ValueType>(_held); pPlaceholder->assign<Holder<ValueType>, ValueType>(_held);
} }
ValueType _held; ValueType _held;
private:
Holder & operator = (const Holder &);
}; };
ValueHolder* content() const ValueHolder* content() const

View File

@ -369,10 +369,10 @@ namespace Impl {
}; };
template <typename T> template <typename T>
using EnableSigned = typename std::enable_if< std::is_signed<T>::value >::type*; using EnableSigned = typename std::enable_if_t< std::is_signed<T>::value >*;
template <typename T> template <typename T>
using EnableUnsigned = typename std::enable_if< std::is_unsigned<T>::value >::type*; using EnableUnsigned = typename std::enable_if_t< std::is_unsigned<T>::value >*;
} // namespace Impl } // namespace Impl