Merge pull request #2407 from natalie-lang/fiber-asan

.gitmodules

@@ -10,6 +10,3 @@
 [submodule "ext/prism"]
 	path = ext/prism
 	url = https://github.com/ruby/prism
-[submodule "ext/minicoro"]
-	path = ext/minicoro
-	url = https://github.com/edubart/minicoro.git

Rakefile

@@ -665,7 +665,6 @@ def include_paths
   [
     File.expand_path('include', __dir__),
     File.expand_path('ext/tm/include', __dir__),
-    File.expand_path('ext/minicoro', __dir__),
     File.expand_path('build', __dir__),
     File.expand_path('build/onigmo/include', __dir__),
     File.expand_path('build/prism/include', __dir__),

include/minicoro/LICENSE

@@ -0,0 +1,47 @@
+This software is available as a choice of the following licenses. Choose
+whichever you prefer.
+
+===============================================================================
+ALTERNATIVE 1 - Public Domain (www.unlicense.org)
+===============================================================================
+This is free and unencumbered software released into the public domain.
+
+Anyone is free to copy, modify, publish, use, compile, sell, or distribute this
+software, either in source code form or as a compiled binary, for any purpose,
+commercial or non-commercial, and by any means.
+
+In jurisdictions that recognize copyright laws, the author or authors of this
+software dedicate any and all copyright interest in the software to the public
+domain. We make this dedication for the benefit of the public at large and to
+the detriment of our heirs and successors. We intend this dedication to be an
+overt act of relinquishment in perpetuity of all present and future rights to
+this software under copyright law.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+For more information, please refer to <http://unlicense.org/>
+
+===============================================================================
+ALTERNATIVE 2 - MIT No Attribution
+===============================================================================
+Copyright (c) 2021-2023 Eduardo Bart (https://github.com/edubart/minicoro)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of
+this software and associated documentation files (the "Software"), to deal in
+the Software without restriction, including without limitation the rights to
+use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
+of the Software, and to permit persons to whom the Software is furnished to do
+so.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

include/natalie/fiber_object.hpp

@@ -5,7 +5,7 @@
 #include <stdint.h>
 #include <sys/mman.h>
 
-#include "minicoro.h"
+#include "minicoro/minicoro.h"
 
 #include "natalie/array_object.hpp"
 #include "natalie/class_object.hpp"
@@ -78,12 +78,23 @@ class FiberObject : public Object {
     Value storage(Env *) const;
     void swap_to_previous(Env *env, size_t arg_size, Value *arg_data);
 
-    void *start_of_stack() { return m_start_of_stack; }
-
     mco_coro *coroutine() { return m_coroutine; }
     Block *block() { return m_block; }
+
+    // "bottom" -- high address
+    void *start_of_stack() { return m_start_of_stack; }
+    void set_start_of_stack(void *ptr) { m_start_of_stack = ptr; }
+
+    // "top" -- low address
+    void *end_of_stack() { return m_end_of_stack; }
     void set_end_of_stack(void *ptr) { m_end_of_stack = ptr; }
 
+    size_t stack_size() const {
+        assert(m_end_of_stack);
+        return (uintptr_t)m_start_of_stack - (uintptr_t)m_end_of_stack;
+    }
+    void set_stack_size(size_t size) { m_end_of_stack = (void *)((uintptr_t)m_start_of_stack - size); }
+
     void set_status(Status status) { m_status = status; }
     SymbolObject *status(Env *env) {
         switch (m_status) {
@@ -126,6 +137,13 @@ class FiberObject : public Object {
     HashObject *thread_storage() { return m_thread_storage; }
     HashObject *ensure_thread_storage();
 
+    FiberObject *previous_fiber() const { return m_previous_fiber; }
+
+#ifdef __SANITIZE_ADDRESS__
+    void *asan_fake_stack_start() const { return m_asan_fake_stack_start; }
+    void set_asan_fake_stack_start(void *start) { m_asan_fake_stack_start = start; }
+#endif
+
 private:
     friend Args;
     friend ThreadObject;
@@ -140,7 +158,8 @@ class FiberObject : public Object {
     ThreadObject *m_thread { nullptr };
     HashObject *m_thread_storage { nullptr };
 #ifdef __SANITIZE_ADDRESS__
-    void *m_asan_fake_stack { nullptr };
+    void *m_asan_fake_stack_start { nullptr };
+    size_t m_asan_fake_stack_size { 0 };
 #endif
     Status m_status { Status::Created };
     TM::Optional<TM::String> m_file {};

lib/natalie/compiler/backends/cpp_backend.rb

@@ -22,7 +22,6 @@ class CppBackend
       INC_PATHS = [
         File.join(ROOT_DIR, 'include'),
         File.join(ROOT_DIR, 'ext/tm/include'),
-        File.join(ROOT_DIR, 'ext/minicoro'),
         File.join(BUILD_DIR),
         File.join(BUILD_DIR, 'onigmo/include'),
       ].freeze

lib/natalie/compiler/flags.rb

@@ -26,6 +26,7 @@ module Flags
       ].freeze
 
       ASAN_FLAGS = DEBUG_FLAGS + %w[
+        -D NAT_GC_DISABLE=true
         -fsanitize=address
         -fno-omit-frame-pointer
       ]
@@ -40,7 +41,7 @@ module Flags
         -shared
         -rdynamic
         -Wl,-undefined,dynamic_lookup
-      ]
+      ].freeze
     end
   end
 end

src/fiber_object.cpp

@@ -2,10 +2,42 @@
 #include "natalie/thread_object.hpp"
 
 #define MINICORO_IMPL
-#include "minicoro.h"
+#include "minicoro/minicoro.h"
+
+#ifdef __SANITIZE_ADDRESS__
+extern "C" void __sanitizer_start_switch_fiber(void **fake_stack_save, const void *bottom, size_t size);
+extern "C" void __sanitizer_finish_switch_fiber(void *fake_stack_save, const void **bottom_old, size_t *size_old);
+#endif
 
 namespace Natalie {
 
+#ifdef __SANITIZE_ADDRESS__
+#define START_SWITCH_FIBER(from_fiber, to_fiber)                                                               \
+    {                                                                                                          \
+        void *fake_stack_start = nullptr;                                                                      \
+        __sanitizer_start_switch_fiber(&fake_stack_start, to_fiber->start_of_stack(), to_fiber->stack_size()); \
+        from_fiber->set_asan_fake_stack_start(fake_stack_start);                                               \
+    }
+
+#define START_SWITCH_FIBER_FINAL(to_fiber)                                                           \
+    {                                                                                                \
+        __sanitizer_start_switch_fiber(nullptr, to_fiber->start_of_stack(), to_fiber->stack_size()); \
+    }
+
+#define FINISH_SWITCH_FIBER(from_fiber, to_fiber)                                                                        \
+    {                                                                                                                    \
+        void *start_of_stack = nullptr;                                                                                  \
+        size_t stack_size = 0;                                                                                           \
+        __sanitizer_finish_switch_fiber(to_fiber->asan_fake_stack_start(), (const void **)&start_of_stack, &stack_size); \
+        from_fiber->set_start_of_stack(start_of_stack);                                                                  \
+        from_fiber->set_stack_size(stack_size);                                                                          \
+    }
+#else
+#define START_SWITCH_FIBER(from_fiber, to_fiber)
+#define START_SWITCH_FIBER_FINAL(to_fiber)
+#define FINISH_SWITCH_FIBER(from_fiber, to_fiber)
+#endif
+
 thread_local TM::Vector<Value> *tl_current_arg_stack = nullptr;
 
 FiberObject *FiberObject::initialize(Env *env, Value blocking, Value storage, Block *block) {
@@ -40,6 +72,7 @@ FiberObject *FiberObject::initialize(Env *env, Value blocking, Value storage, Bl
     mco_result res = mco_create(&m_coroutine, &desc);
     assert(res == MCO_SUCCESS);
     m_start_of_stack = (void *)((uintptr_t)m_coroutine->stack_base + m_coroutine->stack_size);
+    m_end_of_stack = (void *)((uintptr_t)m_start_of_stack - STACK_SIZE);
 
     return this;
 }
@@ -110,30 +143,31 @@ Value FiberObject::resume(Env *env, Args args) {
         env->raise("FiberError", "attempt to resume the current fiber");
 
     auto suspending_fiber = m_previous_fiber = current();
+
+    // RESUME START SWITCH
+    START_SWITCH_FIBER(suspending_fiber, this);
+
     {
         std::lock_guard<std::recursive_mutex> lock(g_gc_recursive_mutex);
         set_args(args.size(), args.data());
         ThreadObject::current()->m_current_fiber = this;
         tl_current_arg_stack = &m_args_stack;
         ThreadObject::current()->set_start_of_stack(m_start_of_stack);
-
-#ifdef __SANITIZE_ADDRESS__
-        auto fake_stack = __asan_get_current_fake_stack();
-        void *real_stack = __asan_addr_is_in_fake_stack(fake_stack, &args, nullptr, nullptr);
-        suspending_fiber->m_end_of_stack = real_stack ? real_stack : &args;
-        suspending_fiber->m_asan_fake_stack = __asan_get_current_fake_stack();
-#else
         suspending_fiber->m_end_of_stack = &args;
-#endif
     }
 
     auto res = mco_resume(m_coroutine);
     assert(res == MCO_SUCCESS);
 
+    auto new_current = FiberObject::current();
+
+    // YIELD FINISH SWITCH
+    FINISH_SWITCH_FIBER(this, new_current);
+
     if (m_error)
         env->raise_exception(m_error);
 
-    auto fiber_args = FiberObject::current()->args();
+    auto fiber_args = new_current->args();
     if (fiber_args.size() == 0) {
         return NilObject::the();
     } else if (fiber_args.size() == 1) {
@@ -204,7 +238,8 @@ Value FiberObject::storage(Env *env) const {
 
 Value FiberObject::yield(Env *env, Args args) {
     auto current_fiber = FiberObject::current();
-    if (!current_fiber->m_previous_fiber)
+    auto previous_fiber = current_fiber->m_previous_fiber;
+    if (!previous_fiber)
         env->raise("FiberError", "can't yield from root fiber");
 
     {
@@ -214,9 +249,17 @@ Value FiberObject::yield(Env *env, Args args) {
         current_fiber->swap_to_previous(env, args.size(), args.data());
     }
 
+    // YIELD START SWITCH
+    START_SWITCH_FIBER(current_fiber, previous_fiber);
+
     mco_yield(mco_running());
 
-    auto fiber_args = FiberObject::current()->args();
+    auto new_current = FiberObject::current();
+
+    // RESUME FINISH SWITCH
+    FINISH_SWITCH_FIBER(new_current->previous_fiber(), new_current);
+
+    auto fiber_args = new_current->args();
     if (fiber_args.size() == 0) {
         return NilObject::the();
     } else if (fiber_args.size() == 1) {
@@ -279,7 +322,7 @@ NO_SANITIZE_ADDRESS void FiberObject::visit_children_from_stack(Visitor &visitor
 NO_SANITIZE_ADDRESS void FiberObject::visit_children_from_asan_fake_stack(Visitor &visitor, Cell *potential_cell) const {
     void *begin = nullptr;
     void *end = nullptr;
-    void *real_stack = __asan_addr_is_in_fake_stack(m_asan_fake_stack, potential_cell, &begin, &end);
+    void *real_stack = __asan_addr_is_in_fake_stack(m_asan_fake_stack_start, potential_cell, &begin, &end);
 
     if (!real_stack) return;
 
@@ -314,6 +357,11 @@ void fiber_wrapper_func(mco_coro *co) {
     auto user_data = (coroutine_user_data *)co->user_data;
     auto env = user_data->env;
     auto fiber = user_data->fiber;
+    auto previous_fiber = fiber->previous_fiber();
+    (void)previous_fiber;
+
+    // RESUME FINISH SWITCH
+    FINISH_SWITCH_FIBER(previous_fiber, fiber);
 
     {
         std::lock_guard<std::recursive_mutex> lock(Natalie::g_gc_recursive_mutex);
@@ -350,5 +398,9 @@ void fiber_wrapper_func(mco_coro *co) {
     } else {
         fiber->swap_to_previous(env, 1, &return_arg);
     }
+
+    // YIELD START SWITCH (FINAL IMPLICIT YIELD)
+    //__sanitizer_start_switch_fiber(nullptr, previous_fiber->start_of_stack(), previous_fiber->stack_size());
+    START_SWITCH_FIBER_FINAL(previous_fiber);
 }
 }

src/integer.cpp

@@ -78,6 +78,8 @@ Integer::Integer(Integer &&other) {
 }
 
 Integer &Integer::operator=(const Integer &other) {
+    if (is_bignum() && m_bignum != other.m_bignum)
+        delete m_bignum;
     if (other.is_fixnum()) {
         m_fixnum = other.m_fixnum;
         m_is_fixnum = true;
@@ -90,6 +92,8 @@ Integer &Integer::operator=(const Integer &other) {
 
 Integer &Integer::operator=(Integer &&other) {
     if (&other == this) return *this;
+    if (is_bignum() && m_bignum != other.m_bignum)
+        delete m_bignum;
     if (other.is_fixnum()) {
         m_fixnum = other.m_fixnum;
         m_is_fixnum = true;