Opened 11 years ago
Closed 4 years ago
#6220 closed defect (fixed)
BACKENDS: OpenBSD - SIMON2 (DOS French) - SIGBUS with an early dialog on mips64el
| Reported by: | SF/tsomi | Owned by: | dwatteau |
|---|---|---|---|
| Priority: | normal | Component: | Port: BSD |
| Version: | Keywords: | ||
| Cc: | Game: | Simon the Sorcerer 2 |
Description
Hi,
This happens on the -current branch of OpenBSD/loongson (which uses the mips64el architecture, LP64, little-endian, strict alignment).
I used the latest git HEAD version (00c0986562f1eb0), with the "--disable-alsa --enable-debug --disable-seq-midi --enable-sndio" configure options, CXXFLAGS set to "-O2 -pipe -g" and the following [1] [2] patches from OpenBSD ports. The compiler is g++ 4.2.1.
I'm getting a SIGBUS with SIMON2DOS French, why is triggered by doing this: - Start a new game - Press Escape to skip the various introduction scenes - When you can start playing, go to the castle - A scene with the 2 guards starts. The game crashes just after Simon says "Qui, moi ?".
Here's gdb output (with "bt full" and "bt"):
Program received signal SIGBUS, Bus error. AGOS::AGOSEngine::setupLocalStringTable (this=0x4a4d0000, mem=0x4bf939ec "J'ai l'impression qu'ils ne sont pas tr=s souriants...", num=440) at engines/agos/string.cpp:251 251 if (--num == 0) #0 AGOS::AGOSEngine::setupLocalStringTable (this=0x4a4d0000, mem=0x4bf939ec "J'ai l'impression qu'ils ne sont pas tr=s souriants...", num=440) at engines/agos/string.cpp:251 No locals. #1 0x0000000007fcbad0 in AGOS::AGOSEngine::loadTextIntoMem (this=0x4a4d0000, stringId=33741) at engines/agos/string.cpp:332 filename = {static _builtinCapacity = 20, _size = 6, _str = 0x7ffe01a8 "TEXT05", { _storage = "TEXT05\000\000\000\000MJ{\024\001\000\200\221p\t", _extern = { _refCount = 0x353054584554, _capacity = 1246560256}}} p = Variable "p" is not available. #0 AGOS::AGOSEngine::setupLocalStringTable (this=0x4a4d0000, mem=0x4bf939ec "J'ai l'impression qu'ils ne sont pas tr=s souriants...", num=440) at engines/agos/string.cpp:251 #1 0x0000000007fcbad0 in AGOS::AGOSEngine::loadTextIntoMem (this=0x4a4d0000, stringId=33741) at engines/agos/string.cpp:332 #2 0x0000000007fcbbe0 in AGOS::AGOSEngine::getLocalStringByID ( this=0x4a4d0000, stringId=33741) at engines/agos/string.cpp:164 #3 0x0000000007fcbf18 in AGOS::AGOSEngine::getStringPtrByID (this=0x4a4d0000, stringId=33741, upperCase=false) at engines/agos/string.cpp:135 #4 0x000000000800a03c in AGOS::AGOSEngine_Simon1::os1_screenTextMsg (this=Variable "this" is not available. ) at engines/agos/script_s1.cpp:345 #5 0x0000000007fc4d7c in AGOS::AGOSEngine_Simon2::executeOpcode (this=Variable "this" is not available. ) at engines/agos/script_s2.cpp:283 #6 0x0000000007fba52c in AGOS::AGOSEngine::runScript (this=0x4a4d0000) at engines/agos/script.cpp:1030 #7 0x0000000007fcede0 in AGOS::AGOSEngine::startSubroutine (this=0x4a4d0000, sub=0x4bf8d008) at engines/agos/subroutine.cpp:569 #8 0x0000000007fbd6ac in AGOS::AGOSEngine::o_process (this=0x4a4d0000) at engines/agos/script.cpp:463 #9 0x0000000007fc4d7c in AGOS::AGOSEngine_Simon2::executeOpcode (this=Variable "this" is not available. ) at engines/agos/script_s2.cpp:283 #10 0x0000000007fba52c in AGOS::AGOSEngine::runScript (this=0x4a4d0000) at engines/agos/script.cpp:1030 #11 0x0000000007fcede0 in AGOS::AGOSEngine::startSubroutine (this=0x4a4d0000, sub=0x4bf8cc28) at engines/agos/subroutine.cpp:569 #12 0x000000000800859c in AGOS::AGOSEngine_Elvira2::oe2_doTable (this=Variable "this" is not available. ) at engines/agos/script_e2.cpp:356 #13 0x0000000007fc4d7c in AGOS::AGOSEngine_Simon2::executeOpcode (this=Variable "this" is not available. ) at engines/agos/script_s2.cpp:283 #14 0x0000000007fba52c in AGOS::AGOSEngine::runScript (this=0x4a4d0000) at engines/agos/script.cpp:1030 #15 0x0000000007fcede0 in AGOS::AGOSEngine::startSubroutine (this=0x4a4d0000, sub=0x4bf893f8) at engines/agos/subroutine.cpp:569 #16 0x0000000007fbd6ac in AGOS::AGOSEngine::o_process (this=0x4a4d0000) at engines/agos/script.cpp:463 #17 0x0000000007fc4d7c in AGOS::AGOSEngine_Simon2::executeOpcode (this=Variable "this" is not available. ) at engines/agos/script_s2.cpp:283 #18 0x0000000007fba52c in AGOS::AGOSEngine::runScript (this=0x4a4d0000) at engines/agos/script.cpp:1030 #19 0x0000000007fcede0 in AGOS::AGOSEngine::startSubroutine (this=0x4a4d0000, sub=0x4bf89de4) at engines/agos/subroutine.cpp:569 #20 0x0000000007fd17cc in AGOS::AGOSEngine::handleVerbClicked ( this=0x4a4d0000, verb=Variable "verb" is not available. ) at engines/agos/verb.cpp:393 #21 0x0000000007fe2c90 in AGOS::AGOSEngine::go (this=0x4a4d0000) at engines/agos/agos.cpp:1063 #22 0x0000000007fa8eb8 in AGOS::AGOSEngine::run (this=0x4a4d0000) at agos.h:223 #23 0x0000000007db74f0 in runGame (plugin=0x4d81a900, system=Variable "system" is not available. ) at base/main.cpp:226 #24 0x0000000007db8a38 in scummvm_main (argc=Variable "argc" is not available. ) at base/main.cpp:452 #25 0x0000000007db5234 in main (argc=1, argv=0x7ffe1960) at backends/platform/sdl/posix/posix-main.cpp:45
[1] http://www.openbsd.org/cgi-bin/cvsweb/ports/games/scummvm/patches/patch-configure [2] http://www.openbsd.org/cgi-bin/cvsweb/ports/games/scummvm/patches/patch-engines_draci_draci_h
Ticket imported from: #3599686. Ticket imported from: bugs/6220.
Attachments (1)
Change History (13)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
AGOSEngine::alignTableMem aligns to size 4 which sounds like it may be too little for mips64el pointers.
(And it's not clear to me at all that alignTableMem is called everywhere it should be.)
comment:3 by , 11 years ago
> Checked the distribution specific patches applied and I doubt that either > are related to this, though it would be nice to have them submitted to > upstream in a less hacky manner.
Well, I'm not the maintainer of this port, but I'll try to get the message across.
> I would suspect that this is being caused by your platform's strict memory > alignment, which I'm not sure we test for
I'm attaching config.h; SCUMM_NEED_ALIGNMENT does get defined.
However... I see common/endian.h has this: #if defined(SCUMM_NEED_ALIGNMENT) && !defined(__mips__)
so what is selected for my arch is in the next #else (l. 312) , but I'm not sure whether this part handles strict alignment?
Because I'm using ScummVM on my loongson since early 2011. And AFAICS, I wasn't experiencing theses crashes before (but I'm not playing SIMON2 very often). I know I had this crash with 1.5.0 too, and I think (not sure) I had it with 1.4.0 as well. But something like 1.2.0 or 1.3.0 was fine, as far I can remember...
I could try a bisect, but building ScummVM with all its engines and debugging takes 4 hours on this box (but I'll do it if it helps).
Oh, and OpenBSD builds stuff with -fstack-protector by default, and it also has PIE by default since the last few months. Don't know if it could matter.
> and for some engines can > guarantee without significant work on the original engine scripting > structures.
I also think I managed to crash SCUMM at some point, but I can't reproduce this for the moment. I'm going to look into that.
Thanks.
comment:4 by , 11 years ago
tsomi: Thank you for the information.
Please don't worry excessively about the patches as we can always look at applying the equivalent changes, but it would be useful for these kind of distribution build issues to be reported to the ScummVM team and resolved cleanly, as other platform ports are likely to benefit! :)
Hmm, so this sounds like it may be a regression. A bisection (even a "gross" one i.e. try building previous releases back to say v1.2.0 and see if the issue still occurs) would be useful as that would indicate whether this is a problem on our side or due to the changes in the OpenBSD system library configuration you mentioned.
To help, you don't need to do a full rebuild with all engines to test just SIMON2, do a build as: ./configure --disable-all-engines --enable-engines=agos && make clean && make
And this will build just the core code and the AGOS engine used for the StS games, which should be much, much faster to build.
comment:5 by , 11 years ago
Note, that the syntax on older releases is slightly different and would be "./configure --disable-all-engines --enable-agos". Please let us know if you reach any conclusions.
comment:6 by , 11 years ago
So I went back to v1.2.0, even v1.1.0, and I'm still having the same crash.
Maybe I've never played SIMON2 much on my loongson. I can't really say, I do know I've played SCUMM games a lot on this machine, though.
Don't know what to do, now. AFAIK installing 2 different releases of OpenBSD on the same machine isn't an easy thing.
I have Debian testing installed on it as well, though. With their version of ScummVM (1.4.1), I don't have the crash. However, there, ScummVM is running in MIPS32 mode...
comment:7 by , 11 years ago
Looking at the code a bit, I would agree with wjp here. The problem is probably that _localStringtable (accessed in line 250) is not aligned to an 8 byte boundary. I *think* that this is required, at least I guess that you have 8 byte pointers on mips64el.
So we will probably need to make sure that alignTableMemory alignes _tablesHeapPtr to an 8 byte boundary instead of an 4 byte boundary. For reference the memory _localStringtable points at is allocated from that in line 324. And there doesn't seem to be any alignTableMemory call there at a quick glance either. So wjp is probably right that it is not called in all relevant places either.
As wjp suggested in IRC we should add asserts which check for for 8 byte alignment, to the places where allocation from _tablesHeapPtr is done. That will help us catch such issues on platforms without strict alignment requirements too. 8 byte is chosen, because it is the size of a pointer on 64 bit systems.
I am not sure whether just changing the alignment from 4 to 8 in alignTableMemory is safe though. It seems the heap size is fixed to _tableMemSize. So increasing the alignment requirements could result in us running out of memory. This already happened for the old item heap on 64 bit systems in the past. See bug #1498158 "AGOS: Itemheap overflow on 64bit systems".
comment:8 by , 11 years ago
Slight clarification: I think that the data _localStringtable *points at* is not aligned to an 8 byte boundary.
comment:9 by , 11 years ago
> at least I guess that you have 8 byte pointers on mips64el. I confirm that.
comment:10 by , 4 years ago
| Component: | Engine: AGOS → Port: BSD |
|---|---|
| Summary: | SIMON2DOS French: SIGBUS with an early dialog on mips64el → BACKENDS: OpenBSD - SIMON2 (DOS French) - SIGBUS with an early dialog on mips64el |
comment:11 by , 4 years ago
I'm the original author of this bug report.
This ought to be fixed with my pull request here:
https://github.com/scummvm/scummvm/pull/2241
which has been merged. This ticket can probably be closed, then.
comment:12 by , 4 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
@dwatteau
Thank you for the feedback.
If the error returns, just reopen the issue.
tsomi: Thank you for your bug report, especially as it has more than sufficient detail to analyse this bug.
Checked the distribution specific patches applied and I doubt that either are related to this, though it would be nice to have them submitted to upstream in a less hacky manner.
As for this issue, from https://en.wikipedia.org/wiki/Unix_signal: "The SIGBUS signal is sent to a process when it causes a bus error. The conditions that lead to the signal being raised are, for example, incorrect memory access alignment or non-existent physical address. "
I would suspect that this is being caused by your platform's strict memory alignment, which I'm not sure we test for and for some engines can guarantee without significant work on the original engine scripting structures.
Unsure how to proceed here, but maybe the AGOS developers and Core Team can take a look and see if there is a packing/alignment issue in AGOS which can be fixed without impacting other code.