Don't deliberately dereference the null pointer

This patch addresses a static analysis issue reported by Cppcheck 2.9
where several member functions of the toml::discard_comment class
defined in the toml/comments.hpp header were implemented to deliberately
dereference the null pointer returned unconditionally by the
always-empty container's data() member function.  This behavior wasn't
technically wrong because those functions all have as precondition that
the container is non-empty so they must never be called on an instance
of toml::discard_comment but we can still be more helpful without
adversely affecting code generation.  Instead of dereferencing the null
pointer, this patch has these functions call an inline private helper
function which is defined to invoke __builtin_unreachable() if available
"and then" throw an exception with a helpful error message.  Even at the
-O1 level, GCC will optimize the code under the assumption that the
function will never be called (i.e. no assembly is emitted), making
failure to ensure this undefined behavior exactly as if the null pointer
had been dereferenced.  However, static analysis will now understand the
programmer's intent and remain silent.  Furthermore, when using the -O0
or -Og levels, GCC won't optimize under this assumption so the exception
will be thrown and might be helpful for debugging.  Compilers that don't
have __builtin_unreachable() won't get any help in determining that the
function must not be called and will have to figure this out by
analyzing the calling code -- which really shouldn't exist in the first
place anyway as the whole point is that these functions must not be
called.
This commit is contained in:
Moritz Klammler 2022-09-14 19:51:57 +02:00
parent 8bb2c63a01
commit cf8a977be2

View File

@ -425,14 +425,14 @@ struct discard_comments
// empty, so accessing through operator[], front/back, data causes address
// error.
reference operator[](const size_type) noexcept {return *data();}
const_reference operator[](const size_type) const noexcept {return *data();}
reference operator[](const size_type) noexcept {never_call("toml::discard_comment::operator[]");}
const_reference operator[](const size_type) const noexcept {never_call("toml::discard_comment::operator[]");}
reference at(const size_type) {throw std::out_of_range("toml::discard_comment is always empty.");}
const_reference at(const size_type) const {throw std::out_of_range("toml::discard_comment is always empty.");}
reference front() noexcept {return *data();}
const_reference front() const noexcept {return *data();}
reference back() noexcept {return *data();}
const_reference back() const noexcept {return *data();}
reference front() noexcept {never_call("toml::discard_comment::front");}
const_reference front() const noexcept {never_call("toml::discard_comment::front");}
reference back() noexcept {never_call("toml::discard_comment::back");}
const_reference back() const noexcept {never_call("toml::discard_comment::back");}
pointer data() noexcept {return nullptr;}
const_pointer data() const noexcept {return nullptr;}
@ -450,6 +450,18 @@ struct discard_comments
const_reverse_iterator rend() const noexcept {return const_iterator{};}
const_reverse_iterator crbegin() const noexcept {return const_iterator{};}
const_reverse_iterator crend() const noexcept {return const_iterator{};}
private:
[[noreturn]] static void never_call(const char *const this_function)
{
#ifdef __has_builtin
# if __has_builtin(__builtin_unreachable)
__builtin_unreachable();
# endif
#endif
throw std::logic_error{this_function};
}
};
inline bool operator==(const discard_comments&, const discard_comments&) noexcept {return true;}