CINE: FW - screen change crash

[Paparazite]

On Amiga, DOS (Fr version and other ?) ScummVM crash when I found and open '​'manhole cover' (2nd screen) in 4315 and I try to return left on 1st screen.

See joined saved game at this point.

[karisal]

I think this may have been a memory corruption problem which got fixed already but not 100% sure.

[karisal]

Replying to karisal:

I think this may have been a memory corruption problem which got fixed already but not 100% sure.

No, it has not yet been fixed. Tried out the provided savegame with the French Amiga version of Future Wars and it surely crashed when walking to the left of the screen.

ScummVM crashes in FWRenderer::drawOverlays() function on the renderOverlay(it); line. Visual Studio 2017 on Windows 7 64-bit said "Access violation reading location 0x00000000.".

void FWRenderer::drawOverlays() {
    ...
    Common::List<overlay>::iterator it;

    for (it = g_cine->_overlayList.begin(); it != g_cine->_overlayList.end(); ++it) {
        renderOverlay(it);
    }
}

So something to do with the overlay list or its iterator being vaporized/pointing to emptiness.

[karisal]

Replying to karisal:

Replying to karisal:

I think this may have been a memory corruption problem which got fixed already but not 100% sure.

No, it has not yet been fixed. Tried out the provided savegame with the French Amiga version of Future Wars and it surely crashed when walking to the left of the screen.

ScummVM crashes in FWRenderer::drawOverlays() function on the renderOverlay(it); line. Visual Studio 2017 on Windows 7 64-bit said "Access violation reading location 0x00000000.".

void FWRenderer::drawOverlays() {
    ...
    Common::List<overlay>::iterator it;

    for (it = g_cine->_overlayList.begin(); it != g_cine->_overlayList.end(); ++it) {
        renderOverlay(it);
    }
}

So something to do with the overlay list or its iterator being vaporized/pointing to emptiness.

Actually it is here it borked:

void FWRenderer::renderOverlay(const Common::List<overlay>::iterator &it) {
	int idx, len, width;
	ObjectStruct *obj;
	AnimData *sprite;
	byte *mask;

	switch (it->type) {
	// color sprite
	case 0:
		if (g_cine->_objectTable[it->objIdx].frame < 0) {
			return;
		}
		sprite = &g_cine->_animDataTable[g_cine->_objectTable[it->objIdx].frame];
		len = sprite->_realWidth * sprite->_height;
		mask = new byte[len];
		memcpy(mask, sprite->mask(), len);

On the last line, the sprite->mask() is NULL and that's used as a source for memcpy. So a possible fix could be to just fill the destination mask with zeroes if the source mask is NULL. Let's see...

[karisal]

The fix for this bug is waiting for this ​pull request to be merged into the 2.2.0 branch.

[sev-]

This has been merged.