[go: up one dir, main page]

Menu

#5704 Tinsel: DW 1 & 2 Segfault on start

Discworld
closed-fixed
Max Horn
5
2011-05-18
2011-05-18
No

Both Discworld 1 & 2 crash ScummVM with this on the terminal:
User picked target 'dw' (gameid 'tinsel')...
Looking for a plugin supporting this gameid... Tinsel
Starting 'Tinsel engine game'
Connected to Alsa sequencer client [14:0]
ALSA client initialised [128:0]
Segmentation fault

And Discworld 2:
User picked target 'dw2-gb' (gameid 'tinsel')...
Looking for a plugin supporting this gameid... Tinsel
Starting 'Tinsel engine game'
Segmentation fault

Running a my own compiled ScummVM built 17/05 on Ubuntu 11.04 amd64:
ScummVM 1.4.0git (May 17 2011 22:58:32)
Features compiled in: Vorbis ALSA SEQ TiMidity RGB zLib Theora

With these compile options:
./configure --enable-myst --enable-riven --enable-sword25 --disable-he --disable-agi --disable-agos --disable-agos2 --disable-cine --disable-cruise --disable-draci --disable-drascula --disable-gob --disable-groovie --disable-hugo --disable-kyra --disable-lure --disable-parallaction --disable-queen --disable-saga --disable-ihnm --disable-sci --disable-sky --disable-teenagent --disable-toon --disable-touche --disable-tucker --disable-debug --disable-mt32emu --disable-mad --disable-flac --disable-fluidsynth --disable-readline --enable-release

Discussion

  • Paul Gilbert

    Paul Gilbert - 2011-05-18
    • assigned_to: nobody --> fingolfin
     
  • Paul Gilbert

    Paul Gilbert - 2011-05-18

    Hi Fingolfin,

    I think the problem was caused by your commit 'Remove unnecessary casts'. This has removed pointer to object operations which are necessary. In background.cpp:165, it's meant to return a pointer to the pDispList field, rather than the pDispList value itself.

    I'm not sure if there's some cool way with git to undo that single commit, so I'll leave it up to you to decide how to proceed.

     
  • digitall

    digitall - 2011-05-18

    Can't replicate this crash with the latest Git master.

    Could you:
    a) try to compile with just "./configure && make clean && make" and see if the crash still occurs.
    b) confirm the git revision you are using i.e. ./scummvm --version from a build without --enable-release.

    If the crash still occurs even with the options in a), then please try updating to the latest Git master and repeating a) and b).

    P.S. If you are trying to disable all engines but a small selection, "./configure --disable-all-engines --enable-myst --enable-riven --enable-sword25" would make for a far more readable command...

     
  • David Rogers

    David Rogers - 2011-05-18

    I updated to the latest git master and did just:
    ./configure && make clean && make

    Version:
    ezekiel@alice:~/scummvm-scummvm-f830d68$ ./scummvm -v
    ScummVM 1.4.0git (May 18 2011 12:31:02)
    Features compiled in: Vorbis ALSA SEQ TiMidity RGB zLib Theora

    And still got segfaults on both discworld games.

    P.S. Thanks for pointing out the disable all engines flag, it makes my command a bit shorter:
    ./configure --disable-all-engines --enable-made --enable-mohawk --enable-myst --enable-riven --enable-scumm --enable-scumm-7-8 --enable-sword1 --enable-sword2 --enable-sword25 --enable-tinsel --disable-debug --disable-mt32emu --disable-mad --disable-flac --disable-fluidsynth --disable-readline --enable-release

     
  • Max Horn

    Max Horn - 2011-05-18

    dreammaster, you are right about deducing the commit which is the cause of this crash, and I reverted it. However, the code it touches is most definitely *not* correct: It casts an OBJECT** to OBJECT*. This is *very* fishy, and should be analyzed on its own.

     
  • Max Horn

    Max Horn - 2011-05-18
    • status: open --> closed-fixed
     
  • Max Horn

    Max Horn - 2011-05-18

    Ah, I see: The PLAYFIELD struct has as first entry an OBJECT pointer, just like the OBJECT itself. So in fact, the dirty trick used by tinsel here is to pretend the PLAYFIELD is an OBJECT, using it as the imaginary HEAD of the linked list of objects PLAYFIELD::pDispList points to.

    I'll document this more explicitly in the code, and we probably still should rewrite the code to avoid that, as it could cause problem with aliasing analysis in compilers.

     
  • Paul Gilbert

    Paul Gilbert - 2011-05-19

    Thanks for looking into further. That makes sense, and explains why the cast was being done, which definitely looked weird.