File 0196-jit-Unregister-unloaded-code-with-GDB-integration.patch of Package erlang
From 17f34a063d0011e4f5de9f9f354fef2030983ab5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20H=C3=B6gberg?= <john@erlang.org>
Date: Fri, 5 Jan 2024 11:16:10 +0100
Subject: [PATCH] jit: Unregister unloaded code with GDB integration
This resolves some random crashes when stepping backwards under rr,
but sadly not all.
---
erts/emulator/beam/beam_bif_load.c | 1 +
erts/emulator/beam/jit/arm/beam_asm.hpp | 10 +-
.../emulator/beam/jit/arm/beam_asm_global.cpp | 8 +-
erts/emulator/beam/jit/asm_load.c | 2 +-
erts/emulator/beam/jit/beam_asm.h | 3 +-
erts/emulator/beam/jit/beam_jit_common.cpp | 12 ++-
erts/emulator/beam/jit/beam_jit_main.cpp | 5 +-
erts/emulator/beam/jit/beam_jit_metadata.cpp | 101 +++++++++++++-----
erts/emulator/beam/jit/x86/beam_asm.hpp | 10 +-
.../emulator/beam/jit/x86/beam_asm_global.cpp | 8 +-
erts/emulator/beam/module.h | 1 +
11 files changed, 106 insertions(+), 55 deletions(-)
diff --git a/erts/emulator/beam/beam_bif_load.c b/erts/emulator/beam/beam_bif_load.c
index 7fe9e9960c..03d4f48961 100644
--- a/erts/emulator/beam/beam_bif_load.c
+++ b/erts/emulator/beam/beam_bif_load.c
@@ -2066,6 +2066,7 @@ BIF_RETTYPE erts_internal_purge_module_2(BIF_ALIST_2)
__lsan_unregister_root_region(modp->old.code_hdr,
modp->old.code_length);
# endif
+ beamasm_unregister_metadata(modp->old.metadata);
beamasm_purge_module(modp->old.executable_region,
modp->old.writable_region,
modp->old.code_length);
diff --git a/erts/emulator/beam/jit/arm/beam_asm.hpp b/erts/emulator/beam/jit/arm/beam_asm.hpp
index 415f114e9e..1e78abfc2c 100644
--- a/erts/emulator/beam/jit/arm/beam_asm.hpp
+++ b/erts/emulator/beam/jit/arm/beam_asm.hpp
@@ -1105,7 +1105,7 @@ public:
void codegen(char *buff, size_t len);
- void register_metadata(const BeamCodeHeader *header);
+ void *register_metadata(const BeamCodeHeader *header);
ErtsCodePtr getCode(unsigned label);
ErtsCodePtr getLambda(unsigned index);
@@ -1829,9 +1829,9 @@ protected:
}
};
-void beamasm_metadata_update(std::string module_name,
- ErtsCodePtr base_address,
- size_t code_size,
- const std::vector<AsmRange> &ranges);
+void *beamasm_metadata_insert(std::string module_name,
+ ErtsCodePtr base_address,
+ size_t code_size,
+ const std::vector<AsmRange> &ranges);
void beamasm_metadata_early_init();
void beamasm_metadata_late_init();
diff --git a/erts/emulator/beam/jit/arm/beam_asm_global.cpp b/erts/emulator/beam/jit/arm/beam_asm_global.cpp
index 84cbf0a515..ff1c0e924c 100644
--- a/erts/emulator/beam/jit/arm/beam_asm_global.cpp
+++ b/erts/emulator/beam/jit/arm/beam_asm_global.cpp
@@ -79,10 +79,10 @@ BeamGlobalAssembler::BeamGlobalAssembler(JitAllocator *allocator)
.name = code.labelEntry(labels[val.first])->name()});
}
- beamasm_metadata_update("global",
- (ErtsCodePtr)getBaseAddress(),
- code.codeSize(),
- ranges);
+ (void)beamasm_metadata_insert("global",
+ (ErtsCodePtr)getBaseAddress(),
+ code.codeSize(),
+ ranges);
/* `this->get_xxx` are populated last to ensure that we crash if we use
* them instead of labels in global code. */
diff --git a/erts/emulator/beam/jit/asm_load.c b/erts/emulator/beam/jit/asm_load.c
index bf97d32987..adfcf61631 100644
--- a/erts/emulator/beam/jit/asm_load.c
+++ b/erts/emulator/beam/jit/asm_load.c
@@ -1133,7 +1133,7 @@ void beam_load_finalize_code(LoaderState *stp,
}
/* Register debug / profiling info with external tools. */
- beamasm_register_metadata(stp->ba, stp->code_hdr);
+ inst_p->metadata = beamasm_register_metadata(stp->ba, stp->code_hdr);
erts_seal_module(inst_p);
diff --git a/erts/emulator/beam/jit/beam_asm.h b/erts/emulator/beam/jit/beam_asm.h
index 595d579aa8..70f0121846 100644
--- a/erts/emulator/beam/jit/beam_asm.h
+++ b/erts/emulator/beam/jit/beam_asm.h
@@ -59,7 +59,8 @@ void beamasm_codegen(void *ba,
const BeamCodeHeader *in_hdr,
const BeamCodeHeader **out_exec_hdr,
BeamCodeHeader **out_rw_hdr);
-void beamasm_register_metadata(void *ba, const BeamCodeHeader *hdr);
+void *beamasm_register_metadata(void *ba, const BeamCodeHeader *header);
+void beamasm_unregister_metadata(void *handle);
void beamasm_purge_module(const void *executable_region,
void *writable_region,
size_t size);
diff --git a/erts/emulator/beam/jit/beam_jit_common.cpp b/erts/emulator/beam/jit/beam_jit_common.cpp
index 7013f25c8d..4fbadb68b5 100644
--- a/erts/emulator/beam/jit/beam_jit_common.cpp
+++ b/erts/emulator/beam/jit/beam_jit_common.cpp
@@ -357,7 +357,7 @@ BeamModuleAssembler::BeamModuleAssembler(BeamGlobalAssembler *_ga,
}
}
-void BeamModuleAssembler::register_metadata(const BeamCodeHeader *header) {
+void *BeamModuleAssembler::register_metadata(const BeamCodeHeader *header) {
#ifndef WIN32
const BeamCodeLineTab *line_table = header->line_table;
@@ -472,10 +472,12 @@ void BeamModuleAssembler::register_metadata(const BeamCodeHeader *header) {
.stop = (ErtsCodePtr)(code.baseAddress() + code.codeSize()),
.name = module_name + "::codeFooter"});
- beamasm_metadata_update(module_name,
- (ErtsCodePtr)code.baseAddress(),
- code.codeSize(),
- ranges);
+ return beamasm_metadata_insert(module_name,
+ (ErtsCodePtr)code.baseAddress(),
+ code.codeSize(),
+ ranges);
+#else
+ return NULL;
#endif
}
diff --git a/erts/emulator/beam/jit/beam_jit_main.cpp b/erts/emulator/beam/jit/beam_jit_main.cpp
index 000b3c6c70..f976d6031e 100644
--- a/erts/emulator/beam/jit/beam_jit_main.cpp
+++ b/erts/emulator/beam/jit/beam_jit_main.cpp
@@ -623,9 +623,10 @@ extern "C"
out_rw_hdr);
}
- void beamasm_register_metadata(void *instance, const BeamCodeHeader *hdr) {
+ void *beamasm_register_metadata(void *instance,
+ const BeamCodeHeader *header) {
BeamModuleAssembler *ba = static_cast<BeamModuleAssembler *>(instance);
- ba->register_metadata(hdr);
+ return ba->register_metadata(header);
}
Uint beamasm_get_header(void *instance, const BeamCodeHeader **hdr) {
diff --git a/erts/emulator/beam/jit/beam_jit_metadata.cpp b/erts/emulator/beam/jit/beam_jit_metadata.cpp
index 10e82fdf37..54e7cba5c3 100644
--- a/erts/emulator/beam/jit/beam_jit_metadata.cpp
+++ b/erts/emulator/beam/jit/beam_jit_metadata.cpp
@@ -131,34 +131,31 @@ static void beamasm_init_late_gdb() {
entry->symfile_size = sizeof(struct debug_info);
/* Insert into linked list */
- entry->next_entry = __jit_debug_descriptor.first_entry;
- if (entry->next_entry) {
- entry->next_entry->prev_entry = entry;
- } else {
- entry->prev_entry = nullptr;
+ erts_mtx_lock(&__jit_debug_descriptor.mutex);
+
+ if (__jit_debug_descriptor.first_entry) {
+ ASSERT(__jit_debug_descriptor.first_entry->prev_entry == nullptr);
+ __jit_debug_descriptor.first_entry->prev_entry = entry;
}
- /* Insert into linked list */
- erts_mtx_lock(&__jit_debug_descriptor.mutex);
+ entry->prev_entry = nullptr;
entry->next_entry = __jit_debug_descriptor.first_entry;
- if (entry->next_entry) {
- entry->next_entry->prev_entry = entry;
- } else {
- entry->prev_entry = nullptr;
- }
+ __jit_debug_descriptor.first_entry = entry;
/* Register with gdb */
+ ASSERT(__jit_debug_descriptor.relevant_entry == nullptr);
__jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
- __jit_debug_descriptor.first_entry = entry;
__jit_debug_descriptor.relevant_entry = entry;
__jit_debug_register_code();
+ __jit_debug_descriptor.relevant_entry = nullptr;
+
erts_mtx_unlock(&__jit_debug_descriptor.mutex);
}
-static void beamasm_update_gdb_info(std::string module_name,
- ErtsCodePtr base_address,
- size_t code_size,
- const std::vector<AsmRange> &ranges) {
+static void *beamasm_insert_gdb_info(std::string module_name,
+ ErtsCodePtr base_address,
+ size_t code_size,
+ const std::vector<AsmRange> &ranges) {
Sint symfile_size = sizeof(struct debug_info) + module_name.size() + 1;
for (const auto &range : ranges) {
@@ -221,21 +218,58 @@ static void beamasm_update_gdb_info(std::string module_name,
/* Insert into linked list */
erts_mtx_lock(&__jit_debug_descriptor.mutex);
- entry->next_entry = __jit_debug_descriptor.first_entry;
- if (entry->next_entry) {
- entry->next_entry->prev_entry = entry;
- } else {
- entry->prev_entry = nullptr;
+
+ if (__jit_debug_descriptor.first_entry) {
+ ASSERT(__jit_debug_descriptor.first_entry->prev_entry == nullptr);
+ __jit_debug_descriptor.first_entry->prev_entry = entry;
}
+ entry->prev_entry = nullptr;
+ entry->next_entry = __jit_debug_descriptor.first_entry;
+ __jit_debug_descriptor.first_entry = entry;
+
/* register with gdb */
+ ASSERT(__jit_debug_descriptor.relevant_entry == nullptr);
__jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
- __jit_debug_descriptor.first_entry = entry;
__jit_debug_descriptor.relevant_entry = entry;
__jit_debug_register_code();
+ __jit_debug_descriptor.relevant_entry = nullptr;
+
erts_mtx_unlock(&__jit_debug_descriptor.mutex);
+
+ /* Return a reference to our debugger entry so we can remove it when
+ * unloading. */
+ return entry;
}
+static void beamasm_delete_gdb_info(void *metadata) {
+ jit_code_entry *entry = (jit_code_entry *)metadata;
+
+ erts_mtx_lock(&__jit_debug_descriptor.mutex);
+
+ /* Unlink current module from the entry list. */
+ if (entry->prev_entry) {
+ entry->prev_entry->next_entry = entry->next_entry;
+ } else {
+ __jit_debug_descriptor.first_entry = entry->next_entry;
+ }
+
+ if (entry->next_entry) {
+ entry->next_entry->prev_entry = entry->prev_entry;
+ }
+
+ /* unregister with gdb */
+ ASSERT(__jit_debug_descriptor.relevant_entry == nullptr);
+ __jit_debug_descriptor.action_flag = JIT_UNREGISTER_FN;
+ __jit_debug_descriptor.relevant_entry = entry;
+ __jit_debug_register_code();
+ __jit_debug_descriptor.relevant_entry = nullptr;
+
+ erts_mtx_unlock(&__jit_debug_descriptor.mutex);
+
+ free((void *)entry->symfile_addr);
+ free(entry);
+}
#endif /* HAVE_GDB_SUPPORT */
#ifdef HAVE_LINUX_PERF_SUPPORT
@@ -525,15 +559,26 @@ void beamasm_metadata_late_init() {
#endif
}
-void beamasm_metadata_update(std::string module_name,
- ErtsCodePtr base_address,
- size_t code_size,
- const std::vector<AsmRange> &ranges) {
+void *beamasm_metadata_insert(std::string module_name,
+ ErtsCodePtr base_address,
+ size_t code_size,
+ const std::vector<AsmRange> &ranges) {
#ifdef HAVE_LINUX_PERF_SUPPORT
perf.update(ranges);
#endif
#ifdef HAVE_GDB_SUPPORT
- beamasm_update_gdb_info(module_name, base_address, code_size, ranges);
+ return beamasm_insert_gdb_info(module_name,
+ base_address,
+ code_size,
+ ranges);
+#else
+ return NULL;
+#endif
+}
+
+void beamasm_unregister_metadata(void *metadata) {
+#ifdef HAVE_GDB_SUPPORT
+ beamasm_delete_gdb_info(metadata);
#endif
}
diff --git a/erts/emulator/beam/jit/x86/beam_asm.hpp b/erts/emulator/beam/jit/x86/beam_asm.hpp
index eed39447a1..1b74628529 100644
--- a/erts/emulator/beam/jit/x86/beam_asm.hpp
+++ b/erts/emulator/beam/jit/x86/beam_asm.hpp
@@ -1186,7 +1186,7 @@ public:
void codegen(char *buff, size_t len);
- void register_metadata(const BeamCodeHeader *header);
+ void *register_metadata(const BeamCodeHeader *header);
ErtsCodePtr getCode(unsigned label);
ErtsCodePtr getLambda(unsigned index);
@@ -1663,9 +1663,9 @@ protected:
}
};
-void beamasm_metadata_update(std::string module_name,
- ErtsCodePtr base_address,
- size_t code_size,
- const std::vector<AsmRange> &ranges);
+void *beamasm_metadata_insert(std::string module_name,
+ ErtsCodePtr base_address,
+ size_t code_size,
+ const std::vector<AsmRange> &ranges);
void beamasm_metadata_early_init();
void beamasm_metadata_late_init();
diff --git a/erts/emulator/beam/jit/x86/beam_asm_global.cpp b/erts/emulator/beam/jit/x86/beam_asm_global.cpp
index bb62938b99..ccd89c8749 100644
--- a/erts/emulator/beam/jit/x86/beam_asm_global.cpp
+++ b/erts/emulator/beam/jit/x86/beam_asm_global.cpp
@@ -80,10 +80,10 @@ BeamGlobalAssembler::BeamGlobalAssembler(JitAllocator *allocator)
.name = code.labelEntry(labels[val.first])->name()});
}
- beamasm_metadata_update("global",
- (ErtsCodePtr)getBaseAddress(),
- code.codeSize(),
- ranges);
+ (void)beamasm_metadata_insert("global",
+ (ErtsCodePtr)getBaseAddress(),
+ code.codeSize(),
+ ranges);
#endif
/* `this->get_xxx` are populated last to ensure that we crash if we use them
diff --git a/erts/emulator/beam/module.h b/erts/emulator/beam/module.h
index 6bbadbd505..63e8e111a1 100644
--- a/erts/emulator/beam/module.h
+++ b/erts/emulator/beam/module.h
@@ -34,6 +34,7 @@ struct erl_module_instance {
const void *executable_region;
void *writable_region;
+ void *metadata;
/* Protected by code modification permission. */
int unsealed;
--
2.35.3
