mupen64plus / mupen64plus-video-glide64mk2

Video plugin for Mupen64Plus 2.0 based on 10th anniversary release code from gonetz
28 stars 37 forks source link

Don't accept n64 textures larger than target 3dfx textures #51

Closed ghost closed 9 years ago

Narann commented 9 years ago

Look ok to me but I would have a quick overview of what this is supposed to fix?

As this situation should never really appear (should it?), or only on custom textures (am I right?), I also suggest to print an informative error message (maybe the texture name).

I'm just afraid some users wouldn't see their textures and have no idea why it fail.

What do you think?

PS: Keep in mind I don't have the whole code in mind so you have a better understanding of the process. I just want to inform the user if the plugin is doing something it is not supposed to do.

ghost commented 9 years ago

This is only for the in rom textures. And this happens when the rdp emulation is incorrect. This would just crash the emulator.

richard42 commented 9 years ago

It would be nice to throw a warning or error debug message when this happens, but there are a lot of places in the code where you added the bailouts. Perhaps a #define macro to check the value for < 0 and throw a DebugMessage and return.

Narann commented 9 years ago

This commit has been reverted back due to #54. Commit: c025a891a142552d45e5bb11583a5b8f6fedb5d3

ghost commented 9 years ago

Why is it better to have a crash?

Narann commented 9 years ago

I guess everybody don't have the crash.

ghost commented 9 years ago

But it so easy to reproduce and I've posted this multiple times (see #30):

CFLAGS="-g3 -fsanitize=undefined -fsanitize=address" OPTFLAGS="" make -C projects/unix all
mupen64plus --gfx ./projects/unix/mupen64plus-video-glide64mk2.so $ROM

The best thing is that I just found another buffer underflow just by starting Kirby and pressing "start". This time because following line in rdp.cpp doesn't check for rdp.ci_count <= 0

if (rdp.frame_buffers[rdp.ci_count-1].status == ci_copy_self && (rdp.timg.addr >= rdp.cimg) && (rdp.timg.addr < rdp.ci_end))
richard42 commented 9 years ago

Sorry, I didn't know about the crash or I would have looked into it deeper.

Narann commented 9 years ago

@conchurnavid created a ticket for it: https://github.com/mupen64plus/mupen64plus-video-glide64mk2/issues/58