Menu
▾
▴
#6345 TINSEL: DW2 is broken on big endian hardware
closed-fixed
5
2014-08-22
2013-06-08
No
As far as I can tell, both Discworld and Discworld II may be broken on big endian hardware since commit ce87175bede46c1bb938b73484e1db05212defbd.
TINSEL: Revert the BE -> LE resource conversion for DW1 Mac
scummvm 1.5.0 works, 1.6.0 doesn't. Both Games work if I check out af667771a918370988cc656412e2ddea3d1d44a3. I'm using N32 binaries on IRIX with the MIPSPro compilers.
discworld (detected as CD/DOS/English)fails with an assertion:
$ ./scummvm
WARNING: Could not find theme 'scummmodern' falling back to builtin!
User picked target 'dw-cd' (gameid 'tinsel')...
Looking for a plugin supporting this gameid... Tinsel
Starting 'Tinsel engine game'
Assertion failed: handle < g_numHandles, file engines/tinsel/handle.cpp, line 329, pid 65049
discworld II (detected as CD/DOS/English (GB)) dies with a segfault:
Process 65090 (scummvm) stopped on signal SIGSEGV: Segmentation violation (handler sig_fixup_mask) at [Tinsel::GetBytes(const unsigned char*,const Tinsel::WorkaroundEntry*&,int&,unsigned int):540 +0x68,0x1049ba24]
540 tmp = code[ip++ * (TinselV0 ? 4 : 1)];
(dbx) active thread 0x10000
Thread 0x10000 is active
(dbx) where
Thread 0x10000
> 0 Tinsel::GetBytes(const unsigned char*,const Tinsel::WorkaroundEntry*&,int&,unsigned int)(scriptCode = 0x113e0e38, wkEntry = 0x7ffb6b04, ip = 0x7ffb6b00, numBytes = 0) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/pcode.cpp":540, 0x1049ba24]
1 Tinsel::Interpret(Common::CoroBaseContext*&,Tinsel::INT_CONTEXT*)(coroParam = 0x10bad4c0, ic = 0x10b36f18) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/pcode.cpp":608, 0x1049bf70]
2 Tinsel::SceneTinselProcess(Common::CoroBaseContext*&,const void*)(coroParam = 0x10bbbb60, param = 0x10bbbb84) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/scene.cpp":176, 0x104d946c]
3 Common::CoroutineScheduler::schedule(void)(this = 0x10bc3950) ["/usr/people/canavan/src/scummvm/git/scummvm/common/coroutines.cpp":231, 0x1080c37c]
4 Tinsel::TinselEngine::NextGameCycle(void)(this = 0x10bb6568) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/tinsel.cpp":1032, 0x1048cd10]
5 Tinsel::TinselEngine::run(void)(this = 0x10bb6568, <no name> = 0x7ffb7538) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/tinsel.cpp":980, 0x1048b6ec]
6 ::runGame(const PluginSubclass<MetaEngine>*,OSystem&,const Common::String&)(<no name> = 0x7ffb7cf8, plugin = 0x10a22528, system = 0x10a28fb0, edebuglevels = 0x7ffb7738) ["/usr/people/canavan/src/scummvm/git/scummvm/base/main.cpp":226, 0x100e4190]
7 ::scummvm_main(argc = 1, argv = 0x7ffb7ee4) ["/usr/people/canavan/src/scummvm/git/scummvm/base/main.cpp":452, 0x100e57a4]
8 ::main(argc = 1, argv = 0x7ffb7ee4) ["/usr/people/canavan/src/scummvm/git/scummvm/backends/platform/sdl/posix/posix-main.cpp":45, 0x100e0410]
9 __start() ["/xlv55/kudzu-apr12/work/irix/lib/libc/libc_n32_M4/csu/crt1text.s":177, 0x100db1a8]
Even when compiling with -g0 (no optimization), I haven't been able to convice the debugger to provide any useable info about local/stack variables in Tinsel::GetBytes.
Just to be sure, you have the DOS versions of DW1 and DW2? And do you have their 'platform' in the 'Edit game' dialog for those entries set to DOS?
Since in theory I don't think commit ce87175bede46c1bb938b73484e1db05212defbd has any effect on DOS versions of DW1/2.
Autodetection has set the platform to DOS. The CDs for both DW and DW2 bear "PC CDROM" labels, the boxes list "IBM-compatible (PC|computer)" in the requirements section.
I don't have a BE system to test, so could you please try and bisect this?
As wjp said, commit b05fa7f20414d6a7571a9ba52f542e527f598c62 doesn't seem to be the cause of this...
Hm... if you are talking about commit, there's one thing that I found:
Inside palette.cpp:
// get hardware palette pointer
- pPalette = (const PALETTE *)LockMem(pDACtail->pal.hRGBarray);
+ pPalette = (const PALETTE *)LockMem(FROM_32(pDACtail->pal.hRGBarray));
I tried to make sure that I didn't add any endianess handling in that commit, but it seems that line slipped through by accident :(
If that is the offending commit, could you try changing that line to what it was? i.e.
pPalette = (const PALETTE *)LockMem(pDACtail->pal.hRGBarray);
I'm not sure if I've interpreted the output of git log correctly. A round of git bisect identifies this release as the cause:
c90d56355fa0bbcdd3122f3e376e5609422338b3 is the first bad commit
commit c90d56355fa0bbcdd3122f3e376e5609422338b3
Author: Filippos Karapetis <bluegr@gmail.com>
Date: Mon Dec 10 17:18:56 2012 +0200
TINSEL: Simplify the scene entrance handling code
This also reverts the rest of the BE resource handling code
:040000 040000 acfc6e90597bacc597c3673e559a69356adc1195 705e229062666e136e601323e74d8fbea6f82d36 M engines
manually testing show that
git checkout c90d56355fa0bbcdd3122f3e376e5609422338b3 is bad
git checkout c6cf4827d719c1833ce4d7e108410db81f00c358 is bad.
So can you verify that:
da971e38a565a52af92c4d419546820c8af9955b works
while
c6cf4827d719c1833ce4d7e108410db81f00c358 doesn't work?
ading the FROM_32() to the pPalette assignment in HEAD doesn't change anything - scummvm still crashes.
I may be able to provide access to a big endian system for testing, however that one would be somewhat slowish with only 2x 400MHz CPUs.
There is the method described here which may be of help to allow you to run a BE VM machine on a fast LE (x86) native machine:
http://wiki.scummvm.org/index.php/HOWTO-Debug-Endian-Issues
da971e38a565a52af92c4d419546820c8af9955b
and
c6cf4827d719c1833ce4d7e108410db81f00c358 both work.
My claim below that c6cf... is bad appears to be a cut-and-paste error.
c90d56355fa0bbcdd3122f3e376e5609422338b3 doesn't work
Just checked with gcc 4.7, same problem as with the MIPSPro compilers.
canavan: Thank you for the further debugging information.
Can you try running a Git bisection to locate the exact point where the breakage/regression was introduced please?
If you are not familar with bisection, see:
http://git-scm.com/book/en/Git-Tools-Debugging-with-Git
No new surprises running git bisect again:
scummvm :) $ git bisect good
c90d56355fa0bbcdd3122f3e376e5609422338b3 is the first bad commit
commit c90d56355fa0bbcdd3122f3e376e5609422338b3
Author: Filippos Karapetis <bluegr@gmail.com>
Date: Mon Dec 10 17:18:56 2012 +0200
TINSEL: Simplify the scene entrance handling code
This also reverts the rest of the BE resource handling code
:040000 040000 acfc6e90597bacc597c3673e559a69356adc1195 705e229062666e136e601323e74d8fbea6f82d36 M engines
canavan: Thanks for doing that... as suspected. Could you also try reverting / splitting down the change chunks in that commit (This will have to be a manual process as I don't think Git offers that option) and see which of the logical changes is provoking this regression?
@canavan: I've fixed the regression in that commit in commit 4b69071. Please, try fetching the newest changes on master and try again. Many thanks for your bisecting work :)
I've just tested DW1 on debian/powerppc, and I can confirm that commit fixed the crash.
4b69071 does indeed fix the assertion when starting DW1, but DW2 still segfaults here. None of the "obvious" (to me, e.g. the handling of hProcess, numProcess, hScript, hMusicScript) other changes appear to fix DW2 while keeping DW1 usable.
Can you provide any more information? What kind of error are you getting? Could you please help in bisecting the last commit that works with DW2? You can apply commit 4b69071 while bisecting - IIRC, that part of the code hasn't been touched by any of the subsequent commits
I assume (from the lack of responses) that DW1 has been fixed on BE systems now, thus I'm changing the bug description to only mention DW2
Apologies for the late response. DW1 is indeed working after 4b69071. Even with 4b69071 applied, git bisect still points at c90d56355fa0bbcdd3122f3e376e5609422338b3 as the culprit for the DW2 crash on startup.
c90d56355fa0bbcdd3122f3e376e5609422338b3 is the first bad commit
commit c90d56355fa0bbcdd3122f3e376e5609422338b3
Author: Filippos Karapetis <bluegr@gmail.com>
Date: Mon Dec 10 17:18:56 2012 +0200
TINSEL: Simplify the scene entrance handling code
This also reverts the rest of the BE resource handling code
:040000 040000 acfc6e90597bacc597c3673e559a69356adc1195 705e229062666e136e601323e74d8fbea6f82d36 M engines
The backtrace still looks the same:
> 0 Tinsel::GetBytes(const unsigned char*,const Tinsel::WorkaroundEntry*&,int&,unsigned int)(scriptCode = <illegal>, wkEntry = <illegal>, ip = <illegal>, numBytes = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/pcode.cpp":541, 0x1016cc60]
1 Tinsel::Interpret(Common::CoroBaseContext*&,Tinsel::INT_CONTEXT*)(coroParam = <illegal>, ic = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/pcode.cpp":608, 0x1016ce9c]
2 Tinsel::SceneTinselProcess(Common::CoroBaseContext*&,const void*)(coroParam = <illegal>, param = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/scene.cpp":177, 0x1019d5b0]
3 Common::CoroutineScheduler::schedule(void)(this = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/common/coroutines.cpp":231, 0x104003e8]
4 Tinsel::TinselEngine::NextGameCycle(void)(this = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/tinsel.cpp":1031, 0x10160614]
5 Tinsel::TinselEngine::run(void)(this = <illegal>, <no name> = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/engines/tinsel/tinsel.cpp":976, 0x1015f638]
6 ::runGame(const PluginSubclass<MetaEngine>*,OSystem&,const Common::String&)(<no name> = <illegal>, plugin = <illegal>, system = <illegal>, edebuglevels = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/base/main.cpp":229, 0x10090a38]
7 ::scummvm_main(argc = 0, argv = 0x1054c330) ["/usr/people/canavan/src/scummvm/git/scummvm/base/main.cpp":474, 0x10091d6c]
8 ::main(argc = <illegal>, argv = <illegal>) ["/usr/people/canavan/src/scummvm/git/scummvm/backends/platform/sdl/posix/posix-main.cpp":45, 0x1008db88]
9 __start() ["/xlv55/kudzu-apr12/work/irix/lib/libc/libc_n32_M4/csu/crt1text.s":177, 0x100872e8]
bluegr: I've tried to debug this on a BE machine a while ago, but got lost in the maze of commits and reverts mixed together around the commit canavan's bisect points to. Can you summarize what these commits are meant to do?