diff --git a/.build/build b/.build/build new file mode 100755 index 0000000..e1109b4 --- /dev/null +++ b/.build/build @@ -0,0 +1,8 @@ +#!/bin/sh -ef + +cd "$(dirname "$0")"/.. + +mkdir -p build +cd build +cmake .. +cmake --build . diff --git a/.build/build-mingw.ps1 b/.build/build-mingw.ps1 new file mode 100644 index 0000000..842584b --- /dev/null +++ b/.build/build-mingw.ps1 @@ -0,0 +1,44 @@ +<# +.DESCRIPTION +Builds Kaitai Struct C++ runtime library and unit tests on Windows using MinGW + +Requires: +- MinGW installed and available in the command prompt +- cmake/ctest available +- GTest installed, path passed in `-GTestPath` +#> + +[CmdletBinding()] +param ( + [Parameter(Mandatory=$true)] + [string] $GTestPath +) + +# Standard boilerplate +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" +$PSDefaultParameterValues['*:ErrorAction']='Stop' + +# Go to repo root +$repoRoot = (Resolve-Path "$PSScriptRoot\..").Path +Push-Location $repoRoot + +$null = New-Item build -ItemType Directory -Force +cd build + +cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE -G "MinGW Makefiles" .. +cmake --build . --config Debug + +# MinGW Makefiles build generates the following output: +# +# - build/libkaitai_struct_cpp_stl_runtime.dll +# - build/libkaitai_struct_cpp_stl_runtime.dll.a +# - build/tests/unittest.exe +# +# unittest.exe links dynamically against GTest DLLs and `libkaitai_struct_cpp_stl_runtime.dll`, and it +# will need all of them in same directory as .exe to run it, so we copy it. + +cp $GTestPath\debug\bin\*.dll tests +cp libkaitai_struct_cpp_stl_runtime.dll tests\unittest.exe + +Pop-Location diff --git a/.build/build-msvc.ps1 b/.build/build-msvc.ps1 new file mode 100644 index 0000000..fdab4ff --- /dev/null +++ b/.build/build-msvc.ps1 @@ -0,0 +1,34 @@ +<# +.DESCRIPTION +Builds Kaitai Struct C++ runtime library and unit tests on Windows using MS Visual C++ (MSVC) + +Requires: +- MSVC native tools installed and available in the command prompt +- cmake/ctest available (normally installed with MSVC native tools) +- GTest installed, path passed in `-GTestPath` +#> + +[CmdletBinding()] +param ( + [Parameter(Mandatory=$true)] + [string] $GTestPath +) + +# Standard boilerplate +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" +$PSDefaultParameterValues['*:ErrorAction']='Stop' + +# Go to repo root +$repoRoot = (Resolve-Path "$PSScriptRoot\..").Path +Push-Location $repoRoot + +$null = New-Item build -ItemType Directory -Force +cd build + +cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE .. +cmake --build . --config Debug +cp $GTestPath\debug\bin\*.dll tests\Debug +cp Debug\kaitai_struct_cpp_stl_runtime.dll tests\Debug + +Pop-Location diff --git a/.build/run-unittest b/.build/run-unittest new file mode 100755 index 0000000..0ffa351 --- /dev/null +++ b/.build/run-unittest @@ -0,0 +1,6 @@ +#!/bin/sh -ef + +cd "$(dirname "$0")"/.. + +cd build/tests +./unittest diff --git a/.build/run-unittest.ps1 b/.build/run-unittest.ps1 new file mode 100644 index 0000000..47fbc6b --- /dev/null +++ b/.build/run-unittest.ps1 @@ -0,0 +1,30 @@ +<# +.DESCRIPTION +Runs unit tests on Windows +#> + +# Standard boilerplate +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" +$PSDefaultParameterValues['*:ErrorAction']='Stop' + +# Go to repo root +$repoRoot = (Resolve-Path "$PSScriptRoot\..").Path +Push-Location $repoRoot + +cd build + +# Use ctest +#ctest -C Debug --output-on-failure + +# Run gtest-generated binary directly, produces more detailed output +if (Test-Path './tests/Debug/unittest.exe') { + ./tests/Debug/unittest.exe +} elseif (Test-Path './tests/unittest.exe') { + ./tests/unittest.exe +} else { + Write-Error "Unable to find the unittest executable." + exit 1 +} + +Pop-Location diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..bd80e15 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,62 @@ +name: Unit tests + +on: + push: + branches: + - master + pull_request: {} + +jobs: + linux: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: restore + run: | + sudo apt-get install -y libgtest-dev + - name: build + run: .build/build + - uses: actions/upload-artifact@v3 + with: + name: build + path: | + build + - name: unittest + run: .build/run-unittest + + windows-msvc: + runs-on: windows-latest + # NB: see https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md for what's available in this image + steps: + - uses: actions/checkout@v3 + - name: restore + run: | + C:\vcpkg\bootstrap-vcpkg.bat + C:\vcpkg\vcpkg install gtest:x64-windows + - name: build + run: .build\build-msvc.ps1 -GTestPath C:\vcpkg\installed\x64-windows + - uses: actions/upload-artifact@v3 + with: + name: build + path: | + build + - name: unittest + run: .build\run-unittest.ps1 + + windows-mingw64: + runs-on: windows-latest + steps: + - uses: actions/checkout@v3 + - name: restore + run: | + C:\vcpkg\bootstrap-vcpkg.bat + C:\vcpkg\vcpkg install gtest:x64-mingw-dynamic + - name: build + run: .build\build-mingw.ps1 -GTestPath C:\vcpkg\installed\x64-mingw-dynamic + - uses: actions/upload-artifact@v3 + with: + name: build + path: | + build + - name: unittest + run: .build\run-unittest.ps1 diff --git a/kaitai/kaitaistream.cpp b/kaitai/kaitaistream.cpp index 6e9a142..a5b69b8 100644 --- a/kaitai/kaitaistream.cpp +++ b/kaitai/kaitaistream.cpp @@ -9,7 +9,7 @@ #define __BYTE_ORDER BYTE_ORDER #define __BIG_ENDIAN BIG_ENDIAN #define __LITTLE_ENDIAN LITTLE_ENDIAN -#elif defined(_MSC_VER) // !__APPLE__ +#elif defined(_MSC_VER) || defined(__MINGW32__) #include #define __LITTLE_ENDIAN 1234 #define __BIG_ENDIAN 4321 @@ -17,7 +17,7 @@ #define bswap_16(x) _byteswap_ushort(x) #define bswap_32(x) _byteswap_ulong(x) #define bswap_64(x) _byteswap_uint64(x) -#elif defined(__QNX__) // __QNX__ +#elif defined(__QNX__) #include #include #define bswap_16(x) ENDIAN_RET16(x) @@ -26,7 +26,7 @@ #define __BYTE_ORDER BYTE_ORDER #define __BIG_ENDIAN BIG_ENDIAN #define __LITTLE_ENDIAN LITTLE_ENDIAN -#else // !__APPLE__ or !_MSC_VER or !__QNX__ +#else // !__APPLE__ or !_MSC_VER or !__MINGW32__ or !__QNX__ #include #include #endif @@ -692,6 +692,10 @@ std::string kaitai::kstream::bytes_to_str(std::string src, std::string src_enc) } #elif defined(KS_STR_ENCODING_NONE) std::string kaitai::kstream::bytes_to_str(std::string src, std::string src_enc) { + // Silence the compiler warning about src_enc being unused - https://stackoverflow.com/a/1486931 + // Sure, it's being unused in this version of this function, but it is obviously used in other implementations + (void) src_enc; + return src; } #else diff --git a/kaitai/kaitaistream.h b/kaitai/kaitaistream.h index 3a25a7d..668787d 100644 --- a/kaitai/kaitaistream.h +++ b/kaitai/kaitaistream.h @@ -245,8 +245,40 @@ class kstream { char buf[std::numeric_limits::digits10 + 5]; if (val < 0) { buf[0] = '-'; - // get absolute value without undefined behavior (https://stackoverflow.com/a/12231604) - unsigned_to_decimal(-static_cast(val), &buf[1]); + + // NB: `val` is negative and we need to get its absolute value (i.e. minus `val`). However, since + // `int64_t` uses two's complement representation, its range is `[-2**63, 2**63 - 1] = + // [-0x8000_0000_0000_0000, 0x7fff_ffff_ffff_ffff]` (both ends inclusive) and thus the naive + // `-val` operation will overflow for `val = std::numeric_limits::min() = + // -0x8000_0000_0000_0000` (because the result of `-val` is mathematically + // `-(-0x8000_0000_0000_0000) = 0x8000_0000_0000_0000`, but the `int64_t` type can represent at + // most `0x7fff_ffff_ffff_ffff`). And signed integer overflow is undefined behavior in C++. + // + // To avoid undefined behavior for `val = -0x8000_0000_0000_0000 = -2**63`, we do the following + // steps for all negative `val`s: + // + // 1. Convert the signed (and negative) `val` to an unsigned `uint64_t` type. This is a + // well-defined operation in C++: the resulting `uint64_t` value will be `val mod 2**64` (`mod` + // is modulo). The maximum `val` we can have here is `-1` (because `val < 0`), a theoretical + // minimum we are able to support would be `-2**64 + 1 = -0xffff_ffff_ffff_ffff` (even though + // in practice the widest standard type is `int64_t` with the minimum of `-2**63`): + // + // * `static_cast(-1) = -1 mod 2**64 = 2**64 + (-1) = 0xffff_ffff_ffff_ffff = 2**64 - 1` + // * `static_cast(-2**64 + 1) = (-2**64 + 1) mod 2**64 = 2**64 + (-2**64 + 1) = 1` + // + // 2. Subtract `static_cast(val)` from `2**64 - 1 = 0xffff_ffff_ffff_ffff`. Since + // `static_cast(val)` is in range `[1, 2**64 - 1]` (see step 1), the result of this + // subtraction will be mathematically in range `[0, (2**64 - 1) - 1] = [0, 2**64 - 2]`. So the + // mathematical result cannot be negative, hence this unsigned integer subtraction can never + // wrap around (which wouldn't be a good thing to rely upon because it confuses programmers and + // code analysis tools). + // + // 3. Since we did mathematically `(2**64 - 1) - (2**64 + val) = -val - 1` so far (and we wanted + // to do `-val`), we add `1` to correct that. From step 2 we know that the result of `-val - 1` + // is in range `[0, 2**64 - 2]`, so adding `1` will not wrap (at most we could get `2**64 - 1 = + // 0xffff_ffff_ffff_ffff`, which is still in the valid range of `uint64_t`). + + unsigned_to_decimal((std::numeric_limits::max() - static_cast(val)) + 1, &buf[1]); } else { unsigned_to_decimal(val, buf); }