Fix incorrect number of elements being passed to renderScreenSpaceTriangles

[fzurita]

I caught this while using ASAN (https://developer.android.com/ndk/guides/asan) to try to catch memory issues.

Without this fix, we are accessing more vertexes in m_dmaVertices than there are available. renderScreenSpaceTriangles seems to expect elements rather than vertexes.

[fzurita]

There is another issue that I found with ASAN:

https://github.com/gonetz/GLideN64/blob/e9a6f258afd7988f64a990d49be545219fb71e4c/src/Textures.cpp#L1123

We are reading way past the end of TMEM in at least one case.

The one case I found where this happens is in the Mario Tennis screen right before the match start where they show players side by side. We are going about twice the size of TMEM

It almost seems like pLine here should be replaced with something derived from tmptex.width: https://github.com/gonetz/GLideN64/blob/e9a6f258afd7988f64a990d49be545219fb71e4c/src/Textures.cpp#L1116

[fzurita]

Both of these commits are just to point out the memory access issues. Please feel free to ignore them and provide more correct fixes if necessary.

[gonetz]

I made proper fix for the issue with renderScreenSpaceTriangles : fd29c43 However, I can't reproduce the problem with out of bounds TMEM read. I have bounds check enabled for debug build, but it does not report about any issue "in the Mario Tennis screen right before the match start where they show players side by side".

[fzurita]

I have updated the pull request with the code I'm running to debug the issue.

This is the backtrace when the problem is detected by ASAN:

#0 0x2898e4 /lib/arm64/libmupen64plus-video-GLideN64.so
GetRGBA5551_RGBA5551(unsigned long long*, unsigned short, unsigned short, unsigned char)
        /src/Textures.cpp:214:12
#1 0x292f60 /lib/arm64/libmupen64plus-video-GLideN64.so
TextureCache::_getTextureDestData(CachedTexture&, unsigned int*, graphics::Parameter, unsigned int (*)(unsigned long long*, unsigned short, unsigned short, unsigned char), unsigned short*)
        /src/Textures.cpp:0:0
#2 0x298370 /lib/arm64/libmupen64plus-video-GLideN64.so
TextureCache::_loadAccurate(unsigned int, CachedTexture*)
        /src/Textures.cpp:1486:3
#3 0x29c67c /lib/arm64/libmupen64plus-video-GLideN64.so
TextureCache::update(unsigned int)
        /src/Textures.cpp:1927:3
#4 0x238ff4 /lib/arm64/libmupen64plus-video-GLideN64.so
GraphicsDrawer::_updateTextures() const
        /src/GraphicsDrawer.cpp:631:20
#5 0x23940c /lib/arm64/libmupen64plus-video-GLideN64.so
GraphicsDrawer::_updateStates(DrawingState) const
        /src/GraphicsDrawer.cpp:663:3
#6 0x23dde4 /lib/arm64/libmupen64plus-video-GLideN64.so
GraphicsDrawer::drawTexturedRect(GraphicsDrawer::TexturedRectParams const&)
        /src/GraphicsDrawer.cpp:1214:4
#7 0x22ea80 /lib/arm64/libmupen64plus-video-GLideN64.so
gDPTextureRectangle(float, float, float, float, int, short, short, float, float, bool)
        /src/gDP.cpp:932:9
#8 0x2698c4 /lib/arm64/libmupen64plus-video-GLideN64.so
_ProcessDList()
        /src/RSP.cpp:59:3
RSP_ProcessDList()
        /src/RSP.cpp:182:3

And this is the logging that I added in the pull request when the condition is detected:

2022-01-16 09:24:55.364 16587-16621/? E/GLideN64: Textures.cpp:1126,ERROR, "tx=1023 tmemfinal=325 tmem=256 pline=69 ty=1 x=1024, clamp=1023, final=580"

Video: https://drive.google.com/file/d/1Wtlst7cMp690lFT104JEHRGUYqYnHtz1/view?usp=drivesdk

The game exits when the issue is detected.

[gonetz]

I have updated the pull request with the code I'm running to debug the issue.

Cool, thanks! I'll try it.

[gonetz]

Yes, I caught the out of bounds read with your check, exactly at the same place as in your video. MS bounds checker sucks :( The problem itself is quite unusual. It seems that the game developers tried to implement some kind of frame buffer effect here. The game sets texture images to current color and depth buffers and renders texrects, which cover all the screen. However, gDPSetTextureImage calls do not followed by any texture load commands, see attached log. That is TMEM is not updated, at all. All texrects render the same images. Plus, the game sets tile sizes for texrect tiles to 1024x256 and does not change it to the end of that "effect". The plugin tries to load texture of that size and eventually reads out of TMEM range. I think, it is very rare case. I added simple check for texture size, which should prevent such situations: 7417144f6 log.txt

[fzurita]

I don't see a need to keep this open anymore. All issues seem to be fixed.