Merge remote-tracking branch 'topic/bbannier/coverity'

CHANGES

@@ -1,3 +1,39 @@
+1.13.0-dev.64 | 2025-01-10 09:55:22 +0100
+
+  * Address a FIXME. (Benjamin Bannier, Corelight)
+
+  * Prevent exception in generic `noexcept` function. (Benjamin Bannier, Corelight)
+
+  * Avoid theoretical exception in widely used helper function. (Benjamin Bannier, Corelight)
+
+  * Prevent destructor from throwing. (Benjamin Bannier, Corelight)
+
+  * Avoid throwing never handled exception in low-level helper function. (Benjamin Bannier, Corelight)
+
+  * Prevent exception escape. (Benjamin Bannier, Corelight)
+
+  * Drop dereference of end iterator. (Benjamin Bannier, Corelight)
+
+  * Prevent potential nullptr dereference. (Benjamin Bannier, Corelight)
+
+  * Prevent copies in various places. (Benjamin Bannier, Corelight)
+
+    This patch removes copies in different places by e.g., leveraging const
+    lifetime expansion or move semantics. We also clean up getters to return
+    new values less often.
+
+  * Prevent division by zero which is UB. (Benjamin Bannier, Corelight)
+
+  * Clearly mark up unreachable code. (Benjamin Bannier, Corelight)
+
+  * Add missing `break` in `switch` case. (Benjamin Bannier, Corelight)
+
+  * Drop redundant string copy. (Benjamin Bannier, Corelight)
+
+  * Remove copyright end year in source files [skip CI]. (Benjamin Bannier, Corelight)
+
+  * Remove copyright years [skip CI]. (Evan Typanski, Corelight)
+
 1.13.0-dev.47 | 2025-01-08 17:48:23 +0100
 
   * Fix update Fedora links in installation docs. (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-1.13.0-dev.47
+1.13.0-dev.64

hilti/runtime/include/types/reference.h

@@ -261,13 +261,13 @@ class ValueReference {
      */
     ValueReference& operator=(ValueReference&& other) noexcept {
         if ( &other != this ) {
-            // We can't move the actual value as other references may be
-            // referring to it.
-            *_get() = *other._get();
-
-            // Some implementations for `std::variant` do not have a `noexpect`
-            // move assignment operator.
+            // Not all types wrapped in a `ValueReference` might have a
+            // `noexcept` (move) assignment operator.
             try {
+                // We can't move the actual value as other references may be
+                // referring to it.
+                *_get() = *other._get();
+
                 other._ptr = nullptr;
             } catch ( ... ) {
                 cannot_be_reached();

hilti/runtime/src/debug-logger.cc

@@ -1,5 +1,7 @@
 // Copyright (c) 2020-now by the Zeek Project. See LICENSE for details.
 
+#include <type_traits>
+
 #include <hilti/rt/logging.h>
 #include <hilti/rt/util.h>
 
@@ -42,7 +44,12 @@ void detail::DebugLogger::print(std::string_view stream, std::string_view msg) {
         }
     }
 
-    auto indent = std::string(i->second * 2, ' ');
+    // Theoretically the ident computation could overflow, but in that case we
+    // would have already run into trouble elsewhere (e.g., giant strings from
+    // huge ident widths). Instead perform the computation with overflow which
+    // for unsigned integers is defined and wraps around.
+    static_assert(std::is_unsigned_v<std::remove_reference_t<decltype(i->second.Ref())>>);
+    auto indent = std::string(i->second.Ref() * 2, ' ');
     (*_output) << fmt("[%s] %s%s", stream, indent, msg) << '\n';
     _output->flush();
 }

hilti/runtime/src/main.cc

@@ -5,6 +5,7 @@
 
 #include <getopt.h>
 
+#include <exception>
 #include <iostream>
 
 #include <hilti/rt/libhilti.h>
@@ -51,4 +52,7 @@ int HILTI_WEAK hilti::main(int argc, char** argv) {
     return 0;
 }
 
-int HILTI_WEAK main(int argc, char** argv) { return hilti::main(argc, argv); }
+int HILTI_WEAK main(int argc, char** argv) try { return hilti::main(argc, argv); } catch ( const std::exception& e ) {
+    hilti::rt::fatalError(hilti::rt::fmt("terminating with uncaught exception of type %s: %s",
+                                         hilti::rt::demangle(typeid(e).name()), e.what()));
+}

hilti/runtime/src/types/regexp.cc

@@ -265,7 +265,7 @@ void regexp::detail::CompiledRegExp::_compileOne(std::string pattern, int idx) {
 }
 
 RegExp::RegExp(const std::vector<std::string>& patterns, regexp::Flags flags) {
-    auto key = (patterns.empty() ? std::string() : join(patterns, "|") + "|" + flags.cacheKey());
+    const auto& key = (patterns.empty() ? std::string() : join(patterns, "|") + "|" + flags.cacheKey());
     auto& ptr = detail::globalState()->regexp_cache[key];
 
     if ( ! ptr )

hilti/runtime/src/types/stream.cc

@@ -251,7 +251,7 @@ View View::advanceToNextData() const {
 
     // If the position is already not in a gap we can directly compute a view at it.
     if ( c && ! c->isGap() )
-        return View(i, _end);
+        return View(std::move(i), _end);
 
     std::optional<Offset> last_end; // Offset of the end of the last seen chunk.
 

hilti/runtime/src/util.cc

@@ -433,12 +433,13 @@ std::string hilti::rt::strftime(const std::string& format, const hilti::rt::Time
 
 hilti::rt::Time hilti::rt::strptime(const std::string& buf, const std::string& format) {
     tm time;
-    auto end = ::strptime(buf.c_str(), format.c_str(), &time);
+    const char* end = ::strptime(buf.data(), format.c_str(), &time);
 
     if ( ! end )
         throw InvalidArgument("could not parse time string");
 
-    if ( end != &*buf.end() )
+    auto consumed = std::distance(buf.data(), end);
+    if ( static_cast<decltype(buf.size())>(consumed) != buf.size() )
         throw InvalidArgument(hilti::rt::fmt("unparsed remainder after parsing time string: %s", end));
 
     // If the struct tm object was obtained from POSIX strptime or equivalent

hilti/toolchain/bin/hilti-config.cc

@@ -2,11 +2,13 @@
 //
 // Outputs paths and flags for using HILTI.
 
+#include <exception>
 #include <iostream>
 #include <list>
 #include <string>
 
 #include <hilti/autogen/config.h>
+#include <hilti/base/logger.h>
 #include <hilti/base/util.h>
 
 using namespace std;
@@ -48,8 +50,7 @@ static void join(std::vector<U>& a, const std::vector<V>& b) {
     a.insert(a.end(), b.begin(), b.end());
 }
 
-// NOLINTNEXTLINE(bugprone-exception-escape)
-int main(int argc, char** argv) {
+int main(int argc, char** argv) try {
     bool want_debug = false;
     bool want_dynamic_linking = false;
 
@@ -223,4 +224,7 @@ int main(int argc, char** argv) {
     cout << hilti::util::join(result.begin(), result.end(), " ") << '\n';
 
     return 0;
+} catch ( const std::exception& e ) {
+    hilti::logger().fatalError(hilti::util::fmt("terminating with uncaught exception of type %s: %s",
+                                                hilti::util::demangle(typeid(e).name()), e.what()));
 }

hilti/toolchain/bin/hiltic.cc

@@ -1,9 +1,11 @@
 // Copyright (c) 2020-now by the Zeek Project. See LICENSE for details.
 
+#include <exception>
+
 #include <hilti/compiler/init.h>
 #include <hilti/hilti.h>
 
-int main(int argc, char** argv) {
+int main(int argc, char** argv) try {
     hilti::init();
     hilti::Driver driver("hiltic", hilti::util::currentExecutable());
 
@@ -22,4 +24,7 @@ int main(int argc, char** argv) {
     }
 
     return 0;
+} catch ( const std::exception& e ) {
+    hilti::logger().fatalError(hilti::util::fmt("terminating with uncaught exception of type %s: %s",
+                                                hilti::util::demangle(typeid(e).name()), e.what()));
 }

hilti/toolchain/include/ast/types/name.h

@@ -51,7 +51,7 @@ class Name : public UnqualifiedType {
     node::Properties properties() const final {
         auto p =
             node::Properties{{"id", _id}, {"builtin", _builtin}, {"resolved-type", to_string(_resolved_type_index)}};
-        return UnqualifiedType::properties() + p;
+        return UnqualifiedType::properties() + std::move(p);
     }
 
     static auto create(ASTContext* ctx, const ID& id, Meta meta = {}) {

hilti/toolchain/include/base/util.h

@@ -607,7 +607,7 @@ constexpr auto to_string(Enum value, const Value<Enum> (&values)[Size]) {
         if ( v.value == value )
             return v.name;
 
-    throw std::out_of_range(std::to_string(static_cast<int>(value)));
+    rt::internalError(fmt("enum value '%s' out of range", static_cast<int>(value)));
 };
 
 } // namespace enum_

hilti/toolchain/src/ast/ast-context.cc

@@ -439,8 +439,9 @@ void ASTContext::replace(Declaration* old, Declaration* new_) {
     _declarations_by_index[index.value()] = new_;
     new_->setDeclarationIndex(index);
 
-    if ( auto n = new_->tryAs<declaration::Type>() ) {
-        auto o = old->tryAs<declaration::Type>();
+    auto* n = new_->tryAs<declaration::Type>();
+    auto* o = old->tryAs<declaration::Type>();
+    if ( n && o ) {
         n->type()->type()->setDeclarationIndex(index);
         replace(o->type()->type(), n->type()->type());
     }
@@ -869,7 +870,7 @@ void ASTContext::_dumpState(const logging::DebugStream& stream) {
         auto n = _types_by_index[idx];
         assert(n->isRetained());
 
-        auto id = n->typeID() ? n->typeID() : ID("<no-type-id>");
+        const auto& id = n->typeID() ? n->typeID() : ID("<no-type-id>");
         HILTI_DEBUG(stream,
                     fmt("[%s] %s [%s] (%s)", ast::TypeIndex(idx), id, n->typename_(), n->location().dump(true)));
     }
@@ -918,7 +919,7 @@ void ASTContext::_dumpStats(const logging::DebugStream& stream, std::string_view
 
     logger().debugPushIndent(stream);
     for ( const auto& [type, num] : live_by_type ) {
-        if ( static_cast<double>(num) / static_cast<double>(live) > 0.01 )
+        if ( live != 0 && static_cast<double>(num) / static_cast<double>(live) > 0.01 )
             HILTI_DEBUG(stream, fmt("- %s: %" PRIu64, type, num));
     }
     logger().debugPopIndent(stream);

hilti/toolchain/src/ast/declarations/field.cc

@@ -10,7 +10,7 @@ using namespace hilti::declaration;
 node::Properties declaration::Field::properties() const {
     auto p =
         node::Properties{{"cc", _cc ? to_string(*_cc) : "<unset>"}, {"linked-type", to_string(_linked_type_index)}};
-    return Declaration::properties() + p;
+    return Declaration::properties() + std::move(p);
 }
 
 std::string declaration::Field::_dump() const {

hilti/toolchain/src/ast/node.cc

@@ -64,9 +64,10 @@ std::string Node::renderSelf(bool include_location) const {
     if ( ! props.empty() )
         sprops = util::fmt(" <%s>", util::join(props, " "));
 
-    auto location = (include_location && meta().location()) ? util::fmt(" (%s)", meta().location().dump(true)) : "";
+    const auto& location =
+        (include_location && meta().location()) ? util::fmt(" (%s)", meta().location().dump(true)) : "";
     auto no_inherit_scope = (inheritScope() ? "" : " (no-inherit-scope)");
-    auto parent = (_parent ? util::fmt(" [parent %s]", identity(_parent)) : " [no parent]");
+    const auto& parent = (_parent ? util::fmt(" [parent %s]", identity(_parent)) : " [no parent]");
 
     auto s = util::fmt("%s%s%s%s%s", name(this), sprops, parent, no_inherit_scope, location);
 

hilti/toolchain/src/ast/operator-registry.cc

@@ -36,7 +36,7 @@ void Registry::initPending(Builder* builder) {
         if ( op->hasOperands() ) { // only register if to be instantiated by the resolver through its operands
             _operators_by_kind[op->kind()].push_back(op.get());
             if ( op->kind() == Kind::MemberCall ) {
-                auto id = op->signature().operands->op1()->type()->type()->as<type::Member>()->id();
+                const auto& id = op->signature().operands->op1()->type()->type()->as<type::Member>()->id();
                 _operators_by_method[id].push_back(op.get());
             }
 

hilti/toolchain/src/ast/operators/bitfield.cc

@@ -28,7 +28,7 @@ QualifiedType* _itemType(Builder* builder, const Expressions& operands) {
 }
 
 void _checkName(expression::ResolvedOperator* op) {
-    auto id = op->op1()->as<expression::Member>()->id();
+    const auto& id = op->op1()->as<expression::Member>()->id();
     if ( ! op->op0()->type()->type()->as<type::Bitfield>()->bits(id) )
         op->addError(util::fmt("bitfield type does not have attribute '%s'", id));
 }

hilti/toolchain/src/ast/operators/generic.cc

@@ -2,7 +2,6 @@
 
 #include <string>
 #include <utility>
-#include <vector>
 
 #include <hilti/ast/builder/builder.h>
 #include <hilti/ast/ctors/bool.h>
@@ -202,7 +201,8 @@ class Unpack : public Operator {
         else if ( data_type->isA<type::Bitfield>() ) {
             if ( args.size() >= 2 && args.size() <= 3 ) {
                 auto arg1 = args[1]->type()->type()->cxxID();
-                auto arg2 = (args.size() > 2 ? args[2]->type()->type()->cxxID() : ID("::hilti::rt::integer::BitOrder"));
+                const auto& arg2 =
+                    (args.size() > 2 ? args[2]->type()->type()->cxxID() : ID("::hilti::rt::integer::BitOrder"));
                 if ( arg1 && arg1 == ID("::hilti::rt::ByteOrder") && arg2 &&
                      arg2 == ID("::hilti::rt::integer::BitOrder") )
                     return;

hilti/toolchain/src/ast/operators/struct.cc

@@ -1,7 +1,6 @@
 // Copyright (c) 2020-now by the Zeek Project. See LICENSE for details.
 
 #include <string>
-#include <vector>
 
 #include <hilti/ast/builder/builder.h>
 #include <hilti/ast/expressions/member.h>
@@ -59,7 +58,7 @@ QualifiedType* _itemType(Builder* builder, const Expressions& operands) {
 }
 
 void _checkName(expression::ResolvedOperator* op, bool check_optional = false) {
-    auto id = op->op1()->as<expression::Member>()->id();
+    const auto& id = op->op1()->as<expression::Member>()->id();
     auto t = op->op0()->type()->type();
 
     if ( auto x = t->tryAs<type::ValueReference>() )

hilti/toolchain/src/ast/operators/tuple.cc

@@ -1,6 +1,5 @@
 // Copyright (c) 2020-now by the Zeek Project. See LICENSE for details.
 
-#include <memory>
 #include <string>
 #include <vector>
 
@@ -119,7 +118,7 @@ class Member : public Operator {
     }
 
     QualifiedType* result(Builder* builder, const Expressions& operands, const Meta& meta) const final {
-        auto id = operands[1]->as<expression::Member>()->id();
+        const auto& id = operands[1]->as<expression::Member>()->id();
         auto tt = operands[0]->type()->type()->tryAs<type::Tuple>();
         if ( ! tt )
             return builder->qualifiedType(builder->typeUnknown(), Constness::Const);
@@ -132,7 +131,7 @@ class Member : public Operator {
     }
 
     void validate(expression::ResolvedOperator* n) const final {
-        auto id = n->op1()->as<expression::Member>()->id();
+        const auto& id = n->op1()->as<expression::Member>()->id();
         auto tt = n->op0()->type()->type()->tryAs<type::Tuple>();
         if ( ! tt ) {
             n->addError("unknown tuple element");

hilti/toolchain/src/ast/operators/union.cc

@@ -1,7 +1,5 @@
 // Copyright (c) 2020-now by the Zeek Project. See LICENSE for details.
 
-#include <string>
-#include <utility>
 #include <vector>
 
 #include <hilti/ast/builder/builder.h>
@@ -25,7 +23,7 @@ QualifiedType* itemType(Builder* builder, const Expressions& operands) {
 }
 
 void checkName(expression::ResolvedOperator* op) {
-    auto id = op->op1()->as<expression::Member>()->id();
+    const auto& id = op->op1()->as<expression::Member>()->id();
     if ( ! op->op0()->type()->type()->as<type::Union>()->field(id) )
         op->addError(util::fmt("type does not have field '%s'", id));
 }

hilti/toolchain/src/ast/scope.cc

@@ -84,7 +84,7 @@ std::vector<Scope::Referee> Scope::_findID(const Scope* scope, const ID& id, boo
 
         std::string nt;
         std::tie(h, nt) = util::rsplit1(h, "::");
-        t = (! t.empty() && ! nt.empty() && t != "$ $" ? util::fmt("%s::%s", nt, t) : nt);
+        t = (! t.empty() && ! nt.empty() && t != "$ $" ? util::fmt("%s::%s", nt, t) : std::move(nt));
     }
 }
 

hilti/toolchain/src/base/util.cc

@@ -45,9 +45,7 @@ std::vector<std::string> util::split(std::string s, const std::string& delim) {
 
         l.push_back(s.substr(0, p));
 
-        // FIXME: Don't understand why directly assigning to s doesn't work.
-        std::string t = s.substr(p + delim.size(), std::string::npos);
-        s = t;
+        s = s.substr(p + delim.size(), std::string::npos);
     }
 
     l.push_back(s);