x86_64 patch

Discuss the future of Warzone 2100 with us.
Troman
Trained
Trained
Posts: 424
Joined: 12 Aug 2006, 15:40
Contact:

Re: x86_64 patch

Post by Troman »

I attached the patch.

I have never actually used it, so there may be some bugs. And looks like I forgot to make eventSetContextVar() calls to pass data's address.

Here's my eventSetContextVar implementation, I probably forgot to include it into the patch before reverting the changes:

Code: Select all

// Set a global variable value for a context
BOOL eventSetContextVar(SCRIPT_CONTEXT *psContext, UDWORD index, INTERP_VAL *data)
{
	INTERP_VAL	*psVal;

	if (!eventGetContextVal(psContext, index, &psVal))
	{
		return FALSE;
	}

	if (psVal->type != data->type)
	{
		ASSERT( FALSE, "eventSetContextVar: Variable type mismatch (%d/%d)", psVal->type, data->type);
		return FALSE;
	}

	// Store the data
	memcpy(psVal, data, sizeof(INTERP_VAL));

	return TRUE;
}
Attachments

[The extension has been deactivated and can no longer be displayed.]

Sign Up for Beta-Testing:
?topic=1617.0
pseudonym404
Trained
Trained
Posts: 35
Joined: 08 Nov 2006, 02:06

Re: x86_64 patch

Post by pseudonym404 »

Cheers :)

Load/save routines will need changing ("TODO: use union" makes sense, looks like I get to do that ;) )
Troman
Trained
Trained
Posts: 424
Joined: 12 Aug 2006, 15:40
Contact:

Re: x86_64 patch

Post by Troman »

Oh, looks like we have a volunteer.  ;D

I have that on my todo, but I doubt i'll get to that anytime soon.
Sign Up for Beta-Testing:
?topic=1617.0
pseudonym404
Trained
Trained
Posts: 35
Joined: 08 Nov 2006, 02:06

Re: x86_64 patch

Post by pseudonym404 »

The only thing left that's preventing getting into the game (gameplay itself, getting the menu system working pretty much just needed void* treap keys) is the union needs to be initialised to 0 before assigning anything to it (getting invalid pointers that look like 0xc63ad8ef00000000, so if (pointer) checks are true when they shouldn't be)

Currently, if I add an extra .ival !=0 check in scrvUpdateBasePointers, I do get to play the game briefly, but it segfaults after a short whole, I think for the same reason (haven't investigated that one much, as I'm still trying to solve this issue).

I'm thinking a memset(val, 0, sizeof(INTERP_VAL)) would be neater than .oval = 0 (maybe in the future another type will be larger), but where? Is there anywhere where this can be done globally, or do I simply have to track them all down?

edit: nevermind.. stack.c and event.c (the ones in event.c were the major issue) ;)

edit: incidentally, is the background for the text version of the cutscenes supposed to be orange?
Last edited by pseudonym404 on 23 Nov 2006, 19:46, edited 1 time in total.
Troman
Trained
Trained
Posts: 424
Joined: 12 Aug 2006, 15:40
Contact:

Re: x86_64 patch

Post by Troman »

pseudonym404 wrote: Currently, if I add an extra .ival !=0 check in scrvUpdateBasePointers, I do get to play the game briefly, but it segfaults after a short whole, I think for the same reason (haven't investigated that one much, as I'm still trying to solve this issue).
What are you trying to achieve with this?
pseudonym404 wrote: I'm thinking a memset(val, 0, sizeof(INTERP_VAL)) would be neater than .oval = 0 (maybe in the future another type will be larger), but where?
Pay attention that this way you reset the type to VAL_BOOL. Maybe it's better to  memset only the union itself, otherwise you will probably have to set the type explicitly back to what it used to be.
pseudonym404 wrote: Is there anywhere where this can be done globally, or do I simply have to track them all down?

edit: nevermind.. stack.c and event.c (the ones in event.c were the major issue) ;)
Are you referring to eventNewContext() and resetLocalVars() in event.c?
pseudonym404 wrote: edit: incidentally, is the background for the text version of the cutscenes supposed to be orange?
No, actually it should be black (and it is black usually), but sometimes it does become orange for some reason.
Sign Up for Beta-Testing:
?topic=1617.0
pseudonym404
Trained
Trained
Posts: 35
Joined: 08 Nov 2006, 02:06

Re: x86_64 patch

Post by pseudonym404 »

Troman wrote: What are you trying to achieve with this?
I was just checking if the least significant 32 bits was null to see if it got past the point where it was currently segfaulting (it did)
Troman wrote: Pay attention that this way you reset the type to VAL_BOOL. Maybe it's better to  memset only the union itself, otherwise you will probably have to set the
type explicitly back to what it used to be.
Yeah, I've made sure I don't lose the type (although when first initialising in stack.c, i've left it initialised to BOOL, this doesn't seem to be a problem, but I can change it if you like).
Troman wrote: Are you referring to eventNewContext() and resetLocalVars() in event.c?
Yep :)
Troman wrote: No, actually it should be black (and it is black usually), but sometimes it does become orange for some reason.
I'll leave that alone for now as it's not amd64 specific (incidentally, it looks like it's the same colour as the fog colour)
Troman
Trained
Trained
Posts: 424
Joined: 12 Aug 2006, 15:40
Contact:

Re: x86_64 patch

Post by Troman »

pseudonym404 wrote: Yeah, I've made sure I don't lose the type (although when first initialising in stack.c, i've left it initialised to BOOL, this doesn't seem to be a problem, but I can change it if you like).
When just created it shouldn't really matter, as long as it's not VAL_STRING or any other pointer.
pseudonym404 wrote: I'll leave that alone for now as it's not amd64 specific (incidentally, it looks like it's the same colour as the fog colour)
Yes I think that's background fog.
Sign Up for Beta-Testing:
?topic=1617.0
pseudonym404
Trained
Trained
Posts: 35
Joined: 08 Nov 2006, 02:06

Re: x86_64 patch

Post by pseudonym404 »

ok, single player seems to be working, load/save works, and multiplayer seems to be working (tested briefly on local loopback), there's still stuff left to fix, but the attached patch is just enough to get warzone going on amd64 (i'm going to look at the rest, but multiple smaller patches seems like a better idea) :)
Attachments

[The extension has been deactivated and can no longer be displayed.]

User avatar
DevUrandom
Regular
Regular
Posts: 1690
Joined: 31 Jul 2006, 23:14

Re: x86_64 patch

Post by DevUrandom »

You check for sizeof(void*)==sizeof(UDWORD) and assert only if this is not true... Shouldn't we assert on all systems just to notify the devs? Because in release builds there wont appear any asserts, so the end user will never get any "crashes"...

(I'll have a more thorough look at the rest of the patch tomorrow.)
pseudonym404
Trained
Trained
Posts: 35
Joined: 08 Nov 2006, 02:06

Re: x86_64 patch

Post by pseudonym404 »

I was a little undecided about that, I changed that bit a few times..

In the end, I opted for a debug message on platforms where it won't segfault later, but whichever you think is best :)
User avatar
DevUrandom
Regular
Regular
Posts: 1690
Joined: 31 Jul 2006, 23:14

Re: x86_64 patch

Post by DevUrandom »

Eg I (and I think most of the others) don't have a x86_64 system, so they will never get a notification "the hard way". And actually I don't read the debuglogs often and for sure not fully. (I guess others don't either?) So best to find all these cases of potential is IMO to do it the hard way on all systems so the devs know it early and hopefully fix it early too. (Just my opinion.)

- In some places the indention is changed from tabs to spaces. Please take care this doesn't happen the next time...
- In case assert is disabled for NDEBUG we need to return... (from scriptTypeIsPointer())
+//this is sizeof(FACTORY) the largest at present 11-2-99 - increased AB 22-04-99
+//depends on pointer sizes - LB 22-11-06
*g* (This code makes ppl use it's funny notations. ;) )
Last edited by DevUrandom on 25 Nov 2006, 02:52, edited 1 time in total.
pseudonym404
Trained
Trained
Posts: 35
Joined: 08 Nov 2006, 02:06

Re: x86_64 patch

Post by pseudonym404 »

DevUrandom wrote: So best to find all these cases of potential is IMO to do it the hard way on all systems so the devs know it early and hopefully fix it early too. (Just my opinion.)
Makes sense :)
DevUrandom wrote: - In some places the indention is changed from tabs to spaces. Please take care this doesn't happen the next time...
Oops.. I had tab marks turned off in kate (I have no idea why), back on now :)
DevUrandom wrote: - In case assert is disabled for NDEBUG we need to return... (from scriptTypeIsPointer())
Ah, mistakenly thought assert would always halt execution
DevUrandom wrote: *g* (This code makes ppl use it's funny notations. ;) )
:)
Troman
Trained
Trained
Posts: 424
Joined: 12 Aug 2006, 15:40
Contact:

Re: x86_64 patch

Post by Troman »

I noticed there are some pointer arithmetics with void * pointer, actually it shouldn't have any size, and MSVC chokes on it, I added char* casts instead, because that's what I think GCC does on default with it. Plase have a look at it, I don't know if that can have any drawbacks on x86_64.
Sign Up for Beta-Testing:
?topic=1617.0
User avatar
DevUrandom
Regular
Regular
Posts: 1690
Joined: 31 Jul 2006, 23:14

Re: x86_64 patch

Post by DevUrandom »

Troman: ? Could you explain that in more detail? void* does have a sizeof 0 on MSVC??

PS: Your patch is commited, pseudonym.
Last edited by DevUrandom on 25 Nov 2006, 21:07, edited 1 time in total.
Troman
Trained
Trained
Posts: 424
Joined: 12 Aug 2006, 15:40
Contact:

Re: x86_64 patch

Post by Troman »

Pointer arithmetics is simply not allowed with void pointers, since its size is undetermined (I looked it up and seems like gcc assumes that it's (char*) on default, but it should still complain), you have to implicitly cast to some other type. sizeof on a void pointer would probably not compile ta all, at least on MSVC.
Sign Up for Beta-Testing:
?topic=1617.0
Post Reply