Skip to content

Commit

Permalink
Add initial big endian support
Browse files Browse the repository at this point in the history
The largest deterrents to big endian compatibility were:

- In the public API, the unions meant for aliasing and the bit fields:
    - In the case of KeyDownEvent, detecting key presses would become broken. The solution is as simple as reversing the field order in CharScanType.
    - In the case of MessageEvent, reading from 'infoChar', 'infoInt', etc. would not work as expected when the value had been initialized through 'infoPtr', such as when using the 'message' function. It is required to add padding before the non-pointer fields. A concise way of achieving this is by using bit fields. This comes at the expense of not being able to take the address of these fields anymore, but I have found no examples of such usage.
    - In the case of TColorBIOS and TColorRGB, the red and blue components would become swapped. A simple solution is to reorder the bit fields.
- In the internal classes:
    - In TCellChar, the 'moveInt' method does bit-casting, which is fine as long as the parameter itself is the result of bit-casting from a byte array. However, 'moveChar' was implemented with an invocation to 'moveInt', breaking this rule. As a result, none of the single-byte characters that went through 'moveChar' would be displayed.
    - The TermColor and colorconv_r structs, which had manual bit-casting optimizations. This would result in colors not being displayed at all. Rather than changing the struct's layout, in this case it seems more convenient to simply add code to to reverse the casted bytes when necessary.

Some other issues that I found were:

- The internal method 'utf32To8' does bit-casting from an integer to a byte array, so bytes have to be reversed.
- In tvdemo, reversing the mouse buttons would not work because the address of 'TEventQueue::mouseReverse' was being treated as an address to a int32_t, even though it is a Boolean.
- In the far2l terminal extensions, integer values should always be in little-endian format, so bytes have to be reversed.
- The 'AsciiChar' member of 'KEY_EVENT_RECORD' is meant to alias the least significant byte of 'UnicodeChar', so padding has to be added.
- In the internal method 'fast_btoa', a little-endian-dependent optimization has been replaced by an endian-safe one.

The code related to streaming and resource files has not been modified, so files generated in a little endian system may be unreadable in a big endian system and viceversa. This is not so important, though, since it is highly discouraged to use these features.

I also took the oportunity to add tests for several of these points and even a GitHub Actions job to run these tests automatically in a big endian system.

See magiblot/turbo#65.
Fixes #134. Closes #136.

Co-authored-by: MookThompson <[email protected]>
  • Loading branch information
magiblot and MookThompson committed Jan 21, 2024
1 parent bbea05c commit f933af5
Show file tree
Hide file tree
Showing 21 changed files with 282 additions and 37 deletions.
37 changes: 28 additions & 9 deletions .github/workflows/cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@ name: Build

on: [push, pull_request]

env:
BUILD_TYPE: Release

jobs:
build-linux-gcc:
name: Linux (GCC) (Unity)
Expand All @@ -13,11 +10,11 @@ jobs:
- uses: actions/checkout@v3

- name: Install Dependencies
run: sudo apt -y install libncursesw5-dev libgpm-dev
run: sudo apt -y install libncursesw5-dev libgpm-dev libgtest-dev

- name: Configure CMake
shell: bash
run: cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DTV_REDUCE_APP_SIZE=ON -DTV_LIBRARY_UNITY_BUILD=ON
run: cmake . -DCMAKE_BUILD_TYPE=Release -DTV_BUILD_TESTS=ON -DTV_REDUCE_APP_SIZE=ON -DTV_LIBRARY_UNITY_BUILD=ON

- name: Build
shell: bash
Expand All @@ -42,7 +39,7 @@ jobs:
env:
CC: gcc-5
CXX: g++-5
run: cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE
run: cmake . -DCMAKE_BUILD_TYPE=Release

- name: Build
shell: bash
Expand All @@ -62,12 +59,34 @@ jobs:
env:
CC: clang
CXX: clang++
run: cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DTV_OPTIMIZE_BUILD=OFF
run: cmake . -DCMAKE_BUILD_TYPE=Release -DTV_OPTIMIZE_BUILD=OFF

- name: Build
shell: bash
run: cmake --build . -j$(nproc)

build-linux-big-endian:
name: Linux (GCC) (Big Endian)
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v3

- name: Build in container
uses: uraimo/run-on-arch-action@v2
with:
arch: s390x
distro: alpine_latest
dockerRunArgs: |
--volume "${PWD}:/app"
env: |
LC_CTYPE: C.UTF-8
shell: /bin/sh
install: apk update && apk add make cmake g++ ncurses-dev gtest-dev linux-headers
run: |
cd /app
cmake . -DCMAKE_BUILD_TYPE= -DTV_BUILD_USING_GPM=OFF -DTV_BUILD_EXAMPLES=OFF -DTV_BUILD_TESTS=ON -DTV_LIBRARY_UNITY_BUILD=ON
cmake --build . -j$(nproc)
build-windows-msvc32:
name: Windows (MSVC) (Win32)
runs-on: windows-latest
Expand Down Expand Up @@ -160,7 +179,7 @@ jobs:

- name: Configure CMake
shell: bash
run: cmake . -G "MinGW Makefiles" -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_CXX_FLAGS="-static" -DTV_OPTIMIZE_BUILD=OFF
run: cmake . -G "MinGW Makefiles" -DCMAKE_BUILD_TYPE= -DCMAKE_CXX_FLAGS="-static" -DTV_LIBRARY_UNITY_BUILD=ON

- name: Build
shell: bash
Expand Down Expand Up @@ -211,7 +230,7 @@ jobs:

- name: Configure CMake
shell: bash
run: cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE
run: cmake . -DCMAKE_BUILD_TYPE=Release

- name: Build
shell: bash
Expand Down
6 changes: 4 additions & 2 deletions examples/tvdemo/tvdemo3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,12 @@ void TVDemo::mouse()

if (mouseCage != 0)
{
int32_t mouseReverse = TEventQueue::mouseReverse;
mouseCage->helpCtx = hcOMMouseDBox;
mouseCage->setData(&(TEventQueue::mouseReverse));
mouseCage->setData(&mouseReverse);
if (deskTop->execView(mouseCage) != cmCancel)
mouseCage->getData(&(TEventQueue::mouseReverse));
mouseCage->getData(&mouseReverse);
TEventQueue::mouseReverse = mouseReverse;
}
destroy( mouseCage );

Expand Down
22 changes: 22 additions & 0 deletions include/tvision/colors.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,34 @@ inline TColorAttr reverseAttribute(TColorAttr attr)
struct TColorRGB
{
uint32_t
#ifndef TV_BIG_ENDIAN
b : 8,
g : 8,
r : 8,
_unused : 8;
#else
_unused : 8,
r : 8,
g : 8,
b : 8;
#endif

TV_TRIVIALLY_CONVERTIBLE(TColorRGB, uint32_t, 0xFFFFFF)
constexpr inline TColorRGB(uint8_t r, uint8_t g, uint8_t b);
};

constexpr inline TColorRGB::TColorRGB(uint8_t r, uint8_t g, uint8_t b) :
#ifndef TV_BIG_ENDIAN
b(b),
g(g),
r(r),
_unused(0)
#else
_unused(0),
r(r),
g(g),
b(b)
#endif
{
}

Expand All @@ -90,11 +104,19 @@ constexpr inline TColorRGB::TColorRGB(uint8_t r, uint8_t g, uint8_t b) :
struct TColorBIOS
{
uint8_t
#ifndef TV_BIG_ENDIAN
b : 1,
g : 1,
r : 1,
bright : 1,
_unused : 4;
#else
_unused : 4,
bright : 1,
r : 1,
g : 1,
b : 1;
#endif

TV_TRIVIALLY_CONVERTIBLE(TColorBIOS, uint8_t, 0xF)
};
Expand Down
4 changes: 4 additions & 0 deletions include/tvision/compat/windows/windows.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,11 @@ typedef struct _KEY_EVENT_RECORD {
WORD wVirtualScanCode;
union {
WCHAR UnicodeChar;
#ifndef TV_BIG_ENDIAN
CHAR AsciiChar;
#else
struct { WCHAR : 8*sizeof(WCHAR) - 8, AsciiChar : 8; };
#endif
} uChar;
DWORD dwControlKeyState;
} KEY_EVENT_RECORD;
Expand Down
7 changes: 7 additions & 0 deletions include/tvision/internal/ansidisp.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <tvision/tv.h>

#include <internal/termdisp.h>
#include <internal/endian.h>

namespace tvision
{
Expand All @@ -30,6 +31,9 @@ struct TermColor

TermColor& operator=(uint32_t val) noexcept
{
#ifdef TV_BIG_ENDIAN
reverseBytes(val);
#endif
memcpy(this, &val, sizeof(*this));
return *this;
static_assert(sizeof(*this) == 4, "");
Expand All @@ -38,6 +42,9 @@ struct TermColor
{
uint32_t val;
memcpy(&val, this, sizeof(*this));
#ifdef TV_BIG_ENDIAN
reverseBytes(val);
#endif
return val;
}
TermColor(uint8_t aIdx, TermColorTypes aType) noexcept
Expand Down
32 changes: 32 additions & 0 deletions include/tvision/internal/endian.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#ifndef TVISION_ENDIAN_H
#define TVISION_ENDIAN_H

#include <stdint.h>

namespace tvision
{

// Optimization: the 'reverseBytes' methods are written in such a way that the
// compiler will likely replace them with a BSWAP instruction or equivalent.

inline void reverseBytes(uint16_t &val)
{
val = (val << 8) | (val >> 8);
}

inline void reverseBytes(uint32_t &val)
{
val = ((val << 8) & 0xFF00FF00U) | ((val >> 8) & 0x00FF00FF);
val = (val << 16) | (val >> 16);
}

inline void reverseBytes(uint64_t &val)
{
val = ((val << 8) & 0xFF00FF00FF00FF00ULL) | ((val >> 8) & 0x00FF00FF00FF00FFULL);
val = ((val << 16) & 0xFFFF0000FFFF0000ULL) | ((val >> 16) & 0x0000FFFF0000FFFFULL);
val = (val << 32) | (val >> 32);
}

} // namespace tvision

#endif // TVISION_ENDIAN_H
16 changes: 7 additions & 9 deletions include/tvision/internal/strings.h
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
#ifndef TVISION_STRINGS_H
#define TVISION_STRINGS_H

#ifndef _TV_VERSION
#include <tvision/tv.h>
#endif

namespace tvision
{

template<class Int>
inline constexpr Int string_as_int(TStringView s) noexcept
// CAUTION: It is not endian-safe to reinterpret the result as an array of bytes.
{
Int res = 0;
for (size_t i = 0; i < min(s.size(), sizeof(res)); ++i)
// CAUTION: Assumes Little Endian.
res |= uint64_t(uint8_t(s[i])) << 8*i;
return res;
}
Expand All @@ -32,15 +30,15 @@ struct alignas(4) btoa_lut_elem_t
using btoa_lut_t = constarray<btoa_lut_elem_t, 256>;

inline char *fast_btoa(uint8_t value, char *buffer) noexcept
// Pre: the capacity of 'buffer' is at least 4 bytes.
{
extern const btoa_lut_t btoa_lut;
const auto &lut = (btoa_lut_elem_t (&) [256]) btoa_lut;
// CAUTION: Assumes Little Endian.
// We can afford to write more bytes into 'buffer' than digits.
uint32_t asInt;
memcpy(&asInt, &lut[value], 4);
memcpy(buffer, &asInt, 4);
return buffer + (asInt >> 24);
// Optimization: read and write the whole LUT entry at once in order to
// minimize memory accesses. We can afford to write more bytes into 'buffer'
// than digits.
memcpy(buffer, &lut[value], 4);
return buffer + (uint8_t) buffer[3];
static_assert(sizeof(btoa_lut_elem_t) == 4, "");
}

Expand Down
7 changes: 6 additions & 1 deletion include/tvision/internal/utf8.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#ifndef TVISION_UTF8_H
#define TVISION_UTF8_H

#include <internal/strings.h>
#include <tvision/tv.h>

#include <internal/endian.h>

#include <stdint.h>
#include <string.h>
Expand Down Expand Up @@ -66,6 +68,9 @@ inline size_t utf32To8(uint32_t u32, char u8[4]) noexcept
(( u32 & 0b00111111) | 0b10000000) << 24;
length = 4;
}
#ifdef TV_BIG_ENDIAN
reverseBytes(asInt);
#endif
memcpy(u8, &asInt, 4);
return length;
}
Expand Down
5 changes: 3 additions & 2 deletions include/tvision/scrncell.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,14 @@ struct TCellChar

inline void TCellChar::moveChar(char ch)
{
moveInt((uchar) ch);
memset(this, 0, sizeof(*this));
_text[0] = ch;
}

inline void TCellChar::moveInt(uint32_t mbc, bool wide)
// Pre: 'mbc' is a bit-casted multibyte-encoded character.
{
memset(this, 0, sizeof(*this));
// CAUTION: Assumes Little Endian.
memcpy(_text, &mbc, sizeof(mbc));
_flags = -int(wide) & fWide;
}
Expand Down
17 changes: 17 additions & 0 deletions include/tvision/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,15 @@ inline void TMouse::registerHandler( unsigned mask, void (_FAR *func)() )

struct CharScanType
{
#if !defined( TV_BIG_ENDIAN )
uchar charCode;
uchar scanCode;
#else
// Due to the reverse byte order, swap the fields in order to preserve
// the aliasing with KeyDownEvent::keyCode.
uchar scanCode;
uchar charCode;
#endif
};

struct KeyDownEvent
Expand Down Expand Up @@ -222,11 +229,21 @@ struct MessageEvent
union
{
void *infoPtr;
#if !defined( TV_BIG_ENDIAN )
int32_t infoLong;
ushort infoWord;
short infoInt;
uchar infoByte;
char infoChar;
#else
// Due to the reverse byte order, add padding at the beginning to
// preserve the aliasing with infoPtr.
struct { intptr_t : 8*sizeof(infoPtr) - 32, infoLong : 32; };
struct { uintptr_t : 8*sizeof(infoPtr) - 16, infoWord : 16; };
struct { intptr_t : 8*sizeof(infoPtr) - 16, infoInt : 16; };
struct { uintptr_t : 8*sizeof(infoPtr) - 8, infoByte : 8; };
struct { intptr_t : 8*sizeof(infoPtr) - 8, infoChar : 8; };
#endif
};
};

Expand Down
2 changes: 2 additions & 0 deletions include/tvision/ttypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ typedef long int32_t;
typedef uchar uint8_t;
typedef ushort uint16_t;
typedef ulong uint32_t;
typedef long intptr_t;
typedef ulong uintptr_t;
#endif

#ifdef __BORLANDC__
Expand Down
12 changes: 12 additions & 0 deletions source/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ target_compile_definitions(${PROJECT_NAME} PRIVATE
TVISION_NO_STL
)

include(TestBigEndian)

TEST_BIG_ENDIAN(IS_BIG_ENDIAN)

if (IS_BIG_ENDIAN)
tv_message(STATUS "Big endian system detected")
target_compile_definitions(${PROJECT_NAME} PUBLIC
TV_BIG_ENDIAN
)
endif()

# Dependencies

if (NOT WIN32)
Expand Down Expand Up @@ -172,6 +183,7 @@ if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.16.0" AND TV_OPTIMIZE_BUILD)
file(GLOB_RECURSE TVSOURCE_NOUNITY "${CMAKE_CURRENT_LIST_DIR}/tvision/s*.cpp")
list(APPEND TVSOURCE_NOUNITY "${CMAKE_CURRENT_LIST_DIR}/tvision/new.cpp")
list(REMOVE_ITEM TVSOURCE_NOUNITY
"${CMAKE_CURRENT_LIST_DIR}/tvision/snprintf.cpp"
"${CMAKE_CURRENT_LIST_DIR}/tvision/stddlg.cpp"
"${CMAKE_CURRENT_LIST_DIR}/tvision/strmstat.cpp"
"${CMAKE_CURRENT_LIST_DIR}/tvision/syserr.cpp"
Expand Down
4 changes: 4 additions & 0 deletions source/platform/ansidisp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,11 @@ struct alignas(8) colorconv_r
colorconv_r() = default;
colorconv_r(TermColor aColor, TColorAttr::Style aExtraFlags=0) noexcept
{
// Optimization: do bit-casting manually, just like with TermColor.
uint64_t val = aColor | (uint64_t(aExtraFlags) << 32);
#ifdef TV_BIG_ENDIAN
reverseBytes(val);
#endif
memcpy(this, &val, 8);
static_assert(sizeof(*this) == 8, "");
}
Expand Down
Loading

0 comments on commit f933af5

Please sign in to comment.