runtime: remove usage of weak_ptr in attach manager

runtime/src/attach/attach_manager/frida_attach_manager.cpp

@@ -76,7 +76,7 @@ int frida_attach_manager::attach_at(void *func_addr, callback_variant &&cb)
 		// Create a frida attach entry
 		itr = internal_attaches
 			      .emplace(func_addr,
-				       std::make_shared<
+				       std::make_unique<
 					       frida_internal_attach_entry>(
 					       func_addr,
 					       (attach_type)cb.index(),
@@ -97,11 +97,12 @@ int frida_attach_manager::attach_at(void *func_addr, callback_variant &&cb)
 	auto inserted_attach_entry =
 		this->attaches
 			.emplace(ent.self_id,
-				 std::make_shared<frida_attach_entry>(
+				 std::make_unique<frida_attach_entry>(
 					 std::move(ent)))
 			.first;
-	inner_attach->user_attaches.push_back(inserted_attach_entry->second);
-	inserted_attach_entry->second->internal_attaches = inner_attach;
+	inner_attach->user_attaches.push_back(
+		inserted_attach_entry->second.get());
+	inserted_attach_entry->second->internal_attach = inner_attach.get();
 	return result;
 }
 
@@ -140,20 +141,19 @@ int frida_attach_manager::destroy_attach(int id)
 {
 	void *drop_func_addr = nullptr;
 	if (auto itr = attaches.find(id); itr != attaches.end()) {
-		if (auto p = itr->second->internal_attaches.lock(); p) {
-			auto &user_attaches = p->user_attaches;
-			auto tail =
-				std::remove_if(user_attaches.begin(),
-					       user_attaches.end(),
-					       [&](const auto &v) -> bool {
-						       return v.lock().get() ==
-							      itr->second.get();
-					       });
-			user_attaches.resize(tail - user_attaches.begin());
-			attaches.erase(itr);
-			if (p->user_attaches.empty()) {
-				drop_func_addr = p->function;
-			}
+		auto p = itr->second->internal_attach;
+
+		auto &user_attaches = p->user_attaches;
+		auto tail =
+			std::remove_if(user_attaches.begin(),
+				       user_attaches.end(),
+				       [&](const auto &v) -> bool {
+					       return v == itr->second.get();
+				       });
+		user_attaches.resize(tail - user_attaches.begin());
+		attaches.erase(itr);
+		if (p->user_attaches.empty()) {
+			drop_func_addr = p->function;
 		}
 	} else {
 		spdlog::error("Unable to find attach id {}", id);
@@ -184,10 +184,8 @@ int frida_attach_manager::destroy_attach_by_func_addr(const void *func)
 	if (auto itr = internal_attaches.find((void *)func);
 	    itr != internal_attaches.end()) {
 		auto uattaches = itr->second->user_attaches;
-		for (auto p : uattaches) {
-			if (auto attach_entry = p.lock(); attach_entry) {
-				attaches.erase(attach_entry->self_id);
-			}
+		for (auto attach_entry : uattaches) {
+			attaches.erase(attach_entry->self_id);
 		}
 		internal_attaches.erase(itr);
 		return 0;
@@ -255,8 +253,8 @@ frida_internal_attach_entry::frida_internal_attach_entry(
 			    this, nullptr);
 		    err < 0) {
 			spdlog::error(
-				"Failed to execute frida replace for function {:x}, when attaching filter",
-				(uintptr_t)function);
+				"Failed to execute frida replace for function {:x}, when attaching filter, err={}",
+				(uintptr_t)function, err);
 			throw std::runtime_error("Failed to attach filter");
 		}
 		override_return_callback = override_return_set_callback(
@@ -272,8 +270,8 @@ frida_internal_attach_entry::frida_internal_attach_entry(
 			    this, nullptr);
 		    err < 0) {
 			spdlog::error(
-				"Failed to execute frida replace for function {:x}, when attaching replace",
-				(uintptr_t)function);
+				"Failed to execute frida replace for function {:x}, when attaching replace, err={}",
+				(uintptr_t)function, err);
 			throw std::runtime_error("Failed to attach replace");
 		}
 	}
@@ -297,25 +295,21 @@ frida_internal_attach_entry::~frida_internal_attach_entry()
 
 bool frida_internal_attach_entry::has_replace_or_override() const
 {
-	for (auto p : user_attaches) {
-		if (auto v = p.lock(); v) {
-			if (v->get_type() == attach_type::REPLACE ||
-			    v->get_type() == attach_type::UPROBE_OVERRIDE) {
-				return true;
-			}
+	for (auto v : user_attaches) {
+		if (v->get_type() == attach_type::REPLACE ||
+		    v->get_type() == attach_type::UPROBE_OVERRIDE) {
+			return true;
 		}
 	}
 	return false;
 }
 
 bool frida_internal_attach_entry::has_uprobe_or_uretprobe() const
 {
-	for (auto p : user_attaches) {
-		if (auto v = p.lock(); v) {
-			if (v->get_type() == attach_type::UPROBE ||
-			    v->get_type() == attach_type::URETPROBE) {
-				return true;
-			}
+	for (auto v : user_attaches) {
+		if (v->get_type() == attach_type::UPROBE ||
+		    v->get_type() == attach_type::URETPROBE) {
+			return true;
 		}
 	}
 	return false;
@@ -324,13 +318,10 @@ bool frida_internal_attach_entry::has_uprobe_or_uretprobe() const
 base_attach_manager::replace_callback &
 frida_internal_attach_entry::get_replace_callback() const
 {
-	for (auto p : user_attaches) {
-		if (auto v = p.lock(); v) {
-			if (v->get_type() == attach_type::REPLACE) {
-				return std::get<
-					base_attach_manager::replace_callback>(
-					v->cb);
-			}
+	for (auto v : user_attaches) {
+		if (v->get_type() == attach_type::REPLACE) {
+			return std::get<base_attach_manager::replace_callback>(
+				v->cb);
 		}
 	}
 	spdlog::error(
@@ -342,13 +333,10 @@ frida_internal_attach_entry::get_replace_callback() const
 base_attach_manager::filter_callback &
 frida_internal_attach_entry::get_filter_callback() const
 {
-	for (auto p : user_attaches) {
-		if (auto v = p.lock(); v) {
-			if (v->get_type() == attach_type::UPROBE_OVERRIDE) {
-				return std::get<
-					base_attach_manager::filter_callback>(
-					v->cb);
-			}
+	for (auto v : user_attaches) {
+		if (v->get_type() == attach_type::UPROBE_OVERRIDE) {
+			return std::get<base_attach_manager::filter_callback>(
+				v->cb);
 		}
 	}
 	spdlog::error(
@@ -360,24 +348,19 @@ frida_internal_attach_entry::get_filter_callback() const
 void frida_internal_attach_entry::iterate_uprobe_callbacks(
 	const pt_regs &regs) const
 {
-	for (auto p : user_attaches) {
-		if (auto v = p.lock(); v) {
-			if (v->get_type() == attach_type::UPROBE) {
-				std::get<(int)attach_type::UPROBE>(v->cb)(regs);
-			}
+	for (auto v : user_attaches) {
+		if (v->get_type() == attach_type::UPROBE) {
+			std::get<(int)attach_type::UPROBE>(v->cb)(regs);
 		}
 	}
 }
 
 void frida_internal_attach_entry::iterate_uretprobe_callbacks(
 	const pt_regs &regs) const
 {
-	for (auto p : user_attaches) {
-		if (auto v = p.lock(); v) {
-			if (v->get_type() == attach_type::URETPROBE) {
-				std::get<(int)attach_type::URETPROBE>(v->cb)(
-					regs);
-			}
+	for (auto v : user_attaches) {
+		if (v->get_type() == attach_type::URETPROBE) {
+			std::get<(int)attach_type::URETPROBE>(v->cb)(regs);
 		}
 	}
 }

runtime/src/attach/attach_manager/frida_attach_manager.hpp

@@ -22,7 +22,7 @@ class frida_attach_entry {
 	base_attach_manager::callback_variant cb;
 	void *function;
 
-	std::weak_ptr<class frida_internal_attach_entry> internal_attaches;
+	class frida_internal_attach_entry *internal_attach;
 	friend class frida_attach_manager;
 	friend class frida_internal_attach_entry;
 
@@ -44,7 +44,7 @@ class frida_attach_entry {
 class frida_internal_attach_entry {
 	void *function;
 	GumInterceptor *interceptor;
-	std::vector<std::weak_ptr<frida_attach_entry> > user_attaches;
+	std::vector<frida_attach_entry *> user_attaches;
 	GumInvocationListener *frida_gum_invocation_listener = nullptr;
 
 	friend class frida_attach_manager;
@@ -96,8 +96,8 @@ class frida_attach_manager final : public base_attach_manager {
     private:
 	GumInterceptor *interceptor;
 	int next_id = 1;
-	std::unordered_map<int, std::shared_ptr<frida_attach_entry> > attaches;
-	std::unordered_map<void *, std::shared_ptr<frida_internal_attach_entry> >
+	std::unordered_map<int, std::unique_ptr<frida_attach_entry> > attaches;
+	std::unordered_map<void *, std::unique_ptr<frida_internal_attach_entry> >
 		internal_attaches;
 
 	friend class frida_internal_attach_entry;

runtime/unit-test/attach/test_filter_attach.cpp

@@ -1,6 +1,7 @@
 #include <catch2/catch_test_macros.hpp>
 #include <cstdint>
 #include <attach/attach_manager/frida_attach_manager.hpp>
+#include <spdlog/spdlog.h>
 #if !defined(__x86_64__) && defined(_M_X64)
 #error Only supports x86_64
 #endif
@@ -14,6 +15,12 @@ __bpftime_func_to_filter(uint64_t a, uint64_t b)
 	return (a << 32) | b;
 }
 
+__attribute__((__noinline__, optnone, noinline)) static uint64_t
+call_filter_func(uint64_t a, uint64_t b)
+{
+	return __bpftime_func_to_filter(a, b);
+}
+
 TEST_CASE("Test attaching filter programs and revert")
 {
 	frida_attach_manager man;
@@ -37,7 +44,7 @@ TEST_CASE("Test attaching filter programs and revert")
 		});
 	REQUIRE(id >= 0);
 	REQUIRE(__bpftime_func_to_filter(1, 2) == (((uint64_t)1 << 32) | 2));
-	REQUIRE(__bpftime_func_to_filter(a, b) == a + b);
+	REQUIRE(call_filter_func(a, b) == a + b);
 
 	// Revert it
 	SECTION("Revert by id")

runtime/unit-test/attach/test_replace_attach.cpp

@@ -1,17 +1,23 @@
 #include <catch2/catch_test_macros.hpp>
 #include <attach/attach_manager/frida_attach_manager.hpp>
+#include <spdlog/spdlog.h>
 #if !defined(__x86_64__) && defined(_M_X64)
 #error Only supports x86_64
 #endif
 
 using namespace bpftime;
-__attribute__((__noinline__)) extern "C" uint64_t
+__attribute__((__noinline__, optnone, noinline)) extern "C" uint64_t
 __bpftime_func_to_replace(uint64_t a, uint64_t b)
 {
 	// Forbid inline
 	asm("");
 	return (a << 32) | b;
 }
+__attribute__((__noinline__, optnone, noinline)) static uint64_t
+call_replace_func(uint64_t a, uint64_t b)
+{
+	return __bpftime_func_to_replace(a, b);
+}
 TEST_CASE("Test attaching replace programs and revert")
 {
 	frida_attach_manager man;
@@ -30,7 +36,7 @@ TEST_CASE("Test attaching replace programs and revert")
 					       return regs.si + regs.di;
 				       });
 	REQUIRE(id >= 0);
-	REQUIRE(__bpftime_func_to_replace(a, b) == a + b);
+	REQUIRE(call_replace_func(a, b) == a + b);
 	REQUIRE(invoke_times == 1);
 	// Revert it
 	invoke_times = 0;

runtime/unit-test/attach_with_ebpf/test_attach_filter_with_ebpf.cpp

@@ -6,14 +6,14 @@
 using namespace bpftime;
 
 // This is the original function to hook.
-__attribute__((__noinline__)) extern "C" int
+__attribute__((__noinline__, optnone)) extern "C" int
 __bpftime_attach_filter_with_ebpf__my_function(const char *str, char c,
 					       long long parm1)
 {
 	asm("");
 	// buggy code: not check str is NULL
 	int i = str[0];
-	spdlog::info("origin func: Args: %s, %c, %d", str, c, i);
+	spdlog::info("origin func: Args: {}, {}, {}", str, c, i);
 	return (int)parm1;
 }