Open baryluk opened 4 years ago
baryluk
commented
4 years ago BTW. I don't get this warning when compiling vfesession.o with povray latest-stable aka 3.7.0.8 on the same machine and using same toolchain and flags. So it looks like a regression.
baryluk
commented
4 years ago Yes, it looks like the issue is that in 3.7.0.8 the POV_LONG is defined uncondiotnally as long long on POSIX.
But now it is instead long long on 32-bit platforms, to make it 64-bit; and long on 64-bit platforms, to make it not 128-bit, and also 64-bit.
This happens in ./unix/povconfig/syspovconfig_posix.h , unix/povconfig/syspovconfig_gnu.h and in unix/povconfig/syspovconfig_bsd.h and in unix/povconfig/syspovconfig_osx.h in similar way. In fact all these files are identical, and it is a bit silly to even have them and cluster source code.
#if defined(_POSIX_V6_LPBIG_OFFBIG) || defined(_POSIX_V6_LP64_OFF64)
// long is at least 64 bits.
#define POV_LONG long
#elif defined(_POSIX_V6_ILP32_OFFBIG) || defined(_POSIX_V6_ILP32_OFF32)
// long is 32 bits.
#define POV_LONG long long
#else
// Unable to detect long size at compile-time, assuming less than 64 bits.
#define POV_LONG long long
#endifThe problem is that this is not correct on few levels.
First: long long can be bigger than 64-bits on 32-bit platforms. In fact even long can be bigger than 64-bit on 64-bit platforms. In theory at least.
Second: You use these macros incorrectly, it should be:
#if (defined(_POSIX_V7_LPBIG_OFFBIG) && (_POSIX_V7_LPBIG_OFFBIG == 1)) || (defined(_POSIX_V7_LP64_OFF32) && (_POSIX_V7_LP64_OFF32 == 1))
// long is at exactly 64 bits.
#define POV_LONG long
#elif (defined(_POSIX_V_7_ILP32_OFFBIG) && (_POSIX_V_7_ILP32_OFFBIG == 1)) || (defined(_POSIX_V7_ILP32_OFF32) && (_POSIX_V7_ILP32_OFF32 == 1))
// long is exactly 32 bits, but long long is guaranteed to be at least 64-bit.
// lets hope it is not bigger (it probably isn't).
#define POV_LONG long long
#else
// Unable to detect long size at compile-time, assuming less than 64 bits.
#define POV_LONG long long
#endifAdditional check against 1 is needed, as these macros can be defined by equal -1 to indicate lack of support.
For example on m68k-linux-gnu triplet:
/usr/include/m68k-linux-gnu/bits/environments.h:
#if __WORDSIZE == 64
(snip)
#else /* __WORDSIZE == 32 */
/* By default we have 32-bit wide `int', `long int', pointers and `off_t'
and all platforms support LFS. */
# define _POSIX_V7_ILP32_OFF32 1
# define _POSIX_V7_ILP32_OFFBIG 1
# define _POSIX_V6_ILP32_OFF32 1
# define _POSIX_V6_ILP32_OFFBIG 1
# define _XBS5_ILP32_OFF32 1
# define _XBS5_ILP32_OFFBIG 1
/* We optionally provide an environment with the above size but an 64-bit
side `off_t'. Therefore we don't define _POSIX_V7_ILP32_OFFBIG. */
/* We can never provide environments with 64-bit wide pointers. */
# define _POSIX_V7_LP64_OFF64 -1
# define _POSIX_V7_LPBIG_OFFBIG -1
# define _POSIX_V6_LP64_OFF64 -1
# define _POSIX_V6_LPBIG_OFFBIG -1
# define _XBS5_LP64_OFF64 -1
# define _XBS5_LPBIG_OFFBIG -1
/* CFLAGS. */
#define __ILP32_OFFBIG_CFLAGS "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64"
#endif /* __WORDSIZE == 32 */This is a standard compliant behaviour and not a bug in gcc or glibc.
Third: Even that fixed macros is not 100% bullet proof, due to long long possibly being bigger than 64-bits even on 32-bit platforms. In general it shouldn't be a big issue, but in fact it can.
Third: It is all unnecessary. You are already including proper POSIX headers that automatically provide necessary types. Namely uint32_t, uint64_r and int64_t. Considering you need C++11 and other reasonably modern libraries and compilers, really it will always be available on POSIX when compiling povray.
Just use #define POV_LONG int64_t, it should work fine.
#include <stdint.h> in C (since C99), and #include <cstdint> in C++ (since C++11). It is probably also fine to include the C one in C++.
Over even better replace all uses of POV_LONG in the entire source try with int64_t if possible. Similar with POV_ULONG to uint64_t. For platforms that don't provide that types, define them using some macros just for these platforms.
PS. In Debian there is even a patch to detect the incorrect detection of POV_LONG at runtime.
https://salsa.debian.org/debian/povray/blob/master/debian/patches/verify_POV_LONG_is_64bit.patch
I highly recommend to simply remove POV_LONG and use int64_t.
c-lipka
commented
3 years ago Those files date back to a time when C++11 was still a new thing, and POV-Ray tried to avoid requiring it, to be compatible with older compilers. We simply haven't found the time and energy yet to fully leverage C++11 features all throughout our code.
As for int64_t, that is not an option, as it is optional (no pun intended) according to the C++11 and later standards. Or, more specifically, it is either mandatory or prohibited, depending on the details of the hardware architecture. Instead, int_least64_t is the proper type to use.
c-lipka
commented
3 years ago For the records: As this applies to an exotic platform, we probably don't want to tackle it in the upcoming v3.8.0 release. It does limit the set of compatible platforms though, so we probably do want to address it in a later v3.8 maintenance release.
Summary
Compiling povray, on m68k machine leads to compiler warning
Environment
Unix Build Command Sequence
Unix Configure Output
Steps to Reproduce
Another option:
The exact versions of libraries (like boost) used doesn't really matter here, as can be seen below.
These are main dependencies I used:
Expected Behavior
No integer overflows of literals.
Actual Behavior
Suggested Solution
POV_LONGtype change? Preprocessing section for different integer widths?./configuredetecting properly sizes of integer types?