Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 158646 - scorched3d not 64bit clean
Summary: scorched3d not 64bit clean
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: scorched3d
Version: rawhide
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 161694 FE5Target
TreeView+ depends on / blocked
 
Reported: 2005-05-24 15:27 UTC by Jeremy Katz
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-02-11 21:54:19 UTC


Attachments (Terms of Use)
Preliminary x86_64 fixes (deleted)
2005-05-24 19:07 UTC, Toshio Kuratomi
no flags Details | Diff
my patch, for reference (deleted)
2005-05-25 20:14 UTC, Jeremy Katz
no flags Details | Diff

Description Jeremy Katz 2005-05-24 15:27:00 UTC
Fails to rebuild on x86_64 due to int/pointer mismatches

http://extras64.linux.duke.edu/failed/development/scorched3d/37.2-3/x86_64/scorched3d-37.2-3.failure.log

Comment 1 Toshio Kuratomi 2005-05-24 19:07:18 UTC
Created attachment 114792 [details]
Preliminary x86_64 fixes

Here's a preliminary patch.  It gets scorched3d to build and run as far as the
menu and preferences.  I don't know about getting further -- I don't have a
working 3d card right now and I'm getting errors about GLX that I assume mean I
need that.  (If not, I'll open another ticket or research upstream.)

I think there are some problems with this patch but I need someone more
knowledgable about 64bit issues to confirm.  When we have code like:

int foo(void *ptr) {
 return (int)ptr;
}

I think it should be converted to:
long foo(void *ptr) {
 return (long)ptr;
}

This is what's been done in this patch.  I think it also has to address calling
code:

void main() {
 int temp;
 temp = foo();
}

and change to:

void main() {
 long temp;

 temp = foo();
}

Which hasn't been done yet.

Also, should pointers all be unsigned long?  Or does long suffice?

Comment 2 Jeremy Katz 2005-05-25 17:02:09 UTC
Yes, pointers are long on 64bit arches.

A different approach to take might be to do like glib does and define a macro
like the following (and this is the anaconda version)
/* 64 bit platforms, definitions courtesy of glib */
#if defined (__x86_64__) || defined(__ia64__) || defined(__alpha__) ||
defined(__powerpc64__) || defined(__sparc64__) || defined(__s390x__)
#define POINTER_TO_INT(p)  ((int) (long) (p))
#define INT_TO_POINTER(i)  ((void *) (long) (i))
#else
#define POINTER_TO_INT(p)  ((int) (p))
#define INT_TO_POINTER(i)  ((void *) (i))
#endif

Then you just use that instead of the casting and it handles things properly
without having to change any of the return types (similar macros can be done for
the unsigned versions as well)

Comment 3 Jeremy Katz 2005-05-25 20:07:22 UTC
Hmmm, even with these changes it still doesn't really work beyond the settings
dialog.

For now, I'm thinking an excludearch might make sense.  The code looks like it's
littered with int/long assumptions that you really can't make :(

Comment 4 Jeremy Katz 2005-05-25 20:14:00 UTC
Created attachment 114850 [details]
my patch, for reference

Comment 5 Panu Matilainen 2005-05-31 09:38:43 UTC
Hmm.. Scorced CVS doesn't seem to have anything to help the situation either,
best to make it excludearch for now as I don't expect to have this sorted out by
FC4 release by any means.

Feel free to commit that excudearch change, I still don't have CVS access :-/

Comment 6 John Ellson 2005-06-12 15:36:11 UTC
Re comment #2.  Why not just "#include <stdint.h>" and use "intptr_t" ?


Comment 7 Jeremy Katz 2005-06-12 16:00:42 UTC
Because a number of upstream projects aren't ready to commit to C99.  That may
be (slowly) changing as I think the new Debian stable should have a new enough
compiler, but it has traditionally been a problem as I've pushed 64bit fixes
upstream.  To the point that I stopped trying to go the c99 route

Comment 8 Ville Skyttä 2005-10-11 19:23:17 UTC
I've updated the devel branch to 39.1 in CVS but not tagged yet, could someone 
check if it still needs patching for ExcludeArch before I request builds of 
it?  Feel free to commit whatever is needed. 
  
(You'll need a new openal-devel from devel CVS to build 39.1; it's been built  
but not yet published.)  

Comment 9 Hans de Goede 2006-02-05 08:54:05 UTC
I've taken ownership of scorched3d and will look into this (as time permits).

Comment 10 Wart 2006-02-08 04:52:02 UTC
I just managed to build this on FC-4 x86_64 using the devel sources.  I only had
to change BR: wxGTK-devel to wxGTK2-devel to get it to build on FC4.

I was able to start it up and begin a game without any problems.

Comment 11 Hans de Goede 2006-02-08 21:26:16 UTC
Hi Wart,

Thanks for looking, I know it compiles and runs kinda sorta in local play mode,
but internet play is (was?) severly hoosed.

It has taken me a lot of fiddling and looking at the code to get this fixed, but
I'm about to commit a new 64bit patch to fe-cvs devel tree which should also fix
internet play, both hosting a server and as a client. If you want I can give you
the gory details, I need to type them out for upstream anyways.


Comment 12 Hans de Goede 2006-02-10 13:27:42 UTC
This is fixed in FE-CVS devel tree now, I'll psuh an update when I also have
fixed the security issue reported in bug 161694.

As promised here is an explanation of the fix as send upstream:

The problem is NetServer.cpp and the destinationID's used elsewhere, destination
ID's are unsigned int and thus are 32 bits even on a 64 bit arch. Since these
also get exchanged over the network and I did't want to break (network)
compatibility with the existing release I've choosen to keep the destinationID's
unsigned int, which also minimizes changes outside of NetServer.cpp . The
problem is that NetServer.cpp generates this ideas by casting TCPsocket to
unsigned int, as TCPsocket is a ptr to a struct TCPsocket_, this means that
NetServer is putting ptr's into ints this won't work on 64 bits since ints are
only 32 bits and addresses are 64 bit, thus this will loose a part of the address.

NetServer uses NetServerRead todo most of the work and when NetServer gets
called it uses a std::map<TCPsocket, NetServerRead *> to find the NetServerRead
to use. I've changed this to an std::map<unsigned int, NetServerRead *>. And I
generate unique ideas in addClient. Thus removing the ugly and on 64 bit defect
storing of TCPsocket addresses in unsigned int. For the rare case where the
TCPsocket (getIpAddress) is actually needed by NetServer I've added a
getSocket() to NetServerRead, so NetServer can get to the socket asociated with
the destinationID.

Since NetServerRead and NetServerXXXProtocol need access to both the socket and
the destinationId and those are no longer the same I've extended NetServerRead
to get passed both the socket and the destID on create and I've extended the
relevant NetServerXXXProtocol methods to have both a socket and a destID as
parameters.

Besides that there are a few places where s3d stores integers in pointers, but
except for needing a cast to compile on 64 bit this is no problem.


Comment 13 Hans de Goede 2006-02-11 21:54:19 UTC
scorched3d-39.1-2 has been built for devel / FC5 which fixes this.



Note You need to log in before you can comment on or make changes to this bug.