Opened 8 years ago

Last modified 5 years ago

#10505 reopened Bugs

Use gcc/clang built-in endianity macros (too)

Reported by: Jan Hudec <bulb@…> Owned by: René Rivera
Milestone: To Be Determined Component: predef USE GITHUB
Version: Boost 1.56.0 Severity: Problem
Keywords: Cc:

Description

Endian detection in boost/predef/other/endian.h does consider the gcc/clang built-in __BYTE_ORDER__ macros and consequently fails to detect endianity of some (usually mobile) platforms.

E.g. on Android I get _BYTE_ORDER defined to _LITTLE_ENDIAN, but _LITTLE_ENDIAN is not defined, leading to all three conditions (_BYTE_ORDER == _BIG_ENDIAN, _BYTE_ORDER == _LITTLE_ENDIAN and _BYTE_ORDER == _PDP_ENDIAN being satisfied and yielding conflicting definitions.

The __BYTE_ORDER__ is provided by gcc and clang compilers themselves and therefore should always be there no matter how broken the rest of standard library is. What I am not sure is whether they should be used in preference to any other method or as a fallback (in which case definedness of the _BIG_ENDIAN, _LITTLE_ENDIAN etc. macros also has to be tested before trying to use them)

Change History (9)

comment:1 by Jan Hudec <bulb@…>, 8 years ago

I was looking wrong. Apparently it does fall back to those macros, but because the preceding code does not check that the complete suite of macros (not just _BYTE_ORDER, but also _BIG_ENDIAN/_LITTLE_ENDIAN/_PDP_ENDIAN), it is never used.

comment:2 by Jan Hudec <bulb@…>, 8 years ago

Ah, and the code using the built-in macros is the same as I had to patch in the earlier versions anyway as it still does not use __BYTE_ORDER__ anyway.

comment:3 by René Rivera, 8 years ago

I don't understand what you are saying in the comments. What is the root of the problem? Can you check the current develop branch to see if addresses the issues?

comment:4 by René Rivera, 8 years ago

Status: newassigned

comment:5 by René Rivera, 7 years ago

Resolution: obsolete
Status: assignedclosed

Closing for lack of followup.

comment:6 by anonymous, 7 years ago

Resolution: obsolete
Status: closedreopened

The current version fixes the part of the problem that the checks on in endian.h lines 72-99 now won't trigger in the errorneous Bionic libc case when _BYTE_ORDER is defined, but _LITTLE_ENDIAN is not.

However, the gcc 4.6 toolchain (which we still need, because binaries built with newer ones don't work on Android < 2.3 and our code otherwise works on 2.1 just fine) does not define __LITTLE_ENDIAN__, so the fallbacks further down on lines 106 and 124 don't trigger on x86 (on arm the __ARMEL__ define is found).

So I would still suggest adding the test for the older gcc macros:

#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \
		defined(__ORDER_LITTLE_ENDIAN__) && defined(__ORDER_PDP_ENDIAN__)
#    if (__BYTE_ORDER == __ORDER_BIG_ENDIAN__)
#        undef BOOST_ENDIAN_BIG_BYTE
#        define BOOST_ENDIAN_BIG_BYTE BOOST_VERSION_NUMBER_AVAILABLE
#    endif
#    if (__BYTE_ORDER == __ORDER_LITTLE_ENDIAN__)
#        undef BOOST_ENDIAN_LITTLE_BYTE
#        define BOOST_ENDIAN_LITTLE_BYTE BOOST_VERSION_NUMBER_AVAILABLE
#    endif
#    if (__BYTE_ORDER == __ORDER_PDP_ENDIAN__)
#        undef BOOST_ENDIAN_LITTLE_WORD
#        define BOOST_ENDIAN_LITTLE_WORD BOOST_VERSION_NUMBER_AVAILABLE
#    endif
#endif

Since things depend on these in the gcc-provided headers, they should be reliable in gcc and clang.

Sorry, I missed the question on new year's eve.

PS: I wanted to make the line references to github, but trac won't accept it.

in reply to:  6 ; comment:7 by René Rivera, 7 years ago

Replying to anonymous:

The current version fixes the part of the problem that the checks on in endian.h lines 72-99 now won't trigger in the errorneous Bionic libc case when _BYTE_ORDER is defined, but _LITTLE_ENDIAN is not.

However, the gcc 4.6 toolchain (which we still need, because binaries built with newer ones don't work on Android < 2.3 and our code otherwise works on 2.1 just fine) does not define __LITTLE_ENDIAN__, so the fallbacks further down on lines 106 and 124 don't trigger on x86 (on arm the __ARMEL__ define is found).

So I would still suggest adding the test for the older gcc macros:

#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \
		defined(__ORDER_LITTLE_ENDIAN__) && defined(__ORDER_PDP_ENDIAN__)
#    if (__BYTE_ORDER == __ORDER_BIG_ENDIAN__)
#        undef BOOST_ENDIAN_BIG_BYTE
#        define BOOST_ENDIAN_BIG_BYTE BOOST_VERSION_NUMBER_AVAILABLE
#    endif
#    if (__BYTE_ORDER == __ORDER_LITTLE_ENDIAN__)
#        undef BOOST_ENDIAN_LITTLE_BYTE
#        define BOOST_ENDIAN_LITTLE_BYTE BOOST_VERSION_NUMBER_AVAILABLE
#    endif
#    if (__BYTE_ORDER == __ORDER_PDP_ENDIAN__)
#        undef BOOST_ENDIAN_LITTLE_WORD
#        define BOOST_ENDIAN_LITTLE_WORD BOOST_VERSION_NUMBER_AVAILABLE
#    endif
#endif

Since things depend on these in the gcc-provided headers, they should be reliable in gcc and clang.

Sorry, I missed the question on new year's eve.

PS: I wanted to make the line references to github, but trac won't accept it.

I still how your code above works. You are checking for defined(__BYTE_ORDER__). But are actually checking for __BYTE_ORDER equality to the __ORDER_* macros. Is the whole thing supposed to be a third alternative in the GNU libc branch? As in __BYTE_ORDER__ should get used if neither __BYTE_ORDER nor _BYTE_ORDER are defined?

in reply to:  7 comment:8 by Jan Hudec <bulb@…>, 7 years ago

Replying to grafik:

I still how your code above works. You are checking for defined(__BYTE_ORDER__). But are actually checking for __BYTE_ORDER equality to the __ORDER_* macros. Is the whole thing supposed to be a third alternative in the GNU libc branch? As in __BYTE_ORDER__ should get used if neither __BYTE_ORDER nor _BYTE_ORDER are defined?

Yes, exactly, it's a third alternative. It is exactly analogous to the first two, but its advantage is that it is built into (sufficiently recent versions of) the gcc and clang compilers themselves, so it works even on systems that don't define the _BYTE_ORDER and __BYTE_ORDER or don't define them completely as is the case of Android.

comment:9 by James E. King, III, 5 years ago

This issue should be resolved or moved into github issues.

Note: See TracTickets for help on using tickets.