News:

Use the "Forum Search"
It may help you to find anything in the forum ;).

r5908 - 64-bit incompatibility

Started by Ters, September 23, 2012, 10:52:33 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Ters

I tried compiling a 64-bit executable from r5908 with gcc 4.5.4, but got errors in boden/grund.cc due to loss of precision when casting a pointer to long. It appears that the cast is to produce a magic number unique to that instance, so the fact that one can not recover the pointer after casting to long and back isn't that much of an issue, but there is no longer any guarantee that the magic number is unique. When treating pointers as number, I believe the correct type to use is size_t, not long. How feasible is it to use size_t for these magic numbers. On 32-bit architectures, there should be no difference, except maybe the signedness, while 64-bit platforms might not suffer too much from increased data size.

Edit:
I only get this problem when targetting 64-bit Windows, not when targetting 64-bit Linux, which explains why I haven't seen this error before. Apparently Microsoft retained 4-byte longs for compatibility reasons, while Linux only did that for ints.

TurfIt

I fail to see how r5908 could cause this... and grund.cc hasn't been touched in a while. Most recent pointer tricks I see is r5889 and gebaeude.cc ?

Ters

It's not a bug introduced in r5908. I just added the revision number I discovered it in, so people know which revision should be able to reproduce the problem. The problem might date from pre-historic times for all I know. It's not restricted to grund.cc either, but almost everything that can have an associated window. There are also some similar but unrelated problems with electric grids.

I've tried out my suggestion, and got an executable that managed to load up and run a game, but I haven't done extensive testing yet.

prissi

Anyway, the usual answer is: Do not compile for 64bit. It consumers much more memory (since the structs/class were optimized for 32 bit) and thus runs slower.

But concerning the window magics: The could be easily changed to size_t without any impact. The only "interesting" question is: Will the enum+size_t result in size_t?

Most of the windows do not use pointers, but rather has their personal hash. But anyway, test next revuision.

TurfIt

re: r5937 - size_t is unsigned. Compiler now spits out even more signed/unsigned comparison warnings... And logic like the below needs attention...  -1


-gui_frame_t *win_get_magic(long magic)
+gui_frame_t *win_get_magic(size_t magic)
{
   if(magic!=-1  &&  magic!=0) {

prissi

Could you prepare a patch for that, please. I am rather busy with 2000 other things.

TurfIt

What, only 2000?  ;D
I think ptrdiff_t rather than size_t is correct here. So did that...

@Ters: The window saving/loading looks problematic, please test that specifically.

Ters

As for comparision, I noticed a (unsigned long)cnv > 1 somewhere. That was a bit spooky.

prissi

Thanks for the patch.

This pointer to int comparison stems from loading; The variable is used to indicate the search state of the convoi. Same for the window id during loading: Those windows with pointers are not restored during reloading.

Ters

I have done some more testing on what has been done. The 64-bit version can load a game saved with a 32-bit version, and the other way around. Tree and convoy windows close when the associated object is removed from map. I wasn't aware of the concept of window saving and loading. Does that even work in any case? If so, how?

There are still some more things I've mentioned that still don't compile when targetting 64-bit Windows, but they might be harder to fix properly. 32-bit longs on 64-bit architecture is probably most likely to occur on architectures with backwards compatibility, so I guess the need for supporting those with a 64-bit build isn't as great, while true 64-bit platforms where a 32-bit release is impossible would probably have 64-bit longs. Though it is a bit safer to use the most correct type for any given thing.

TurfIt

Windows are restored at simworld.cc:5272. Normally only in mulitplayer, but simply comment out the if( ::restoreUI) check to easily test single player. Any windows that are open when the game is saved should be restored.

Different sized types should be able to be handled in simtypes.h. Modify the typedefs as needed.

prissi

As far as I know, intel has no penalty using sint32 instead of sint64 in x64 mode, when 32 bit are ok. It profits anyway from the larger number of registers. And for the same behaviour in network mode, using same type sizes are most helpful ...