ASan fixes

Rakefile

@@ -103,7 +103,7 @@ task test_self_hosted_full: %i[bootstrap build_test_support] do
 end
 
 desc 'Test that some representative code runs with the AddressSanitizer enabled'
-task test_asan: [:build_sanitized, 'bin/nat'] do
+task test_asan: [:build_sanitized, :build_test_support, 'bin/nat'] do
   sh 'ruby test/asan_test.rb'
 end
 

include/natalie/random_object.hpp

@@ -49,6 +49,7 @@ class RandomObject : public Object {
         auto old_seed = default_random->seed();
         auto new_seed = IntegerObject::convert_to_native_type<nat_int_t>(env, seed);
         default_random->m_seed = new_seed;
+        delete default_random->m_generator;
         default_random->m_generator = new std::mt19937(new_seed);
         return old_seed;
     }

lib/socket.cpp

@@ -207,7 +207,7 @@ Value Addrinfo_getaddrinfo(Env *env, Value self, Args &&args, Block *block) {
     Defer freeinfo { [&res] { freeaddrinfo(res); } };
 
     auto output = new ArrayObject {};
-    for (addrinfo *rp = res; rp != nullptr; rp = rp->ai_next) {
+    for (struct addrinfo *rp = res; rp != nullptr; rp = rp->ai_next) {
         auto entry = self->send(env, "new"_s, { new StringObject { reinterpret_cast<const char *>(rp->ai_addr), rp->ai_addrlen } });
         if (rp->ai_canonname)
             entry->ivar_set(env, "@canonname"_s, new StringObject { rp->ai_canonname });
@@ -290,7 +290,7 @@ Value Addrinfo_initialize(Env *env, Value self, Args &&args, Block *block) {
             host = new StringObject { "0.0.0.0" };
 
         struct addrinfo hints { };
-        struct addrinfo *getaddrinfo_result;
+        struct addrinfo *getaddrinfo_result = nullptr;
 
         if (afamily)
             hints.ai_family = afamily;
@@ -342,9 +342,12 @@ Value Addrinfo_initialize(Env *env, Value self, Args &&args, Block *block) {
             service_str,
             &hints,
             &getaddrinfo_result);
+
         if (s != 0)
             env->raise("SocketError", "getaddrinfo: {}", gai_strerror(s));
 
+        Defer freeinfo { [&getaddrinfo_result] { freeaddrinfo(getaddrinfo_result); } };
+
         if (self->ivar_get(env, "@pfamily"_s)->is_nil())
             self->ivar_set(env, "@pfamily"_s, Value::integer(getaddrinfo_result->ai_family));
 
@@ -378,8 +381,6 @@ Value Addrinfo_initialize(Env *env, Value self, Args &&args, Block *block) {
         default:
             env->raise("SocketError", "getaddrinfo: unrecognized result");
         }
-
-        freeaddrinfo(getaddrinfo_result);
     }
 
     return self;
@@ -1191,14 +1192,15 @@ Value Socket_pack_sockaddr_in(Env *env, Value self, Args &&args, Block *block) {
     else
         service_str = service->to_str(env)->string();
 
-    struct addrinfo *addr;
+    struct addrinfo *addr = nullptr;
     auto result = getaddrinfo(host->as_string_or_raise(env)->c_str(), service_str.c_str(), &hints, &addr);
+
     if (result != 0)
         env->raise("SocketError", "getaddrinfo: {}", gai_strerror(result));
 
-    auto packed = new StringObject { reinterpret_cast<const char *>(addr->ai_addr), addr->ai_addrlen, Encoding::ASCII_8BIT };
+    Defer freeinfo { [&addr] { freeaddrinfo(addr); } };
 
-    freeaddrinfo(addr);
+    auto packed = new StringObject { reinterpret_cast<const char *>(addr->ai_addr), addr->ai_addrlen, Encoding::ASCII_8BIT };
 
     return packed;
 }
@@ -1309,7 +1311,7 @@ static String Socket_family_to_string(int family) {
     }
 }
 
-static int Socket_getaddrinfo_result_port(addrinfo *result) {
+static int Socket_getaddrinfo_result_port(struct addrinfo *result) {
     switch (result->ai_family) {
     case AF_INET: {
         auto sockaddr = reinterpret_cast<sockaddr_in *>(result->ai_addr);
@@ -1384,15 +1386,18 @@ Value Socket_s_getaddrinfo(Env *env, Value self, Args &&args, Block *) {
     else
         service = servname->as_string_or_raise(env)->string();
 
-    struct addrinfo *result;
+    struct addrinfo *result = nullptr;
     int s = getaddrinfo(
         host.is_empty() ? nullptr : host.c_str(),
         service.c_str(),
         &hints,
         &result);
+
     if (s != 0)
         env->raise("SocketError", "getaddrinfo: {}", gai_strerror(s));
 
+    Defer freeinfo([&result] { freeaddrinfo(result); });
+
     auto ary = new ArrayObject;
 
     do {
@@ -1411,8 +1416,6 @@ Value Socket_s_getaddrinfo(Env *env, Value self, Args &&args, Block *) {
         result = result->ai_next;
     } while (result);
 
-    freeaddrinfo(result);
-
     return ary;
 }
 
@@ -1573,7 +1576,7 @@ Value TCPSocket_initialize(Env *env, Value self, Args &&args, Block *block) {
 
     auto domain = AF_INET;
     if (host->is_string() && !host->as_string()->is_empty()) {
-        addrinfo *info;
+        struct addrinfo *info = nullptr;
         const auto result = getaddrinfo(host->as_string()->c_str(), nullptr, nullptr, &info);
         if (result != 0) {
             if (result == EAI_SYSTEM)
@@ -1629,7 +1632,7 @@ Value TCPServer_initialize(Env *env, Value self, Args &&args, Block *block) {
 
     auto domain = AF_INET;
     if (hostname->is_string() && !hostname->as_string()->is_empty()) {
-        addrinfo *info;
+        struct addrinfo *info = nullptr;
         const auto result = getaddrinfo(hostname->as_string()->c_str(), nullptr, nullptr, &info);
         if (result != 0) {
             if (result == EAI_SYSTEM)

src/kernel_module.cpp

@@ -516,6 +516,12 @@ Value KernelModule::spawn(Env *env, Args &&args) {
 
     Vector<char *> new_env;
 
+    Defer free_new_env([&]() {
+        for (auto str : new_env) {
+            free(str);
+        }
+    });
+
     if (args.size() >= 1 && (args.at(0)->is_hash() || args.at(0)->respond_to(env, "to_hash"_s))) {
         auto hash = args.shift()->to_hash(env);
         for (auto ep = environ; *ep; ep++)
@@ -532,12 +538,6 @@ Value KernelModule::spawn(Env *env, Args &&args) {
 
     args.ensure_argc_at_least(env, 1);
 
-    Defer free_new_env([&]() {
-        for (auto str : new_env) {
-            free(str);
-        }
-    });
-
     if (args.size() == 1) {
         auto arg = args.at(0)->to_str(env);
         auto splitter = new RegexpObject { env, "\\s+" };

src/string_object.cpp

@@ -3731,7 +3731,13 @@ Value StringObject::convert_float() {
         if (p == -1)
             p = m_string.find((char)toupper(delimiter));
 
-        if (p != -1 && (m_string[p - 1] == '_' || m_string[p + 1] == '_'))
+        if (p == -1)
+            return true;
+
+        if (p > 0 && m_string[p - 1] == '_')
+            return false;
+
+        if (p < (ssize_t)m_string.length() && m_string[p + 1] == '_')
             return false;
 
         return true;

test/asan_test.rb

@@ -23,7 +23,16 @@
           Dir[
             'spec/language/*_spec.rb',
             'test/natalie/**/*_test.rb',
+            # fixed:
+            'spec/core/kernel/Float_spec.rb',
+            'spec/core/kernel/srand_spec.rb',
+            'spec/core/process/spawn_spec.rb',
+            'spec/core/random/new_seed_spec.rb',
+            'spec/core/random/srand_spec.rb',
             'spec/core/string/crypt_spec.rb',
+            'spec/library/yaml/dump_spec.rb',
+            'spec/library/yaml/load_spec.rb',
+            'spec/library/yaml/to_yaml_spec.rb',
           ].to_a
         else
           # runs nightly -- all tests
@@ -34,37 +43,36 @@
         end
 
 TESTS_TO_SKIP = [
-  'test/natalie/libnat_test.rb', # too slow, times out frequently
-  'test/natalie/thread_test.rb', # calls GC.start, but we're not ready for that
-  'test/natalie/gc_test.rb', # calls GC.enable, but we're not ready for that
-  'spec/library/socket/basicsocket/do_not_reverse_lookup_spec.rb', # getaddrinfo leak
-  'spec/library/socket/ipsocket/getaddress_spec.rb', # getaddrinfo leak
-  'spec/library/socket/socket/getaddrinfo_spec.rb', # getaddrinfo leak
-  'spec/library/socket/tcpsocket/initialize_spec.rb', # getaddrinfo leak
-  'spec/library/socket/tcpserver/new_spec.rb', # getaddrinfo leak
-  'spec/library/socket/tcpserver/sysaccept_spec.rb', # getaddrinfo leak
-  'spec/library/socket/tcpsocket/setsockopt_spec.rb', # getaddrinfo leak
-  'spec/library/socket/ipsocket/peeraddr_spec.rb', # getaddrinfo leak
-  'spec/library/socket/tcpsocket/recv_nonblock_spec.rb', # getaddrinfo leak
-  'spec/library/socket/udpsocket/bind_spec.rb', # getaddrinfo leak
-  'spec/library/socket/tcpsocket/open_spec.rb', # getaddrinfo leak
-  'spec/library/socket/udpsocket/write_spec.rb', # getaddrinfo leak
-  'spec/library/socket/ipsocket/recvfrom_spec.rb', # getaddrinfo leak
-  'spec/library/socket/ipsocket/addr_spec.rb', # getaddrinfo leak
-  'spec/library/yaml/unsafe_load_spec.rb', # heap buffer overflow in Natalie::StringObject::convert_float()
-  'spec/library/yaml/load_spec.rb', # heap buffer overflow in Natalie::StringObject::convert_float()
-  'spec/library/yaml/dump_spec.rb', # heap buffer overflow in Natalie::StringObject::convert_float()
-  'spec/library/yaml/to_yaml_spec.rb', # heap buffer overflow in Natalie::StringObject::convert_float()
-  'spec/core/process/spawn_spec.rb', # leak in KernelModule::spawn
-  'spec/core/process/fork_spec.rb', # spec timeout
-  'spec/core/kernel/fork_spec.rb', # spec timeout
-  'spec/core/kernel/Float_spec.rb', # heap buffer overflow in Natalie::StringObject::convert_float()
-  'spec/core/kernel/srand_spec.rb', # leak in Natalie::RandomObject::srand
-  'spec/core/random/new_seed_spec.rb', # leak in Natalie::RandomObject::srand
-  'spec/core/random/srand_spec.rb', # leak in Natalie::RandomObject::srand
-  'spec/core/process/uid_spec.rb', # not sure why this breaks
-  'spec/core/process/euid_spec.rb', # not sure why this breaks
-  'spec/core/process/egid_spec.rb', # not sure why this breaks
+  # calls GC.start/GC.enable, but we're not ready for that
+  'test/natalie/thread_test.rb',
+  'test/natalie/gc_test.rb',
+
+  # getaddrinfo "leak"
+  # https://bugs.kde.org/show_bug.cgi?id=448991
+  # https://bugzilla.redhat.com/show_bug.cgi?id=859717
+  # My understanding is that it is a single object that is internal to glibc and never freed.
+  'spec/library/socket/ipsocket/getaddress_spec.rb',
+  'spec/library/socket/socket/getaddrinfo_spec.rb',
+  'spec/library/socket/tcpsocket/initialize_spec.rb',
+  'spec/library/socket/tcpserver/new_spec.rb',
+  'spec/library/socket/tcpserver/sysaccept_spec.rb',
+  'spec/library/socket/tcpsocket/setsockopt_spec.rb',
+  'spec/library/socket/ipsocket/peeraddr_spec.rb',
+  'spec/library/socket/tcpsocket/recv_nonblock_spec.rb',
+  'spec/library/socket/udpsocket/bind_spec.rb',
+  'spec/library/socket/tcpsocket/open_spec.rb',
+  'spec/library/socket/udpsocket/write_spec.rb',
+  'spec/library/socket/ipsocket/recvfrom_spec.rb',
+  'spec/library/socket/ipsocket/addr_spec.rb',
+
+  # spec timeout, hangs on waitpid
+  'spec/core/process/fork_spec.rb',
+  'spec/core/kernel/fork_spec.rb',
+
+   # some issue to do with ptrace + Docker privileges
+  'spec/core/process/uid_spec.rb',
+  'spec/core/process/euid_spec.rb',
+  'spec/core/process/egid_spec.rb',
 ].freeze
 
 describe 'Sanitizers tests' do