Open maierlars opened 8 months ago
Indeed, the allocation is to small.
constexpr static std::size_t sizeof_inner_n(count_t count)
{
auto const inner_size = immer_offsetof(impl_t, d.data.inner.buffer) +
sizeof(inner_t::buffer) * count;
return std::max(sizeof(node_t), inner_size);
}
Fix the issue at hand. However, I think there are many more places where not enough memory is allocated for the objects that are constructed.
That buffer
object is never used when N=0 as it used to construct in it the actual elements of the node (it also does not have constructor nor touches the memory unless we do it explicitly).
I see the solution in @jsteemann PR and that should fix the UB, but at the cost of some memory waste. Maybe a solution is to take the buffer
type completely out of the struct, and append it (with correct alignment) after the struct "manually".
That could work, indeed. I might give it a try later today.
@maierlars did you find time to try that solution?
It could help a lot with this: https://github.com/google/oss-fuzz/pull/12087/files/bc7e6e5181614ea5ab41c9dce7613cc420686b5d..50591a9fcf22e37316e5f18a4e047f4a31a377cf
I'm sorry, there was so much else to do. I totally forgot about it. :(
Are you still interested in trying to fix it? :)
Please don't hold back if you want to give it a try. You seem eager to attack it ;)
Not really... I'm super busy and won't be able to do it this month. Just saw the comments on the other ticket and thought of this. Feel free to fix it first :) I'll let you know here when/if I have time to work on it myself.
Our sanitizer runs have produced the following errors. UBSan claims that an object is constructed in a memory region that is too small for the object.
Details
``` /immer/immer/detail/hamts/node.hpp:224:26: runtime error: constructor call on address 0x60300009a540 with insufficient space for an object of type 'node_t' (aka 'immer::detail::hamts::node, std::shared_ptr>, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::hash_key, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::equal_key, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5>')
0x60300009a540: note: pointer points here
00 00 00 00 be be be be be be be be be be be be be be be be be be be be be be be be 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:224:26 in
/immer/immer/detail/hamts/node.hpp:229:12: runtime error: member access within address 0x60300009a540 with insufficient space for an object of type 'node_t' (aka 'immer::detail::hamts::node, std::shared_ptr>, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::hash_key, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::equal_key, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5>')
0x60300009a540: note: pointer points here
00 00 00 00 01 00 00 00 be be be be be be be be be be be be be be be be be be be be 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:229:12 in
/immer/immer/detail/hamts/node.hpp:229:12: runtime error: member access within address 0x60300009a540 with insufficient space for an object of type 'impl_t' (aka 'immer::detail::csl::member_two, std::shared_ptr>, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::hash_key, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::equal_key, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5>::impl_data_t, immer::detail::csl::member>>::type::ownee>::type>::type>::type')
0x60300009a540: note: pointer points here
00 00 00 00 01 00 00 00 be be be be be be be be be be be be be be be be be be be be 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:229:12 in
/immer/immer/detail/hamts/node.hpp:229:17: runtime error: member access within address 0x60300009a548 with insufficient space for an object of type 'immer::detail::hamts::node, std::shared_ptr>, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::hash_key, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::equal_key, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5>::impl_data_t'
0x60300009a548: note: pointer points here
be be be be be be be be be be be be be be be be be be be be 00 00 00 00 00 00 00 00 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:229:17 in
/immer/immer/detail/hamts/node.hpp:229:19: runtime error: member access within address 0x60300009a548 with insufficient space for an object of type 'data_t'
0x60300009a548: note: pointer points here
be be be be be be be be be be be be be be be be be be be be 00 00 00 00 00 00 00 00 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:229:19 in
/immer/immer/detail/hamts/node.hpp:229:24: runtime error: member access within address 0x60300009a548 with insufficient space for an object of type 'inner_t'
0x60300009a548: note: pointer points here
be be be be be be be be be be be be be be be be be be be be 00 00 00 00 00 00 00 00 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:229:24 in
/immer/immer/detail/hamts/node.hpp:230:12: runtime error: member access within address 0x60300009a540 with insufficient space for an object of type 'node_t' (aka 'immer::detail::hamts::node, std::shared_ptr>, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::hash_key, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::equal_key, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5>')
0x60300009a540: note: pointer points here
00 00 00 00 01 00 00 00 be be be be 00 00 00 00 be be be be be be be be be be be be 00 00 00 00
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /immer/immer/detail/hamts/node.hpp:230:12 in
/immer/immer/detail/hamts/node.hpp:230:12: runtime error: member access within address 0x60300009a540 with insufficient space for an object of type 'impl_t' (aka 'immer::detail::csl::member_two, std::shared_ptr>, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::hash_key, immer::map, std::shared_ptr, arangodb::consensus::Node::TransparentHash, arangodb::consensus::Node::TransparentEqual, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>>::equal_key, immer::memory_policy>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5>::impl_data_t, immer::detail::csl::member>>::type::ownee>::type>::type>::type')
0x60300009a540: note: pointer points here
00 00 00 00 01 00 00 00 be be be be 00 00 00 00 be be be be be be be be be be be be 00 00 00 00
```
A backtrace is also available:
I had a closed look at the code in question:
The size of the allocated memory is calculated using
size_of_inner_n
in the same file, which looks like thisNow the part that confuses me: The type
impl_t
isSince
refs_t
andownee_t
are empty classes, they are optimized away bycombine_standard_layout_t
.impl_data_t
remains and is defined asFor now lets ignore what happens if
collision_t
which depends onT
is actually bigger thaninner_t
. Ifmake_inner_n
is called withn = 0
, it does not allocate enough memory, because the memberbuffer
is no longer accounted for.This indeed happens here:
Any ideas whether this bogus or, if the above analysis is correct, how it can be fixed? My first guess would be to patch
sizeof_inner_n
and always usemax(1, n)
instead ofn
.