[Scummvm-devel] Fwd: PS2 : stack madness

Max Horn max at quendi.de
Tue Apr 7 10:38:58 CEST 2009

Hi folks,

below an email Sunmax sent me, with my replies inbetween. Attached is  
also the patch I sent him back then, against the 0.13 branch.

Anfang der weitergeleiteten E-Mail:

> Von: "sunmax\@libero\.it" <sunmax at libero.it>
> Datum: 7. April 2009 07:08:55 MESZ
> An: "max" <max at quendi.de>
> Betreff: Re: [Scummvm-devel] PS2 : stack madness
> Hey Max,
> thank you very much for taking care of this! So here is what I did:
> - rebuilt all my cross-tools and libs from scratch using latest SDK
> - built from a fresh ScummVM 0.13.x with your fix: no luck
> - built from that same branch + your patch : no luck
> - built from that same branch + your patch + re-enabled defaultVal :
>   no luck
>   OTOH the great thing is that now the compiler does not ICE anymore
>   and we don't need a PS2-only workaround -> I like your patch!

OK... It's still not really how I would want the code to look, but if  
it fixes the ICE, it might be worth committing. If I don't hear  
complaints about the patch (e.g. from Tanoku or LordHoto), I'll apply  
it to trunk & branch during the weekend or so.

> As per your suggestion I took the address of a local var just before
> the crash and the address is insane:
> &s = 0x01FFE234  [in a working context]
> &s = 0x0049B7B4  [in a going-to-crash context]
> Both SP and FP are messed up.

Good, that looks indeed like a stack smasher (at least it is unlikely  
that we use 1.8 GB of stack space ... ;). Well, it is of course *bad*,  
but *good* is that the technique seems to work.

Still could be a stack overflow that causes this, though. The stack  
contains return addresses (and saved SP values), after all. When it  
overflows (or gets corrupted in some other way), then when you return  
from the current function, the SP gets restored -- and in this case,  
apparently to a bogus value.

> It would be very useful to have a fully (or some way) working gdb  
> here,
> so I partnered with a Portuguese developer from ps2dev to resurrect  
> it.
> He already did some work on it, together we should have it ready soon.

Cool :)
> Do you think the backtrace is going to be reliable with a messed  
> stack?

Probably not, if the stack *really* is messed. If the stack is not  
messed but "only" overflowed, then GDB might help you catch that. E.g.  
you could set a watchpoint on the SP value, triggering as soon as it  
leaves a certain bound. This way you might be able to either find out  
where it gets smashed/overflown.

> Even with your patch and changing bunch of stuff it crashes a lot in:
> graphics/cursorman.cpp @ 46
> Something that happens before that is causing the stack corruption
>   (see a previous e-mail for my reverse + by-hand backtrace)
> Is it possible that something that changed between 0.12.0 and 0.13.0
> might use much more stack than in the past? Any hint?

Sure it is possible. No idea what that would be, though.

> Did anybody run valgrind with ScummVM on a powerful desktop and tried
> to start COMI?

No idea... let's see if "anybody" is going to answer (don't hold your  
breath... :).

> Why it happens mostly with COMI?

Because COMI uses a lot more RAM than any other SCUMM game? A stack  
overflow happens when the stack (coming from one end of RAM) and the  
heap (coming from the other end of RAM) collide. So if you have lots  
of stack use *and* lots of RAM use, a stack overflows (with resulting  
stack smashing) is much more likely to occur.

> Is the theme handled differently in COMI because of the resolution?

COMI has nothing to do with theme handling, in fact it doesn't even  
*know* what a theme is ;). That's all up to the GUI code. Which indeed  
may do things differently in a different resolution (such as take up  
more memory, since obviously screen buffers require more memory...).

> Is it using more stack?

No idea, but one could test this idea this on a desktop machine with a  
working debugger, I would say... Anybody with a debugger and COMI  
could, in fact ;).

> Can we force COMI to start (well, kinda...) at 320x240 and see
> if this makes a difference?

Not really.

> I am still positive that we are trimming it down and will catch it
> eventually!

Yeah, we'll squish 'em ;)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: gui-drawcallback-hack.patch
Type: application/octet-stream
Size: 4451 bytes
Desc: not available
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20090407/d48583e3/attachment.obj>

More information about the Scummvm-devel mailing list