endian bug in <machine/_types.h> ?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

endian bug in <machine/_types.h> ?

foo64
NDK r8c

I tracked down a nasty bug in third party code, but I think it's actually the NDK's fault. I'm posting here so someone can confirm or deny it.

<machine/_types.h> gets indirectly included from a system header (for example, from <csignal>) in one of my headers. At the bottom, there's this bit of code:

#ifdef __ARMEB__
#define _BYTE_ORDER _BIG_ENDIAN
#else
#define _BYTE_ORDER _LITTLE_ENDIAN
#endif

Since __ARMEB__ is never defined, that code sets _BYTE_ORDER = _LITTLE_ENDIAN. The only problem is that _LITTLE_ENDIAN does not exist at this point, so it's defining _BYTE_ORDER to nothing! Not the same as #undef'ing it though (this is important later)

The third party library's "types" header detects endianness like this:

#if (defined(_BYTE_ORDER) && (_BYTE_ORDER == _BIG_ENDIAN))
#define SF_BYTE_ORDER          SF_BIG_ENDIAN
#else
#define SF_BYTE_ORDER          SF_LITTLE_ENDIAN
#endif

Seems pretty straight forward. I know that Android is little endian, so I was shocked to see SF_BIG_ENDIAN being defined *most* of the time. Huh?

Here's what I think is happening: Since _BYTE_ORDER is defined as nothing and _BIG_ENDIAN doesn't exist, the preprocessor considers them to be equal. I think technically it treats them both as 0, but I could be wrong.

My current workaround is to add -D_LITTLE_ENDIAN=1234 to my GCC flags. I got the idea by looking at how <sys/endian.h> and <linux/byteorder/little_endian.h> define _LITTLE_ENDIAN as 1234. I'm not sure it's the best method, but it works.

It seems wrong to me that <machine/_types.h> uses _LITTLE_ENDIAN and _BIG_ENDIAN without defining them. It should check that they're defined, or even better, include a header that does define them.

Lastly, the correct endianness was *sometimes* detected by simply not including any system headers from a few CPP files. That way _BYTE_ORDER wasn't even defined, which caused the #else clause to evaluate in the third party header.

--
You received this message because you are subscribed to the Google Groups "android-ndk" group.
To view this discussion on the web visit https://groups.google.com/d/msg/android-ndk/-/xMWasrcjHwAJ.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/android-ndk?hl=en.
Reply | Threaded
Open this post in threaded view
|

Re: endian bug in <machine/_types.h> ?

dtx0
I have cscope installed and used "cscope -bqkR" and "cscope -d" to catalog the code and search it. Remember that we're dealing with the preprocessor and that object macros (for example) are identified and have a replacement token list, if there is one.

Here's some code that can be used to illustrate what I mean:

[HeaderA.h]
#define X Y

[HeaderB.h]
#include "HeaderA.h"
#define Y 103

[main.c]
#include <stdio.h>
#include "HeaderB.h"

#define FIRST(a) (a)
#define SECOND(X, Y) ((FIRST(X)) * (THIRD(Y)))
#define THIRD(b) (5 + (b))

int main(int argc, const char * const argv[]) {
printf("%d\n", X); /* result should be 103 */
printf("%d\n", SECOND(10, 3)); /* should be (10 * (3 + 5)) = 80 */
return (0);
}


------OUTPUT------
> 103 
> 80
>
....................................................................................................

So, despite the fact that the macro "THIRD" is defined after "SECOND", everything works out. There's no bug. Note: the simple  sample code written by me was done sloppily because my designated driver just arrived (yay, beer!).


If you look at endian.h, the following lines which are commented explain everything:

 27 /*
 28  * Generic definitions for little- and big-endian systems.  Other endianesses
 29  * has to be dealt with in the specific machine/endian.h file for that port.
 30  *
 31  * This file is meant to be included from a little- or big-endian port's
 32  * machine/endian.h after setting _BYTE_ORDER to either 1234 for little endian
 33  * or 4321 for big..
 34  */

from endian.h:

#ifndef _SYS_ENDIAN_H_
#define _SYS_ENDIAN_H_
 
#include <sys/cdefs.h> 
#include <machine/_types.h>

#define _LITTLE_ENDIAN  1234
#define _BIG_ENDIAN     4321
#define _PDP_ENDIAN     3412



On Friday, January 4, 2013 4:48:46 PM UTC-8, foo64 wrote:
NDK r8c

I tracked down a nasty bug in third party code, but I think it's actually the NDK's fault. I'm posting here so someone can confirm or deny it.

<machine/_types.h> gets indirectly included from a system header (for example, from <csignal>) in one of my headers. At the bottom, there's this bit of code:

#ifdef __ARMEB__
#define _BYTE_ORDER _BIG_ENDIAN
#else
#define _BYTE_ORDER _LITTLE_ENDIAN
#endif

Since __ARMEB__ is never defined, that code sets _BYTE_ORDER = _LITTLE_ENDIAN. The only problem is that _LITTLE_ENDIAN does not exist at this point, so it's defining _BYTE_ORDER to nothing! Not the same as #undef'ing it though (this is important later)

The third party library's "types" header detects endianness like this:

#if (defined(_BYTE_ORDER) && (_BYTE_ORDER == _BIG_ENDIAN))
#define SF_BYTE_ORDER          SF_BIG_ENDIAN
#else
#define SF_BYTE_ORDER          SF_LITTLE_ENDIAN
#endif

Seems pretty straight forward. I know that Android is little endian, so I was shocked to see SF_BIG_ENDIAN being defined *most* of the time. Huh?

Here's what I think is happening: Since _BYTE_ORDER is defined as nothing and _BIG_ENDIAN doesn't exist, the preprocessor considers them to be equal. I think technically it treats them both as 0, but I could be wrong.

My current workaround is to add -D_LITTLE_ENDIAN=1234 to my GCC flags. I got the idea by looking at how <sys/endian.h> and <linux/byteorder/little_endian.h> define _LITTLE_ENDIAN as 1234. I'm not sure it's the best method, but it works.

It seems wrong to me that <machine/_types.h> uses _LITTLE_ENDIAN and _BIG_ENDIAN without defining them. It should check that they're defined, or even better, include a header that does define them.

Lastly, the correct endianness was *sometimes* detected by simply not including any system headers from a few CPP files. That way _BYTE_ORDER wasn't even defined, which caused the #else clause to evaluate in the third party header.

--
You received this message because you are subscribed to the Google Groups "android-ndk" group.
To view this discussion on the web visit https://groups.google.com/d/msg/android-ndk/-/HRhLLlHeE90J.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/android-ndk?hl=en.