TINSEL: immediate SIGBUS on strict-alignment architectures

[dwatteau]

This is on OpenBSD/loongson 6.8, a MIPS64, little-endian system with strict-alignment constraints. This is the same setup I used in #6220. sizeof(void *) == 8.

When unaligned access are done, OpenBSD doesn't intercept them, so it usually gives a hard SIGBUS. SCUMM_NEED_ALIGNMENT is defined.

This is with the dw-dos-cd-demo-en.zip file from the Demos page, with ScummVM 2.2.1pre1.

The game immediately crashes at startup, before anything is ever displayed.

Thread 1 received signal SIGBUS, Bus error.
Tinsel::FadeProcess (coroParam=@0x13990adf10: 0x13990b2800, param=0x13990adf3c)
    at engines/tinsel/faders.cpp:109
109             _ctx->pPalette = (PALETTE *)LockMem(pFade->pPalQ->hPal);

(gdb) bt full
#0  Tinsel::FadeProcess (coroParam=@0x13990adf10: 0x13990b2800, param=0x13990adf3c) at engines/tinsel/faders.cpp:109
        _ctx = 0x13990b2800
        tmpHolder = {_ctx = @0x13990adf10}
#1  0x0000000f8982288c in Common::CoroutineScheduler::schedule (this=0x144c12f900) at common/coroutines.cpp:232
        pProc = 0x13990adf00
        pNext = <optimized out>
        i = <optimized out>
#2  0x00000013eeba1060 in Tinsel::TinselEngine::NextGameCycle (this=0x13ea7b8200) at engines/tinsel/tinsel.cpp:1032
No locals.
#3  0x00000013eeba09bc in Tinsel::TinselEngine::run (this=0x13ea7b8200) at engines/tinsel/tinsel.cpp:977
        timerVal = 153809
#4  0x0000000f8946e954 in runGame (plugin=<optimized out>, system=..., edebuglevels=...) at base/main.cpp:292
        dir = {<Common::ArchiveMember> = {_vptr$ArchiveMember = 0xf89b31bd0 <vtable for Common::FSNode+16>}, 
          _realNode = {<Common::SafeBool<Common::SharedPtr<AbstractFSNode>, Common::impl::no_base<Common::SharedPtr<AbstractFSNode> > >> = {<Common::impl::no_base<Common::SharedPtr<AbstractFSNode> >> = {<No data fields>}, <No data fields>}, _refCount = 0x13f580c840, _deletion = 0x13f580d4c0, _pointer = 0x1433eb6f80}}
        target = {static npos = 4294967295, static _builtinCapacity = 20, _size = 10, 
          _str = 0xfffffc8af8 "dw-demo-cd", {_storage = "dw-demo-cd\000\000\000\000\000\000\000\000\000", 
            _extern = {_refCount = 0x2d6f6d65642d7764, _capacity = 25699}}}
        err = {_code = Common::kNoError, _desc = {static npos = 4294967295, static _builtinCapacity = 20, 
            _size = 8, _str = 0xfffffc8ad0 "No error", {
              _storage = "No error\000\000\000\000\000\000\000\000\070\214\374\377", _extern = {
                _refCount = 0x726f727265206f4e, _capacity = 0}}}}
        caption = {static npos = 4294967295, static _builtinCapacity = 20, _size = 31, 
          _str = 0x144f8d5ae0 "Discworld (CD Demo/DOS/English)", {
            _storage = "\200oRb\024\000\000\000 \000\000\000\377\000\000\000\227\315", <incomplete sequence \324>, 
            _extern = {_refCount = 0x1462526f80, _capacity = 32}}}
        tokenizer = {_str = {static npos = 4294967295, static _builtinCapacity = 20, _size = 0, 
            _str = 0xfffffc8a18 "", {
              _storage = "\000!z\t\000\000\000\000\nO[10\000\000\000\240", <incomplete sequence \360\235>, 
              _extern = {_refCount = 0x97a2100, _capacity = 828067594}}}, _delimiters = {static npos = 4294967295, 
            static _builtinCapacity = 20, _size = 2, _str = 0xfffffc8a40 " ,", {
              _storage = " ,\000mmvm\000\212\370\226\211\017\000\000\000\005\v\000", _extern = {
                _refCount = 0x6d766d6d002c20, _capacity = 2308372618}}}, _tokenBegin = 0, _tokenEnd = 0}
        previousLanguage = {static npos = 4294967295, static _builtinCapacity = 20, _size = 1, 
          _str = 0xfffffc8a70 "C", {_storage = "C\000\233\241\023", '\000' <repeats 11 times>, "`\243\233\241", 
            _extern = {_refCount = 0x13a19b0043, _capacity = 0}}}
        gameKeymaps = {_capacity = 1, _size = 1, _storage = 0x13996bfbc0}
        result = {_code = 21, _desc = {static npos = 4294967295, static _builtinCapacity = 20, _size = 1649075712, 
            _str = 0x0, {_storage = " \000\000\000\017\000\000\000\005\v\000\000\000\000\000\000 s\263\211", 
              _extern = {_refCount = 0xf00000020, _capacity = 2821}}}}
        engine = 0x9f5cf63571a28a60
        metaEngine = <optimized out>
        keymapper = 0x13ff5b4f00
#5  scummvm_main (argc=<optimized out>, argv=<optimized out>) at base/main.cpp:561
        result = {_code = 3562524055, _desc = {static npos = 4294967295, static _builtinCapacity = 20, 
            _size = 2573983264, _str = 0x13a19b9d70 <__isthreaded> "\001", {
              _storage = "#\000\000\000\000\000\000\000\227\315W\324\361\353\246M\300\322", <incomplete sequence \350>, _extern = {_refCount = 0x23, _capacity = 3562524055}}}}
        chainedGame = {static npos = 4294967295, static _builtinCapacity = 20, _size = 0, _str = 0xfffffc8a18 "", {
            _storage = "\000!z\t\000\000\000\000\nO[10\000\000\000\240", <incomplete sequence \360\235>, _extern = {
              _refCount = 0x97a2100, _capacity = 828067594}}}
        saveSlot = <optimized out>
        plugin = <optimized out>
        specialDebug = {static npos = 4294967295, static _builtinCapacity = 20, _size = 0, _str = 0xfffffc8948 "", {
            _storage = "\000!z\t\000\000\000\000\nO[10\000\000\000\240", <incomplete sequence \360\235>, _extern = {
              _refCount = 0x97a2100, _capacity = 828067594}}}
        command = {static npos = 4294967295, static _builtinCapacity = 20, _size = 10, 
          _str = 0xfffffc8920 "dw-demo-cd", {_storage = "dw-demo-cd\000\235\023\000\000\000\000$\302\234", 
            _extern = {_refCount = 0x2d6f6d65642d7764, _capacity = 2634048611}}}
        settings = {_nodePool = {<Common::FixedSizeMemoryPool<80, 10>> = {<Common::MemoryPool> = {_chunkSize = 80, 
                _pages = {_capacity = 0, _size = 0, _storage = 0x0}, _next = 0xfffffc85b0, _chunksPerPage = 8}, 
              _storage = "\000\206\374\377\377\000\000\000X\021\233\241\023\000\000\000`P\215O\024\000\000\000؄\353\334\023", '\000' <repeats 19 times>, "\020\231j\027\024\000\000\000`\243\233\241\023\000\000\000\204\016\225\241\023\000\000\000\\\016\225\241\023\000\000\000P\206\374\377\377\000\000\000P'\272\036\024\000\000\000\240\024\313\311\023\000\000\000\227\315W\324\361\353\246M@\324Jb\024\000\000\000p\235\233\241\023\000\000\000#\000\000\000\000\000\000\000X\021\233\241\023\000\000\000`P\215O\024\000\000\000؄\353\334\023\000\000\000\240\206\374\377\377\000\000\000\210\207\374\377\377\000\000\000`\243\233\241\023\000\000\000\020\207\374\377\377\000\000\000`\243\233\241\023\000\000\000\214\034"...}, <No data fields>}, _defaultVal = {static npos = 4294967295, static _builtinCapacity = 20, _size = 0, 
            _str = 0xfffffc88e0 "", {_storage = "\000s^O\024\000\000\000\274E[O\024\000\000\000\000\000\000", 
              _extern = {_refCount = 0x144f5e7300, _capacity = 1331381692}}}, _storage = 0x13ae182900, _mask = 15, 
          _size = 0, _deleted = 0, _hash = {<No data fields>}, _equal = {<No data fields>}}
        res = {_code = Common::kNoError, _desc = {static npos = 4294967295, static _builtinCapacity = 20, 
            _size = 8, _str = 0xfffffc8570 "No error", {_storage = "No error\000rror\000\000\000\b\206\374\377", 
              _extern = {_refCount = 0x726f727265206f4e, _capacity = 1869771264}}}}
        system = <optimized out>
#6  0x0000000f89468dc8 in main (argc=-361004544, argv=0x9f5cf63571a28a60)
    at backends/platform/sdl/posix/posix-main.cpp:45
        res = <optimized out>

(gdb) where
#0  Tinsel::FadeProcess (coroParam=@0x13990adf10: 0x13990b2800, param=0x13990adf3c) at engines/tinsel/faders.cpp:109
#1  0x0000000f8982288c in Common::CoroutineScheduler::schedule (this=0x144c12f900) at common/coroutines.cpp:232
#2  0x00000013eeba1060 in Tinsel::TinselEngine::NextGameCycle (this=0x13ea7b8200) at engines/tinsel/tinsel.cpp:1032
#3  0x00000013eeba09bc in Tinsel::TinselEngine::run (this=0x13ea7b8200) at engines/tinsel/tinsel.cpp:977
#4  0x0000000f8946e954 in runGame (plugin=<optimized out>, system=..., edebuglevels=...) at base/main.cpp:292
#5  scummvm_main (argc=<optimized out>, argv=<optimized out>) at base/main.cpp:561
#6  0x0000000f89468dc8 in main (argc=-361004544, argv=0x9f5cf63571a28a60)
    at backends/platform/sdl/posix/posix-main.cpp:45

(gdb) list
104		if (TinselV2)
105			// Note that this palette is being faded
106			FadingPalette(pFade->pPalQ, true);
107	
108		// get pointer to palette - reduce pointer indirection a bit
109		_ctx->pPalette = (PALETTE *)LockMem(pFade->pPalQ->hPal);
110	
111		for (_ctx->pColMult = pFade->pColorMultTable; *_ctx->pColMult >= 0; _ctx->pColMult++) {
112			// go through all multipliers in table - until a negative entry
113	

[dwatteau]

quick and incomplete diff removing the first SIGBUSes

[dwatteau]

Here are some more findings…

It appears that I can know unbreak most of the first scenes of dw-demo-cd if I use memcpy() to copy the *param stuff, instead of just a plain cast.

The attached diff shows the first occurrences where I changed this. But there are a lot of them, and I'm not at ease with what I'm doing and the coroutines nearby.

So I'm not sure I could submit a real PR fixing this. But if someone else wants to look into this, here's a first attempt.

[eriktorbjorn]

Does it still happen with the current development version? I see there was a recent change that might be indirectly related to this issue:

​https://github.com/scummvm/scummvm/commit/640f2d6654e5cd4d546a6bbe8c0d967a03ee868f

[dwatteau]

New gdb output on 6b21b0910987

[dwatteau]

Yes, it still SIGBUSes on 6b21b0910987, see the new gdb attachment. The memcpy() trick above could probably work around this, but I'm not sure it's the cleanest way to handle this.

(note: there's nothing critical for me here; it's a machine that's almost only useful to catch strict-alignment portability problems, but I'm not sure any "real world" device is really impacted, except maybe the Dreamcast?)

[eriktorbjorn]

I thought the problem had something to do with packed structures, but I can't see that the ones in the lines where it crashes are. I'm probably overlooking something obvious, though.

[dwatteau]

This has effectively been fixed by lephilousophe in commit 6a1abd09f1959b1a2123832128f85f06d0b91b54 ("COMMON: Make sure coroutine Process parameter is well aligned") as part of PR#5148 (which has also been backported to the 2.7.1 release).

Running my Discworld games on that MIPS device (or on x86 with -fsanitize=alignment), I'm not seeing any memory alignment issue anymore. No need for the memcpy() calls I've suggested above.

Closing this then, thanks!