x86_64 patch

[Giel]

On a users request I've started looking into making warzone 64 bit safe. Stupidly enough I started out with 2.0.7, as taking the -m32 stuff in ./configure into account I thought that you (upstream) had kinda abandoned 64 bit. Luckily after only 3 hours of work  I hit the big script problem (which you seem to have nicely fixed) and then I did a (second) more thorough google search on warzone and 64 bit and found this thread.

Just to be sure you know. The 2.0 branch is _not_ and probably will not be 64bit safe. In fact we hardly even maintain the 2.0 branch anymore (only high priority and easy bugfixes make it in). The current development trunk is being worked on much more actively (and although not 100% 64bit safe, it is much better than 2.0.x).

So if there is an area I can help in let me know, also I wonder what the status is, I've read the entire Forum thread, but still a status update would be nice, reading this thread I get the impression that things are almost done / finished.

Currently spotting 64bit problems is our greatest task in making code 64bit safe. These will mainly occur in save/load to/from disk code and networking code. Of which the latter (i.e. networking code) is currently being worked on quite heavily so problems there are most likely to be spotted and solved fast. So you're greatest chance of discovering 64bit errors are in save/load code. A large portion of this save/load code is in src/game.c (be warned though: obfuscated C's winning code can probably be found there).

[HansdeGoede]

Currently spotting 64bit problems is our greatest task in making code 64bit safe. These will mainly occur in save/load to/from disk code and networking code. Of which the latter (i.e. networking code) is currently being worked on quite heavily so problems there are most likely to be spotted and solved fast. So you're greatest chance of discovering 64bit errors are in save/load code. A large portion of this save/load code is in src/game.c (be warned though: obfuscated C's winning code can probably be found there).

Erm, I did a full audit of the 2.0 code for the use of long values (one of the big culprits when loading binary stuff from disk) and I've found no 64 bit problematic uses of long in the save/load code. Actually the only 64 problematic uses of long was the use in the netmangle / unmangle code, which is no longer present in the svn trunk. Ofcourse there could still be use of int's as pointers, but load/save code is an unlikely case, and normally the compiler catches these and issues a warning.

[Watermelon]

Erm, I did a full audit of the 2.0 code for the use of long values (one of the big culprits when loading binary stuff from disk) and I've found no 64 bit problematic uses of long in the save/load code. Actually the only 64 problematic uses of long was the use in the netmangle / unmangle code, which is no longer present in the svn trunk. Ofcourse there could still be use of int's as pointers, but load/save code is an unlikely case, and normally the compiler catches these and issues a warning.

The pointer address to uint32 casts problems in load/save are already fixed iirc,the loading/saving problems are mainly due to different size of different type on different operating systems,e.g:I couldn't load a save game saved on 64bit linux under 32bit windows.

I believe most of the loading/saving problems reported on bug tracker are not relevant to 64bit,rather it's due to some problems in campaign scripting stuff and event save via scripts.

The netcode is a pain in the ass,it freaks out almost immediately when starting a mp game with 2 pc with different OSs or same os with different 'bit'.Hopefully EvilGuru and per will get it sorted soon.

[HansdeGoede]

The pointer address to uint32 casts problems in load/save are already fixed iirc,the loading/saving problems are mainly due to different size of different type on different operating systems,e.g:I couldn't load a save game saved on 64bit linux under 32bit windows.

Hmm, thats strange, the usual suspect here is the use of longs, which are 32 bit on i386 and 64 bits on x86_64 (and byte order when going from i386 to for example powerpc), but as said no longs are being used, so that leaves us with alignment issues as probable cause. I assume that the laod/save code dumps c-structs directly to disks? If so do these structs contain pointers (which will hopefully be reinitialized after load), notice that pointers will be 64 bit on a 64 bit arch and 32 bit on i386. So if pointers are saved to disk, then some padding needs to be added on i386 to make i386 / x86_64 compatible. Also notice that these pointers and other 64 bit variables like long long / int64 will get aligned to 64 bit. Last but not least the struct itself will get padded so that its size becomes a multiple of 64 bits on 64 bits archs.

If you for example have a struct like this:

Code: Select all

struct example1 {
   int a;
   int b;
   int c;
   void *ptr;
   int d;
};

Then in memory this will look like this on i386:

Code: Select all

struct i386_in_mem {
   int32 a;
   int32 b;
   int32 c;
   int32 ptr;
   int32 d;
};

And in memory this will look like this on x86_64:

Code: Select all

   int32 a;
   int32 b;
   int32 c;
   int32 pad1;
   int64 ptr;
   int32 d;
   int32 pad2;
};

[Watermelon]

Hmm, thats strange, the usual suspect here is the use of longs, which are 32 bit on i386 and 64 bits on x86_64 (and byte order when going from i386 to for example powerpc), but as said no longs are being used, so that leaves us with alignment issues as probable cause. I assume that the laod/save code dumps c-structs directly to disks? If so do these structs contain pointers (which will hopefully be reinitialized after load), notice that pointers will be 64 bit on a 64 bit arch and 32 bit on i386. So if pointers are saved to disk, then some padding needs to be added on i386 to make i386 / x86_64 compatible. Also notice that these pointers and other 64 bit variables like long long / int64 will get aligned to 64 bit. Last but not least the struct itself will get padded so that its size becomes a multiple of 64 bits on 64 bits archs.

yes it dumps the struct in memory directly into binary files on disk,though the dumping of pointers should already be avoided in save/loading,because the saved struct only holds IDs to certain object in data txt file(or a linear C array in memory know as 'some stats'),or IDs to droids in saved object list.I think per is working on a portable binary save format,not sure when/how it will be done though.

If you for example have a struct like this:

Code: Select all

struct example1 {
   int a;
   int b;
   int c;
   void *ptr;
   int d;
};

Then in memory this will look like this on i386:

Code: Select all

struct i386_in_mem {
   int32 a;
   int32 b;
   int32 c;
   int32 ptr;
   int32 d;
};

And in memory this will look like this on x86_64:

Code: Select all

   int32 a;
   int32 b;
   int32 c;
   int32 pad1;
   int64 ptr;
   int32 d;
   int32 pad2;
};

yea that could be the problem if some pointer got dumped to disk,the loading on 32bit OS(64bit save file) turned into a quasi-infinite 'for loop' because it tried to access the padding 32bit which has a value of 0xFFFFFFFF or some uninitialized value I guess.

[Giel]

Hmm, thats strange, the usual suspect here is the use of longs, which are 32 bit on i386 and 64 bits on x86_64 (and byte order when going from i386 to for example powerpc), but as said no longs are being used, so that leaves us with alignment issues as probable cause. I assume that the laod/save code dumps c-structs directly to disks? If so do these structs contain pointers (which will hopefully be reinitialized after load), notice that pointers will be 64 bit on a 64 bit arch and 32 bit on i386. So if pointers are saved to disk, then some padding needs to be added on i386 to make i386 / x86_64 compatible. Also notice that these pointers and other 64 bit variables like long long / int64 will get aligned to 64 bit. Last but not least the struct itself will get padded so that its size becomes a multiple of 64 bits on 64 bits archs.

Well I don't think we use any longs, but we use plenty of pointers though to cause alignment issues. In arrays due to the size difference and in single structs with pointers on the end due to padding issues and/or displaced databytes in the pointer itself.

If you for example have a struct like this:

Code: Select all

struct example1 {
   int a;
   int b;
   int c;
   void *ptr;
   int d;
};

More problematic IMO is the fact that sizeof(struct example1) will vary between 32bit and 64bit which will break code that saves/loads arrays of "struct example1". One example is the previous code of writeFXData and readFXData (before r2378).

That code dumped a struct to disk which at the end had a pointer (pointer to an IMD; i.e. a 3D model), from this pointer a 32bit hash was computed (based on the filename from which the IMD was originally loaded), this hash was then stored in the pointer.

We then dumped an array of these structs (with 32bit hashes stored in pointers which can be either 32 or 64 bit long). Now if we would store these values as little endian this would cause no problems if the array was 1 entry long. But fact is that we (try to) store _everything_ as big endian, so apart from alignment and padding issues this code could never produce portable save files (simply because of the size difference in the pointers causing the actual hash value to shift 4 bytes backwards).

[pseudonym404]

Unfortunately, I've somehow wound up catsitting this week (so I'm not at my machine, and can't get at a 64 bit machine until I get back),  but my basic plan of action was to clean up the few remaining pointer/integer cast warnings, and move on to persuading a 32 bit build to load save games from a 64 bit build and vice versa (I suspect much comparing of save games in a hex editor).

The large majority of cast warnings are now simply due to menu code storing integers in void pointers (pUserData, declared in widgbase.h and widget.h), which obviously works with pointers larger than the integer type (they're never written to disk), but is generating ~90 warnings.

Other than menu code, there's the animobj/hashtabl code, which is only potentially a problem in that by hashing only half of a pointer on 64 bit systems, it increases the chances of producing identical hashes for different pointer values (my last hashtabl patch should fix that); and game.c, which I'd only just briefly looked at before setting off (but look for values assigned using FIXME_CAST_ASSIGN, they're the ones that're reusing pointers for integer values).

I haven't looked at the netcode yet, as I've been mostly working on getting single player stable (I'd never played the game before I started submitting patches at the beginning of this thread, and would quite like to complete single player)

Current SVN is playable single player on amd64, with a few minor graphical glitches (purple flicker on background in menus while loading/saving, flickering between orange and black background ingame, and occasional 'messed up' walls (looks like polys have mixed up vertices).

[Giel]

but my basic plan of action was to clean up the few remaining pointer/integer cast warnings, and move on to persuading a 32 bit build to load save games from a 64 bit build and vice versa (I suspect much comparing of save games in a hex editor).

Well I'm quite confident that the *.gam files, at least for version 35 and 7 of the savegame file format, are entirely 64bit safe. Version 35 only uses fixed-width integers (and no pointers) and character arrays to store its data in.

Now as for the *.bjo files (found in subdirectories of savegames) I'm certain that fxstate.bjo, score.bjo and visstate.bjo are 64bit safe. I haven't (yet) looked at the code that writes out the rest series of *.bjo files though.

The large majority of cast warnings are now simply due to menu code storing integers in void pointers (pUserData, declared in widgbase.h and widget.h), which obviously works with pointers larger than the integer type (they're never written to disk), but is generating ~90 warnings.

Yes, I think everyone who's worked on Warzone's source code will agree that the GUI code is a complete mess. Storing integer values or boolean values in pointers is argueably a _bad_ thing to do.

Current SVN is playable single player on amd64, with a few minor graphical glitches (purple flicker on background in menus while loading/saving, flickering between orange and black background ingame, and occasional 'messed up' walls (looks like polys have mixed up vertices).

I can't remember ever encountering this kind of errors. If you know of ways to reproduce this consistently then you might consider filing a bug report.

PS You might want to submit your patches to the patchtracker, that way we can keep track of patches (whether they're committed or not, etc.).

[DevUrandom]

Some minor warning cleanups.

adding *.tab.c to BUILD_SOURCES and CLEANFILES before *.tab.h in applicable Makefile.am files causes *.tab.c to be the file that triggers the %.tab.h %.tab.c: %.y rule, resulting in the command run being "bison -y  -d -o[...].tab.c [...].y", not "bison -y  -d -o[...].tab.h [...].y", which (aside from being correct syntax ) stops "warning: conflicting outputs to file `[...].tab.h'"

chat_parser.y had %token _T_A declared twice, second instance removed

audp_parser.y had trailing | signs (presumably from when NULL wasn't commented out)

Applied in r2426.