Opened 9 years ago

Closed 9 years ago

Last modified 10 months ago

#4848 closed defect (fixed)

COMMON: fatal assertion when starting most engines

Reported by: SF/khalek Owned by: fingolfin
Priority: normal Component: Ports
Keywords: Cc:
Game:

Description

$ scummvm -v
ScummVM 1.1.0 (Apr 5 2010 20:04:52)
Features compiled in: Vorbis FLAC MP3 RGB zLib

gcc version 3.3.5 (propolice)

OpenBSD/i386 4.7-current

When trying to add comi/sq3demo/dott but not sword1/sword from either the command line or the launcher I see:

assertion "_size == old_size" failed: file "common/hashmap.h", line 432, function "expandStorage"

(gdb) bt
#0 0x0efd6a4d in kill () from /usr/lib/libc.so.53.1
#1 0x0f0222cb in abort () at /usr/src/lib/libc/stdlib/abort.c:68
#2 0x0efadbb7 in __assert2 (file=0x3c001eeb "common/hashmap.h", line=432,
func=0x3c183e47 "expandStorage", failedexpr=0x3c001f5d "_size == old_size")
at /usr/src/lib/libc/gen/assert.c:52
#3 0x1c5305b0 in Common::HashMap<Common::String, Common::FSNode, Common::IgnoreCase_Hash, Common::IgnoreCase_EqualTo>::expandStorage ()
#4 0x1c530274 in Common::HashMap<Common::String, Common::FSNode, Common::IgnoreCase_Hash, Common::IgnoreCase_EqualTo>::lookupAndCreateIfMissing ()
#5 0x1c52ffa0 in Common::HashMap<Common::String, Common::FSNode, Common::IgnoreCase_Hash, Common::IgnoreCase_EqualTo>::getVal ()
#6 0x1c5a1e35 in detectGame ()
#7 0x1c5a01d1 in AdvancedMetaEngine::detectGames ()
#8 0x1c0152fe in EngineManager::detectGames ()
#9 0x1c5b4d97 in GUI::LauncherDialog::addGame ()
#10 0x1c5b6857 in GUI::LauncherDialog::handleCommand ()
#11 0x1c5b77a1 in GUI::CommandSender::sendCommand ()
#12 0x1c5ea3e4 in GUI::ButtonWidget::handleMouseUp ()
#13 0x1c5abf26 in GUI::Dialog::handleMouseUp ()
#14 0x1c5ad1e0 in GUI::GuiManager::runLoop ()
#15 0x1c5abb22 in GUI::Dialog::runModal ()
#16 0x1c00aa0f in launcherDialog ()
#17 0x1c00c4d2 in scummvm_main ()
#18 0x1c008884 in main ()

I can provide a trace with symbols if it helps.

Ticket imported from: #2982206. Ticket imported from: bugs/4848.

Change History (11)

comment:1 Changed 9 years ago by fingolfin

Could you please run "make test" to see if that shows the same issue?

Do you have an alternate C compiler such as GCC 3.3.6 or GCC 4.x to test compilation with? This way could narrow down whether the issue occurs with only this specific compiler + OS combination.

comment:2 Changed 9 years ago by SF/khalek

make test works with 3.3.5

The problem seems to go away with GCC 4.2.4 indeed. So where now? What versions of GCC is ScummVM supporting? I found a note about compilation with 2.95 being broken but not much else.

comment:3 Changed 9 years ago by fingolfin

We do support GCC 3.x, and it works just fine on several ports. Otherwise I would have closed your report right away.

Now we need to find out what's wrong there, e.g. a bug in the compiler or some subtle bug in the hashmap code. I am right now trying to find a system for which I can get hold of a GCC 3.3.5, or at least something close to it. You are welcome to experiment with the hashmap code, too, by the way.

comment:4 Changed 9 years ago by fingolfin

I installed GCC 3.3.6 on a Debian 4.0 machine, and tried to reproduce the issue, but no such luck. In particular, I downloaded the sq3 demo from our webpage, and then added it. Note that this is an almost virgin system, i.e. with a fresh scummvm config. Since the config data is also stored in hashmaps, this might be relevant ... or not, as it's not completely clear to me which hashmap the error occurs in.
A stack trace from debug build including line numbers would be quite helpful.
Also, you could try to see what happens if you temporarily remove/rename your config file, causing ScummVM to create a new one.

I could look into setting up an OpenBSD virtual machine for testing, I guess. Is your gcc the default compiler for OpenBSD 4.7?

comment:5 Changed 9 years ago by fingolfin

Owner: set to fingolfin
Resolution: fixed
Status: newclosed

comment:6 Changed 9 years ago by fingolfin

I installed OpenBSD 4.6 in a virtual machine and was able to reproduce the issue there. It had nothing to do with the hashmap, rather, the problem was that apparently this particular gcc version choked on some code in engines/advancedDetector.cpp and produced incorrect machine code. I modified the source code in question slightly, which made the crash go away for me; though I did not perform further testing.

Note that this issue did not occur with gcc 3.3.6 on Debian 4.0, so it seems likely that this has either been fixed between 3.3.5 and 3.3.6, or maybe it is caused by some of the OpenBSD specific changes applied to this gcc.

comment:7 Changed 9 years ago by SF/khalek

Thanks a lot for going to so much effort to look into this!

the gcc-local man page documents local changes on OpenBSD. The most significant being the stack protector. OpenBSD is also very aggressive in the way it allocates and frees memory to try and catch problems/misbehaviour.

I suspect a problem with stack protector, but am really busy at the moment. Will hopefully get a chance to look into this some more over the weekend.

comment:8 Changed 9 years ago by fingolfin

OK. So what happened was that the "extra" param to detectGame was of type "const Common::String", and a char * was passed in. This should create a "temporary" object of this type, which lives until the function returns. BUt apparently, it was dead by the time extra was used in detectGames for a lookup in a hashmap. I.e. it looks as if the object was deallocated to early.
The fix changed the param to be of type "const Common::String &".

comment:9 Changed 9 years ago by SF/khalek

I don't see it with -fno-stack-protector with 3.3.5, I need to put together a smaller test case to track this down. I'm curious to see if this occurs with -fstack-protector with the libssp based gcc 4.x releases as well.

comment:10 Changed 9 years ago by fingolfin

Please do so. Maybe it's a bug in our code after all, although I completely fail to see how...

BTW, it might be interesting to try and see what happens if you use type "Common::String" instead of "const Common::String" or "const Common::String &" (i.e. use a non-const non-ref)

comment:11 Changed 10 months ago by digitall

Component: Ports
Note: See TracTickets for help on using tickets.