luciusDXL / TheForceEngine

Modern "Jedi Engine" replacement supporting Dark Forces, mods, and in the future Outlaws.
https://TheForceEngine.github.io
GNU General Public License v2.0
994 stars 73 forks source link

Linux (Arch Linux) x86_64 - Segfault when launching game #430

Closed V0rt3x667 closed 3 months ago

V0rt3x667 commented 4 months ago

Hi, I can compile and run version v1.09.540 with no issues, however when compiling either v1.10.000 or the current master branch I get a segfault error when running the executable. I am using an old Dell Latitude with Intel graphics so I am enquiring if the requirements for OpenGL or Vulkan (If used) have changed?

The commands I use to compile theforceengine are:

cmake . \ -B"build" \ -G"Ninja" \ -DCMAKE_BUILD_RPATH_USE_ORIGIN="ON" \ -DCMAKE_BUILD_TYPE="Release" \ -DCMAKE_C_COMPILER="clang" \ -DCMAKE_CXX_COMPILER="clang++" \ -DCMAKE_EXE_LINKER_FLAGS_INIT="-fuse-ld=lld" \ -DCMAKE_INSTALL_PREFIX="/opt/archypie/ports/forceengine" \ -DCMAKE_MODULE_LINKER_FLAGS_INIT="-fuse-ld=lld" \ -DCMAKE_SHARED_LINKER_FLAGS_INIT="-fuse-ld=lld" \ -Wno-dev ninja -C build

If no hardware requirements have changed please let me know and I will provide a backtrace. Thanks

luciusDXL commented 3 months ago

There should be no change in hardware requirements. Though its possible that I added extra checks somewhere, but that shouldn't cause a crash. Do you have a GPU capable of at least OpenGL 3.3 features? Can you show your log file (the_force_engine_log.txt)?

mlauss2 commented 3 months ago

Try building with gnu ld please, i.e. drop all that "-fuse-ld=lld" stuff.

EDIT: I can't even build it with lld, just gnu ld and mold. (i just added "-fuse-ld=mold" to the cxxflags). What did you do to get lld to link the final binary? EDIT2: If you have a new enough CMake (3.29+), you could simply add "-DCMAKE_LINKER_TYPE=LLD" and drop all that "-fuse-ld=lld" stuff altogether. Still can't link it with lld though, it wants a ".lib" file for SDL2

V0rt3x667 commented 3 months ago

You have to sed the following to build with lld: sed "s|#pragma comment(lib, \"SDL2main.lib\")|//#pragma comment(lib, \"SDL2main.lib\")|g" -i "./TheForceEngine/main.cpp"

I previously tested with mold. I can try again with GCC & LD and get back to you.

V0rt3x667 commented 3 months ago

v1.10.000 builds and runs fine with gcc/ld but not with clang/ld or clang/lld. I have attached the logfile for the clang/ld build. Thanks for your help.

the_force_engine_log.txt

mlauss2 commented 3 months ago

Ah, thanks for the hint, indeed this line should be moved under a "#ifdef _WIN32" clause.

I was now able to build TFE with clang-18.1.8 and lld-18.1.8 and from a quick test it runs just fine. After your build finishes, does it start if you run "build/theforceengine" ?

Can you please do an "addr2line -e /path/to/theforceengine.exe 0x5d7604992fad" (i.e. address 003 of the crashlog, where the real crash happens), or run it under gdb and get a backtrace please? Although, if you say it runs when built with gcc/ld, your system might just have a bad clang/lld combo.

Thanks!

V0rt3x667 commented 3 months ago

I built theforceengine with the Debug flag set. I hope the back trace below is sufficient, it is for the clang/lld combo. I tried the addr2line option but it just returned ??:0

[Main] The Force Engine v1.10.000+                                                                                                                    

Program received signal SIGSEGV, Segmentation fault.
Downloading source file /usr/src/debug/glibc/glibc/string/../sysdeps/x86_64/multiarch/strlen-sse2.S
__strlen_sse2 () at ../sysdeps/x86_64/multiarch/strlen-sse2.S:142                                                                                     
142     movdqu  (%rax), %xmm4
(gdb) bt
#0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/strlen-sse2.S:142
#1  0x000055555568d4fe in FileUtil::findFileObjectNoCase (filename=0x4000000000 <error: Cannot access memory at address 0x4000000000>, objisdir=true)
    at /home/aaron/GitHub/ArchyPie-Setup/tmp/build/forceengine/TheForceEngine/TFE_FileSystem/fileutil-posix.cpp:266
#2  0x000055555568d491 in FileUtil::directoryExits (path=0x4000000000 <error: Cannot access memory at address 0x4000000000>, 
    outPath=outPath@entry=0x0) at /home/aaron/GitHub/ArchyPie-Setup/tmp/build/forceengine/TheForceEngine/TFE_FileSystem/fileutil-posix.cpp:231
#3  0x000055555574268c in TFE_Settings::autodetectGamePaths ()
    at /home/aaron/GitHub/ArchyPie-Setup/tmp/build/forceengine/TheForceEngine/TFE_Settings/settings.cpp:232
#4  0x00005555557423e4 in TFE_Settings::readFromDisk ()
    at /home/aaron/GitHub/ArchyPie-Setup/tmp/build/forceengine/TheForceEngine/TFE_Settings/settings.cpp:256
#5  0x00005555557422dd in TFE_Settings::init (firstRun=@0x7fffffffdc5f: false)
    at /home/aaron/GitHub/ArchyPie-Setup/tmp/build/forceengine/TheForceEngine/TFE_Settings/settings.cpp:124
#6  0x00005555557b8a4d in main (argc=1, argv=0x7fffffffe798) at /home/aaron/GitHub/ArchyPie-Setup/tmp/build/forceengine/TheForceEngine/main.cpp:568 
mlauss2 commented 3 months ago

Please apply this patch and rebuild:

index 9724175e..d5a7db01 100644
--- a/TheForceEngine/TFE_Settings/settings.cpp
+++ b/TheForceEngine/TFE_Settings/settings.cpp
@@ -227,6 +227,7 @@ namespace TFE_Settings
                                }
                        }
 #endif
+#ifdef _WIN32
                        // If the registry approach fails, just try looking in the various hardcoded paths.
                        if (!pathValid)
                        {
@@ -242,6 +243,7 @@ namespace TFE_Settings
                                        }
                                }
                        }
+#endif
                }
                writeToDisk();
        }
V0rt3x667 commented 3 months ago

Hi, the patch fixes the issue. Thanks.

mlauss2 commented 3 months ago

From my POV the "issue" is in your toolchain. Your lld does stupid things with perfectly fine string constants that happen to be windows paths. I cannot reproduce it though with my vanilla lld/clang setup, I'm inclined to believe that this is either system- or distro specific. But it's not a TFE problem.

V0rt3x667 commented 3 months ago

v1.09.540 builds and runs just fine with the same tool chain. Its fine if you do not wish to pursue the issue further as the patch you provided works. I doubt many will be using the latest versions of clang & lld anyway. It works fine with gcc & ld as is so if you want to close the issue that is fine. Thanks for your help on this.

mlauss2 commented 3 months ago

if your gnu ld and mold work, only lld does not, while I cannot reproduce this at all, I see no reason to pursue this any further to work around a single broken lld linker.

It looks like your lld misplaces the windows path names array ("c_darkForcesLocations[]" in TheForceEngine/TFE_Settings/gameSettingsData.h) or computes the wrong location since the address of the path passed to the crashing function seems way off (the 0x004000000000), it should point to the start of a constant string pool in memory. This was probably exposed by the additional strings added to TFE since the last release.

mlauss2 commented 3 months ago

I'm curious, does this change anything?

index bdcde75b..003fc717 100644
--- a/TheForceEngine/TFE_Settings/gameSourceData.h
+++ b/TheForceEngine/TFE_Settings/gameSourceData.h
@@ -71,7 +71,7 @@ namespace TFE_Settings
                "OUTLAWS.LAB",                  // Game_Outlaws
        };

-       static const char* c_darkForcesLocations[] =
+       static const char* const c_darkForcesLocations[] =
        {
                // C drive
                "C:/Program Files (x86)/Steam/steamapps/common/dark forces/Game/",
@@ -88,7 +88,7 @@ namespace TFE_Settings
                "D:/Program Files (x86)/GOG.com/Star Wars - Dark Forces/",
                "D:/GOG Games/Star Wars - Dark Forces/",
        };
-       static const char* c_outlawsLocations[] =
+       static const char* const c_outlawsLocations[] =
        {
                // C drive
                "C:/Program Files (x86)/Steam/steamapps/common/outlaws/",
@@ -103,7 +103,7 @@ namespace TFE_Settings
        };
        static const u32 c_hardcodedPathCount = TFE_ARRAYSIZE(c_darkForcesLocations);

-       static const char** c_gameLocations[] =
+       static const char* const * c_gameLocations[] =
        {
                c_darkForcesLocations,
                c_outlawsLocations
diff --git a/TheForceEngine/TFE_Settings/settings.cpp b/TheForceEngine/TFE_Settings/settings.cpp
index 9724175e..43b5cfac 100644
--- a/TheForceEngine/TFE_Settings/settings.cpp
+++ b/TheForceEngine/TFE_Settings/settings.cpp
@@ -231,7 +231,7 @@ namespace TFE_Settings
                        if (!pathValid)
                        {
                                // Try various possible locations.
-                               const char** locations = c_gameLocations[gameId];
+                               const char* const * locations = c_gameLocations[gameId];
                                for (u32 i = 0; i < c_hardcodedPathCount; i++)
                                {
                                        if (FileUtil::directoryExits(locations[i]))
V0rt3x667 commented 3 months ago

Hi, applying the patch above in addition to the previous one has not broken anything and work just fine. Thanks.

mlauss2 commented 3 months ago

Please test this new patch alone (without the other).

V0rt3x667 commented 3 months ago

I have tested the patch by itself and it works fine.

mlauss2 commented 3 months ago

Interesting. We can add that to the TFE codebase, but I still prefer your lld getting fixed :) I'll add this to my "pile of little fixes PR". Thanks for testing!

V0rt3x667 commented 3 months ago

Thanks again for your help.