griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

BLI: improve exception safety of memory utils

Browse Source 
Even if we do not use exception in many places in Blender, our core C++ library
should become exception safe. Otherwise, we don't even have the option
to work with exceptions if we decide to do so.
This commit is contained in:
Jacques Lucke
2020-07-06 09:08:53 +02:00
parent 703a73fa84
commit 572c48cf98
5 changed files with 212 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

116
source/blender/blenlib/BLI_memory_utils.hh
View File

@@ -19,6 +19,9 @@
/** \file
* \ingroup bli
* Some of the functions below have very similar alternatives in the standard library. However, it
* is rather annoying to use those when debugging. Therefore, some more specialized and easier to
* debug functions are provided here.
*/
#include <memory>
@@ -28,10 +31,39 @@
namespace blender {
/**
* Call the destructor on n consecutive values. For trivially destructible types, this does
* nothing.
*
* Exception Safety: Destructors shouldn't throw exceptions.
*
* Before:
* ptr: initialized
* After:
* ptr: uninitialized
*/
template<typename T> void destruct_n(T *ptr, uint n)
{
static_assert(std::is_nothrow_destructible_v<T>,
"This should be true for all types. Destructors are noexcept by default.");
/* This is not strictly necessary, because the loop below will be optimized away anyway. It is
* nice to make behavior this explicitly, though. */
if (std::is_trivially_destructible<T>::value) {
return;
}
for (uint i = 0; i < n; i++) {
ptr[i].~T();
}
}
/**
* Call the default constructor on n consecutive elements. For trivially constructible types, this
* does nothing.
*
* Exception Safety: Strong.
*
* Before:
* ptr: uninitialized
* After:
@@ -45,36 +77,23 @@ template<typename T> void default_construct_n(T *ptr, uint n)
return;
}
for (uint i = 0; i < n; i++) {
new ((void *)(ptr + i)) T;
uint current = 0;
try {
for (; current < n; current++) {
new ((void *)(ptr + current)) T;
}
}
}
/**
* Call the destructor on n consecutive values. For trivially destructible types, this does
* nothing.
*
* Before:
* ptr: initialized
* After:
* ptr: uninitialized
*/
template<typename T> void destruct_n(T *ptr, uint n)
{
/* This is not strictly necessary, because the loop below will be optimized away anyway. It is
* nice to make behavior this explicitly, though. */
if (std::is_trivially_destructible<T>::value) {
return;
}
for (uint i = 0; i < n; i++) {
ptr[i].~T();
catch (...) {
destruct_n(ptr, current);
throw;
}
}
/**
* Copy n values from src to dst.
*
* Exception Safety: Basic.
*
* Before:
* src: initialized
* dst: initialized
@@ -92,6 +111,8 @@ template<typename T> void initialized_copy_n(const T *src, uint n, T *dst)
/**
* Copy n values from src to dst.
*
* Exception Safety: Strong.
*
* Before:
* src: initialized
* dst: uninitialized
@@ -101,14 +122,23 @@ template<typename T> void initialized_copy_n(const T *src, uint n, T *dst)
*/
template<typename T> void uninitialized_copy_n(const T *src, uint n, T *dst)
{
for (uint i = 0; i < n; i++) {
new ((void *)(dst + i)) T(src[i]);
uint current = 0;
try {
for (; current < n; current++) {
new ((void *)(dst + current)) T(src[current]);
}
}
catch (...) {
destruct_n(dst, current);
throw;
}
}
/**
* Move n values from src to dst.
*
* Exception Safety: Basic.
*
* Before:
* src: initialized
* dst: initialized
@@ -126,6 +156,8 @@ template<typename T> void initialized_move_n(T *src, uint n, T *dst)
/**
* Move n values from src to dst.
*
* Exception Safety: Basic.
*
* Before:
* src: initialized
* dst: uninitialized
@@ -135,8 +167,19 @@ template<typename T> void initialized_move_n(T *src, uint n, T *dst)
*/
template<typename T> void uninitialized_move_n(T *src, uint n, T *dst)
{
for (uint i = 0; i < n; i++) {
new ((void *)(dst + i)) T(std::move(src[i]));
static_assert(std::is_nothrow_move_constructible_v<T>,
"Ideally, all types should have this property. We might have to remove this "
"limitation of a real reason comes up.");
uint current = 0;
try {
for (; current < n; current++) {
new ((void *)(dst + current)) T(std::move(src[current]));
}
}
catch (...) {
destruct_n(dst, current);
throw;
}
}
@@ -144,6 +187,8 @@ template<typename T> void uninitialized_move_n(T *src, uint n, T *dst)
* Relocate n values from src to dst. Relocation is a move followed by destruction of the src
* value.
*
* Exception Safety: Basic.
*
* Before:
* src: initialized
* dst: initialized
@@ -161,6 +206,8 @@ template<typename T> void initialized_relocate_n(T *src, uint n, T *dst)
* Relocate n values from src to dst. Relocation is a move followed by destruction of the src
* value.
*
* Exception Safety: Basic.
*
* Before:
* src: initialized
* dst: uninitialized
@@ -177,6 +224,8 @@ template<typename T> void uninitialized_relocate_n(T *src, uint n, T *dst)
/**
* Copy the value to n consecutive elements.
*
* Exception Safety: Basic.
*
* Before:
* dst: initialized
* After:
@@ -192,6 +241,8 @@ template<typename T> void initialized_fill_n(T *dst, uint n, const T &value)
/**
* Copy the value to n consecutive elements.
*
* Exception Safety: Strong.
*
* Before:
* dst: uninitialized
* After:
@@ -199,8 +250,15 @@ template<typename T> void initialized_fill_n(T *dst, uint n, const T &value)
*/
template<typename T> void uninitialized_fill_n(T *dst, uint n, const T &value)
{
for (uint i = 0; i < n; i++) {
new ((void *)(dst + i)) T(value);
uint current = 0;
try {
for (; current < n; current++) {
new ((void *)(dst + current)) T(value);
}
}
catch (...) {
destruct_n(dst, current);
throw;
}
}

120
tests/gtests/blenlib/BLI_memory_utils_test.cc Normal file
View File

@@ -0,0 +1,120 @@
#include "BLI_memory_utils.hh"
#include "BLI_strict_flags.h"
#include "testing/testing.h"
namespace blender {
struct MyValue {
static inline int alive = 0;
MyValue()
{
if (alive == 15) {
throw std::exception();
}
alive++;
}
MyValue(const MyValue &other)
{
if (alive == 15) {
throw std::exception();
}
alive++;
}
~MyValue()
{
alive--;
}
};
TEST(memory_utils, DefaultConstructN_ActuallyCallsConstructor)
{
constexpr int amount = 10;
TypedBuffer<MyValue, amount> buffer;
EXPECT_EQ(MyValue::alive, 0);
default_construct_n(buffer.ptr(), amount);
EXPECT_EQ(MyValue::alive, amount);
destruct_n(buffer.ptr(), amount);
EXPECT_EQ(MyValue::alive, 0);
}
TEST(memory_utils, DefaultConstructN_StrongExceptionSafety)
{
constexpr int amount = 20;
TypedBuffer<MyValue, amount> buffer;
EXPECT_EQ(MyValue::alive, 0);
EXPECT_THROW(default_construct_n(buffer.ptr(), amount), std::exception);
EXPECT_EQ(MyValue::alive, 0);
}
TEST(memory_utils, UninitializedCopyN_ActuallyCopies)
{
constexpr int amount = 5;
TypedBuffer<MyValue, amount> buffer1;
TypedBuffer<MyValue, amount> buffer2;
EXPECT_EQ(MyValue::alive, 0);
default_construct_n(buffer1.ptr(), amount);
EXPECT_EQ(MyValue::alive, amount);
uninitialized_copy_n(buffer1.ptr(), amount, buffer2.ptr());
EXPECT_EQ(MyValue::alive, 2 * amount);
destruct_n(buffer1.ptr(), amount);
EXPECT_EQ(MyValue::alive, amount);
destruct_n(buffer2.ptr(), amount);
EXPECT_EQ(MyValue::alive, 0);
}
TEST(memory_utils, UninitializedCopyN_StrongExceptionSafety)
{
constexpr int amount = 10;
TypedBuffer<MyValue, amount> buffer1;
TypedBuffer<MyValue, amount> buffer2;
EXPECT_EQ(MyValue::alive, 0);
default_construct_n(buffer1.ptr(), amount);
EXPECT_EQ(MyValue::alive, amount);
EXPECT_THROW(uninitialized_copy_n(buffer1.ptr(), amount, buffer2.ptr()), std::exception);
EXPECT_EQ(MyValue::alive, amount);
destruct_n(buffer1.ptr(), amount);
EXPECT_EQ(MyValue::alive, 0);
}
TEST(memory_utils, UninitializedFillN_ActuallyCopies)
{
constexpr int amount = 10;
TypedBuffer<MyValue, amount> buffer;
EXPECT_EQ(MyValue::alive, 0);
{
MyValue value;
EXPECT_EQ(MyValue::alive, 1);
uninitialized_fill_n(buffer.ptr(), amount, value);
EXPECT_EQ(MyValue::alive, 1 + amount);
destruct_n(buffer.ptr(), amount);
EXPECT_EQ(MyValue::alive, 1);
}
EXPECT_EQ(MyValue::alive, 0);
}
TEST(memory_utils, UninitializedFillN_StrongExceptionSafety)
{
constexpr int amount = 20;
TypedBuffer<MyValue, amount> buffer;
EXPECT_EQ(MyValue::alive, 0);
{
MyValue value;
EXPECT_EQ(MyValue::alive, 1);
EXPECT_THROW(uninitialized_fill_n(buffer.ptr(), amount, value), std::exception);
EXPECT_EQ(MyValue::alive, 1);
}
EXPECT_EQ(MyValue::alive, 0);
}
} // namespace blender

4
tests/gtests/blenlib/BLI_vector_test.cc
View File

@@ -499,7 +499,7 @@ class TypeConstructMock {
{
}
TypeConstructMock(TypeConstructMock &&other) : move_constructed(true)
TypeConstructMock(TypeConstructMock &&other) noexcept : move_constructed(true)
{
}
@@ -513,7 +513,7 @@ class TypeConstructMock {
return *this;
}
TypeConstructMock &operator=(TypeConstructMock &&other)
TypeConstructMock &operator=(TypeConstructMock &&other) noexcept
{
if (this == &other) {
return *this;

1
tests/gtests/blenlib/CMakeLists.txt
View File

@@ -63,6 +63,7 @@ BLENDER_TEST(BLI_math_geom "bf_blenlib")
BLENDER_TEST(BLI_math_matrix "bf_blenlib")
BLENDER_TEST(BLI_math_vector "bf_blenlib")
BLENDER_TEST(BLI_memiter "bf_blenlib")
BLENDER_TEST(BLI_memory_utils "bf_blenlib")
BLENDER_TEST(BLI_path_util "${BLI_path_util_extra_libs}")
BLENDER_TEST(BLI_polyfill_2d "bf_blenlib")
BLENDER_TEST(BLI_set "bf_blenlib")

4
tests/gtests/functions/FN_cpp_type_test.cc
View File

@@ -50,7 +50,7 @@ struct TestType {
other.value = copy_constructed_from_value;
}
TestType(TestType &&other)
TestType(TestType &&other) noexcept
{
value = move_constructed_value;
other.value = move_constructed_from_value;
@@ -63,7 +63,7 @@ struct TestType {
return *this;
}
TestType &operator=(TestType &&other)
TestType &operator=(TestType &&other) noexcept
{
value = move_assigned_value;
other.value = move_assigned_from_value;