Skip to content
This repository was archived by the owner on Mar 22, 2023. It is now read-only.

Commit 22fa029

Browse files
lukaszstolarczuklukaszstolarczuk
authored
Merge pull request #1112 from igchor/ubsan_fixes
Ubsan fixes
2 parents b9e4440 + fc4117b commit 22fa029

4 files changed

Lines changed: 49 additions & 27 deletions

File tree

‎include/libpmemobj++/experimental/radix_tree.hpp‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,15 +2852,17 @@ radix_tree<Key, Value, BytesView>::radix_tree_iterator<
28522852
IsConst>::radix_tree_iterator(leaf_ptr leaf_, tree_ptr tree)
28532853
: leaf_(leaf_), tree(tree)
28542854
{
2855+
assert(tree);
28552856
}
28562857

28572858
template <typename Key, typename Value, typename BytesView>
28582859
template <bool IsConst>
28592860
template <bool C, typename Enable>
28602861
radix_tree<Key, Value, BytesView>::radix_tree_iterator<
28612862
IsConst>::radix_tree_iterator(const radix_tree_iterator<false> &rhs)
2862-
: leaf_(rhs.leaf_)
2863+
: leaf_(rhs.leaf_), tree(rhs.tree)
28632864
{
2865+
assert(tree);
28642866
}
28652867

28662868
template <typename Key, typename Value, typename BytesView>
@@ -2871,6 +2873,8 @@ typename radix_tree<Key, Value,
28712873
BytesView>::radix_tree_iterator<IsConst>::operator*() const
28722874
{
28732875
assert(leaf_);
2876+
assert(tree);
2877+
28742878
return *leaf_;
28752879
}
28762880

@@ -2882,6 +2886,8 @@ typename radix_tree<Key, Value,
28822886
BytesView>::radix_tree_iterator<IsConst>::operator->() const
28832887
{
28842888
assert(leaf_);
2889+
assert(tree);
2890+
28852891
return leaf_;
28862892
}
28872893

@@ -2903,6 +2909,9 @@ void
29032909
radix_tree<Key, Value, BytesView>::radix_tree_iterator<IsConst>::assign_val(
29042910
basic_string_view<typename V::value_type, typename V::traits_type> rhs)
29052911
{
2912+
assert(leaf_);
2913+
assert(tree);
2914+
29062915
auto pop = pool_base(pmemobj_pool_by_ptr(leaf_));
29072916

29082917
if (rhs.size() <= leaf_->value().capacity() && !tree->mt.get(false)) {
@@ -2988,6 +2997,7 @@ bool
29882997
radix_tree<Key, Value, BytesView>::radix_tree_iterator<IsConst>::try_increment()
29892998
{
29902999
assert(leaf_);
3000+
assert(tree);
29913001

29923002
constexpr auto direction = radix_tree::node::direction::Forward;
29933003
auto parent_ptr = leaf_->parent.load_acquire();
@@ -3035,6 +3045,7 @@ bool
30353045
radix_tree<Key, Value, BytesView>::radix_tree_iterator<IsConst>::try_decrement()
30363046
{
30373047
constexpr auto direction = radix_tree::node::direction::Reverse;
3048+
assert(tree);
30383049

30393050
while (true) {
30403051
if (!leaf_) {
@@ -3198,18 +3209,16 @@ template <typename Key, typename Value, typename BytesView>
31983209
Key &
31993210
radix_tree<Key, Value, BytesView>::leaf::key()
32003211
{
3201-
return *reinterpret_cast<Key *>(this + 1);
3212+
auto &const_key = const_cast<const leaf *>(this)->key();
3213+
return *const_cast<Key *>(&const_key);
32023214
}
32033215

32043216
template <typename Key, typename Value, typename BytesView>
32053217
Value &
32063218
radix_tree<Key, Value, BytesView>::leaf::value()
32073219
{
3208-
auto key_dst = reinterpret_cast<char *>(this + 1);
3209-
auto val_dst = reinterpret_cast<Value *>(
3210-
key_dst + total_sizeof<Key>::value(key()));
3211-
3212-
return *reinterpret_cast<Value *>(val_dst);
3220+
auto &const_value = const_cast<const leaf *>(this)->value();
3221+
return *const_cast<Value *>(&const_value);
32133222
}
32143223

32153224
template <typename Key, typename Value, typename BytesView>
@@ -3224,10 +3233,12 @@ const Value &
32243233
radix_tree<Key, Value, BytesView>::leaf::value() const
32253234
{
32263235
auto key_dst = reinterpret_cast<const char *>(this + 1);
3227-
auto val_dst = reinterpret_cast<const Value *>(
3228-
key_dst + total_sizeof<Key>::value(key()));
3236+
auto key_size = total_sizeof<Key>::value(key());
3237+
auto padding = detail::align_up(key_size, alignof(Value)) - key_size;
3238+
auto val_dst =
3239+
reinterpret_cast<const Value *>(key_dst + padding + key_size);
32293240

3230-
return *reinterpret_cast<const Value *>(val_dst);
3241+
return *val_dst;
32313242
}
32323243

32333244
template <typename Key, typename Value, typename BytesView>
@@ -3348,12 +3359,16 @@ radix_tree<Key, Value, BytesView>::leaf::make(pointer_type parent,
33483359
auto key_size = total_sizeof<Key>::value(std::get<I1>(first_args)...);
33493360
auto val_size =
33503361
total_sizeof<Value>::value(std::get<I2>(second_args)...);
3362+
auto padding = detail::align_up(key_size, alignof(Value)) - key_size;
33513363
auto ptr = static_cast<persistent_ptr<leaf>>(
3352-
a.allocate(sizeof(leaf) + key_size + val_size));
3364+
a.allocate(sizeof(leaf) + key_size + padding + val_size));
33533365

33543366
auto key_dst = reinterpret_cast<Key *>(ptr.get() + 1);
33553367
auto val_dst = reinterpret_cast<Value *>(
3356-
reinterpret_cast<char *>(key_dst) + key_size);
3368+
reinterpret_cast<char *>(key_dst) + padding + key_size);
3369+
3370+
assert(reinterpret_cast<uintptr_t>(key_dst) % alignof(Key) == 0);
3371+
assert(reinterpret_cast<uintptr_t>(val_dst) % alignof(Value) == 0);
33573372

33583373
new (ptr.get()) leaf();
33593374
new (key_dst) Key(std::forward<Args1>(std::get<I1>(first_args))...);

‎tests/CMakeLists.txt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,11 @@ if(NOT USE_ASAN)
210210
add_test_generic(NAME ptr_arith TRACERS none)
211211
endif()
212212

213-
build_test(p_ext p_ext/p_ext.cpp)
214-
add_test_generic(NAME p_ext TRACERS none pmemcheck)
213+
if(NOT USE_UBSAN)
214+
# We want to test overflow which is reported by UBSAN
215+
build_test(p_ext p_ext/p_ext.cpp)
216+
add_test_generic(NAME p_ext TRACERS none pmemcheck)
217+
endif()
215218

216219
if(NOT WIN32)
217220
build_test(shared_mutex_posix mutex/shared_mutex_posix.cpp)
@@ -582,8 +585,11 @@ if(TEST_CONCURRENT_HASHMAP)
582585
build_test(concurrent_hash_map_singlethread concurrent_hash_map/concurrent_hash_map_singlethread.cpp)
583586
add_test_generic(NAME concurrent_hash_map_singlethread TRACERS none memcheck pmemcheck)
584587

585-
build_test(concurrent_hash_map_layout concurrent_hash_map/concurrent_hash_map_layout.cpp)
586-
add_test_generic(NAME concurrent_hash_map_layout TRACERS none)
588+
if(NOT USE_UBSAN)
589+
# ASSERT_ALIGNED_FIELD is not compatible with UBSAN
590+
build_test(concurrent_hash_map_layout concurrent_hash_map/concurrent_hash_map_layout.cpp)
591+
add_test_generic(NAME concurrent_hash_map_layout TRACERS none)
592+
endif()
587593

588594
if(GDB_FOUND)
589595
build_test(concurrent_hash_map_rehash_break concurrent_hash_map_rehash_break/concurrent_hash_map_rehash_break.cpp)

‎tests/common/map_wrapper.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ struct test_bytes_view<
5353

5454
struct test_bytes_view_int {
5555
test_bytes_view_int(const int *v)
56-
: v((unsigned)(*v + std::numeric_limits<int>::max() + 1))
56+
: v(static_cast<unsigned>(*v) + std::numeric_limits<int>::max() + 1)
5757
{
5858
}
5959

‎tests/external/libcxx/map/is_transparent.h‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//
77
//===----------------------------------------------------------------------===//
88
//
9-
// Copyright 2020, Intel Corporation
9+
// Copyright 2020-2021, Intel Corporation
1010
//
1111
// testing transparent
1212
//
@@ -170,31 +170,32 @@ struct transparent_less_not_referenceable {
170170
struct heterogeneous_bytes_view {
171171
heterogeneous_bytes_view(const int *value)
172172
{
173-
v = (unsigned)(*value + (std::numeric_limits<int>::max)() + 1);
173+
v = static_cast<unsigned>(*value) +
174+
(std::numeric_limits<int>::max)() + 1;
174175
}
175176

176177
heterogeneous_bytes_view(const C2Int *value)
177178
{
178-
v = (unsigned)(value->get() +
179-
(std::numeric_limits<int>::max)() + 1);
179+
v = static_cast<unsigned>(value->get()) +
180+
(std::numeric_limits<int>::max)() + 1;
180181
}
181182

182183
heterogeneous_bytes_view(const Moveable *value)
183184
{
184-
v = (unsigned)(value->get() +
185-
(std::numeric_limits<int>::max)() + 1);
185+
v = static_cast<unsigned>(value->get()) +
186+
(std::numeric_limits<int>::max)() + 1;
186187
}
187188

188189
heterogeneous_bytes_view(const MoveableWrapper *value)
189190
{
190-
v = (unsigned)(value->get().get() +
191-
(std::numeric_limits<int>::max)() + 1);
191+
v = static_cast<unsigned>(value->get().get()) +
192+
(std::numeric_limits<int>::max)() + 1;
192193
}
193194

194195
heterogeneous_bytes_view(const PrivateConstructor *value)
195196
{
196-
v = (unsigned)(value->get() +
197-
(std::numeric_limits<int>::max)() + 1);
197+
v = static_cast<unsigned>(value->get()) +
198+
(std::numeric_limits<int>::max)() + 1;
198199
}
199200

200201
size_t

0 commit comments

Comments
 (0)