Skip to content

Commit

Permalink
Add -fsanitize=address for memory leak and out-of-bounds detection.
Browse files Browse the repository at this point in the history
  • Loading branch information
mutouyun committed Oct 22, 2023
1 parent ec602f5 commit 872b97f
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 49 deletions.
5 changes: 2 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -DNDEBUG")
if(NOT MSVC)
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O2")
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_DEBUG} -g -rdynamic")
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g -rdynamic -fsanitize=address")
endif()

if (MSVC)
Expand All @@ -22,8 +22,7 @@ if (MSVC)
CMAKE_CXX_FLAGS_RELEASE
CMAKE_C_FLAGS
CMAKE_C_FLAGS_DEBUG
CMAKE_C_FLAGS_RELEASE
)
CMAKE_C_FLAGS_RELEASE)
if (LIBIPC_USE_STATIC_CRT)
foreach(CompilerFlag ${CompilerFlags})
string(REPLACE "/MD" "/MT" ${CompilerFlag} "${${CompilerFlag}}")
Expand Down
42 changes: 26 additions & 16 deletions include/libpmr/small_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ class holder<Value, false> : public holder_base {

namespace detail {

/// \struct holder_type_base
/// \brief Defines the holder type operation interface.
struct holder_type_base {
virtual ~holder_type_base() noexcept = default;
virtual std::size_t size() const noexcept = 0;
Expand All @@ -213,6 +215,8 @@ struct holder_type_base {
virtual void dest(void *p, std::size_t n) const noexcept = 0;
};

/// \struct template <typename Value> holder_type
/// \brief Defines generalization operations for different data types.
template <typename Value>
struct holder_type : holder_type_base {
std::size_t size() const noexcept override { return sizeof(Value); }
Expand Down Expand Up @@ -240,6 +244,10 @@ struct holder_info {
std::size_t count;
};

struct holder_data : holder_info {
alignas(std::max_align_t) char ___data;
};

template <typename Value>
std::size_t full_sizeof(std::size_t count) noexcept {
return ::LIBIMP::round_up(sizeof(detail::holder_info), alignof(std::max_align_t))
Expand All @@ -252,29 +260,31 @@ std::size_t full_sizeof(holder_info const *info) noexcept {
}

void *value_ptr(holder_info *info) noexcept {
return reinterpret_cast<void *>(
::LIBIMP::round_up(reinterpret_cast<std::size_t>(info + 1), alignof(std::max_align_t)));
if (info == nullptr) return nullptr;
return &static_cast<holder_data *>(info)->___data;
}

void const *value_ptr(holder_info const *info) noexcept {
return reinterpret_cast<void const *>(
::LIBIMP::round_up(reinterpret_cast<std::size_t>(info + 1), alignof(std::max_align_t)));
if (info == nullptr) return nullptr;
return &static_cast<holder_data const *>(info)->___data;
}

class holder_info_ptr {
/// \class holder_data_builder
/// \brief The data pointer builder.
class holder_data_builder {
allocator const &alloc_;
holder_info **pptr_;
holder_data **pptr_;
std::size_t size_;

public:
holder_info_ptr(allocator const &alloc, holder_info *(&ptr), std::size_t sz) noexcept
holder_data_builder(allocator const &alloc, holder_data *(&ptr), std::size_t sz) noexcept
: alloc_(alloc)
, pptr_ (&ptr)
, size_ (::LIBIMP::round_up(sizeof(holder_info), alignof(std::max_align_t)) + sz) {
*pptr_ = static_cast<holder_info *>(alloc_.allocate(size_));
, size_ (detail::full_sizeof<::LIBIMP::byte>(sz)) {
*pptr_ = static_cast<holder_data *>(alloc_.allocate(size_));
}

~holder_info_ptr() noexcept {
~holder_data_builder() noexcept {
if (pptr_ == nullptr) return;
alloc_.deallocate(*pptr_, size_);
*pptr_ = nullptr;
Expand All @@ -284,11 +294,11 @@ class holder_info_ptr {
return (pptr_ != nullptr) && (*pptr_ != nullptr);
}

holder_info *operator->() const noexcept {
holder_data *operator->() const noexcept {
return *pptr_;
}

holder_info_ptr &operator=(holder_info const &rhs) noexcept {
holder_data_builder &operator=(holder_data const &rhs) noexcept {
if (!*this) return *this;
**pptr_ = rhs;
return *this;
Expand All @@ -308,7 +318,7 @@ class holder_info_ptr {
template <>
class holder<void, true> : public holder_base {

detail::holder_info info_;
detail::holder_data info_;

public:
template <typename Value>
Expand Down Expand Up @@ -385,7 +395,7 @@ class holder<void, true> : public holder_base {
template <>
class holder<void, false> : public holder_base {

detail::holder_info *info_ptr_;
detail::holder_data *info_ptr_;

public:
holder() noexcept
Expand All @@ -397,7 +407,7 @@ class holder<void, false> : public holder_base {
std::enable_if_t<alignof(Value) <= alignof(std::max_align_t), bool> = true>
holder(allocator const &alloc, ::LIBIMP::types<Value>, std::size_t n) : holder() {
LIBIMP_LOG_("holder<void, false>");
detail::holder_info_ptr info_p{alloc, info_ptr_, sizeof(Value) * n};
detail::holder_data_builder info_p{alloc, info_ptr_, sizeof(Value) * n};
if (!info_p) {
log.error("The destination information-pointer failed to be constructed."
" type size = ", sizeof(Value),
Expand Down Expand Up @@ -452,7 +462,7 @@ class holder<void, false> : public holder_base {
LIBIMP_LOG_();
auto *des = ::LIBIMP::construct<holder>(p);
if (!valid()) return;
detail::holder_info_ptr info_p{alloc, des->info_ptr_, this->sizeof_type() * this->count()};
detail::holder_data_builder info_p{alloc, des->info_ptr_, this->sizeof_type() * this->count()};
if (!info_p) {
log.error("The destination information-pointer failed to be constructed."
" type size = ", this->sizeof_type(),
Expand Down
16 changes: 9 additions & 7 deletions src/libipc/platform/posix/shm_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ result<int> shm_open_fd(std::string const &name, mode::type type) noexcept {
log.error("name is empty.");
return std::make_error_code(std::errc::invalid_argument);
}

/// \brief Open the object for read-write access.
// For portable use, a shared memory object should be identified by name of the form /somename.
// see: https://man7.org/linux/man-pages/man3/shm_open.3.html
std::string op_name = '/' + name;
// Open the object for read-write access.
int flag = O_RDWR;
switch (type) {
case mode::open:
Expand All @@ -73,13 +75,13 @@ result<int> shm_open_fd(std::string const &name, mode::type type) noexcept {
return std::make_error_code(std::errc::invalid_argument);
}

/// \brief Create/Open POSIX shared memory bject
int fd = ::shm_open(name.c_str(), flag, S_IRUSR | S_IWUSR |
S_IRGRP | S_IWGRP |
S_IROTH | S_IWOTH);
// Create/Open POSIX shared memory bject.
int fd = ::shm_open(op_name.c_str(), flag, S_IRUSR | S_IWUSR |
S_IRGRP | S_IWGRP |
S_IROTH | S_IWOTH);
if (fd == posix::failed) {
auto err = sys::error();
log.error("failed: shm_open(name = ", name,
log.error("failed: shm_open(name = ", op_name,
", type = ", type,
"). error = ", err);
return err;
Expand Down
4 changes: 2 additions & 2 deletions src/libipc/platform/win/mmap_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ result<HANDLE> mmap_open(std::string const &file, std::size_t size, mode::type t
return std::make_error_code(std::errc::invalid_argument);
}

/// \brief Opens a named file mapping object.
// Opens a named file mapping object.
auto try_open = [&]() -> result<HANDLE> {
HANDLE h = ::OpenFileMapping(FILE_MAP_ALL_ACCESS, FALSE, t_name.c_str());
if (h == NULL) {
Expand All @@ -83,7 +83,7 @@ result<HANDLE> mmap_open(std::string const &file, std::size_t size, mode::type t
return h;
};

/// \brief Creates or opens a named or unnamed file mapping object for a specified file.
// Creates or opens a named or unnamed file mapping object for a specified file.
auto try_create = [&]() -> result<HANDLE> {
HANDLE h = ::CreateFileMapping(INVALID_HANDLE_VALUE, detail::get_sa(), PAGE_READWRITE | SEC_COMMIT,
/// \remark dwMaximumSizeHigh always 0 here.
Expand Down
2 changes: 1 addition & 1 deletion src/libipc/shm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ shared_memory::shared_memory(shared_memory &&other) noexcept
: shm_(std::exchange(other.shm_, nullptr)) {}

shared_memory &shared_memory::operator=(shared_memory &&rhs) & noexcept {
this->shm_ = std::exchange(rhs.shm_, nullptr);
shared_memory(std::move(rhs)).swap(*this);
return *this;
}

Expand Down
4 changes: 3 additions & 1 deletion src/libpmr/memory_resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "libimp/detect_plat.h"
#include "libimp/system.h"
#include "libimp/log.h"
#include "libimp/aligned.h"

#include "libpmr/memory_resource.h"

Expand Down Expand Up @@ -52,7 +53,8 @@ void *new_delete_resource::allocate(std::size_t bytes, std::size_t alignment) no
}
#if defined(LIBIMP_CPP_17)
/// \see https://en.cppreference.com/w/cpp/memory/c/aligned_alloc
return std::aligned_alloc(alignment, bytes);
/// \remark The size parameter must be an integral multiple of alignment.
return std::aligned_alloc(alignment, ::LIBIMP::round_up(bytes, alignment));
#else
if (alignment <= alignof(std::max_align_t)) {
/// \see https://en.cppreference.com/w/cpp/memory/c/malloc
Expand Down
10 changes: 7 additions & 3 deletions test/imp/test_imp_byte.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@
TEST(byte, construct) {
{
imp::byte b;
SUCCEED();
}
{
imp::byte b{};
EXPECT_EQ(int(b), 0);
}
{
imp::byte b {123};
imp::byte b{123};
EXPECT_EQ(int(b), 123);
}
{
imp::byte b {65535};
imp::byte b{65535};
EXPECT_EQ(int(b), 255);
EXPECT_EQ(std::int8_t(b), -1);
}
{
imp::byte b {65536};
imp::byte b{65536};
EXPECT_EQ(int(b), 0);
}
}
Expand Down
45 changes: 29 additions & 16 deletions test/pmr/test_pmr_small_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,18 @@ TEST(small_storage, holder_copy_move_construct) {
EXPECT_FALSE((std::is_move_assignable<pmr::holder<void, false>>::value));
}

TEST(small_storage, holder_copy_move) {
struct foo {
int i;
foo(int i) : i(i) {}
foo(foo const &rhs) : i(rhs.i) {}
foo(foo &&rhs) : i(std::exchange(rhs.i, 0)) {}
};
namespace {

struct foo {
int i;
foo(int i) : i(i) {}
foo(foo const &rhs) : i(rhs.i) {}
foo(foo &&rhs) : i(std::exchange(rhs.i, 0)) {}
};

} // namespace

TEST(small_storage, holder_copy_move_object_stack) {
pmr::allocator alc;
pmr::holder<foo, true> h1(alc, 1);
pmr::holder<foo, true> h2, h3;
Expand All @@ -61,7 +65,10 @@ TEST(small_storage, holder_copy_move) {
h1.destroy(alc);
h2.destroy(alc);
h3.destroy(alc);
}

TEST(small_storage, holder_copy_move_object_heap) {
pmr::allocator alc;
pmr::holder<foo, false> h4(alc, 1);
pmr::holder<foo, false> h5, h6;
h4.copy_to(alc, &h5);
Expand All @@ -73,17 +80,20 @@ TEST(small_storage, holder_copy_move) {
h4.destroy(alc);
h5.destroy(alc);
h6.destroy(alc);
}

TEST(small_storage, holder_copy_move_array_stack) {
pmr::allocator alc;
void *ph1 = std::malloc(pmr::holder<void, true>::full_sizeof<int>(10));
void *ph2 = std::malloc(pmr::holder<void, true>::full_sizeof<int>(10));
void *ph3 = std::malloc(pmr::holder<void, true>::full_sizeof<int>(10));
auto *h7 = ::new (ph1) pmr::holder<void, true>(alc, ::LIBIMP::types<int>{}, 10);
auto *h8 = ::new (ph2) pmr::holder<void, true>;
auto *h9 = ::new (ph2) pmr::holder<void, true>;
h7->copy_to(alc, h8);
auto *h8 = static_cast<pmr::holder<void, true> *>(ph2);
auto *h9 = static_cast<pmr::holder<void, true> *>(ph3);
h7->copy_to(alc, ph2);
EXPECT_EQ(h7->count(), 10);
EXPECT_EQ(h8->count(), 10);
h7->move_to(alc, h9);
h7->move_to(alc, ph3);
EXPECT_EQ(h7->count(), 0);
EXPECT_EQ(h9->count(), 10);
h7->destroy(alc);
Expand All @@ -92,7 +102,10 @@ TEST(small_storage, holder_copy_move) {
std::free(ph1);
std::free(ph2);
std::free(ph3);
}

TEST(small_storage, holder_copy_move_array_heap) {
pmr::allocator alc;
pmr::holder<void, false> h10(alc, ::LIBIMP::types<int>{}, 10);
pmr::holder<void, false> h11, h12;
h10.copy_to(alc, &h11);
Expand All @@ -110,14 +123,14 @@ TEST(small_storage, sizeof) {
EXPECT_EQ(sizeof(pmr::holder_null), sizeof(void *));
EXPECT_EQ(sizeof(pmr::holder<int, true>), sizeof(void *) + imp::round_up(sizeof(int), alignof(void *)));
EXPECT_EQ(sizeof(pmr::holder<int, false>), sizeof(void *) + sizeof(void *));
EXPECT_EQ(sizeof(pmr::holder<void, true>), sizeof(void *) + sizeof(pmr::detail::holder_info));
EXPECT_EQ(sizeof(pmr::holder<void, true>), imp::round_up(sizeof(void *) + sizeof(pmr::detail::holder_data), alignof(std::max_align_t)));
EXPECT_EQ(sizeof(pmr::holder<void, false>), sizeof(void *) + sizeof(void *));

// pmr::small_storage<4> s1;
EXPECT_TRUE(sizeof(pmr::small_storage<16>) > 16);
EXPECT_TRUE(sizeof(pmr::small_storage<64>) > 64);
EXPECT_TRUE(sizeof(pmr::small_storage<512>) > 512);
EXPECT_TRUE(sizeof(pmr::small_storage<4096>) > 4096);
EXPECT_GT(sizeof(pmr::small_storage<16>) , 16);
EXPECT_GT(sizeof(pmr::small_storage<64>) , 64);
EXPECT_GT(sizeof(pmr::small_storage<512>) , 512);
EXPECT_GT(sizeof(pmr::small_storage<4096>), 4096);
}

TEST(small_storage, construct) {
Expand Down

0 comments on commit 872b97f

Please sign in to comment.