AK: Implement ShortString for big-endian

[sideeffect42]

This PR flips the ShortString struct order for big-endian systems.

Further, I added a test case to assert that if a ShortString is interpreted as a pointer it will be odd.

---

This is a copy of LadybirdBrowser/ladybird#416 and has not been tested in SerenityOS, because I don't have a system running SerenityOS.

[nico]

I'd strongly prefer if we didn't bother with big endian. It's fringe enough that the intersection and fringe OS should be pretty close to empty.

(Edit: to be clear, this is just my personal opinion, not project consensus.)

[sideeffect42]

@nico, I don't think the question of big-endian support is solely about it being fringe or not.
Some hobbyist OSes specifically target special systems because people find it fun.
It's also not just PowerPC, also ARM, MIPS, s390, SPARC etc. can be run in big-endian mode.

Some projects (e.g. OpenBSD) also target big-endian platforms to discover bugs or potential security issues earlier.
I'm going so far to argue that in a sane code base endianness should not be a problem at all. Endianness-issues mostly occur when input is not sanitized properly, people get confused with the data type they're working with, or trying to implement optimizations relying on a specific implementation detail.

/2c

Of course the project is free to decide whether or not they want to take my patches.
I just wanted to be nice and post PRs for the changes I had implemented for Ladybird anyway.

[spholz]

Adding big-endian support for lagom will probably not be that complicated.
But the kernel currently assumes little endian in many places, e.g. multiple drivers use bit-field structs to represent hardware registers. There also is very likely a lot of code that is not using the LittleEndian<> wrapper, where appropriate.

I did actually try to compile serenity for big-endian RISC-V some time ago (just for fun). But that resulted in a lot of compilation errors in the kernel (mostly in the NVMe driver, which uses the endian conversion functions incorrectly).

Technically aarch64 and riscv64 also have big-endian variants, but we also don't support those either.
(side note: the riscv64 psABI says that bit-fields "are packed in little-endian fashion", but gcc doesn't seem to respect that when targeting big-endian RISC-V)

[DanShaders]

I'm also in favor of cherry picking big-endian patches. My main argument is not having two diverging AK implementations that will be a big pain to cherry pick patches between.

As for the kernel side of things, if we ever decide to port Serenity to BE arch, I don't think it will be hard to patch Clang and/or GCC to provide [[serenity::little_endian_layout]] or some for bit-fields (I even already know where exactly to patch Clang to get this behavior :))

[nico]

Some hobbyist OSes specifically target special systems because people find it fun.

That's an extremely fair point!

The one thing I'd reply to that is that while it's fun for some, it's not necessarily fun for others, but supporting big-endian is kind of an all-or-nothing deal. For example, when you want to do a high-performance bitstream reader, you might want to make assumptions about endianness, and that's a lot easier if there's just one endianness.

(It my mind, adding support for big-endian is a little like adding support for architectures where char isn't 8 bits, say.)

(Again, this is just my personal opinion etc)

My main argument is not having two diverging AK implementations that will be a big pain to cherry pick patches between.

Independent of this PR, I think this is a very bad argument, since with this argument we'll want to cherry-pick over more or less anything – except for their NIH work. And since we won't pick the NIH bits, we'll have cherry-picking pain sooner or later anyways. (In fact, we already do, due to skia-specific changes in LibWeb.) Cherry-picks will become rarer and harder over time. That's the nature of forks. I don't think we should spend a lot of energy fighting this.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

[stale]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!