Hacker News Reader

Comment by EdSchouten | Hacker News Reader

In my opinion bugs related to endianness expose a design flaw in the C/C++ programming languages, or at least in the way people tend to use those languages. It isn't that hard to write code that works well with any kind of endianness, and you don't need stuff like __BYTE_ORDER__ for that. For example, if I wanted to read a little endian 32-bit integer value from a byte stream, I could write a function like this:

    uint32_t read_int32(unsigned char *b) {
        return uint32_t(b[0]) |
               (uint32_t(b[1]) << 8) |
               (uint32_t(b[2]) << 16) |
               (uint32_t(b[3]) << 24);
    }

Which, according to Godbolt.org, at -O2 or higher compiles to:

    read_int32(unsigned char*):
        mov     eax, DWORD PTR [rdi]
        ret

No magic needed!

View on HN

Yes, it's totally possible to write good abstractions for this sort of basic operation, which handle both endianness and alignment. (QEMU's version of them uses memcpy(), again relying on the compiler to optimise this to a simple load where the host CPU supports unaligned accesses.) I agree that doing that kind of thing so code can be endian-agnostic and keeping the ifdefs to a minimum is better style and easier to understand.

But if the big-endian case isn't run through CI then bugs will still creep in, where people forget to use the abstraction and just cast a buffer pointer to do a 32-bit read, or where they don't keep the distinction between 'a 32 bit value in host endianness' and 'a 32-bit value in host-independent endianness (eg network data, or pixel data)' straight and forget a conversion step. QEMU's testing on s390 and ppc64 catches a steady trickle of this kind of bug before it goes into the codebase.

(You can catch unaligned-access bugs on x86 with clang/gcc address-sanitizer, I think, but it can't help with endianness bugs.)

It certainly doesn't help that the "wrong" way is featured in almost every textbook, tutorial and article I've seen in the last 15 years.

Your example is actually for the most favourable case because there are native 32-bit types. I've seen countless attempts to implement three-byte records via custom Uint24 classes that overload operators and abstract endianness (in various buggy ways) instead of just reading three damn bytes in a uint32_t. Supposedly in the name of readability ("I want to be explicit that this stores a 24-bit variable, not a 32-bit variable, so I'm going to copy-paste these 200 lines of either template magic or line noise, I'm not sure, but see how readable it is now? Oh yeah it's still 32-bit underneath, yeah, I'm not sure what happens if you write a 32-bit variable there, I think it breaks. This is way better than a typedef though, 'cause, uh, C++-17?)

For a long time, I struggled with endianness, because I thought that bitshifting and masking would be different on BE vs. LE:

    // wrong
    uint32_t num = ...;
    // num is set to 0x10203040
    num >> 24 == 0x10; // On little endian
    num >> 24 == 0x10; // On big endian

Of course that is wrong, and the second one is always the case. In a way, you could say C/C++ is big endian - the higher valued byte is first (left in English reading order). In fact, if you avoid reinterpreting via casting / punning / memcpy, you don't have to think about it at all [1].

One thing I have to do frustratingly often is to implement a bunch of functions like "read_uint32_le", it is a pity that there is no comprehensive standard.

([1] On the other hand, I miss the good old days where you could treat C like "high level assembler", a pointer was just a number that indexed memory, and you could reinterpret bytes with impunity - sometimes had to to get decent performance!)

I clicked around a little, and it seems "icc 21.1.9", "x64 msvc x19.28", "risc-v clang (trunk)", "armv7-a clang (trunk)" compile it to four separate byte loads, it looks like. It's not that I don't trust my compiler (usually clang) to optimize it this way, it's just that the level of variability between compilers makes me think I shouldn't really rely on this behaving this way.

I wonder what the arch difference is between the clangs that do it and the ones that don't, I don't know much about arm and risc-v.

ARM does not support non-aligned reads, and can not safely change four byte loads into a one 32-bit load unless it can guarantee the address is aligned to 4 bytes.

Not sure about RISC-V but I would not be surprised if the same applies there.

It used to be the case that Arm could not handle unaligned loads, but that functionality was added in v6 of the architecture (many years ago now) and so is present in most Arm cores around today including all the 64-bit ones.

(Some instructions insist on aligned data, like atomic read-modify-write operations and some types of SIMD insn, but x86 has a few needs-alignment special cases too. Basic "just load/store data" insns that the compiler will emit are fine.)

> Not sure about RISC-V but I would not be surprised if the same applies there.

It's probably a case of lowest common denominator.

Unalligned access is discouraged but allowed by the spec... But it's allowed to raise an exception. But not required. Cheaper to optimise to avoid that circumstance.

> For example, if I wanted to read a little endian 32-bit integer value from a byte stream, I could write a function like this

But for the longest time people would basically dump the raw bytes of their structs from / to the network whenever possible, e.g.

    struct my_data {
        int foo, bar;
        float x,y,z;
        long dontdothat;
    } data;

    write_bytes_to_the_network((const unsigned char*) &data, sizeof(my_data));

This not only leads to the endianess issues, but also alignment/packing.

But yes, people often start that way and regret only when it is too late and the files are in the wild.

s/compiles to/may or may not compile to/

Exactly. You might not be so lucky on a different platform.

Imho a better way to handle this would be to use a macro or special function that comes with the compiler, assuming it exists of course.

That looks like it could result in unaligned access and hence a crash on sparc64.

How? We’re just reading from a byte array.

The optimized code assumes it can read a full 32 bit value at once. So at least that is only valid on a platform that doesn't assume that 32 bit loads are aligned and the actual compiled code would look a lot worse on platforms that do.

Thus, it can't compile that way for sparc64.

That’s fine, GCC on sparc will generate the sequence to do an unaligned read with the right endianess. If that means reading bytes and shifting them, so be it.

Exactly. And even if we want to optimize access for those architectures, there surely must be ways to do that. Maybe compilers already do that if you use alignas(int32_t) on the byte array at the call site and ensure that read_int32() function of mine gets inlined?

No they won’t, if not in very basic cases that can’t be generalized as a rule of thumb solution. Your suggestion bargains correctness with performance.

If you know you are reading aligned memory and you need to do it with a single word memory access, then you need to use intNN_t* and use htonl or similar functions to byteswap. There’s no way around it.

The compiler can generate a test for alignment, and then a branch between two pieces of code. However, if the test and branch branch causes pipeline stalls due to bad prediction, it could be worse than just running the byte-banging code unconditionally.

I.e. "if that means doing what the programmer literally expressed, so be it."

But that code isn't portable for a different endian machine. To write portable endian clean code, you want to write functions/macros named like:

  cpu_to_le32(),
  cpu_to_be32(),
  be32_to_cpu(),
  le32_to_cpu(),

etc. (and similar for 16, and 64 bit quantities) similar to what's described here: https://kernelnewbies.org/EndianIssues)

These functions will either swap, or not, depending on the endianness of the target machine, and this decision is made at compile time, so the "no swap" case imposes no performance penalty.)

That way, the intent is clear, and the code works on big endian and little endian. So instead of what you propose, which I imagine you might use via:

  x = some_value; /* say this is big endian, and we want it little endian, because we're on Intel. */
  uint32_t y = read_int((char *) &x);

you'd have:

  y = be32_to_cpu(x);

Now the intent is clear, and the code is portable, x is big endian, and we want native CPU (whether that be big endian or little endian).

Likewise, you could write:

  x = cpu_to_be32(y);

say before writing it out to disk or network for some purpose that requires big endian.

This is similar to htonl(), ntoh(), htons(), ntohs(). (host to network/network to host long/short).

> But that code isn't portable for a different endian machine.

Except it is. On x86, which is little endian, compilers are smart enough to convert it to a mov.

> These functions will either swap, or not, depending on the endianness of the target machine

So will the above. Here's the MIPS little endian version:

    read_int32(unsigned char*):
        lwl     $2,3($4)
        j       $31
        lwr     $2,0($4)

This doesn't swap. The MIPS big endian version:

    read_int32(unsigned char*):
        lbu     $3,1($4)
        lbu     $2,2($4)
        sll     $5,$3,8
        lbu     $6,0($4)
        sll     $3,$2,16
        lbu     $2,3($4)
        or      $3,$5,$3
        or      $3,$3,$6
        sll     $2,$2,24
        j       $31
        or      $2,$3,$2

This swaps and ors. What part of this do you not consider portable between different-endian machines?

>But that code isn't portable for a different endian machine.

But it is. That's the whole point, to not bother with weird endian conversion macros.

It isn't. It always swaps. Most times you're dealing with endianness, you want to convert to or from native CPU to or from big or little endian. Code that always swaps regardless of native endianness will silently do the wrong thing if compiled on a machine of different endianness.

You're telling me that what the linux kernel guys do is "weird endian conversion macros?" I don't believe you.

You are wrong.

It doesn't always swap. Look at the assembly I posted above comparing mips64le to mips64be.

The code takes the byte at b[n] and shifts it (8.n)-bits. These are the same bytes on both little endian and big endian machines. They read the bytes in the same order (little endian), and convert them to the native order.

You can also see very clearly that it doesn't always swap by looking at the simple x86_64 assembly output that the was posted where no shifts or swaps are performed. Just a mov instruction.

Read the assembly. Or better yet, try the code out for yourself.

It's also about performance. The snippet assumes the byte order of the buffer or byte stream is little endian so only covers two of four cases. If you want to read a little-endian byte stream on a big-endian machine, of course you need swapping. But, considering portability, if that byte stream had been written by the big-endian machine before (or came directly from network) it would be in big-endian ordering and swapping would be wrong.

Assuming LE ordering on the writer side, you would need to swap again on a BE machine. Usually, however, you want to store in your target byte order, though. One could argue this is irrelevant as long as the memory doesn't leave your application, of course. Yet, by explicitly having macros _to_cpu, _to_be, _to_le and so on you make all of this explicit.

The snippet assumes the data being read is little endian, yes. You can make a different snippet if you want to read big endian. You use an equivalent snippet for the writing side, and boom. You never need any #ifdefs in those functions. Is it actually better than using functions like you describe? Maybe. A part of me does love the elegance of using the exact same code on any platform regardless of the native byte order. This construct was suggested/encouraged by Rob Pike: https://commandcenter.blogspot.com/2012/04/byte-order-fallac...

Ah, ok. Clearly I'm not awake yet.

Reply on news.ycombinator.com