libretro / flycast

Flycast is a multiplatform Sega Dreamcast emulator. NOTE: No longer actively developed, use upstream repo for libretro from now on - https://github.com/flyinghead/flycast
http://reicast.com
GNU General Public License v2.0
155 stars 77 forks source link

gdi.cpp load_gdi corrupts track paths when trying to join them to current dir #13

Closed i30817 closed 6 years ago

i30817 commented 7 years ago

edit: see comment https://github.com/libretro/reicast-emulator/issues/13#issuecomment-374440729

Using tosec-iso verified rips, this core crashes early and often if almost any game i try is ran from the command line.

Usually the crash is some variation of `Loaded /home/i30817/.config/retroarch/bios/dc/dc_flash.bin as nvram

Using Recompiler Sh4 Reset recSh4 Init @@ ngen_ResetBlocks() Freeing fpcb

GDI : 3 tracks file[1] "track01.bin": FAD:0, CTRL:4, SSIZE:2352, OFFSET:0 RetroArch [libretro INFO] :: Verify Failed : file!=0 in RawTrackFile -> core/imgread/common.h : 269 DEBUGBREAK! `

For not compressed games. The file 'track01.bin' obviously exists and is in the same dir as RA was started. Anyone else experiencing this bugginess?

inactive123 commented 7 years ago

It works fine on my Linux machine. You probably are missing the right BIOS files at the right location.

i30817 commented 7 years ago

not really, i can boot bios fine (if i provide a 'not existing' gdi file). bios is sitting fine and writable on ~/.config/retroarch/bios/dc e10c53c2f8b90bab96ead2d368858623 dc_boot.bin 0a93f7940c455905bea6e392dfde92a4 dc_flash.bin

Also i tried both recompilers. shrug.

The path to the gdi do have spaces, but i assume no program is barbaric enough to not work because of that nowadays. I'll try anyway.

edit: nope.

i30817 commented 7 years ago

I believe i found generally where the bug might be. Maybe.

This only happens if i try to start the game (like i do on the other cores) from the cmd line instead of the RA GUI.

retroarch -v -L /usr/lib/x86_64-linux-gnu/libretro/reicast_libretro.so disc.gdi crashes right away with a message like above.

retroarch -v -L /usr/lib/x86_64-linux-gnu/libretro/reicast_libretro.so disc.g shows the bios menu (bogus name apparently required to bypass RA 'don't do anything' behaviour.

and simply retroarch and loading from 'load content' shows the bios and after setting the date loads the game.

Some games still crash after the Dreamcast splash (Tomb Raider Last Revelations for example) but the message is different. The TOSEC I used to validate is from TOSEC - DAT Pack - Complete (2444) (TOSEC-v2017-04-23).zip, if that is important.

Hope it's reproducible on your end, and that it helps.

i30817 commented 7 years ago

BTW, I believe i've found the reason the tombraider games won't work (which is not what this issue is about but i mentioned it). As is mentioned on emugen only Demul and Makaron can run optimized Windows CE that some Dreamcast games used to interface with the console (directx...). Since two of those are the tomb raider games, that mystery is solved (probably). Needs to be fixed upstream and is probably a lot of work.

crocket commented 6 years ago

Does this issue still exist? Compile the latest commit or install the latest stable version to check it out.

i30817 commented 6 years ago

I'm actually unsure if it's the same problem now because i converted my dreamcast games to .chd and now they crash even from the menu. To remove variables I extracted a game (record of the lodoss war, which i know booted before with the TOSEC style dumps that were converted to chd) with chdman (into a toc and a bin). From the menu i found out that toc isn't actually a recognized extension by the reicast core, and converted the toc file with toc2cue (in spite of the warnings).

Now, the converted cue from the cmd line: https://gist.github.com/i30817/2fd26680b2f6fb03a792f6b26a4f3f89

the crash from the menu and loadcontent with the reicast core: https://gist.github.com/i30817/972f09ffc271faddf453b2a207ddad66

It still crashes in both, so as i said, i'm unsure if it was chdman screwing up.

BTW, both bios files are present in the core info with the right crcs,

Redream core actually can't start too (for other reason [ERROR] Error(s): /usr/lib/x86_64-linux-gnu/libretro/redream_libretro.so: undefined symbol: descramble).

crocket commented 6 years ago

Did you compile the latest commit of reicast-libretro?

crocket commented 6 years ago

I converted a .gdi file to chd by executing sdlmame-chdman createcd -i xxx.gdi -o xxx.chd, and reicast-libretro crashes when it loads chd. But, it can load the original .gdi file and its associated bin files.

i30817 commented 6 years ago

just installed from the ppa. I can't compile a big project like RA atm.

The logs say which version of RA ([INFO] RetroArch 1.7.1 (Git 9721c7c)) was used, though not the core except which .so it tried to load.

i30817 commented 6 years ago

Yeah, i no longer have the original .gdi.

Though some of the .chd files convert back into .gdi+bin+raw so those might work, without that other bug interfering, i'll try it out.

edit: this seems to be caused by the extension that chdman -o (out) parameter has. If it has no extension it creates a TOC file, if it has a gdi, it creates a gdi.

i30817 commented 6 years ago

Ok, crazy taxi this time. Converted to gdi with -o ct.gdi as said on previous post

cmd line crash: https://gist.github.com/i30817/81326477d8397fdd80d8aa0e9114213c

works fine from menu load content: https://gist.github.com/i30817/1d3a47f28260008ee7793d30bfd8fa5f

bonus, chd crash cmd line: https://gist.github.com/i30817/d47e52c15d2f4eb2ab64fa73a8474b56

crash loading chd from the GUI as load content: https://gist.github.com/i30817/dd87816b140a20349196b657382ffe58

Seems like chd is not working here at all and the situation hasn't changed for the original bug.

crocket commented 6 years ago

Is this a duplicate of https://github.com/libretro/reicast-emulator/issues/19?

i30817 commented 6 years ago

I don't think it's a duplicate. The original bug refers to bare gdi files not working on the cmd line and working from the GUI and i that didn't change, i just noticed that gdi is not working at all too.

The ppa i'm using is the testing ppa: http://ppa.launchpad.net/libretro/testing/ubuntu

my machine is xenial so the last change should be (if i'm not mistaken): 2018-03-19 10:01

if you want to check the current code. I updated the machine with apt-get update ; apt-get upgrade yesterday.

i30817 commented 6 years ago

edit: i used apt-get update; apt-get upgrade again and the main RA program updated, testing again for if the recent X11 fix affected this (because the cmd line crash is apparently X related

Protocol error: bad 3 (Window); Sequence Number 11

Nope. Tested it and same error.

crocket commented 6 years ago

Is this a duplicate of https://github.com/reicast/reicast-emulator/issues/1084?

In any case, I think you should compile the latest possible version of reicast-libretro and launch a game with it.

Because I'm on Gentoo Linux, compiling the latest commit is as easy as executing emerge games-emulation/reicast-libretro-9999. -9999 means the latest git commit.

i30817 commented 6 years ago

No. The test files from the converted chd to gdi don't have spaces, being named ct.gdi, ct01.bin, ct02.raw, ct03.bin.

But to make sure, i removed the space on the path: /media/i30817/Huggin/Downloads/Untitled\ Folder/

didn't make a difference.

This seems like a linux and possibly X.org specific error ( assuming the Protocol error message is a cause not a symptom ) so if you're trying to duplicate it on windows, it's quite likely you won't be able to reproduce.

crocket commented 6 years ago

What is the exact version of your reicast-libretro? Does your gdi file point to non-existent bin files?

My reicast-libretro was compiled from https://github.com/libretro/reicast-emulator/commit/fff2eb2f92251db82cb763d3ab1d19be4a705f95

i30817 commented 6 years ago

Core information on the GUI doesn't have the version, but when you load the core it says on the lower left: 1.7.1 - Reicast 0.1

(which i highly doubt is accurate).

The loaded .so md5sum is: 02637c1582e741102fa000f324fa97da /usr/lib/x86_64-linux-gnu/libretro/reicast_libretro.so

aptitude show libretro-reicast shows:

Package: libretro-reicast
State: installed Automatically installed: no Multi-Arch: same Version: 0.1-r201802121201-fff2eb2-7~ubuntu16.04.1 Priority: optional Section: games Maintainer: Sérgio Benjamim sergio_br2@yahoo.com.br Architecture: amd64 Uncompressed Size: 20,2 M Depends: retroarch | libretro-frontend, libc6 (>= 2.15), libgcc1 (>= 1:3.0), libgl1-mesa-glx | libgl1, libstdc++6 (>= 5.2) Description: Libretro wrapper for Reicast This wrapper makes Reicast API compatible with libretro, thus allowing its use with libretro frontends, such as RetroArch.

Reicast is a Sega Dreamcast emulator, based on the old nullDC.

20180212 should be a date, from the ppa recipe.

crocket commented 6 years ago

I suspect the deb package was built improperly. You should try building reicast_libretro.so on your machine.

The build instructions are on README.md. It's easy.

After make, execute retroarch -L /path/to/your-reicast_libretro.so /path/to/game

i30817 commented 6 years ago

We shall see. I'm building now.

Anyway to build with debug symbols to see what's happening if it does crash (and does that need a RA debug build in addition to the core build, which is something i'll probably not bother with?)

edit: same similar crash (no protocol window error this time). How do i build debug again?

i30817 commented 6 years ago

GDI : 3 tracks file[1] "ct01.bin": FAD:0, CTRL:4, SSIZE:2352, OFFSET:0 [libretro INFO] Verify Failed : file!=0 in RawTrackFile -> core/imgread/common.h : 269

It's the common error. Which doesn't make much sense. It parsed the .gdi alright to have found it has 3 tracks, but can't find the first track? this track exists (before you think i'm a total idiot). And it loads from the GUI. Maybe i should modify the .gid file to add a './' before the track name to handhold this?

edit: didn't help, neither did changing it to a absolute path. Something is seriously wrong here. Maybe the destructor of the object that contains the file was triggered and the error is a unreported and ignored error previously.

i30817 commented 6 years ago

I tested all you wanted:

  1. it's not a missing bios issue, i used the same XDG_CONFIG_HOME in all the logs (as you can confirm by checking the first line) and i verified on the GUI that dc_boot.bin and dc_nvmem.bin have the right md5sum and the logs themselves confirm the files were found.
  2. it's not a 'ppa build error' because i built the thing and loaded the built .so with -L and it has the same issue.
  3. it's not a gdi error because i tested the same gdi on the GUI and the cmd line and in the first it worked and in the latter it didn't.
  4. it's not a path name, filename in the .gdi or space error in path.
  5. i created a new config by omitting XDG_CONFIG_HOME (there wasn't anything on ~/.config/retroarch since i had long deleted it for filling that partition), copied the bios files there, edited the retroarch.cfg system path to point to the bios file, everything as close to 'reset' as possible. The same error occurred.

So i'd ask that you believe that the error exists, and isn't my or the ppa build fault. You can't really reproduce it yourselves? it's easy, just start a .gdi from the console with the core loaded with -L

retroarch -L /usr/lib/x86_64-linux-gnu/libretro/reicast_libretro.so game.gdi should do it (or whatever path to your built reicast_libretro.so file).

It's also not something that just happens in 'some' games, though i don't want to extract many more chd to (re)confirm that

crocket commented 6 years ago

When I tried to load soulcalibur gdi on command line, I saw the following error message.

SIGSEGV @ 0x556e6ac7d35f (signal_handler + 0x0xffffd64869d28e1f) ... 0x7f25eb834000 -> was not in vram
[libretro INFO] Fatal error : segfault
 in signal_handler -> core/libretro/common.cpp : 391
DEBUGBREAK!

I executed the command again, and the game runs.

i30817 commented 6 years ago

That's pretty different from here. In my case it's consistently that 'Verify Failed : file!=0' which from the code seems to indicate that the given track file is null (which is complete nonsense because in the log line exactly before it showed the filename of the track file - and this filename is derived from the .gdi not direct user input).

Any way to build debug to see if something else is shown? I doubt a backtrace will show anything useful (because it's a null check) but...

i30817 commented 6 years ago

Ok i found something messing around. I introduced some prints to see what's what and it didn't make a difference, but something i did by accident did.

if i start the game with: retroarch -L reicast_libretro.so ../ct.gdi (that is, with the game .gdi not in the same directory as the current dir) it loaded fine from the cmd line. If i start the game with: retroarch -L reicast_libretro.so ct.gdi (i copied the built reicast so to the game .gdi dir to make sure it wasn't that), or retroarch -L a/reicast_libretro.so a/ct.gdi (from one dir above) it crashed as usual.

Soooo, this crashing only occurs if you're trying to start from the cmd line with the current directory or dir above the same as the game gdi. This honestly looks like a buffer mangling in path strings.

crocket commented 6 years ago

I executed retroarch -L /usr/lib64/libretro/reicast_libretro.so ./xxx.gdi, and it crashes.

It seems reicast and reicast-libretro have issues with file path handling.

Reicast removes spaces in file names before reading files, so I can't open files whose file paths contain spaces. reicast-libretro cannot load gdi files in the current directory.

I think you should rename the issue.

i30817 commented 6 years ago

Well, what do you suggest. It's not a 'space in path' problem - since the .gdi path has no spaces, it's a 'path handling is FUBAR' problem.

BTW, it's likely that (this) problem is on: reicast-emulator/core/imgread/gdi.cpp:Disc* gdi_parse(const char* file)

There is a lot of path manipulation going on other.

It pisses me off that i have to do make clean in order to have changes i code reflect on compilation too (or maybe it was just me screwing up). Other projects i've seen could build incrementally and changes in the code files would be reflected on the next make without make clean, annoying.

i30817 commented 6 years ago

Ok watch this screw up. I added a very simple print on the end of the gdi_parse function on gid.cpp at line 93 (after the strcpy here) strcpy(pathptr, track_filename.c_str()); printf ( "FILE: %s\n", path); t.file = new RawTrackFile(core_fopen(path),OFFSET,t.StartFAD,SSIZE);

(pathptr is a aliasing of end of path : char* pathptr=&path[len]; I think the idea is to join the relative path here, but it's completely screwed up.

Guess what retroarch -L reicast-emulator/reicast_libretro.so ct.gdi outputs on this? If you guessed FILE: ct.ct01.bin, you're right.

Anyway, as expected entering the core build dir and doing instead retroarch -L reicast_libretro.so ../ct.gdi has FILE: ../ct01.bin output and doing the opposite and going a dir above retroarch -L a/reicast-emulator/reicast_libretro.so a/ct.gdi has FILE: a/cct01.bin

So... yeah, someone fix this.

crocket commented 6 years ago

To catch the attention of developers, you should rename the issue so that they can understand the issue right away.

i30817 commented 6 years ago

Problem seems to be this piece of code:


    char path[512];
    strcpy(path,file);
    size_t len=strlen(file);
    while (len>2)
    {
        if (path[len]=='\\' || path[len]=='/')
            break;
        len--;
    }
    len++;
    char* pathptr=&path[len];

This seems like a really really terrible 'multiplatform' way to join relative paths (the path of the gdi to the path inside the gdi for the track). Why the >2 limitation (except if it's trying to halfass 'C:something' which i'm not even sure it's a viable path)? How does this work if someone is a idiot and passes a directory to the core? What happens if the gdi has a path separator that is different from the current platform? What happens if someone passes a path > 512? Why the terrible space skipping in the rest of the code? Does that even work if the .gdi has spaces? Are the other readers just as broken? etc. Please delete this whole code and try again (though i'd be ok with just fixing the loop, the other problems are severe).