Blockmap overflow in amadeus.wad

[Quasar]

This level crashes Eternity by accessing lines[65279] in P_BlockLinesIterator, while there are only 645 lines in the map. I looked at PrBoom+ and it has no code to cleanse such invalid lines from the blockmap, so I can only assume that it is sneaking by via sheer luck, allowing invalid linedef accesses to occur that are only by chance not crashing the program.

Is there any practical way to detect and avoid corrupt blockmap crashes and still act as compatibly as possible?

EDIT: nevermind. Fixed in EE r1001 by adding a routine to verify the blockmap. Is EE the only port which does this?

It seems that amadeus.wad is actually corrupted. There are three of the offending block lists, which should be empty blocks but for a single bit that is flipped in the terminator for each list (FEFF instead of FFFF). There are also two copies of the WAD directory, and a significant amount of garbage data appended after what should be the EOF. I am guessing this file probably lived on a scratched CD or gaussed-out floppy disk, got transmitted over a bad dialup connection, or was generated by one of those lovely old DOS tools that could do such things as corrupt the resident portion of command.com, trash portions of your file system, and even inadvertently delete their own executable file *while running*.

[andrewj]

Quasar said:

EDIT: nevermind. Fixed in EE r1001 by adding a routine to verify the blockmap. Is EE the only port which does this?

Probably. Some ports nowadays completely ignore the blockmap, and build their own on level load, since large maps (over 12000x12000) can overflow the limits of the BLOCKMAP lump (the exact limit is not fixed, it depends on the width and height of the map as well as the number of linedefs).

[Graf Zahl]

Yes, but Eternity can't do that due to the all overriding needs of demo compatibility...

[entryway]

AMADEUS.WAD works fine with prboom-plus and your P_VerifyBlockMap does not detect any issues

AMADEUS.WAD: 65594 31.10.94 13:19
MD5: 8CE2351B70056DE3719DF5756A06692B

Quasar said:

It seems that amadeus.wad is actually corrupted. There are also two copies of the WAD directory, and a significant amount of garbage data appended after what should be the EOF.

AMADEUS2.WAD has these issues, but VerifyBlockMap still does not detect anything

AMADEUS2.WAD: 82795 24.11.94 00:44
MD5: 15E9E5ED877463BACCCCB9B538C5148D

[DaniJ]

One possible solution is to use two blockmaps; load the blockmap from PWAD but use it only for LOS checks for demo compatibility. Generate a new blockmap during map load for use everywhere else.

[Graf Zahl]

DaniJ said:

LOS checks

That's one of the things most ports don't use the blockmap for at all.

[DaniJ]

Yeah I know. Doomsday doesn't either btw. I would have thought it essential to for demo compatibility purposes however.

[Graf Zahl]

It is indeed because the blockmap gets used for all collision detection in the engine.

[Quasar]

The Amadeus WAD *I* have definitely has an issue. There are three block lists in the blockmap which consist of "00 00 FF FE"". When I run the file with Eternity r1001 I get the message "Blockmap error: index >= numlines", and the blockmap is rebuilt to avoid the crash.

Apparently we have different files.

[esselfortium]

Graf Zahl said:

Yes, but Eternity can't do that due to the all overriding needs of demo compatibility...

Not necessarily; couldn't a port implement blockmap rebuilding as an optional feature, like (iirc) ZDoom's option to internally rebuild nodes on all loaded maps?

[Quasar]

esselfortium said:

Not necessarily; couldn't a port implement blockmap rebuilding as an optional feature, like (iirc) ZDoom's option to internally rebuild nodes on all loaded maps?

EE already had this, inherited from MBF. It is only triggered when the blockmap is detectably too small (< 8 bytes), too large (> 0x10000), or now, discernably corrupt or malformed (contains block list offsets out of range, has an incomplete offset table, or has unterminated block lists). And the latter is only done outside of old demos, so if you run this corrupted copy of amadeus.wad with the -vanilla recording parameter, the game will still crash, as I suspect Choco might as well. If Choco does not crash it is only because it is using the zone heap, which sometimes allows you to get away with otherwise illegal accesses.

[Quasar]

Here is a screenshot of the offending data in a copy of the file freshly downloaded from the youfailit mirror of /idgames, which matches the one from gamers.org to which Catopromancy originally linked me:

http://eternity.mancubus.net/pics/crash/amadeus_blockmap_error.jpg

As you can see, two of the corrupt block lists are back to back in the file, and have a single bit flipped, quite likely for a disk or transmission error. It seems highly unlikely that the blockmap builder would malfunction on only 3 empty lists and write the rest of them correctly. The third occurs further down, out of the view of this shot.

The data after the WAD directory is actually the MUS lump and is not an error as I previously thought. There *are* two copies of the WAD directory, but this is probably simply due to a wad utility adding a new directory after inserting the MUS lump.

[entryway]

Quasar said:

http://eternity.mancubus.net/pics/crash/amadeus_blockmap_error.jpg

Hmm, the difference is not big

http://pastebin.com/8C2p1r9E

btw, gzdoom also crashes on amadeus.wad from Archive

[Graf Zahl]

Since I got a report about such a crash for ZDoom, too, I added the checking code but found something interesting:

This created false positives for several IWAD maps and on closer inspection all these cases had the block offset at 0 apparently indicating a block without any lines.

Yet I can't detect any code regarding this, neither in P_VerifyBlockmap nor in P_BlockLinesIterator.

Anyone knows what's up here?

[andrewj]

Do we have the code to Carmack's BSP tool? I've seen his code for reject building, but don't recall seeing the node building code.

[entryway]

Graf Zahl said:

Yet I can't detect any code regarding this, neither in P_VerifyBlockmap nor in P_BlockLinesIterator.

Anyone knows what's up here?

At least you can add runtime check in BlockLinesIterator::Next() to avoid crashes.

if (*list < 0 || *list >= numlines)
  fail;

[Graf Zahl]

andrewj said:

Do we have the code to Carmack's BSP tool? I've seen his code for reject building, but don't recall seeing the node building code.

The code is all in the ISBSP source that's easily found by searching for it.


EDIT: Found the problem. The code did not work as it was and needed some minor changes. Seems it's all fine now.

[andrewj]

FYI, I did google for it, stopped looking after a dozen pages of false positives.

[Graf Zahl]

First link I got for 'idbsp doom':

http://www.filefront.com/7974/IDBSP-v1.0/