Tantei Jinguji Saburo Hai to Diamond crashes with jpn0.pgf

hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.

https://www.ppsspp.org

Other

11.24k stars 2.17k forks source link

Tantei Jinguji Saburo Hai to Diamond crashes with jpn0.pgf #7690

Closed daniel229 closed 3 years ago

daniel229 commented 9 years ago

Remove jpn0.pgf or replace it with the PSP one,it won't crash.it's always happen.

unknownbrackets commented 9 years ago

If you uncomment that debug code, does it trip there?

Although, I guess this is caused by the incorrect sizes of the fonts that come with PPSSPP.

-[Unknown]

daniel229 commented 9 years ago

No,it doesn't trip into the debug code.

unknownbrackets commented 9 years ago

Hmm, does disabling SSE help this time? Just to be sure.

Does it reproduce with #pragma optimize("", off)? The a address looks super invalid, but I'm thinking it's probably just garbage again. That said, idx definitely looks dangerous. If those are real values, that's definitely bad.

-[Unknown]

daniel229 commented 9 years ago

No,disabling SSE and #pragma optimize("", off) won't help,and it also happen in debug build.

hrydgard commented 9 years ago

Okay, so this time it crashes in the debug build too, that's good for debugging.

Looks like idx has become negative somehow. I guess we need even more checks against bad input data (like spline patches that are smaller than 4x4 or something maybe).. can you check spatch.u_count and v_count?

unknownbrackets commented 9 years ago

Yeah, "won't help" is good news. This screenshot except with #pragma optimize("", off) in the code would be awesome:

https://cloud.githubusercontent.com/assets/3481559/7195263/5b31b5d0-e4ef-11e4-8917-6bd98eb0275e.png

Because otherwise any of the variables in the lower left can be lies. So it might not even be negative at all. Turning off optimizations in this file will give us useful numbers.

-[Unknown]

hrydgard commented 9 years ago

The debugger screenshot is of a Debug build, which can be clearly seen. Optimization should already be off.

a looks broken because it's null, which it is because the code read outside the spatch.points array.

daniel229 commented 9 years ago

Add NOTICE_LOG(G3D, "spatch.count_u = %i, spatch.count_v = %i", spatch.count_u, spatch.count_v); right before SimpleVertex *a = spatch.points[idx]; 26:01:675 user_main N[G3D]: Common\SplineCommon.cpp:402 spatch.count_u = 1, spatch.count_v = 1

unknownbrackets commented 9 years ago

Oops, my bad. Didn't realize it was Debug.

Well, yeah, clearly they must be 1 for that to happen then (iv is 0xfffffffd.)

patch_div_s is clamped in that case... but a couple things look questionable with that small a count_u/count_v...

-[Unknown]

hrydgard commented 9 years ago

I simply made it reject patches < 4x4.

daniel229 commented 9 years ago

Still crash,get a different stacktrace

and may get error logs https://gist.github.com/daniel229/90c5a069e1627d5e2eed

https://gist.github.com/daniel229/037e1f850626938816df

unknownbrackets commented 9 years ago

Hmm, looks like garbage. And none of this happens with PSP fonts?

-[Unknown]

daniel229 commented 9 years ago

No,it does not happen with PSP fonts.

hrydgard commented 9 years ago

So, most likely our font is so much too large that some memory gets corrupted and it does bogus gpu commands. The right fix is to fix our font. We should be more resilient, though..

unknownbrackets commented 9 years ago

I agree. For the resilience, we probably need a few more tests. I don't really understand splines or beziers well, though...

In this latest crash, I guess one of the steps became null. fmt = 00000008 - so that means it's one of the nulls in colstep, right? Maybe we should stub those as long as they're possible, to prevent crashes?

327ca4c96eef73b7be9aeff1efe947495dd0220b

-[Unknown]

daniel229 commented 9 years ago

The game still crashes with that change, Stack trace get this one.

>   0000000004cb1278()  未知

unknownbrackets commented 9 years ago

Hmm, no idea what that is. Maybe jit or vertexjit... the trace doesn't go farther?

-[Unknown]

daniel229 commented 9 years ago

No,just get a stack like that address.

unknownbrackets commented 9 years ago

What happens if you change this, in GPU/Common/VertexDecoderCommon.cpp:

if (jitCache && g_Config.bVertexDecoderJit) {

To:

if (jitCache && false) {

Does the crash change? Just wondering if we can get a clean stack.

-[Unknown]

daniel229 commented 9 years ago

Anything useful?

unknownbrackets commented 9 years ago

Well, definitely looks like the source is invalid. ptr_ is not even either, it ends in 9. That's odd, I wonder if we should align vertex addresses to 4.

-[Unknown]

hrydgard commented 9 years ago

I think vertices can legitimately be at odd addresses, as long as all components are byte-sized.

unknownbrackets commented 9 years ago

Ah, right, of course. Well, we still might want to align the pointer to the vertex align size maybe? Should test what really happens...

-[Unknown]

xebra commented 7 years ago

This issue occurs when open the inventory.

From this screen to next screen uljm05529_00000 Crash on this screen uljm05529_00001

I think this issue may relate to using Chinese font instead of Japanese font. Chinese font seems very similar as Japanese one. However it's really different. Chinese Kanji glyph is not same as Japanese one. So I think we need use Japanese font to fix this issue. Also our Japanese font(jpn0.pgf) has no shadow glyphs, so it may be the reason to crash the game. And current our font is very huge.

Anyway, our font is broken, I think it cause this issue. I think this is memory overflow/overwrite issue, so it does not relate to any GPU/CPU issue.

BTW, now I'm developing PGF viewer/converter with GUI on C# based on ttf2pgf to fix any font issue.

nassau-tk commented 4 years ago

@hrydgard @unknownbrackets

I tested on v1.10.3-876 and v1.10.3-878 both on Windows10(64-bit). But,It's not crash both builds version. Reason:v1.10.3-878 was updated the jpn0.pgf to font of Japansese base. But,not related? Maybe? Log has not red code.

@xebra commented on 20 Jan 2017 This issue occurs when open the inventory. From this screen to next screen

@xebra said clash will happen when open the inventory and select the item. But, It's will not happen the case. I'll just report .

(If exists "@xebra's PGF Viewer" then I want it. :smile: )

nassau-tk commented 3 years ago

v1.10.3-1234 I tested latest modified jpn0.pgf. The issue will not happen on this version too.

Should this issue be made Keep OPEN?

Panderner commented 3 years ago

v1.10.3-1234 I tested latest modified jpn0.pgf. The issue will not happen on this version too.

Should this issue be made Keep OPEN?

@nassau-tk still not crashing for real PSP font?

nassau-tk commented 3 years ago

@Panderner

@nassau-tk still not crashing for real PSP font?

What does it means? [This issue will not hapeen if you installed "Orignal PSP font".] I was understanding like above.

And,I tested Original PSP font on PPSSPP just in case. The issue is not happen.

hrydgard commented 3 years ago

Is this still an issue?

nassau-tk commented 3 years ago

Is this still an issue?

I can't reproduce the issue.

unknownbrackets commented 3 years ago

There have been other font related fixes, such as the allocation callbacks. Maybe those changes fixed this.

-[Unknown]

sum2012 commented 3 years ago

v1.10.3-1319-gc546c6032-windows-amd64 test no problem Closing