Stop false positive in clang array OOB sanitiser

[Aidan63]

When the GC runs with all of clangs sanitisers enabled you get lots of false positive errors about OOB array access on mRow. This is because a lot of code accesses the 0th entry of the first array to basically treat it as a single chunk of memory, this does work fine as 2D arrays are just a big chunk of memory.

Clangs sanitisers are tripped up by this though, so I've changed mRow to actually be one big array since there was more access to it as a 1D array than a 2D array. This should make it easier to not lose track of actually important sanitisation errors.

[tobil4sk]

This looks good, I think I also saw some compiler warnings about this

[tobil4sk]

Though, it makes getting a pointer to a single row more awkward. Maybe we can have a helper function for that.

[Aidan63]

Another thought I did have was to add an extra item to the union instead of changing mRow, something like unsigned char mData[IMMIX_LINES * IMMIX_LINE_LEN]. So if you want nice row access you can still use mRow, but if you want to do custom offsets you can use the other union tag.

[tobil4sk]

That seems clean, but I think in c++ it might be undefined behaviour to alias using a union. Maybe it's something that could be done using a simple wrapper class.

[Aidan63]

Accessing anything other than the "active" entry (last written to) is UB per the spec, but the big three compilers all define it as allowed assuming you're compiling with any strict aliasing stuff disabled (hxcpp does).
I've renamed mRow to mData and added a row helper function to the union.

[tobil4sk]

any strict aliasing stuff disabled (hxcpp does).

Where does hxcpp disable strict aliasing? The only target that has -fno-strict-aliasing is android gcc and ios explicitly has -fstrict-aliasing. In general it is not something we want to disable since it makes a lot of optimisations impossible.

[Aidan63]

Hmm, I could have sworn it was disabled for everything.
In general I'm really not fussed about strict aliasing and don't think we should bother with it. It's incredibly easy to break aliasing rules, gcc and clang's warnings are poor and miss many cases, OS' build without it (Linux, BSDs), MSVC supports non of it, and many big libraries require it (libuv).
You find much hand wringing online about it enabling optimisations, but outside of small snippets comparing produced asm you find next to nothing on how it improves over all runtime speed. That, combined with so many things disabling it is rather telling to it's use. Academically interesting, little practical use.

I'm going to continue using reinterpret_cast over memcpy or whatever madness is the "correct" way, there's plenty of code in hxcpp which does type punning and so I don't think it's a concern.

[tobil4sk]

I'm going to continue using reinterpret_cast over memcpy or whatever madness is the "correct" way, there's plenty of code in hxcpp which does type punning and so I don't think it's a concern.

reinterpret_cast is fine in this case because it is aliasing char as int, and char is compatible with any type. It would only be an issue if those same bytes are then reinterpreted differently with an incompatible type.

[Aidan63]

yeah, char and std::byte are special in regards to aliasing. But there's plenty of reinterpreting of unrelated (but byte wise compatible) types, all of the stuff related to socket_in6 in Socket.cpp comes to mind, same with everything related to wchar_t and char16_t on Windows.
Very little of the C/C++ world pays attention to strict type aliasing and I'm not about to start. It's only been in the last few C++ editions where pretty essential functions from an ergonomics view have been added, so I don't think it's being taken seriously.