wieredness with BIG_ENDIAN macro

[josesimoes]

Starting with GCC 5.4.1 the BIG_ENDIAN define seems to be defined internally in the compiler.
This has been reported by @cw2 in #474. I can confirm that I'm seeing this too on my builds.

I've opened a ticket with GNU ARM Embedded Toolchain. Waiting on their comments.
Thinking on a solution for this, I found that CMSIS has a macro defined for this purpose (see "Pre-processor Macros" in http://www.keil.com/pack/doc/cmsis/DSP/html/index.html. It's ARM_MATH_BIG_ENDIAN. As we seem to be moving towards CMSIS and we are already using other macros from there (__FPU_PRESENT for example), suggest that the current BIG_ENDIAN in NETMF is replaced with ARM_MATH_BIG_ENDIAN.

I can submit a PR for this immediately.

[smaillet-ms]

Well that's a pretty crappy turn of affairs... (And yet another reason why I hate preprocessor macros...)

NETMF is not bound to CMSIS nor to ARM so we shouldn't make such assumptions. How about we go with NETMF_TARGET_BIG_ENDIAN? That makes it specific for NETMF AND implies, correctly, that it is for the target.

[cw2]

Just for reference, this is the bug report https://bugs.launchpad.net/gcc-arm-embedded/+bug/1609078 (thanks @josesimoes )

[josesimoes]

The PR is submitted and it has been live tested in hardware with the Discovery4 solution.
I had to replace the little endian too.
A few comments:

• The guys at ARM for GCC were vare adamant that this shouldn't come from the toolchain or GCC compiler.
• Their reasons are strong such as that toolchain macros start with __, I've tried their suggestions and I couldn't find these macros being defined anywhere on the toolchain.
• I spotted something that grabbed my attention: in PR Refactored conditional compilation for PLATFORM_WINDOWS and WIN32 #468, file TinyCLR_PlatformDef.h, lines 10 and following a define for LITTLE_ENDIAN was added. Can this (along with something else) maybe causing this issue?

Right now I don't have the time to revert the code back to a version previous to those changes and retest again. Anyways it's working now and the fix seems perfectly reasonable. So lacking a reason for going deeper into this and not having any further suggestions, I suggest that this is considered as fixed.

PS: because of the still open issue with the "end" symbol (see #474) I had to add that to my test build, although I haven't committed that change.

[cw2]

Well, I have found the cause, it is due to a change in header files in GCC 5.4.1:

arm-none-eabi\include\machine\endian.h contains unconditional

#define BIG_ENDIAN 4321

whilst in GCC 5.3.1 BIG_ENDIAN symbol is defined only when

#if __BSD_VISIBLE
#define LITTLE_ENDIAN   _LITTLE_ENDIAN
#define BIG_ENDIAN  _BIG_ENDIAN
#define PDP_ENDIAN  _PDP_ENDIAN
#define BYTE_ORDER  _BYTE_ORDER
#endif

Thus, the trivial repro case

#include <stdio.h>
int main(void)
{
#ifdef BIG_ENDIAN
#error BIG_ENDIAN
#endif
return 0;
}

works in GCC 5.4.1 if the #include is removed.

Initially, I was only searching through NETMF codebase, I completely forgot the standard headers. (facepalm)

[smaillet-ms]

Well, I think Charlie Brown said it best - "Oh, good grief!"
😞

[josesimoes]

@cw2 nice finding!
Unless there is an incredible coincidence, someone posted this exact same finding, description and test code on the ARM GCC bug report... 😉
Your secret identity has been compromised!! 😆

[josesimoes]

Having no answer from the ARM GCC team suggest the following:

• the workaround in Fixes issues with endian macros (in GCC) #484 will work for now
• leave this issue open
• after this is corrected upstream we'll decide what to do (if the workaround becomes a fix or if any further changes are required)

I'm pointing this out because there are BIG_ENDIAN and LITTLE_ENDIAN macros in the LwIP source that were replaced too. It's not such a big deal but it's a third party source that is in the repo and was changed.