From 1a75580d752ef22e911c0b8de7ead2b3f2ac5aac Mon Sep 17 00:00:00 2001 From: Joel Croteau Date: Wed, 26 Jun 2024 23:00:25 -0600 Subject: [PATCH] Make `push_error` and `push_warning` print name of calling function Currently, `push_error` and `push_warning` give themselve as the calling function, which is not useful for debugging. I have altered these to use the C stacktrace to get the name of the function that called them. This saves having to turn these functions into macros, which would require recompiling every single piece of code that uses them. I have also set it to, when debug symbols are available, use them with `atos` (available on macOS) to find the exact source file and line being called from. It should be possible to the same thing on Linux using `addr2line`, but I don't have a Linux box handy to test that. I am not sure how you would implement this in Windows. I have tested this on macOS Sonoma 14.5 (23F79). It should work on any *nix system. I am not sure about Windows. Fixes #76770 --- core/error/error_macros.cpp | 65 ++++++++++++++++++++++++++++++++ core/error/error_macros.h | 1 + core/variant/variant_utility.cpp | 4 +- editor/editor_log.cpp | 2 +- 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/core/error/error_macros.cpp b/core/error/error_macros.cpp index 8376c0aaf84c..80052a0da979 100644 --- a/core/error/error_macros.cpp +++ b/core/error/error_macros.cpp @@ -34,6 +34,10 @@ #include "core/os/os.h" #include "core/string/ustring.h" +#include +#include +#include + static ErrorHandlerList *error_handler_list = nullptr; void add_error_handler(ErrorHandlerList *p_handler) { @@ -125,6 +129,67 @@ void _err_print_index_error(const char *p_function, const char *p_file, int p_li _err_print_index_error(p_function, p_file, p_line, p_index, p_size, p_index_str, p_size_str, p_message.utf8().get_data(), p_editor_notify, p_fatal); } +struct FunctionInfo { + const char* function; + const char* file; + int line; + String descriptor; +}; + +FunctionInfo calling_function(const char* filter, int frames_to_skip = 0) { + constexpr int kBacktraceDepth = 15; + constexpr int kDemangledBufferSize = 100; + void* backtrace_addrs[kBacktraceDepth]; + static char s_demangled[kDemangledBufferSize]; + Dl_info info; + FunctionInfo result; + + int trace_size = backtrace(backtrace_addrs, kBacktraceDepth); + + int i = frames_to_skip; + do { + ++i; + dladdr(backtrace_addrs[i], &info); + } while (i < trace_size && strstr(info.dli_sname, filter)); + + int status; + char* demangled = abi::__cxa_demangle(info.dli_sname, nullptr, nullptr, &status); + + if (status == 0) { + // Have to do it this way to avoid a memory leak, as abi::__cxa_demangle returns a `malloc`ed c-string. + strncpy(s_demangled, demangled, kDemangledBufferSize); + s_demangled[kDemangledBufferSize - 1] = 0; + result.function = s_demangled; + } else { + result.function = info.dli_sname; + } + free(demangled); + + result.file = info.dli_fname; + result.line = static_cast(info.dli_saddr) - static_cast(info.dli_fbase); + +#ifdef DEBUG_ENABLED + if (OS::get_singleton()->get_name() == "macOS") { + String pipe; + Error error = OS::get_singleton()->execute("atos", { "-o", info.dli_fname, "-l", + String::num_uint64(reinterpret_cast(info.dli_fbase), 16), + String::num_uint64(reinterpret_cast(backtrace_addrs[i]), 16)}, + &pipe); + + if (error == OK) { + result.descriptor = pipe + " - "; + } + } +#endif + + return result; +} + +void _err_print_error_backtrace(const char *filter, const String& p_error, bool p_editor_notify, ErrorHandlerType p_type) { + FunctionInfo info = calling_function(filter, 1); + _err_print_error(info.function, info.file, info.line, "", info.descriptor + p_error, p_editor_notify, p_type); +} + void _err_flush_stdout() { fflush(stdout); } diff --git a/core/error/error_macros.h b/core/error/error_macros.h index ab7dbcbd44b7..c80bb98dfb74 100644 --- a/core/error/error_macros.h +++ b/core/error/error_macros.h @@ -69,6 +69,7 @@ void _err_print_error(const char *p_function, const char *p_file, int p_line, co void _err_print_error(const char *p_function, const char *p_file, int p_line, const String &p_error, const String &p_message, bool p_editor_notify = false, ErrorHandlerType p_type = ERR_HANDLER_ERROR); void _err_print_index_error(const char *p_function, const char *p_file, int p_line, int64_t p_index, int64_t p_size, const char *p_index_str, const char *p_size_str, const char *p_message = "", bool p_editor_notify = false, bool fatal = false); void _err_print_index_error(const char *p_function, const char *p_file, int p_line, int64_t p_index, int64_t p_size, const char *p_index_str, const char *p_size_str, const String &p_message, bool p_editor_notify = false, bool fatal = false); +void _err_print_error_backtrace(const char* filter, const String &p_error, bool p_editor_notify = false, ErrorHandlerType p_type = ERR_HANDLER_ERROR); void _err_flush_stdout(); #ifdef __GNUC__ diff --git a/core/variant/variant_utility.cpp b/core/variant/variant_utility.cpp index 7534a154a1c3..fd5004f2b389 100644 --- a/core/variant/variant_utility.cpp +++ b/core/variant/variant_utility.cpp @@ -1089,7 +1089,7 @@ void VariantUtilityFunctions::push_error(const Variant **p_args, int p_arg_count } } - ERR_PRINT(s); + _err_print_error_backtrace(FUNCTION_STR, s); r_error.error = Callable::CallError::CALL_OK; } @@ -1109,7 +1109,7 @@ void VariantUtilityFunctions::push_warning(const Variant **p_args, int p_arg_cou } } - WARN_PRINT(s); + _err_print_error_backtrace(FUNCTION_STR, s, false, ERR_HANDLER_WARNING); r_error.error = Callable::CallError::CALL_OK; } diff --git a/editor/editor_log.cpp b/editor/editor_log.cpp index 6a016c217a86..2745f0ca04d3 100644 --- a/editor/editor_log.cpp +++ b/editor/editor_log.cpp @@ -49,7 +49,7 @@ void EditorLog::_error_handler(void *p_self, const char *p_func, const char *p_f if (p_errorexp && p_errorexp[0]) { err_str = String::utf8(p_errorexp); } else { - err_str = String::utf8(p_file) + ":" + itos(p_line) + " - " + String::utf8(p_error); + err_str = String::utf8(p_func) + " (" + String::utf8(p_file) + ": " + itos(p_line) + ") - " + String::utf8(p_error); } if (p_editor_notify) {