Build with MSVC pakset checksum mismatch.

[DrSuperGood]

Builds made with MSVC appear to be suffering from a pakset mismatch error with servers.  The checksum for the loaded pakset mismatches the server even though the very same pakset matches the server (the GCC built nightlies using the same file work). Checking for the mismatch starts by returning 10-11 good types which generally points towards a hashing error, possibly only with goods (not sure since it only returns "first 11").

To see how far this error ran I disabled the "join prevention" for pakset mismatch so I could join mismatched servers (it thinks all servers match always). I was able to successfully join the server, build some random lines and am still connected to it after 15 minutes. This means that it clearly is not a major pakset mismatch like it would have me think.

It also raises a minor technical issue that nothing stops you joining a mismatched server. What really should happen is the server evaluates your hash and refuses you if it fails, not the other way around (at least for physically joining). Security aside MSVC builds are producing incorrect checksums for loading the identical pakset as the release/nightly builds.

EDIT: I can confirm this applies to both GDI and SDL2 builds so it is something wrong with the checksum code and not just me building it badly.

[Ters]

It might be something as simple as GCC and MSVC laying out structures differently, and Simutrans then generating the hashcode on the raw bytes. (I haven't looked.)

[Dwachs]

The checksum code has not been touched for months. It does not work with raw memory, it builds the hash only from the crucial properties of an object.

Maybe the endianness is set wrong for you build? Or the windows build loads addons by default? This can be tested: remove the simutrans/addons folder for both client and server.

How can you be sure that the server and client paksets are indeed indentical?

[DrSuperGood]

How can you be sure that the server and client paksets are indeed indentical?

Same installation of Simutrans as I use for playing using the official release versions which are built using GCC. It has no problem joining what so ever and the pakset flags as matching perfectly.

Only difference is that if I load it with a MSVC build (running in same directory as I use for the official release install) it then shows the exact same pakset now as mismatching. I can also confirm that it is not loading the pak64 food addon without my knowledge since there are no food goods available.

The server is not run by me but someone else (moblet) who is probably using a GCC build. I will try running a local MSVC server later and seeing if the checksum matches from the MSVC client.

I also am not booted from the pak64 server if I disable the pak check in the lobby. This means that the game results must be reasonably deterministic between MSVC and GCC builds and that most likely there is something mismatching with the checksum generation process.

Of Interesting note is that it appears my MSVC custom build of the nightly is performing better than the standard GCC build (both GDI and SDL and both the same nightly and release). Where as the GCC build is obtaining a frame rate of 13-17 in a well developed map (fifty's old server from last year), my MSVC GDI build and MSVC SDL2 (no comparison available) build achieves 21-25 fps in the same far out zoom.

However the SDL2 build suffers from very bad map loading performance for some reason (possibly stalls due to updating the UI?!).

[prissi]

Indeed, the warenbauer did not initialized fields (catg_index) which were used for checksum calculation. MSVC fill for debug builds real random values into the memeory, while GCC just nulls them.

[DrSuperGood]

Indeed, the warenbauer did not initialized fields (catg_index) which were used for checksum calculation. MSVC fill for debug builds real random values into the memeory, while GCC just nulls them.

In this case the solution will be to explicitly initialize them with the same value as GCC does (for backwards compatibility purposes).

There are also two other errors I picked up.

At the start of simmain.cc the following is declared.

Code Select Expand


#if defined(_M_X64)  ||  defined(__x86_64__)
#warning "Simutrans is preferably compiled as 32 bit binary!"
#endif

However the macro "#warning" is a GCC macro so will throw a syntax error when building with other compilers.
The simplest solution is to check the compiler with a define statement so the line is not parsed for non GCC compilers.

Code Select Expand


#if defined(_M_X64)  ||  defined(__x86_64__)
#if   __GNUC__
#warning "Simutrans is preferably compiled as 32 bit binary!"
#endif
#endif

This is critical for building a x64 build using MSVC.

The other change is with SDL sound for MSCV. The one standard library import is not defined and so throws usual syntax errors for undeclared names. The following import is required.

Code Select Expand


#include <stdio.h>

If anyone ever wants to use SDL sound with MSCV is another question however it is still good practice for portability.

I have attached a patch with both these changes.

[Ters]

At the start of simmain.cc the following is declared.

Code Select Expand


#if defined(_M_X64)  ||  defined(__x86_64__)
#warning "Simutrans is preferably compiled as 32 bit binary!"
#endif

However the macro "#warning" is a GCC macro so will throw a syntax error when building with other compilers.
The simplest solution is to check the compiler with a define statement so the line is not parsed for non GCC compilers.

Doesn't Microsoft have a way to emit such warnings as well? Because why should it be OK to build 64-bit with MSVC, while GCC users get a warning?

[DrSuperGood]

Doesn't Microsoft have a way to emit such warnings as well?

Yes but it uses a different syntax. You can add one if you want.

[prissi]

MSVC uses anyway no inline assembly at the moment ...

[DrSuperGood]

I believe I found another checksum mismatch. The mismatch with pak128 was reported as being cactuses of sort which are declared as ground objects.

The member "index" used by ground objects appears to be uninitialized during hashing so results in compiler dependent behaviour. Initializing it to a specific value did not help match up with GCC due to GCC doing other stuff with uninitialized variables so I am not sure if this is the entire problem/solution.

[Ters]

If the variable is initialized to a proper value, it doesn't matter what GCC does to uninitialized valiables.

[Dwachs]

There seems to be indeed a problem with this index variable. It should not be part of the checksum.

Edit: in r7439.

[DrSuperGood]

If the variable is initialized to a proper value, it doesn't matter what GCC does to uninitialized valiables.

It does for me trying to test if it is the cause of the problem. I do not have GCC set up to build for Windows.

There seems to be indeed a problem with this index variable. It should not be part of the checksum.

Excellent.