From 4e7a357a479568e4731e1976c0a6c51c12be4111 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Mon, 26 Jun 2017 11:27:18 +0100 Subject: [PATCH] Fix various undefined behaviours detected by UBSan --- src/array_view.hh | 2 +- src/hash.cc | 6 ++++-- src/highlighters.cc | 2 +- src/remote.cc | 8 +++++--- src/string.cc | 11 ++++++++--- src/utils.hh | 12 ++++++------ 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/array_view.hh b/src/array_view.hh index d7dc9fbf..e9c455ff 100644 --- a/src/array_view.hh +++ b/src/array_view.hh @@ -34,7 +34,7 @@ public: template::type> constexpr ArrayView(const std::vector& v) - : m_pointer(&v[0]), m_size(v.size()) {} + : m_pointer(v.data()), m_size(v.size()) {} constexpr ArrayView(const std::initializer_list& v) : m_pointer(v.begin()), m_size(v.size()) {} diff --git a/src/hash.cc b/src/hash.cc index f5d320e2..342e37a7 100644 --- a/src/hash.cc +++ b/src/hash.cc @@ -1,6 +1,7 @@ #include "hash.hh" #include +#include namespace Kakoune { @@ -32,11 +33,12 @@ size_t hash_data(const char* input, size_t len) constexpr uint32_t c2 = 0x1b873593; const int nblocks = len / 4; - const uint32_t* blocks = reinterpret_cast(data + nblocks*4); + const uint8_t* blocks = data + nblocks*4; for (int i = -nblocks; i; ++i) { - uint32_t key = blocks[i]; + uint32_t key; + memcpy(&key, blocks + 4*i, 4); key *= c1; key = rotl(key, 15); key *= c2; diff --git a/src/highlighters.cc b/src/highlighters.cc index 36904b20..ee3917a9 100644 --- a/src/highlighters.cc +++ b/src/highlighters.cc @@ -1014,7 +1014,7 @@ struct LineNumbersHighlighter : Highlighter } private: - void do_highlight(const Context& context, HighlightPass, DisplayBuffer& display_buffer, BufferRange) + void do_highlight(const Context& context, HighlightPass, DisplayBuffer& display_buffer, BufferRange) override { const Face face = get_face("LineNumbers"); const Face face_wrapped = get_face("LineNumbersWrapped"); diff --git a/src/remote.cc b/src/remote.cc index bb577aa2..7b3ffccb 100644 --- a/src/remote.cc +++ b/src/remote.cc @@ -23,7 +23,7 @@ namespace Kakoune { -enum class MessageType : char +enum class MessageType : uint8_t { Unknown, Connect, @@ -54,7 +54,7 @@ public: ~MsgWriter() noexcept(false) { uint32_t count = (uint32_t)m_buffer.size() - m_start; - *reinterpret_cast(m_buffer.data() + m_start + 1) = count; + memcpy(m_buffer.data() + m_start + sizeof(MessageType), &count, sizeof(uint32_t)); } void write(const char* val, size_t size) @@ -172,7 +172,9 @@ public: uint32_t size() const { kak_assert(m_write_pos >= header_size); - return *reinterpret_cast(m_stream.data()+1); + uint32_t res; + memcpy(&res, m_stream.data() + sizeof(MessageType), sizeof(uint32_t)); + return res; } MessageType type() const diff --git a/src/string.cc b/src/string.cc index 36beca1e..89b7c3fe 100644 --- a/src/string.cc +++ b/src/string.cc @@ -22,7 +22,8 @@ String::Data::Data(const char* data, size_t size, size_t capacity) l.size = size; l.capacity = capacity; - memcpy(l.ptr, data, size); + if (data != nullptr) + memcpy(l.ptr, data, size); l.ptr[size] = 0; } else @@ -101,6 +102,9 @@ void String::Data::force_size(size_t new_size) void String::Data::append(const char* str, size_t len) { + if (len == 0) + return; + const size_t new_size = size() + len; reserve(new_size); @@ -147,7 +151,8 @@ void String::Data::set_size(size_t size) void String::Data::set_short(const char* data, size_t size) { s.size = (size << 1) | 1; - memcpy(s.string, data, size); + if (data != nullptr) + memcpy(s.string, data, size); s.string[size] = 0; } @@ -308,7 +313,7 @@ Optional str_to_int_ifp(StringView str) return {}; res = res * 10 + c - '0'; } - return negative ? -(int)res : (int)res; + return negative ? -res : res; } int str_to_int(StringView str) diff --git a/src/utils.hh b/src/utils.hh index 1966f86b..06df21a1 100644 --- a/src/utils.hh +++ b/src/utils.hh @@ -21,8 +21,8 @@ public: static T& instance() { - kak_assert (ms_instance); - return *ms_instance; + kak_assert(ms_instance); + return *static_cast(ms_instance); } static bool has_instance() @@ -33,8 +33,8 @@ public: protected: Singleton() { - kak_assert(not ms_instance); - ms_instance = static_cast(this); + kak_assert(ms_instance == nullptr); + ms_instance = this; } ~Singleton() @@ -44,11 +44,11 @@ protected: } private: - static T* ms_instance; + static Singleton* ms_instance; }; template -T* Singleton::ms_instance = nullptr; +Singleton* Singleton::ms_instance = nullptr; // *** On scope end *** //