[fixed]64 bit integer typedefs have different definitions in different compilation units Mac OS

[edo9300]

I'm building irrlicht 1.9 (even tho the issue shoudl also apply to older irrlicht) with xcode 9.2 on mac os 10.12 and I was having some linking issues when i tried to use utf8ToWchar and wcharToUtf8 inside CIrrDeviceOSX.mm, the issue was about the linker not managing to find the symbol

Code: Select all

__ZN3irr4core11utf8ToWcharEPKcPwm

used from a function whitin the objective c file.
I checked the exported symbols from an irrlicht static library, and the exported symbols from utf8.o are

Code: Select all

0000000000000000 T __ZN3irr4core11utf8ToWcharEPKcPwy
0000000000000257 T __ZN3irr4core11wcharToUtf8EPKwPcy

but

Code: Select all

__ZN3irr4core11utf8ToWcharEPKcPwm

is also listed as used (and it's the one that fails to link).
Putting those symbols through a demangler, it shows that the one that is exported translates to

Code: Select all

_irr::core::utf8ToWchar(char const*, wchar_t*, unsigned long long)

, while the one that is failing to link translates to

Code: Select all

_irr::core::utf8ToWchar(char const*, wchar_t*, unsigned long)

.
This shows that the u64 and s64 types aren't consistent inside irrlicht itself, and highly probably could cause issues even when linking with programs using the library.
The issue arises when depending on the headers included beforehand, the check

Code: Select all

#elif defined(__GNUC__)
#if defined(__WORDSIZE) && __WORDSIZE == 64
typedef unsigned long int			u64;
#else
__extension__ typedef unsigned long long	u64;
#endif

could lead to different results, if no standard c library headers are included,

Code: Select all

__WORDSIZE

won't be defined, and thus

Code: Select all

typedef unsigned long int

will be picked, but if they are included,

Code: Select all

typedef unsigned long int

will be picked instead.
I'm not sure if in the case of CIrrDeviceOSX.mm the header inclusion is caused by some of my modifications, but regardless of that, this happens for example when compiling the sources of the bundled aesGladman (a quick way to test this, is to remove the definition as unsigned long int or put ill formatted code inside that define, and then the code will fail to build), and as i said before, this could lead to linking issues with the various programs.
The way to fix this would be to change the logic to choose the typedef for 64 bit integers and especially not use the compiler reserved flags (with __WORDSIZE being one of them) to determine them.
Needless to say that this would be a highly possible abi breaking change on non windows systems.

[CuteAlien]

Sounds like this bug which I closed a few months ago due to lack of further info: https://sourceforge.net/p/irrlicht/bugs/433
Patches welcome. I can't test OS X as I don't have it, so I completely depend on patches and feedback there (I do check if they make sense and test on Windows/Linux/Android).

[edo9300]

Maybe a not so bad fix could be to make that check for __LP64__ or _LP64 being defined instead of _WORDSIZE, as in that case they would be defined by the compiler directly, rather than being imported by an header. Mac os headers for example check for the existence of __LP64__ to define _WORDSIZE

Code: Select all

#if __LP64__
#define __WORDSIZE 64
#else
#define __WORDSIZE 32
#endif

and of which gcc ensure the definition

__LP64__
_LP64
These macros are defined, with value 1, if (and only if) the compilation is for a target where long int and pointer both use 64-bits and int uses 32-bit.

This check should be enough as this whole branch is entered only if __GNUC__ is defined, in which case __LP64__ should be defined as well. (Also in this case the abi break would highly probably happen on BSD system leaving linux untouched, as on linux long int was already being used.)

[CuteAlien]

I'm still a bit puzzled why __WORDSIZEis still not included in your case after the last fix.
The #include <limits.h> at the start of irrTypes.h should have ensured that it's always included - any idea why that is not working for you?
(edit: Note that this _was_ wrong in Irrlicht 1.8, so this is for example also a bug that happens if library/code accidentally include headers from different versions)

We should never set __WORDSIZE that's just a recipe for disaster (setting standard library defines ourselfes... then any code using Irrlicht which is also trying to check for those defines might get messed up results).

Have to read up on __LP64__first, but lacking time for that right now (doing taxes *sigh* someone kill me please).

[edo9300]

For the inclusion, that's up to the compiler/implementation, as in any case__WORDSIZE isn't standard, if you see mac os sdk headers, you can see it clearly https://github.com/phracker/MacOSX-SDKs ... e/limits.h this is limits.h of the current sdk i'm using to build, and here it is stdint https://github.com/phracker/MacOSX-SDKs ... t.h#L9-L13. As you can see wordsize is being defined here.
In any case, my fix isn't about defining wordsize ourselves, but rather check for that __LP64__ macro directly

Code: Select all

#if defined(_MSC_VER) || ((__BORLANDC__ >= 0x530) && !defined(__STRICT_ANSI__))
typedef unsigned __int64			u64;
#elif defined(__GNUC__)
#if (defined(__LP64__) && __LP64__ == 1) || (defined(_LP64) && _LP64 == 1)
typedef unsigned long int			u64;
#else
__extension__ typedef unsigned long long	u64;
#endif
#else
typedef unsigned long long			u64;
#endif

If we're a bit paranoid we could also still check for __WORDSIZE==64, even if it would be redoundant.

[edo9300]

Btw, I too up until today didn't know anything about that, i've found it out by looking here https://sourceforge.net/p/predef/wiki/Home/, this page specifically https://sourceforge.net/p/predef/wiki/DataModels/. This piece of wiki is very useful .

[CuteAlien]

OK, not sure right now from quick reading if there's a chance that long int is 64 bit while pointers are 32 bit (in which case it wouldn't work).
Documention only says it's set when both are 64 bit, but not if there's a chance that only one of them is.

But I suppose adding to existing check should be safe. So

Code: Select all

#if defined(__WORDSIZE) && __WORDSIZE == 64

would become (I didn't compile&test yet...):

Code: Select all

#if (defined(__LP64__) && __LP64__ == 1) || (defined(_LP64) && _LP64 == 1)  || (defined(__WORDSIZE) && __WORDSIZE == 64)

And I guess I can remove limits.h include again maybe. Or safer to just keep it.

edit: OK, you posted another link while I wrote this. But I really have to get back to my hated taxes or I'll waste yet another day on not doing them, sorry ;-) (edit: I'll be back later... probably...)

[edo9300]

If the __WORDSIZE check remains then the include should remain as well, otherwise it won't be defined under any circumstances unless a previously included header defined it.
Also i know the feeling about wasting time, totally not doing that by dealing with an os i don't even use instead of studying.

[CuteAlien]

Yeah, you are right - keeping the include in there. I hope r6278 in svn trunk works better now.

Thanks for your help!