Skip to content

Commit

Permalink
Make push_error and push_warning print name of calling function
Browse files Browse the repository at this point in the history
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 godotengine#76770
  • Loading branch information
TV4Fun committed Jun 27, 2024
1 parent 1aaa831 commit 1a75580
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 3 deletions.
65 changes: 65 additions & 0 deletions core/error/error_macros.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
#include "core/os/os.h"
#include "core/string/ustring.h"

#include <cxxabi.h>
#include <dlfcn.h>
#include <execinfo.h>

static ErrorHandlerList *error_handler_list = nullptr;

void add_error_handler(ErrorHandlerList *p_handler) {
Expand Down Expand Up @@ -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<const char*>(info.dli_saddr) - static_cast<const char*>(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<uint64_t>(info.dli_fbase), 16),
String::num_uint64(reinterpret_cast<uint64_t>(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);
}
1 change: 1 addition & 0 deletions core/error/error_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -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__
Expand Down
4 changes: 2 additions & 2 deletions core/variant/variant_utility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion editor/editor_log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 1a75580

Please sign in to comment.