Windows crash on unknown discs and error otherwise

[MarcroSoft]

Hi. I am having trouble ripping on windows. If the disc is in the database, it downloads the covers, and then Unable to open "(data)": Invalid data found when processing input!
If the disc is not in the database, it just crashes after printing the line tracks: With unknown discs it creates the folder and writes an empty log file in there, with known discs it doesn't make the folder. Any ideas? Thanks, Marc

[cyanreg]

Does this happen with nightly or 0.9.0?

[MarcroSoft]

It crashes with both of them.

[cyanreg]

Does it crash if you use the -N -A -U flags? If it doesn't, could you check which one stops it from crashing?

[MarcroSoft]

It crashes with all of them.

[cyanreg]

Any chance you could run this under a debugger and see where it crashes? If not, @wiiaboo, think you could take a look at it?

[cyanreg]

ping

[MarcroSoft]

Yes?

[wiiaboo]

Any chance you could run this under a debugger and see where it crashes? If not, @wiiaboo, think you could take a look at it?

Haven't touched any of this (compiling/debugging) in years. Compiling current master or 0.9.0 and testing with a mounted .cue/.wav always seg faults:

winpty gdb --args ./src/cyanrip.exe -d i: -s 0 -o flac -UAN

(gdb) r
Starting program: E:\builds\ab\build\cyanrip-git\build-64bit\src\cyanrip.exe -d i: -s 0 -UAN
[New Thread 32616.0x79f8]
[New Thread 32616.0x13a0]
[New Thread 32616.0x1c14]
Checking i: for cdrom...
                CDROM sensed: DiscSoft Virtual          1.0

Opening drive...

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffddb56950a in msvcrt!_aligned_offset_realloc () from C:\WINDOWS\System32\msvcrt.dll
(gdb) bt
#0  0x00007ffddb56950a in msvcrt!_aligned_offset_realloc () from C:\WINDOWS\System32\msvcrt.dll
#1  0x00007ff6317ed965 in av_bprint_finalize (buf=buf@entry=0x5fb1a0, ret_str=0x5fb160) at src/libavutil/bprint.c:243
#2  0x00007ff63173761d in crip_get_path (ctx=ctx@entry=0xce6100, type=type@entry=CRIP_PATH_LOG,
    create_dirs=create_dirs@entry=1, fmt=<optimized out>, arg=arg@entry=0x0) at ../src/cyanrip_main.c:1272
#3  0x00007ff631735253 in cyanrip_log_init (ctx=ctx@entry=0xce6100) at ../src/cyanrip_log.c:327
#4  0x00007ff631c52db9 in main (argc=<optimized out>, argv=<optimized out>) at ../src/cyanrip_main.c:1798

Can't test older releases without also building an older ffmpeg, as 0.8.1 and earlier don't build with 6.x.

[cyanreg]

Thanks a lot for testing. Could you test the new build https://github.com/cyanreg/cyanrip/actions/runs/5032862828/jobs/9026732893 once it completes? It might help.

[wiiaboo]

Seems to segfault as well. My own build has more or less the same backtrace:

Starting program: E:\builds\ab\build\cyanrip-git\build-64bit\src\cyanrip.exe -d d: -s 0 -UAN
[New Thread 12624.0xdd0]
[New Thread 12624.0x7050]
[New Thread 12624.0x16d0]
Checking d: for cdrom...
                CDROM sensed: DiscSoft Virtual          1.0

Opening drive...

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffddb56950a in msvcrt!_aligned_offset_realloc () from C:\WINDOWS\System32\msvcrt.dll
(gdb) bt
#0  0x00007ffddb56950a in msvcrt!_aligned_offset_realloc () from C:\WINDOWS\System32\msvcrt.dll
#1  0x00007ff6ed2ed9e5 in av_bprint_finalize (buf=0x5fb1a0, ret_str=0x5fb160) at src/libavutil/bprint.c:243
#2  0x00007ff6ed23761d in crip_get_path ()
#3  0x00007ff6ed23c793 in cyanrip_cue_init ()
#4  0x00007ff6ed752e49 in main ()

The automated build doesn't have debug symbols, so it's not really useful, but seems to segfault at a different point?

Starting program: E:\cyanrip.exe -d d: -s 0 -N -o flac -UAN
[New Thread 16816.0x7004]
[New Thread 16816.0x1788]
[New Thread 16816.0x3e5c]
Checking d: for cdrom...
                CDROM sensed: DiscSoft Virtual          1.0

Opening drive...

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffddb56950a in msvcrt!_aligned_offset_realloc () from C:\WINDOWS\System32\msvcrt.dll
(gdb) bt
#0  0x00007ffddb56950a in msvcrt!_aligned_offset_realloc () from C:\WINDOWS\System32\msvcrt.dll
#1  0x00007ff6ab33d8d5 in opus_custom_decoder_create ()
#2  0x00007ff6ab23761d in ?? ()
#3  0x00007ff6ab235253 in ?? ()
#4  0x00007ff6ab72fd09 in opus_custom_decoder_create ()
#5  0x00007ff6ab2312ee in ?? ()
#6  0x00007ff6ab231406 in ?? ()
#7  0x00007ffddb037614 in KERNEL32!BaseThreadInitThunk () from C:\WINDOWS\System32\kernel32.dll
#8  0x00007ffddc1826a1 in ntdll!RtlUserThreadStart () from C:\WINDOWS\SYSTEM32\ntdll.dll
#9  0x0000000000000000 in ?? ()

[wiiaboo]

Might also be something wrong with my method/setup (mounting a .cue/.wav using DaemonTools), even an ancient build that I posted to HydrogenAudio is segfaulting. I remember being able to read directly from a bin/cue, but I can't remember how I managed it back then. And I don't really have an actual CD drive I can test with.

[cyanreg]

Does the ancient build crash in the same way?

[wiiaboo]

Yeah, that's why I thought it might be something to with how I'm testing.

[cyanreg]

Had someone else investigate. The crash does happen in what appears to be the same place. I'm guessing Microsoft broke something, maybe? It seems to be AVBprint related, could you investigate?

[wiiaboo]

Ideally it would be someone actually knowledgeable with ffmpeg/Windows C stuff, like rossy, if you could get to him. Can't help there.

[rossy]

I think this call to stat is causing buf.str to be overwritten with an invalid pointer.

https://github.com/cyanreg/cyanrip/blob/c099d425073ee7fcb2035e79128a8cc704e36721/src/cyanrip_main.c#L1263-L1264

The problem is, at this point in the code, the definition of struct stat is different to struct stat and struct _stat64 in os_compat.h, because stat is declared as a macro for _stat64 in mingw64 sys/stat.h, but os_compat.h shadows that macro here:

https://github.com/cyanreg/cyanrip/blob/c099d425073ee7fcb2035e79128a8cc704e36721/src/os_compat.h#L63

So win32_stat and _wstat64 are expecting struct _stat64 with a 64-bit st_size field, but they're given a pointer to the smaller struct stat, with a 32-bit st_size field, causing them to overwrite part of AVBPrint buf.

Not sure what an elegant cross-platform solution to this would look like, but this seems to fix it and allows me to successfully rip a CD in Windows: rossy/cyanrip@46fcf6457cab81f6b517d9ff67bfad6fe39500d0

[cyanreg]

@rossy Any way of not hardcoding the contents of struct crip_stat on Windows?

[rossy]

You could probably do something like rossy/cyanrip@b094a9cf4254293e7205d74dc0d11fd0265d5b09

That relies on the assumption that struct stat is the same as struct _stat64, which is the case when you compile with -D_FILE_OFFSET_BITS=64, but meson always adds that define, so it should be safe.

(Edit: Assuming MSVC compatibility isn't a goal, since official SDK headers don't understand _FILE_OFFSET_BITS.)

[cyanreg]

I think that looks fine. Could you add this as a comment and submit it as a PR?

[rossy]

So I just tested the CI build and it seems like that patch only fixes one of two separate bugs. Now, instead of a crash, I get:

Unable to open "(data)": Invalid data found when processing input!

My own debug builds using libs from the MSYS2 repo are unaffected. Could it be something introduced in FFmpeg git master?

[cyanreg]

Does this happen if you disable coverart lookups?

[rossy]

Nope, no error with -U

[cyanreg]

@rossy thanks, could replicate on linux, should be fixed now, could you check?

[MarcroSoft]

Hi, Thanks a lot on working on this issue, I am not good at c or c++ so I'm probably not to much help other than testing... Should I try the latest windows build or is it still not fixed?

[cyanreg]

@MarcroSoft yes, try the latest windows build (https://github.com/cyanreg/cyanrip/releases/download/nightly/cyanrip-win64-latest.exe which was done 1 minute ago), it should work

[rossy]

That build works for me with cover art.