AGI: Mixed-Up Mother Goose (DOS) - Segmentation fault on item return (ScummVM 2.8.1+)

[hugoarpin]

Game: Mixed-Up Mother Goose (DOS)
Md5sum: e524655abf9b96a3b179ffcd1d0f79af LOGDIR
Version: Scummvm v2.8.1 (Arch Linux) & commit 3853dd99b5c

Almost every time an item is returned (see attached screenshot), after the song and text, when a cursor key is pressed to move and dismiss the text box, I get a segmentation fault.

I recall testing (and completing) the game in the past (probably v2.8.0 or maybe earlier) and the bug was not present. I have not tested on other OS (ex: Windows), so I don't know if it is Linux specific.

Here is the gdb output of the error (commit 3853dd99b5c):
Thread 1 "scummvm" received signal SIGSEGV, Segmentation fault.
Agi::GfxMgr::render_BlockEGA (this=0x55555d3248c0, x=3, y=-5, width=146, height=42) at engines/agi/graphics.cpp:599
599 curColor = _activeScreen[offsetVisual++];

PS: Sorry if the error is due to a bad configuration on my part, but at this point I can't see it.

[sluicebox]

Hello! Thank you for reporting this. Also, excellent taste in games!

I can reproduce this crash with Windows release 2.8.1, but so far I am unable to reproduce it with anything else. I touched code that affected some of the the rhymes in 2.8.1, so it seems like it's probably going to be that. I can't explain why it's not happening elsewhere.

If you're able to build from source, that might avoid the problem for now. And if this is something that hasn't really been fixed yet, you'd have to do that once we've fixed it.

In the meantime, it could help narrow this down if you remember any other rhymes you've seen crash. You crashed Crooked Man, and I've crashed Hickory Dickory Dock, any others?

Builds I haven't been able to crash:

• Windows daily build
• Local msvc and gcc Windows builds of master
• Local gcc Windows build of branch-2-8-1
• Local Mac build of branch-2-8-1
• Local Mac build of master
• Mac release 2.8.1

[hugoarpin]

Hi sluicebox,

Thanks for the help! I have learnt of you through the impressive OneShortEye videos. I did not know you also worked on the AGI engine (on top of SCI), amazing!

I have built the recent commits 3853dd99b5c yesterday and 0856bf2003e this morning, both of them segfault on my Arch Linux system.

But interestingly, Jack Horner didn't crash (pie to boy in house at lower right), but Banbury Cross did (horse to town centre).

So as you said, it seems to only affect certain rhymes. I will try to give you an exhaustive list of the crashes tonight (hoping that I will have enough free time).

I have also built commit bc8550ce02a, which is the one you linked to inside the ticket #13820 and it also crashed at the line mentioned in my previous message (engines/agi/graphics.cpp:599 curColor = _activeScreen[offsetVisual++]).

However, when I built the commit just before (8f2127e4184), the text box was a bit messed up because the refactor was not yet complete, but it did not segfault, so it seems that bc8550ce02a could be the cause.

I hope the seemingly somewhat platform-specific nature of the bug will not be too difficult to hunt/fix! Linux can be such a pain sometimes...

PS: I tried this game(s) because it showcased so many engine evolutions (AGI / SCI dithered EGA / SCI VGA talkie / SCI deluxe 640x480). But I must say that I found it quite charming and that it is still a beautiful game for young boys & girls nowadays IMO!

[sluicebox]

Thank you for reminding me that I said I'd pick him up from an airport!

I thought it would be fun to learn AGI this year, and it was, but I also broke this so maybe not *that* amazing =)

The rhymes that crash are the ones from #13820:

• Hickory Dickory Dock
• Crooked Man
• Jack Sprat
• Old Woman Who Lived In A Shoe
• Banbury Cross

Their rhyme text boxes are so big that Sierra placed them over the menu bar up top. That's so unusual that, according to a scan I did, it doesn't happen in other AGI games. The authors of our AGI code understandably didn't expect that, so those message boxes didn't work. I changed the graphics code so that it would draw, but now I see that the code that clears the message box also needs adjustments.

Thanks for bisecting this. This was the first real computer game I ever played. Now I "just" need to sit down with a lot of scratch paper, draw some diagrams, and do some children's arithmetic. (we're doomed!)

[hugoarpin]

Hahaha, don't forget Shorty or he'll make a video to slander you! :p

I can see that you are already way ahead of me for testing the issue. Thank you for explaining simply to me the technical details about the text boxes and menu bar!

I've just built the older v2.8.0 and compared with DOSBox, your work is a clear improvement, as the text box for these rhymes was obviously messed up in ScummVM before!

Still puzzling as to why it occurs (mostly) on Linux, as it seems the issue is deep inside the AGI engine and not so much because of some system interaction, weird!? Oh Linux...

Anyway, I hope this will not take too much of your time and sorry I can't help more, as it would probably take me days (or weeks) to get familiar enough with the AGI engine to be useful!

But if you ever want/need me to test anything on my (wonderful?) Arch Linux system before committing to the ScummVM repo, please send me the patch!

[sluicebox]

It's true, he can cancel me with one star wipe, and rightly so.

I am taking you up on your generous offer. Can you please try applying the attached patch? I have high confidence it will fix the crash.

[hugoarpin]

Hi sluice,

I was able to complete consecutively the 5 rhymes you mentioned previously without any crash, good job man!

However, I don't know if it was bad luck, but once at the end of the old shoe rhyme, scummvm just quit. Weirdly it exited with a return value 0 (EXIT_SUCCESS) and without any segfault.

But it did trigger that warning before stopping:
WARNING: render_Block ignored by clipping. x: 3, y: 131, w: 154, h: 42!

Which looks like it came from the line:
engines/agi/graphics.cpp:515: warning("render_Block ignored by clipping. x: %d, y: %d, w: %d, h: %d", x, y, width, height);

Sorry I was busy last night, but I will try to test more exhaustively tonight/tomorrow, thanks!

[sluicebox]

Thanks for testing, the segfault should be properly fixed now.

​https://github.com/scummvm/scummvm/commit/cb0db51091ddcb67de95bfaa5e0f94a4f619db6d

I don't know how to explain the program suddenly quitting with a success status, but that seems like a different thing, assuming the program wasn't terminated by something external on your machine or a bumped hotkey. Can you try shoe'ing a few more times with the latest code? If it turns out to be a thing, please open another ticket.

Thanks again for reporting!

[sluicebox <22204938+sluicebox@…>]

In cb0db51:

AGI: Fix closing window when overlapping with menu

Fixes out of bounds memory access when closing a window that overlaps
with the menu bar at the top of the screen. This crashed at least some
release builds, including Windows release 2.8.1.

Mixed-Up Mother Goose is the only game known to draw a text window over
the menu bar. When I recently added support for this, I missed that the
code for closing the window needed to be updated too:
bc8550ce02a036e3757d817e963e038463844412

Fixes bug #15241

Big thanks to @hugoarpin for reporting this and testing the fix.

[hugoarpin]

Just tested cb0db51 as best I could, doing many different rhymes combo and a full playthrough, everything works perfectly as far as I can tell!

Maybe I just hit ctrl-q instead of an arrow key on the shoe last time I tested, I was in a bit of rush lol.

Awesome job again sluicebox, many thanks for your time!

PS: On a last note and sorry to ask here, but since you are an SCI lord, can I ask if it is normal that the keyboard movement is so oddly delayed in the SCI EGA version of this game? I thought it was again only on my system, but I tried in dosbox and it seems to be the same, so not a bug? In that case, I don't see how it could be played without a mouse?!

[sluicebox]

I don't know much about the SCI0 version, but you're right that the keyboard input isn't great. I get the same results in the original, so presumably they didn't do a good job of implementing the custom walk behavior from the AGI version in the SCI game scripts. It seems a little less worse if you turn up the speed or tap the keys instead of holding them.

I'll take a look at the scripts eventually and see if there's an easy mistake that can be corrected, but it's more likely it just isn't structured very well.

[hugoarpin]

Big thanks for confirming the keyboard behaviour, take care!