Opened 16 years ago

Closed 16 years ago

Last modified 13 months ago

#8274 closed patch (wontfix)

Make ScummVM compile with GCC 3.3.1

Reported by: SF/johneck Owned by: fingolfin
Priority: normal Component: Port: Linux
Keywords: Cc:
Game:

Description

The latest CVS versions of ScummVM refuses to compile, it dies in all asserts()
where pointers are used.
Example:
byte* tmp = some_function_method_or_other();
assert(tmp);

Will fail with "*byte is not of long int" (or similar). The way to fix this is to:
byte* tmp = some_function_method_or_other();
assert(tmp != NULL);

And atleast it compiles with GCC 3.3.1 now. The patch fixes all these, atleast when
trying to compile for the SDL backend.

Ticket imported from: #794066. Ticket imported from: patches/379.

Attachments (1)

scummvm.patch (19.1 KB ) - added by SF/johneck 16 years ago.
Patch

Download all attachments as: .zip

Change History (13)

comment:1 by fingolfin, 16 years ago

There is no patch attached.

BTW it seems this must be new in GCC 3.3.1, as things work just
fine with 3.3 for me (and quite frankly, the error/warning makes
no sense at all ... a compiler bug?)

by SF/johneck, 16 years ago

Attachment: scummvm.patch added

Patch

comment:2 by SF/johneck, 16 years ago

Strange. I distinctly remember adding the patch... Maybe Opera did something stupid.

Trying again.

comment:3 by SF/johneck, 16 years ago

This is the exact error:

g++ -Wp,-MMD,"scumm/.deps/actor.d",-MQ,"scumm/actor.o",-MP -O -Wall -Wuninitialized
-Wno-long-long -Wno-multichar -Wno-unknown-pragmas -g -ansi -W -Wno-unused-parameter
-pedantic -Wpointer-arith -Wcast-qual -Wcast-align -Wconversion -Wshadow -Wimplicit
-Wundef -Wnon-virtual-dtor -Wno-reorder -Wwrite-strings -fcheck-new -Wctor-dtor-privacy
-DHAVE_CONFIG_H -DUNIX -I. -Icommon -I/usr/include/SDL -D_REENTRANT -c scumm/actor.
cpp -o scumm/actor.o
scumm/actor.cpp: In member function `void Actor::animateCostume()':
scumm/actor.cpp:1038: error: invalid conversion from `byte*' to `long int'
scumm/actor.cpp: In member function `void Actor::animateLimb(int, int)':
scumm/actor.cpp:1070: error: invalid conversion from `byte*' to `long int'
make: *** [scumm/actor.o] Error 1

GCC version:
Reading specs from /usr/lib/gcc-lib/i386-redhat-linux/3.3.1/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info
--enable-shared --enable-threads=posix --disable-checking --with-system-zlib
--enable-__cxa_atexit --host=i386-redhat-linux
Thread model: posix
gcc version 3.3.1 20030811 (Red Hat Linux 3.3.1-1)

comment:4 by SF/khalek, 16 years ago

GCC 3.3.1 worked fine here and the debian prerelease of GCC
3.3.2 also works fine here

comment:5 by SF/johneck, 16 years ago

Hmm...

Might be a problem with the assert.h file then. That is provided by glibc. The glibc on this
system is 2.3.2-71 (RedHat release). This is from Raw Hide (unstable, bleeding edge). I don't
know exactly how much Red Hat patches glibc but this release has parts fetched from the
glibc CVS.

And I don't know what ISO C says about assert but the glibc CVS might be botched or
something. Or the CVS glibc may have found a bug in the assert code and fixed it.
Not sure.

The assert does only have problems with non-POD types it seems.

comment:6 by SF/khalek, 16 years ago

I'm also running glibc 2.3.2-3 obviously the -version in
both cases being the vendor release.

Once again this is the version found in debian sid and it
works fine...

comment:7 by SF/johneck, 16 years ago

I have been playing around some more with this.

A small ugly test program:
#include <stdio.h>
#include <assert.h>

typedef unsigned char byte;

int main(void)
{
byte* nisse = NULL;
assert(nisse);

return 0;
}

compiling with:

gcc -Wall -o test test.c
test.c: In function `main':
test.c:9: warning: passing arg 1 of `__builtin_expect' makes integer from pointer without a
cast

g++ -Wall -o test test.c
test.c: In function `int main()':
test.c:9: error: invalid conversion from `byte*' to `long int'

This is with the newer gcc-3.3.1-2 release from Red Hat.

Reading specs from /usr/lib/gcc-lib/i386-redhat-linux/3.3.1/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info
--enable-shared --enable-threads=posix --disable-checking --with-system-zlib
--enable-__cxa_atexit --host=i386-redhat-linux
Thread model: posix
gcc version 3.3.1 20030814 (Red Hat Linux 3.3.1-2)

A small ugly test program, modified:
#include <stdio.h>
#include <assert.h>

typedef unsigned char byte;

int main(void)
{
byte* nisse = NULL;
assert(nisse != NULL);

return 0;
}

Works fine, both for gcc and g++, no warnings.

I am not sure what ISO C / ISO C++ says about assert(). Is this a Red Hat thing?

I did try to compile a bunch of other programs and so only ScummVM and ClanLib produced
errors here (and in the case of ClanLib only two errors). The compiled stuff includes AbiWord,
Wine, basic GNOME libraries, ...

comment:8 by fingolfin, 16 years ago

ScummVM's use of assert() is 100% valid. Essentially, this
assert(foo);
is functionally equivalent to
if (!(foo)) { ... /* do something */ }
The error you get seems to imply something (compiler, C lib...) on
your system is broken. The error would occur if for some reasons,
assert on your system is not implemented as a macro, but rather
(incorrectly!) as a function with a signature like
void assert(long cond);

comment:9 by SF/johneck, 16 years ago

I know that assert(nisse); is valid C, and I am well aware of what assert do.

I did however figure out what broke it. It seems like RH decided to add __builtin_expect in
the assert macro. This takes a long parameter. C manages but C++ will fail to make the cast
from basically anything to long. The patch hides the problem since now __builtin_expect will
be called with a boolean expression which can be cast without problems.

comment:10 by fingolfin, 16 years ago

Owner: set to fingolfin
Resolution: wontfix
Status: newclosed

comment:11 by fingolfin, 16 years ago

I recommend you upgrade to a system with a non-broken compiler
then. Sorry but it can't be our task to add work arounds for
serious breakage some compiler vendor introduces into his
compiler.

comment:12 by digitall, 13 months ago

Component: Port: Linux
Note: See TracTickets for help on using tickets.