Skip to content

Commit

Permalink
Fix issues found with ASan and UBSan
Browse files Browse the repository at this point in the history
  • Loading branch information
@eliaskosunen
eliaskosunen committed Nov 1, 2023
1 parent a69d578 commit f7332fa
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 20 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ _Released 2023-xx-xx_

* Fix `scn::wrap(std::string_view&&)` being ambiguous on gcc (reported in #83)
* Fix compiler error in `scn::basic_string_view<CharT>::substr` (reported in #86)
* Fix memory safety issues found with ASan and UBsan in
`small_vector`, `detail::utf16::validate_next`, and `detail::get_buffer`.
* Improve error messages given from the float parser

# 1.1.2
Expand Down
5 changes: 2 additions & 3 deletions include/scn/detail/vectored.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ namespace scn {
if (begin >= buf_it->begin() && begin < buf_it->end()) {
break;
}
if (begin == buf_it->end()) {
if (begin == buf_it->end() && buf_it + 1 != s.end()) {
++buf_it;
begin = buf_it->begin();
break;
Expand Down Expand Up @@ -156,8 +156,7 @@ namespace scn {
::scn::custom_ranges::detail::_requires<
provides_buffer_access_concept,
Range,
::scn::ranges::iterator_t<const Range>>::value> {
};
::scn::ranges::iterator_t<const Range>>::value> {};
} // namespace detail

SCN_END_NAMESPACE
Expand Down
5 changes: 3 additions & 2 deletions include/scn/reader/int.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,9 @@ namespace scn {
}

auto it = r.value();
std::basic_string<char_type> str(to_address(it),
s.size());
std::basic_string<char_type> str(
to_address(it),
static_cast<size_t>(std::distance(it, s.end())));
ret = ctx.locale().get_localized().read_num(
tmp, str, static_cast<int>(base));

Expand Down
2 changes: 0 additions & 2 deletions include/scn/unicode/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ namespace scn {
static constexpr const uint16_t trail_surrogate_max = 0xdfff;
static constexpr const uint16_t lead_offset =
lead_surrogate_min - (0x10000u >> 10);
static constexpr const uint32_t surrogate_offset =
0x10000u - (lead_surrogate_min << 10) - trail_surrogate_min;
static constexpr const uint32_t code_point_max = 0x10ffff;

template <typename Octet>
Expand Down
7 changes: 4 additions & 3 deletions include/scn/unicode/utf16.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ namespace scn {
"Invalid utf16 trail surrogate"};
}
++it;
cp = static_cast<code_point>(
static_cast<uint32_t>(lead << 10u) + trail +
surrogate_offset);
auto lead32 = static_cast<uint32_t>(lead) - 0xd800u;
auto trail32 = static_cast<uint32_t>(trail) - 0xdc00u;
cp = static_cast<code_point>(((lead32 << 10) | trail32) +
0x10000u);
return {};
}
if (is_trail_surrogate(lead)) {
Expand Down
24 changes: 21 additions & 3 deletions include/scn/util/memory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ namespace scn {
constexpr bool B = std::is_trivially_copyable<T>::value &&
std::is_pointer<ForwardIt>::value &&
sizeof(T) == 1;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
return uninitialized_fill(first, last, value,
std::integral_constant<bool, B>{});
}
Expand All @@ -186,6 +188,8 @@ namespace scn {
{
using value_type =
typename std::iterator_traits<ForwardIt>::value_type;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
ForwardIt current = first;
for (; current != last; ++current) {
::new (static_cast<void*>(std::addressof(*current))) value_type;
Expand All @@ -198,6 +202,8 @@ namespace scn {
using value_type =
typename std::iterator_traits<ForwardIt>::value_type;
ForwardIt current = first;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
for (; current != last; ++current) {
::new (static_cast<void*>(std::addressof(*current)))
value_type();
Expand All @@ -216,6 +222,9 @@ namespace scn {
using value_type =
typename std::iterator_traits<ForwardIt>::value_type;
ForwardIt current = d_first;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
SCN_EXPECT(detail::to_address(d_first) != nullptr);
for (; first != last; ++first, (void)++current) {
::new (static_cast<void*>(std::addressof(*current)))
value_type(*first);
Expand All @@ -234,6 +243,9 @@ namespace scn {
using value_type =
typename std::iterator_traits<ForwardIt>::value_type;
using pointer = typename std::iterator_traits<ForwardIt>::pointer;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
SCN_EXPECT(detail::to_address(d_first) != nullptr);
auto ptr =
std::memcpy(std::addressof(*d_first), std::addressof(*first),
static_cast<size_t>(std::distance(first, last)) *
Expand All @@ -252,6 +264,9 @@ namespace scn {
{
using value_type =
typename std::iterator_traits<ForwardIt>::value_type;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
SCN_EXPECT(detail::to_address(d_first) != nullptr);
ForwardIt current = d_first;
for (; first != last; ++first, (void)++current) {
::new (static_cast<void*>(std::addressof(*current)))
Expand All @@ -271,6 +286,9 @@ namespace scn {
using value_type =
typename std::iterator_traits<ForwardIt>::value_type;
using pointer = typename std::iterator_traits<ForwardIt>::pointer;
SCN_EXPECT(detail::to_address(first) != nullptr);
SCN_EXPECT(detail::to_address(last) != nullptr);
SCN_EXPECT(detail::to_address(d_first) != nullptr);
auto ptr =
std::memcpy(std::addressof(*d_first), std::addressof(*first),
static_cast<size_t>(std::distance(first, last)) *
Expand All @@ -289,12 +307,12 @@ namespace scn {

erased_storage(T val) noexcept(
std::is_nothrow_move_constructible<T>::value)
: m_ptr(::new (static_cast<void*>(&m_data)) T(SCN_MOVE(val)))
: m_ptr(::new(static_cast<void*>(&m_data)) T(SCN_MOVE(val)))
{
}

erased_storage(const erased_storage& other)
: m_ptr(other ? ::new (static_cast<void*>(&m_data))
: m_ptr(other ? ::new(static_cast<void*>(&m_data))
T(other.get())
: nullptr)
{
Expand All @@ -309,7 +327,7 @@ namespace scn {
}

erased_storage(erased_storage&& other) noexcept
: m_ptr(other ? ::new (static_cast<void*>(&m_data))
: m_ptr(other ? ::new(static_cast<void*>(&m_data))
T(SCN_MOVE(other.get()))
: nullptr)
{
Expand Down
16 changes: 11 additions & 5 deletions include/scn/util/small_vector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ namespace scn {
using reverse_iterator = std::reverse_iterator<pointer>;
using const_reverse_iterator = std::reverse_iterator<const_pointer>;

struct stack_storage : basic_stack_storage<T, StackN> {
};
struct stack_storage : basic_stack_storage<T, StackN> {};
struct heap_storage {
size_type cap{0};
};
Expand Down Expand Up @@ -674,13 +673,15 @@ namespace scn {
private:
stack_storage& _construct_stack_storage() noexcept
{
::new (std::addressof(m_stack_storage)) stack_storage;
::new (static_cast<void*>(std::addressof(m_stack_storage)))
stack_storage;
m_ptr = m_stack_storage.reinterpret_unconstructed_data();
return m_stack_storage;
}
heap_storage& _construct_heap_storage() noexcept
{
::new (std::addressof(m_heap_storage)) heap_storage;
::new (static_cast<void*>(std::addressof(m_heap_storage)))
heap_storage;
m_ptr = nullptr;
return m_heap_storage;
}
Expand All @@ -700,6 +701,9 @@ namespace scn {

void _destruct_elements() noexcept
{
if (!m_ptr) {
return;
}
const auto s = size();
for (size_type i = 0; i != s; ++i) {
m_ptr[i].~T();
Expand All @@ -724,7 +728,9 @@ namespace scn {
auto ptr =
static_cast<pointer>(static_cast<void*>(storage_ptr));
auto n = size();
uninitialized_move(begin(), end(), ptr);
if (!empty()) {
uninitialized_move(begin(), end(), ptr);
}
_destruct();
auto& heap = [this]() -> heap_storage& {
if (is_small()) {
Expand Down
8 changes: 6 additions & 2 deletions test/each/integer_each.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,15 @@ TEST_CASE_TEMPLATE_DEFINE("integer each", T, integer_each_test)
CHECK(tmp == val);
};
auto random_ints = [](size_t n, T a, T b) {
using dist_type =
typename std::conditional<std::is_signed<T>::value, long long,
unsigned long long>::type;
std::default_random_engine rng(std::random_device{}());
std::uniform_int_distribution<T> dist(a, b);
std::uniform_int_distribution<dist_type> dist(
static_cast<dist_type>(a), static_cast<dist_type>(b));
std::vector<T> vec;
std::generate_n(std::back_inserter(vec), n,
[&]() { return dist(rng); });
[&]() { return static_cast<T>(dist(rng)); });
return vec;
};
for (T i = min; i != min + 1000; ++i) {
Expand Down

0 comments on commit f7332fa

Please sign in to comment.