[Scummvm-tracker] [ScummVM] #10116: MM C64 DEMO: Attempting to display the original save/load screen causes a crash.
Colin Snover
trac at scummvm.org
Sat Nov 11 05:42:05 CET 2017
#10116: MM C64 DEMO: Attempting to display the original save/load screen causes a
crash.
-----------------------------+---------------------------
Reporter: robertmegone | Owner: csnover
Type: defect | Status: new
Priority: blocker | Component: Engine: SCUMM
Resolution: | Keywords: reproducible
Game: Maniac Mansion |
-----------------------------+---------------------------
Comment (by csnover):
I seem to be unable to trigger the specific crash mentioned by the OP, but
since the SCUMM save code triggers undefined behaviour, it makes sense
that it would do different things to different people.
This is not legal C++:
{{{#!c++
#define OFFS(type,item) ((uint32)(((ptrdiff_t)(&((type
*)42)->type::item))-42))
// ...later...
OFFS(ScummEngine, _gameMD5[0])
}}}
ScummEngine is not a standard layout type, so this triggers UB in C++14
and earlier, and implementation-defined behaviour in C++17 and later.
It is frustrating to me that this was known UB and compilers actively were
warning about it and instead of eliminating the UB `42` was added to
defeat the heuristic of the compiler.
This code happens to also crash Clang’s UndefinedBehaviorSanitizer code,
which can’t deal with someone trying to get the offset of a member in a
class with a vtable like this.
So in conclusion, this save code needs to be fixed to be C++-compliant,
and I don’t know enough about this engine right now to know how to do that
successfully.
In the meantime it would be helpful to learn from robertmegone a specific
set of reproduction steps to get the array assertion error since I want to
make sure I have not just unmasked a separate issue.
--
Ticket URL: <https://bugs.scummvm.org/ticket/10116#comment:8>
ScummVM <https://bugs.scummvm.org>
ScummVM
More information about the Scummvm-tracker
mailing list