Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Musl libc support, scons detection for libexecinfo #2439

Closed
LinuxUserGD opened this issue Mar 9, 2021 · 3 comments · Fixed by godotengine/godot#63983
Closed

Musl libc support, scons detection for libexecinfo #2439

LinuxUserGD opened this issue Mar 9, 2021 · 3 comments · Fixed by godotengine/godot#63983

Comments

@LinuxUserGD
Copy link

LinuxUserGD commented Mar 9, 2021

Describe the project you are working on

Musl support for Godot

On distributions like Alpine Linux or Gentoo Linux musl support seems to be already feature-complete.
With the use of glibc macros in the Godot source code (for debugging / stack traces) Godot can't be compiled on those systems by default.

This proposal is about modifying the linuxbsd platform implementation to apply glibc macros only if glibc is available on the system.
As an example the basename function which is included in the glibc headers differ from the musl implementation so it might be needed to "reimplement" it in the godot source code itself (see patch below).

Of course the glibc implementation is more widespread on linux desktops but with the support of other libc implementations for linuxbsd (at least at source level) godot would be POSIX-compliant and therefore open for other linux platforms like servers or embedded systems without glibc.

Describe the problem or limitation you are having in your project

Godot currently only runs on Linux distributions with glibc as libc implementation. On distros with musl libc, it won't compile.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Patch the Godot source code so glibc specific features will only be enabled if glibc is available on the system.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Example patch for musl ( all deleted lines of code should contain #ifdef __GLIBC__ instead so it doesn't break compatibility with existing glibc systems running godot):

diff --git a/platform/linuxbsd/crash_handler_linuxbsd.cpp b/platform/linuxbsd/crash_handler_linuxbsd.cpp
index ea0222cb19..845a555706 100644
--- a/platform/linuxbsd/crash_handler_linuxbsd.cpp
+++ b/platform/linuxbsd/crash_handler_linuxbsd.cpp
@@ -38,89 +38,6 @@
 #define CRASH_HANDLER_ENABLED 1
 #endif
 
-#ifdef CRASH_HANDLER_ENABLED
-#include <cxxabi.h>
-#include <dlfcn.h>
-#include <execinfo.h>
-#include <signal.h>
-#include <stdlib.h>
-
-static void handle_crash(int sig) {
-	if (OS::get_singleton() == nullptr) {
-		abort();
-	}
-
-	void *bt_buffer[256];
-	size_t size = backtrace(bt_buffer, 256);
-	String _execpath = OS::get_singleton()->get_executable_path();
-
-	String msg;
-	const ProjectSettings *proj_settings = ProjectSettings::get_singleton();
-	if (proj_settings) {
-		msg = proj_settings->get("debug/settings/crash_handler/message");
-	}
-
-	// Dump the backtrace to stderr with a message to the user
-	fprintf(stderr, "%s: Program crashed with signal %d\n", __FUNCTION__, sig);
-
-	if (OS::get_singleton()->get_main_loop()) {
-		OS::get_singleton()->get_main_loop()->notification(MainLoop::NOTIFICATION_CRASH);
-	}
-
-	fprintf(stderr, "Dumping the backtrace. %s\n", msg.utf8().get_data());
-	char **strings = backtrace_symbols(bt_buffer, size);
-	if (strings) {
-		for (size_t i = 1; i < size; i++) {
-			char fname[1024];
-			Dl_info info;
-
-			snprintf(fname, 1024, "%s", strings[i]);
-
-			// Try to demangle the function name to provide a more readable one
-			if (dladdr(bt_buffer[i], &info) && info.dli_sname) {
-				if (info.dli_sname[0] == '_') {
-					int status;
-					char *demangled = abi::__cxa_demangle(info.dli_sname, nullptr, nullptr, &status);
-
-					if (status == 0 && demangled) {
-						snprintf(fname, 1024, "%s", demangled);
-					}
-
-					if (demangled) {
-						free(demangled);
-					}
-				}
-			}
-
-			List<String> args;
-
-			char str[1024];
-			snprintf(str, 1024, "%p", bt_buffer[i]);
-			args.push_back(str);
-			args.push_back("-e");
-			args.push_back(_execpath);
-
-			String output = "";
-
-			// Try to get the file/line number using addr2line
-			int ret;
-			Error err = OS::get_singleton()->execute(String("addr2line"), args, &output, &ret);
-			if (err == OK) {
-				output.erase(output.length() - 1, 1);
-			}
-
-			fprintf(stderr, "[%ld] %s (%s)\n", (long int)i, fname, output.utf8().get_data());
-		}
-
-		free(strings);
-	}
-	fprintf(stderr, "-- END OF BACKTRACE --\n");
-
-	// Abort to pass the error to the OS
-	abort();
-}
-#endif
-
 CrashHandler::CrashHandler() {
 	disabled = false;
 }
@@ -134,19 +51,8 @@ void CrashHandler::disable() {
 		return;
 	}
 
-#ifdef CRASH_HANDLER_ENABLED
-	signal(SIGSEGV, nullptr);
-	signal(SIGFPE, nullptr);
-	signal(SIGILL, nullptr);
-#endif
-
 	disabled = true;
 }
 
 void CrashHandler::initialize() {
-#ifdef CRASH_HANDLER_ENABLED
-	signal(SIGSEGV, handle_crash);
-	signal(SIGFPE, handle_crash);
-	signal(SIGILL, handle_crash);
-#endif
 }
diff --git a/platform/linuxbsd/os_linuxbsd.cpp b/platform/linuxbsd/os_linuxbsd.cpp
index 09e1f9461c..08bfead7f2 100644
--- a/platform/linuxbsd/os_linuxbsd.cpp
+++ b/platform/linuxbsd/os_linuxbsd.cpp
@@ -44,7 +44,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-
+#include <libgen.h>
 #include <dlfcn.h>
 #include <fcntl.h>
 #include <sys/stat.h>
@@ -302,6 +302,8 @@ static String get_mountpoint(const String &p_path) {
 	return "";
 }
 
+const char *gnu_basename(const char *path) {const char *base = strrchr(path, '/'); return base ? base+1: path;}
+
 Error OS_LinuxBSD::move_to_trash(const String &p_path) {
 	int err_code;
 	List<String> args;
@@ -382,7 +384,8 @@ Error OS_LinuxBSD::move_to_trash(const String &p_path) {
 	// The trash can is successfully created, now we check that we don't exceed our file name length limit.
 	// If the file name is too long trim it so we can add the identifying number and ".trashinfo".
 	// Assumes that the file name length limit is 255 characters.
-	String file_name = basename(p_path.utf8().get_data());
+	String file_name = gnu_basename(p_path.utf8().get_data());
+	
 	if (file_name.length() > 240) {
 		file_name = file_name.substr(0, file_name.length() - 15);
 	}

If this enhancement will not be used often, can it be worked around with a few lines of script?

Providing the patch in a separate "gd_tweaks" repository or fork would be possible but because this has to be applied to the source code, an upstream pull request for the main godot repository would be better.

Is there a reason why this should be core and not an add-on in the asset library?

It can't fix the buildsystem.

@Calinou
Copy link
Member

Calinou commented Mar 9, 2021

You can already build Godot with musl by installing libexecinfo then passing execinfo=yes on the SCons command line. That said, I agree execinfo=yes should be used by default when musl is detected. The question is, how do we detect that the user will be using musl instead of glibc from SCons?

Currently, you'll also need to patch VHACD or disable it using module_vhacd_enabled=no.

@LinuxUserGD
Copy link
Author

LinuxUserGD commented Mar 9, 2021

@Calinou After installing http://distcache.freebsd.org/local-distfiles/itetcu/libexecinfo-1.1.tar.bz2 compilation errors are generally fixed , the patch for glibc basename use is still necessary though.
Edit: Actually the basename function is only used in the godot master branch in platforms/linuxbsd, therefore compiling godot for musl works fine with execinfo=yes in 3.x.

@LinuxUserGD
Copy link
Author

LinuxUserGD commented Jan 25, 2022

gnu_basename function should be fixed by godotengine/godot#51429

@LinuxUserGD LinuxUserGD changed the title Musl libc support, use of glibc specific macros in linuxbsd platform Musl libc support, scons detection for libbacktrace Jan 25, 2022
@LinuxUserGD LinuxUserGD changed the title Musl libc support, scons detection for libbacktrace Musl libc support, scons detection for libexecinfo Apr 29, 2022
@akien-mga akien-mga added this to the 4.0 milestone Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants