segfault while running actfancr (and clones)

[barbudreadmon]

• last output before issue :

RetroArch [libretro INFO] :: Initialize DIP switches. RetroArch [libretro INFO] :: 'Coin A' (4) RetroArch [libretro INFO] :: '(Default) 1 Coin 1 Credits' RetroArch [libretro INFO] :: '3 Coins 1 Credits' RetroArch [libretro INFO] :: '2 Coins 1 Credits' RetroArch [libretro INFO] :: '1 Coin 1 Credits' RetroArch [libretro INFO] :: '1 Coin 2 Credits' RetroArch [libretro INFO] :: 'Coin B' (4) RetroArch [libretro INFO] :: '(Default) 1 Coin 1 Credits' RetroArch [libretro INFO] :: '3 Coins 1 Credits' RetroArch [libretro INFO] :: '2 Coins 1 Credits' RetroArch [libretro INFO] :: '1 Coin 1 Credits' RetroArch [libretro INFO] :: '1 Coin 2 Credits' RetroArch [libretro INFO] :: 'Lives' (2) RetroArch [libretro INFO] :: '(Default) 3' RetroArch [libretro INFO] :: '3' RetroArch [libretro INFO] :: '4' RetroArch [libretro INFO] :: 'Difficulty' (4) RetroArch [libretro INFO] :: '(Default) Normal' RetroArch [libretro INFO] :: 'Easy' RetroArch [libretro INFO] :: 'Normal' RetroArch [libretro INFO] :: 'Hard' RetroArch [libretro INFO] :: 'Hardest' Program received signal SIGSEGV, Segmentation fault.

• this game won't crash on fbalpha2012 (probably because dipswitches are not handled in fbalpha2012)
• "triothep", another game from the same hardware, won't crash

Seems to me this game crash while reading dipswitches, i'm not familiar with this part of the code so it would be really nice if @AkimanBengus could take a look at this issue.

Edit : seems to me like some other games like "armwrest" are suffering the same issue.

[AkimanBengus]

I will try to take a look when I will have time

[barbudreadmon]

I took a look, it segfault on : strncpy(dip_value->friendly_name, dip_value->bdi.szText, sizeof(dip_value->friendly_name)); because dip_value->bdi.szText is null

Adding : if (dip_value->bdi.szText == NULL) continue; seems to solve the issue. @AkimanBengus : Do you think it's okay ?

I pushed this commit : https://github.com/libretro/fbalpha/commit/db69226913551e1468f460fdb0152c49f3085354

[barbudreadmon]

It seems we are supposed to use them :

Those are the default dip settings, for when a game starts that hasn't been started before. Check your parser against ours in burner/win32/inpdipsw.cpp. {0x14, 0xff, 0xff, 0x7f, NULL }, 0x14 is the position # of DrvDips variable in the input structure. one of the next 0xff's is to tell the parser that its the default dips, and one of them is the bitmask (check the struct to be sure), the 0x7f is the actual default dip settings. The description field is usually left NULL for default settings. If you remove this, chances are the game will break. delete config/games/actfancr.ini so it uses the defaults and reload your game and see what I mean.

[barbudreadmon]

Well, games are launching, but i don't think dipswitches for those games are working

[AkimanBengus]

Hello,

I don't think it's correct if I understand good the segfault is causing by the NULL in the dip_value->bdi.szText string when we are parsing the default value. And this is normal that this string is NULL with the pgi representing the default value.

So the line strncpy(dip_value->friendly_name, dip_value->bdi.szText, sizeof(dip_value->friendly_name)); is not necessary for the pgi default value.

But with your code, I think you are skipping the initialisation of the default value So I would do just like this under this code

I'm sorry but I don't think I will have time to this.

if (is_default_value) { dipswitch_core_option_value *default_dip_value = &dip_option->values[0]; default_dip_value->bdi = dip_value->bdi; default_dip_value->pgi = dip_value->pgi; snprintf(default_dip_value->friendly_name, sizeof(default_dip_value->friendly_name), "%s %s", "(Default)", default_dip_value->bdi.szText); } else { strncpy(dip_value->friendly_name, dip_value->bdi.szText, sizeof(dip_value->friendly_name)); }

[barbudreadmon]

Same thing, games are launching, but there are still issues with dipswitches values, pretty obvious in a game like armwrest where "Service mode" became a value for "Rematches".

I wanted to use their code in burner/win32/inpdipsw.cpp to understand how it works, but the amount of windows specific function make it hard to understand for me.

Well, at least for now, those games are playable, and i don't think it breaks dipswitch support in games which worked previously, so i'll wait for some clean fix when you've got some time :).

[barbudreadmon]

My bad, it seems your code is actually the good fix, i see the same values for armwrest dipswitches in fba standalone. Thanks again, i close :).

[barbudreadmon]

I'm reopening this, actually there are still missing dipswitches and wrong default values compared to fba standalone.

[barbudreadmon]

There is more to this issue than i thought : with the fix, alcon still segfault on dipswitches.

[AkimanBengus]

I took a look at the stand alone version for the game 'armwrest' and there is also a problem with the dipswitch 'Rematches' but also with the 'Difficulty'. Is it possible that this issue can come from the code representing the dipswitch in the file 'd_punchout.cpp'

Can you tell me which game is good in fba stand alone and not in libretro ? When I will have time I will look at this game.

When I coded the dipswitch I did know nothing about it. It was only a reverse engineering from the code of the stand alone version. And I'm sure that there still some bugs due to the misunderstanding of the BurnDIPInfo and its values

[barbudreadmon]

I tried a few games up to "C" letter

Games still crashing :

• alcon
• btlkroad

Games launching but dipswitches seems different from standalone :

• actfancr (Demo Sounds + Bonus Life)
• athena (Coin A + Demo Sounds + Bonus Life + Cabinet)
• bermudat (Flip Screen + Coin A)
• calibr50 (Multiple "Licensed To" on standalone)
• canvas (Multiple "Unknown" on standalone)

Also, i asked a few questions to "dink" upstream, perhaps you should ask some too, here is the topic : http://neosource.1emulation.com/forums/index.php?topic=2693.msg22503#msg22503

[AkimanBengus]

Hi @barbudreadmon

I adapted the parser of the DipSwitches to handle some errors present in the BurnDIPInfo description. I corrected the BurnDIPInfo description of the games you provided me. You can see what kind of errors exist by looking at my second commit. I also added some logs to detect all the errors I know, it's possible that there is still unknown things

It will be good to show this to dink to reflect the correction in the upstream.

[barbudreadmon]

dink said thanks :). He also provided more fixes for those drivers.

[AkimanBengus]

You are welcome :-) I hope that for now it will not segfault anymore ! It would be great to be able to launch fbalpha in a batch with all the drivers to see which remain incorrect

[barbudreadmon]

I can, here is the list of dipswitch errors i collect when i run every game in batch mode : http://pastebin.com/raw/iixAc50n Do i submit this list upstream or do you want to check it first ?

[AkimanBengus]

This is not that I don't want, but I haven't many times for this. And I prefer that he look because maybe he will see other errors which I don't know.

About the log, it's important to know that:

• "the line 'n' is useless" isn't very important because it is skipped by both parser (libretro and stand alone)
• "Start of next DIPSWITCH is too early" "'4' values were intended and only '2' were found" These 2 lines happened when the nSetting (nbr of values of a dipswitch) is set with a value greater than the number of line of values. I suppose, in this case, the nSetting is not correct.
• "End of the struct was reached too early" "'4' values were intended and only '2' were found" This is quite the same than above, but it happened at the end of the structure.

I'm worry about this one:

• "'4' values were intended and only '0' were found" I don't know why this happens because the BurnDIPInfo description seems good

[barbudreadmon]

I'm closing, games affected by this issue aren't supposed to crash anymore in fbalpha. Also, dispswitch error logs were sent to upstream and dealt with, .40 will contain the fixes.