Page 1 of 1

Segmentation fault (core dumped) when adding new setting to Setup

Posted: Mon Nov 09, 2020 7:17 pm
by mat
As an example, I've added "Show level preview" option to Setup -> Graphics. I've inserted all needed things to some files (files.c, screens.c, tools.c, system.h). The compilation is ok but when I want to run the compiled game, a message appears: "Segmentation fault (core dumped)". Oddly enough, the game works at the sacrifice of removing other option (for example "Small game graphics").

I know what the message means but I can't find a source of the problem. Could anyone help me? I try to launch on Linux Mate 20.04.1 LTS (Focal Fossa) 64-bit

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Tue Nov 10, 2020 10:36 am
by Holger
Hi mat, to check what went wrong, it would be very helpful to be able to reproduce the problem! To do this, please add a patch (or output of "git diff") and the commit hash against which the patch should be used (if you are not using the latest release version).

A segmentation fault is *very* easy to create in C, and although I assume that most of your changes are just fine, there is probably a tiny little bug with a big effect. :-)

You could also start the game inside "gdb" (the GNU debugger) and type "bt" after the crash to get the back trace, so you should get an idea what went wrong (and where).

If you provide me with the patch, I will have a look at all that.

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Tue Nov 10, 2020 8:35 pm
by mat
Hash: 84a28d5bf05056e1a70eeb37ff6b73aca21c5533
Patch below.

Thank you in advance :)

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Wed Nov 11, 2020 7:42 am
by filbo
I was able to reproduce this, and I think it is not a bug in mat's code!

After a bit of debugging, nothing was making sense, so I did:

$ make clean
$ make

and then it worked fine. It appears the bug is in Holger's makefiles, which do not declare any dependencies on src/lib/system.h, which is easily demonstrated:

$ make
$ touch src/libgame/system.h
$ make

(2nd `make` doesn't build anything)

=====

After a little further investigation, it turns out you MUST run `make depend` before any of this stuff will work!

Holger: please have the Makefile either automatically run `make depend` if src/.depend doesn't exist, or at least error out and tell the user to do so.

I know it's labeled a 'development target', but if someone is compiling at all, they're basically doing development! And I think if they were merely downloading your source tarball and running `make`, this could still screw them up. Suppose tarball 1 and 2 differ only in some header file. They extract tarball 1, `make`, extract 2, `make`, and nothing is compiled. Unless the tarball is created with all timestamps set to tarball-creation-time, which they shouldn't be.

This seems to do it. (It's incomplete as it doesn't check for src/*/.depend, but that doesn't seem important):

Code: Select all

$ git diff -U1 Makefile
diff --git a/Makefile b/Makefile
index 3e5877f..b1da5fa 100644
--- a/Makefile
+++ b/Makefile
@@ -62,3 +62,3 @@ MAKE_CMD_ANDROID = $(MAKE) -C $(ANDROID_DIR)
 
-all:
+all: $(SRC_DIR)/.depend
        @$(MAKE_CMD)
@@ -119,3 +119,3 @@ tags:
 
-depend dep:
+depend dep $(SRC_DIR)/.depend:
        $(MAKE_CMD) depend

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Wed Nov 11, 2020 7:43 am
by filbo
(mat: should be sufficient for you to run `make dep; make`)

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Wed Nov 11, 2020 11:10 am
by mat
filbo wrote: Wed Nov 11, 2020 7:43 am (mat: should be sufficient for you to run `make dep; make`)
It works! :D
Thank you, filbo :)

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Wed Nov 11, 2020 2:20 pm
by Holger
Hi filbo, thank you for sorting things out for mat! :D

Yes, that's right: When modifying the code, you should use "make clean ; make" to rebuild, as there are only rules to rebuild if source files are newer than the compiled (object file) counterparts, but not for source and header files. To prevent having to use "make clean" every time (and always rebuild everything), there's "make depend" (or "make dep") to create the header file dependencies (which should be called every time you change the "include logic" of the header files, that is, change "#include" statements in source files or add or rename header files).

Sorry, mat, that I forgot about this and was thinking about a potential bug in your code! :shock:
I know it's labeled a 'development target', but if someone is compiling at all, they're basically doing development!
When just building the binary from source (without modifying the code itself), you do not need the "make dep".
And I think if they were merely downloading your source tarball and running `make`, this could still screw them up. Suppose tarball 1 and 2 differ only in some header file. They extract tarball 1, `make`, extract 2, `make`, and nothing is compiled. Unless the tarball is created with all timestamps set to tarball-creation-time, which they shouldn't be.
Not sure about the last sentence, but when creating a tarball (like using "tar cvzf foo.tar.gz foo"), the source files always have timestamps that are older than your currently built object or binary files, so "make" will never rebuild the code then (after having extracted a different tarball). So, when extracting different versions over another (which I would rather not do, to prevent a mixture of unused files that do not belong to the current code base), you should always do a "make clean" to ensure everything will be correctly built.
This seems to do it. (It's incomplete as it doesn't check for src/*/.depend, but that doesn't seem important):
This looks like a good solution, thanks! (That way, a manual "make dep" would only be required in the above cases.)

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Thu Nov 12, 2020 1:56 am
by filbo
With the sequential tarball situation I described:

If the user downloaded tarball 1 right around when it was issued, and immediately built from it, their .o files would be dated then. If tarball 2 was issued later than that, presumably at least one source file would be newer, so a new `make` after extracting it on top of the old built directory would rebuild things.

You are right that this whole setup is rickety. If they did the same steps, but were downloading from a historical archive, all of the files in tarball 2 would be older than their .o files, and nothing would be rebuilt.

But for normal simple user operation -- downloading each tarball as you announce it, building, playing, repeating when you announce a new one -- the simple procedure should work. Except won't, if they haven't done `make dep` and you changed any structure shapes in a header.

Noted: this Makefile change is not in 4.3.0.4; nor changes to make 'autowhat' work on levelsets with spaces in their names. I feel like there are probably a couple of other patches or ideas I've sent which I expect to come out in the code eventually. Of course there is no obligation for you to do any of those things! I 'expect' in the sense that past occurrences lead me to believe that they will eventually arrive...

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Sat Nov 14, 2020 5:59 pm
by Holger
If the user downloaded tarball 1 right around when it was issued, and immediately built from it, their .o files would be dated then. If tarball 2 was issued later than that, presumably at least one source file would be newer, so a new `make` after extracting it on top of the old built directory would rebuild things.
Yes, that scenario is somewhat plausible, so something like that might indeed happen.
But for normal simple user operation -- downloading each tarball as you announce it, building, playing, repeating when you announce a new one -- the simple procedure should work. Except won't, if they haven't done `make dep` and you changed any structure shapes in a header.
That's right, but this should not happen, as each package has its version number in the extracted directory name, so extracting into the previous version's directory should never happen (unless you rsync files to the old directory, which sounds crazy ;-) ).

So, if you download, extract and build by typing "make", everything should be fine.

Developing is another thing, and I should point out the need for executing "make dep" whenever header files have changed.
Noted: this Makefile change is not in 4.3.0.4;
No, it was too late for it, as 4.2.0.4 was already on its way to be released (and I try to not add any "last minute" changes anymore, as this went wrong too often). I also think about moving it from "Makefile" to "src/Makefile"...
nor changes to make 'autowhat' work on levelsets with spaces in their names
Sorry, haven't answered that PM yet... I'm afraid that supporting level set names with spaces for the "autoplay", "autotest" etc. commands is not possible in a clean way, as I think that it would currently be impossible to find a generic solution instead of an ugly workaround that works for only some level set names. Just think of commands like "autotest Mines 4 Freaks 001 002 003" or "autotest Fahrenheit 451 001 002 003"... It just does not work in a reliably detectable way, unfortunately. Call me oldschool, but I generally still avoid special characters in file and directory names if there is a probability that I will frequently use them on the command line... that's also the reason for my level set directory name convention like "rnd_cool_caves_01" instead of "R'n'D Cool Caves 01" (which works generally fine with R'n'D, but would need some sort of escaping for the current command line logic).

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Mon Nov 16, 2020 11:12 am
by filbo
For whatever reason, my system's Zelda levels are named 'Legend Of Zelda' (2), so autowhat fails on them. This may be a misconfiguration retained from some years-ago download of the LoZ levels as separate data, possibly not from artsoft.org. Thus my efforts to 'fix' RnD to handle it.

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Mon Nov 16, 2020 12:26 pm
by Holger
As far as I remember, the "original" Zelda level set directory names (and also those from the packages downloadable from artsoft.org) are "zelda" and "zelda2", but I know that there are quite some sets (some downloadable from bd-fans.com, for example) that do contain directory names like "My Cool Levels From 2020" etc., and while this should generally be no problem for "normal users" (which just play levels), it is a problem for the "autotest" (and related) command line tools for which I do not have a proper solution for now.

A solution could be to extend the command line syntax to something like the following:

Before (does not work):

$ ./rocksndiamonds -e "autotest My Cool Levels From 2020 001 002 003"

After (would work):

$ ./rocksndiamonds --command "autotest" --level-set "My Cool Levels From 2020" --levels "001 002 003"

Until then, just renaming the level set in question (to "My_Cool_Levels_From_2020", for example) seems to be the quickest and easiest solution...

Re: Segmentation fault (core dumped) when adding new setting to Setup

Posted: Mon Nov 16, 2020 11:34 pm
by filbo
Renaming levelsets means also renaming ~/.rocksndiamonds/tapes/... directories and I'm not sure what else. Partly I wasn't sure if there were any *.conf files inside the levelset which might break if changed. (Of course I could/should have looked into that...!)

The change I submitted would allow levelsets with spaces in their names, as long as they didn't have space-digit; so the two I'm concerned about, 'Legend Of Zelda' and 'Legend Of Zelda II' would be fine (since luckily the '2' is in Roman Numerals!)

Anyway, since my levelsets with spaces in their names are basically a configuration bug, forget about it, I'll just rename things.