bbbradsmith / hatariB

Libretro core for Hatari emulation of Atari ST STE TT Falcon
GNU General Public License v2.0
20 stars 3 forks source link

segfault when building #13

Closed dmanlfc closed 1 year ago

dmanlfc commented 1 year ago

I'm trying to build hatariB with buildroot for batocera.linux and i get a segfault...

Segmentation fault (core dumped) gmake[4]: *** [src/cpu/CMakeFiles/gencpu.dir/build.make:75: src/cpu/cpudefs.c] Error 139 gmake[4]: *** Deleting file 'src/cpu/cpudefs.c' gmake[4]: Leaving directory '/x86_64/build/libretro-hatari-b-c37df8ed52fd8f52bcfee0a0499b825bc7808154/hatari/build' gmake[3]: *** [CMakeFiles/Makefile2:483: src/cpu/CMakeFiles/gencpu.dir/all] Error 2 gmake[3]: *** Waiting for unfinished jobs....

a couple of other things to note:

  1. Header issues:

there is an error because config.h is not there, twice before & whilst building with CMake

here is the patch to fix the header reference...

diff --git a/hatari/src/includes/main.h b/hatari/src/includes/main.h
index 09342774d8..1ae5c3f07a 100644
--- a/hatari/src/includes/main.h
+++ b/hatari/src/includes/main.h
@@ -8,7 +8,7 @@
 #ifndef HATARI_MAIN_H
 #define HATARI_MAIN_H

-#include "config.h"
+#include "../../build/config.h"

 #include <stdio.h>
 #include <stdlib.h>

but you should create the config.h file using the makefile first too.

  1. There is a compilation error...
[ 38%] Building C object src/CMakeFiles/core.dir/scandir.c.o
../../src/scandir.c: In function ‘scandir’:
../../src/scandir.c:140:31: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
  140 |                 for (i = 0; i < nitems; i++)
      |                               ^
cc1: all warnings being treated as errors

here is the patch i used...

diff --git a/hatari/src/scandir.c b/hatari/src/scandir.c
index 7e089a1729..3af5288b71 100644
--- a/hatari/src/scandir.c
+++ b/hatari/src/scandir.c
@@ -136,7 +136,7 @@ int scandir(const char *dirname, struct dirent ***namelist,
 error_out:
    if (names)
    {
-       int i;
+       size_t i;
        for (i = 0; i < nitems; i++)
            free(names[i]);
        free(names);

or remove the -Werror=sign-compare flag being set.

bbbradsmith commented 1 year ago

My best guess for the segfault would be that your gmake is creating more instances of gcc than it has memory for (a result of -j). Try adding MULTITHREAD= to the make command line.

You will have to explain more about config.h being missing. What error was produced, and at what stage? It is produced by the first cmake step, and only used by the second cmake step (build), so I don't know what you mean by "twice before", or "whilst building".

The warning with scandir I guess hasn't come up yet because all platforms try so far either HAVE_SCANDIR or were WIN32 where there's a different code path. Since this is actually a warning produced by untouched Hatari code, you might want to submit that patch to hatari, if that code hasn't changed since the release version this is based on.

Instead, because this core provides its own alternative to scandir, I think here it would just be better to #define out scandir altogether. I have now done this.

dmanlfc commented 1 year ago

Setting to -j1 doesn't help @bbbradsmith

Yes removing scandir would make sense, we don't get that error when we build the libretro hatari core.

dmanlfc commented 1 year ago

hatariB-cleaned-output.txt

Attached is a build output.

dmanlfc commented 1 year ago

this is the config.h error

/x86_64/host/bin/x86_64-buildroot-linux-gnu-gcc -o build/core/core_osk.o -I/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include -I/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include/SDL2 -lSDL2 -O3 -Wall -Werror -fPIC -D__LIBRETRO__ -DSHORTHASH=\"c37df8e\" -I/build/buildroot/hatari/build -I/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include/SDL2 -c core/core_osk.c 
(cd hatari/build && export CFLAGS="-I/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include -I/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include/SDL2 -lSDL2 -O3 -Wall -Werror -fPIC -D__LIBRETRO__ -DSHORTHASH=\"c37df8e\" -I/build/buildroot/hatari/build -I/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include/SDL2" && cmake .. -DZLIB_INCLUDE_DIR=/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include -DZLIB_LIBRARY=/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/lib -DSDL2_INCLUDE_DIR=/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/include/SDL2 -DSDL2_LIBRARY=/x86_64/host/x86_64-buildroot-linux-gnu/sysroot/usr/lib -DCMAKE_DISABLE_FIND_PACKAGE_Readline=1 -DCMAKE_DISABLE_FIND_PACKAGE_X11=1 -DCMAKE_DISABLE_FIND_PACKAGE_PNG=1 -DCMAKE_DISABLE_FIND_PACKAGE_PortMidi=1 -DCMAKE_DISABLE_FIND_PACKAGE_CapsImage=1 -DENABLE_SMALL_MEM=0 -DENABLE_TRACING=0)
In file included from core/core_config.c:4:
core/../hatari/src/includes/main.h:11:10: fatal error: config.h: No such file or directory
   11 | #include "config.h"
      |          ^~~~~~~~~~
compilation terminated.
make[1]: *** [makefile:121: build/core/core_config.o] Error 1
make[1]: *** Waiting for unfinished jobs....
In file included from core/core_file.c:7:
core/../hatari/src/includes/main.h:11:10: fatal error: config.h: No such file or directory
   11 | #include "config.h"
      |          ^~~~~~~~~~
compilation terminated.
make[1]: *** [makefile:121: build/core/core_file.o] Error 1
bbbradsmith commented 1 year ago

we don't get that error when we build the libretro hatari core.

My core is mostly unrelated to libretro hatari, but I suspect that it still gets the warning, but doesn't treat warnings as errors. At any rate, the correction should really go upstream to hatari itself.

Setting to -j1 doesn't help @bbbradsmith

The log you provided clarified that the segfault comes from running gencpu, which is a utility that gets built and run to generate some of the cpu emulation code. (You can also edit the make file to add tracing to gmake if you'd like to see the actual build commands. It would show you the gencpu command that is run, which might show you how to manually build and execute it.)

See: https://github.com/bbbradsmith/hatariB/blob/main/hatari/src/cpu/gencpu.c

So, I'm not sure what to suggest. You've probably discovered a bug with gencpu.c that has been (un)lucky enough not to affect other platforms yet. Since I can't reproduce the error, it's hard for me to advise what to do about it. I know the code comes from WinUAE, so you might check if there were any reports of a segfault there. You might also try to build the actual Hatari 2.4.1 from source, and see if you get the same segfault. In theory you should, but if not it could indicate some problem I've introduced. I've made no changes at all to that source file, at least.

this is the config.h error

I think this indicates that your version of make treats dependencies and parallel build different than all the others I have tried. The target $(CORE) depends on directories, hatarilib, and $(OBJECTS), in that order explicitly. However, your parallel build is starting to work on $(OBJECTS) before hatarilib.

I will add an explicit hatarilib depenency to the target that builds $(OBJECTS), which should fix this, but this is a new interpretation of parallel build order that I have not yet seen. At any rate, the fix here is not to add that extra path to the header, it is a problem with the order chosen by make.

I have pushed that change to the makefile.

dmanlfc commented 1 year ago

hatari 2.4.1 builds fine here

bbbradsmith commented 1 year ago

Alright, well it's possible that -fPIC is somehow revealing a latent bug. Other than that I don't think there's any other compile changes that should be able to affect it. You could remove -fPIC from the makefile and see if it gets past gencpu, however the final link of hatarib.so will not work without -fPIC. This would just check whether that's the difference. If it's not that, I don't have any other guesses.

Otherwise you could try manually building and running gencpu to see if that makes a difference. If you turn on the trace for cmake (another option you can find in the makefile) it will show each command line, which might help you figure out what to do.

dmanlfc commented 1 year ago

@bbbradsmith it seems related to the buildroot CFLAGS, I have removed them & it get's to 100% Unfortunately I have another problem with linking as it doesn't take our already built SDL2 2.28.2 library. I will try to build using the libraries in the full method...

It would be great to allow an option to use system sdl / zlib etc if present in the system in future. Including full cmake. i.e.

# SDL2
if (ENABLE_SDL2 AND USE_SYSTEM_SDL2)
    find_package(SDL2 REQUIRED)
    add_library(SDL2 INTERFACE)
    target_link_libraries(SDL2 INTERFACE "${SDL2_LIBRARY}")
    target_include_directories(SDL2 INTERFACE "${SDL2_INCLUDE_DIR}")
    add_library(SDL2::SDL2 ALIAS SDL2)
endif()
bbbradsmith commented 1 year ago

You can supply your own SDL2 and ZLIB by providing environment variables which override them. See the makefile here: https://github.com/bbbradsmith/hatariB/blob/71d03d649faf6e14ca3810bd2fbfc3a824cbf226/makefile#L14

Though, if overriding you will probably need to use the .so versions, not the static .a versions, because the static ones are not normally provided with -fPIC.

dmanlfc commented 1 year ago

Yes I did that as it doesn't work with static like you say.

bbbradsmith commented 1 year ago

Anything that static links with a .so has to use -fPIC, but standard packages don't normally provide static libraries for that purpose, and expect .so should always dynamically link with other .so instead.

You should be able to use libSDL2.so rather than libSDL2.a for linking though.

dmanlfc commented 1 year ago

Sorry to confuse you it's building @bbbradsmith, I'm using the shared libraries after trying static, that why I closed it. Thanks for the help.