Commit 7e38a24f authored by Dominik Charousset's avatar Dominik Charousset

Fix unsafe memcmp usage

parent 7b5f55d8
...@@ -45,11 +45,9 @@ public: ...@@ -45,11 +45,9 @@ public:
type_id_t operator[](size_t index) const noexcept; type_id_t operator[](size_t index) const noexcept;
/// Convertes the internal buffer to a ::type_id_list and returns it. /// Convertes the internal buffer to a ::type_id_list and returns it.
/// @pre `push_back` was called at least once type_id_list move_to_list() noexcept;
type_id_list move_to_list();
/// Convertes the internal buffer to a ::type_id_list and returns it. /// Convertes the internal buffer to a ::type_id_list and returns it.
/// @pre `push_back` was called at least once
type_id_list copy_to_list() const; type_id_list copy_to_list() const;
void clear() noexcept { void clear() noexcept {
......
...@@ -69,7 +69,13 @@ public: ...@@ -69,7 +69,13 @@ public:
/// Compares this list to `other`. /// Compares this list to `other`.
int compare(type_id_list other) const noexcept { int compare(type_id_list other) const noexcept {
return memcmp(data_, other.data_, (size() + 1) * sizeof(type_id_t)); // These conversions are safe, because the size is stored in 16 bits.
int s1 = data_[0];
int s2 = other.data_[0];
int diff = s1 - s2;
if (diff == 0)
return memcmp(begin(), other.begin(), s1 * sizeof(type_id_t));
return diff;
} }
/// Returns an iterator to the first type ID. /// Returns an iterator to the first type ID.
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "caf/detail/type_id_list_builder.hpp" #include "caf/detail/type_id_list_builder.hpp"
#include <cstdint> #include <cstdint>
#include <cstdlib>
#include <mutex> #include <mutex>
#include <unordered_set> #include <unordered_set>
...@@ -30,15 +31,17 @@ namespace caf::detail { ...@@ -30,15 +31,17 @@ namespace caf::detail {
namespace { namespace {
struct dyn_type_id_list { struct dyn_type_id_list {
dyn_type_id_list(type_id_t* storage) : storage(storage) { explicit dyn_type_id_list(type_id_t* storage) noexcept : storage(storage) {
CAF_ASSERT(storage != nullptr);
auto first = reinterpret_cast<const uint8_t*>(storage); auto first = reinterpret_cast<const uint8_t*>(storage);
auto last = first + ((storage[0] + 1) * sizeof(type_id_t)); auto last = first + ((storage[0] + 1) * sizeof(type_id_t));
hash = caf::hash::fnv<size_t>::compute(make_span(first, last)); hash = caf::hash::fnv<size_t>::compute(make_span(first, last));
} }
dyn_type_id_list(dyn_type_id_list&& other) dyn_type_id_list(dyn_type_id_list&& other) noexcept
: storage(other.storage), hash(other.hash) { : storage(other.storage), hash(other.hash) {
other.storage = nullptr; other.storage = nullptr;
other.hash = 0;
} }
~dyn_type_id_list() { ~dyn_type_id_list() {
...@@ -49,7 +52,7 @@ struct dyn_type_id_list { ...@@ -49,7 +52,7 @@ struct dyn_type_id_list {
size_t hash; size_t hash;
}; };
bool operator==(const dyn_type_id_list& x, const dyn_type_id_list& y) { bool operator==(const dyn_type_id_list& x, const dyn_type_id_list& y) noexcept {
return type_id_list{x.storage} == type_id_list{y.storage}; return type_id_list{x.storage} == type_id_list{y.storage};
} }
...@@ -122,10 +125,11 @@ type_id_t type_id_list_builder::operator[](size_t index) const noexcept { ...@@ -122,10 +125,11 @@ type_id_t type_id_list_builder::operator[](size_t index) const noexcept {
return storage_[index + 1]; return storage_[index + 1];
} }
type_id_list type_id_list_builder::move_to_list() { type_id_list type_id_list_builder::move_to_list() noexcept {
if (size_ == 0) auto list_size = size();
if (list_size == 0)
return make_type_id_list(); return make_type_id_list();
storage_[0] = static_cast<type_id_t>(size()); storage_[0] = static_cast<type_id_t>(list_size);
// Transfer ownership of buffer into the global cache. If an equivalent list // Transfer ownership of buffer into the global cache. If an equivalent list
// already exists, get_or_set_type_id_buf releases `ptr` and returns the old // already exists, get_or_set_type_id_buf releases `ptr` and returns the old
// buffer. // buffer.
...@@ -135,14 +139,15 @@ type_id_list type_id_list_builder::move_to_list() { ...@@ -135,14 +139,15 @@ type_id_list type_id_list_builder::move_to_list() {
} }
type_id_list type_id_list_builder::copy_to_list() const { type_id_list type_id_list_builder::copy_to_list() const {
if (size_ == 0) auto list_size = size();
if (list_size == 0)
return make_type_id_list(); return make_type_id_list();
storage_[0] = static_cast<type_id_t>(size());
auto vptr = malloc(size_ * sizeof(type_id_t)); auto vptr = malloc(size_ * sizeof(type_id_t));
if (vptr == nullptr) if (vptr == nullptr)
throw std::bad_alloc(); throw std::bad_alloc();
auto copy = reinterpret_cast<type_id_t*>(vptr); auto copy = reinterpret_cast<type_id_t*>(vptr);
memcpy(copy, storage_, size_ * sizeof(type_id_t)); copy[0] = static_cast<type_id_t>(list_size);
memcpy(copy + 1, storage_ + 1, list_size * sizeof(type_id_t));
return type_id_list{get_or_set_type_id_buf(copy)}; return type_id_list{get_or_set_type_id_buf(copy)};
} }
......
...@@ -20,40 +20,52 @@ ...@@ -20,40 +20,52 @@
namespace caf { namespace caf {
void message_builder::clear() noexcept { namespace {
storage_size_ = 0;
types_.clear();
elements_.clear();
}
message message_builder::to_message() const { using namespace detail;
if (empty())
enum to_msg_policy {
move_msg,
copy_msg,
};
template <to_msg_policy Policy, class TypeListBuilder, class ElementVector>
message to_message_impl(size_t storage_size, TypeListBuilder& types,
ElementVector& elements) {
if (storage_size == 0)
return message{}; return message{};
using namespace detail; auto vptr = malloc(sizeof(message_data) + storage_size);
auto vptr = malloc(sizeof(message_data) + storage_size_);
if (vptr == nullptr) if (vptr == nullptr)
throw std::bad_alloc(); throw std::bad_alloc();
auto raw_ptr = new (vptr) message_data(types_.copy_to_list()); message_data* raw_ptr;
if constexpr (Policy == move_msg)
raw_ptr = new (vptr) message_data(types.move_to_list());
else
raw_ptr = new (vptr) message_data(types.copy_to_list());
intrusive_cow_ptr<message_data> ptr{raw_ptr, false}; intrusive_cow_ptr<message_data> ptr{raw_ptr, false};
auto storage = raw_ptr->storage(); auto storage = raw_ptr->storage();
for (auto& element : elements_) for (auto& element : elements)
storage = element->copy_init(storage); if constexpr (Policy == move_msg)
storage = element->move_init(storage);
else
storage = element->copy_init(storage);
return message{std::move(ptr)}; return message{std::move(ptr)};
} }
} // namespace
void message_builder::clear() noexcept {
storage_size_ = 0;
types_.clear();
elements_.clear();
}
message message_builder::to_message() const {
return to_message_impl<copy_msg>(storage_size_, types_, elements_);
}
message message_builder::move_to_message() { message message_builder::move_to_message() {
if (empty()) return to_message_impl<move_msg>(storage_size_, types_, elements_);
return message{};
using namespace detail;
auto vptr = malloc(sizeof(message_data) + storage_size_);
if (vptr == nullptr)
throw std::bad_alloc();
auto raw_ptr = new (vptr) message_data(types_.move_to_list());
intrusive_cow_ptr<message_data> ptr{raw_ptr, false};
auto storage = raw_ptr->storage();
for (auto& element : elements_)
storage = element->move_init(storage);
return message{std::move(ptr)};
} }
} // namespace caf } // namespace caf
...@@ -46,6 +46,7 @@ CAF_TEST(message builder can build messages incrermenetally) { ...@@ -46,6 +46,7 @@ CAF_TEST(message builder can build messages incrermenetally) {
auto msg = builder.to_message(); auto msg = builder.to_message();
CAF_CHECK_EQUAL(builder.size(), 1u); CAF_CHECK_EQUAL(builder.size(), 1u);
CAF_CHECK_EQUAL(msg.types(), make_type_id_list<int32_t>()); CAF_CHECK_EQUAL(msg.types(), make_type_id_list<int32_t>());
CAF_CHECK_EQUAL(to_string(msg.types()), "[int32_t]");
CAF_CHECK_EQUAL(to_string(msg), "(1)"); CAF_CHECK_EQUAL(to_string(msg), "(1)");
} }
STEP("after adding [2, 3], the message is (1, 2, 3)") { STEP("after adding [2, 3], the message is (1, 2, 3)") {
...@@ -55,12 +56,14 @@ CAF_TEST(message builder can build messages incrermenetally) { ...@@ -55,12 +56,14 @@ CAF_TEST(message builder can build messages incrermenetally) {
auto msg = builder.to_message(); auto msg = builder.to_message();
CAF_CHECK_EQUAL(msg.types(), CAF_CHECK_EQUAL(msg.types(),
(make_type_id_list<int32_t, int32_t, int32_t>())); (make_type_id_list<int32_t, int32_t, int32_t>()));
CAF_CHECK_EQUAL(to_string(msg.types()), "[int32_t, int32_t, int32_t]");
CAF_CHECK_EQUAL(to_string(msg), "(1, 2, 3)"); CAF_CHECK_EQUAL(to_string(msg), "(1, 2, 3)");
} }
STEP("moving the content to a message produces the same message again") { STEP("moving the content to a message produces the same message again") {
auto msg = builder.move_to_message(); auto msg = builder.move_to_message();
CAF_CHECK_EQUAL(msg.types(), CAF_CHECK_EQUAL(msg.types(),
(make_type_id_list<int32_t, int32_t, int32_t>())); (make_type_id_list<int32_t, int32_t, int32_t>()));
CAF_CHECK_EQUAL(to_string(msg.types()), "[int32_t, int32_t, int32_t]");
CAF_CHECK_EQUAL(to_string(msg), "(1, 2, 3)"); CAF_CHECK_EQUAL(to_string(msg), "(1, 2, 3)");
} }
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment