Merge pull request #1130 from ijatinydv/fix-hash-big-endian

jvdp1 · web-flow · commit e33b3ebd7a01 · 2026-03-19T23:36:19.000+01:00
Fix test_hash_functions failures on big-endian architectures
diff --git a/.github/workflows/ci_big_endian.yml b/.github/workflows/ci_big_endian.yml
@@ -0,0 +1,68 @@
+name: CI - Big-Endian
+
+on:
+  # Always run on pushes to the main branch
+  push:
+    branches: [master, main]
+  # For PRs, ONLY run if the hash files or this workflow change
+  pull_request:
+    paths:
+      - '.github/workflows/ci_big_endian.yml'
+      - 'src/hash/**'
+      - 'test/hash_functions/**'
+  # Allow manual triggers from the GitHub Actions tab
+  workflow_dispatch:
+
+env:
+  CTEST_TIME_TIMEOUT: "5"  # some failures hang forever
+
+jobs:
+  test-big-endian:
+    runs-on: ubuntu-24.04
+    name: Test on s390x (big-endian)
+    strategy:
+      fail-fast: false
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Build and test on s390x
+        uses: uraimo/run-on-arch-action@v3
+        with:
+          arch: s390x
+          distro: ubuntu24.04
+
+          # Cache Docker image layer in GitHub Package Registry
+          githubToken: ${{ github.token }}
+
+          # Install dependencies (cached in Docker image layer)
+          install: |
+            apt-get update -q -y
+            apt-get install -q -y gfortran gcc g++ cmake python3-pip ninja-build git
+            pip3 install --break-system-packages fypp
+
+          # Build and run big-endian tests
+          run: |
+            echo "=== Architecture Info ==="
+            uname -m
+            echo "Byte order: $(python3 -c 'import sys; print(sys.byteorder)')"
+
+            echo "=== Compiler Version ==="
+            gfortran --version
+
+            echo "=== CMake Configure ==="
+            cmake -G Ninja \
+              -DCMAKE_BUILD_TYPE=Release \
+              -DCMAKE_MAXIMUM_RANK:String=4 \
+              -DFIND_BLAS:STRING=FALSE \
+              -S . -B build
+
+            echo "=== Build (Targeted) ==="
+            cmake --build build --target test_hash_functions --parallel
+
+            echo "=== Run Big-Endian Tests ==="
+            ctest --test-dir build \
+              -R hash_functions \
+              --output-on-failure \
+              --no-tests=error
diff --git a/doc/specs/stdlib_hash_procedures.md b/doc/specs/stdlib_hash_procedures.md
@@ -394,6 +394,16 @@ generating seeds for `universal_mult_hash`.
 All assume a two's complement sign bit, and no out of
 range checks.
 
+**Endianness note:** Both `pengy_hash` and `spooky_hash` will produce
+different hash values on big-endian processors (e.g., s390x, SPARC,
+PowerPC) compared to little-endian processors (e.g., x86, ARM, RISC-V)
+for the same input bytes. The hash quality is equally good on both
+architectures, but the outputs are not cross-architecture portable.
+Do not use these hashes for cross-architecture verification (e.g.,
+hashing a file on an x86 system and verifying the hash on an IBM
+mainframe). The 32-bit hashes `nmhash32`, `nmhash32x`, and
+`water_hash` produce identical results on all architectures.
+
 The `stdlib_hash_32bit_fnv` and `stdlib_hash_64bit_fnv`
 submodules each provide implementations of the FNV-1 and FNV-1A
 algorithms in the form of two separate overloaded functions: `FNV_1`
@@ -1366,6 +1376,9 @@ performance on long keys. It passes all the SMHasher tests, and has
 no known bad seeds.
 It is a *pure* function for integer arrays, and an *elemental*
 function for character strings.
+Note that `pengy_hash` will produce different hash values on
+big-endian and little-endian processors for the same input. The hash
+quality is equally good on both architectures.
 
 ##### Example
 
diff --git a/test/hash_functions/test_hash_functions.f90 b/test/hash_functions/test_hash_functions.f90
@@ -41,6 +41,10 @@ subroutine collect_hash_functions(testsuite)
             , new_unittest("water_hash", test_water_hash) &
             , new_unittest("pengy_hash", test_pengy_hash) &
             , new_unittest("spooky_hash", test_spooky_hash) &
+            , new_unittest("hash_determinism", test_hash_determinism) &
+            , new_unittest("hash_distribution", test_hash_distribution) &
+            , new_unittest("nmhash32_kat", test_nmhash32_kat) &
+            , new_unittest("nmhash32x_kat", test_nmhash32x_kat) &
             ]
 
     end subroutine collect_hash_functions
@@ -49,10 +53,11 @@ subroutine test_little_endian(error)
         !> Error handling
         type(error_type), allocatable, intent(out) :: error
  
-        ! Test for endianness
-
-        call check(error, little_endian, "The processor is not Little-Endian")
-        if (allocated(error)) return
+        ! Skip test on big-endian systems instead of failing
+        if (.not. little_endian) then
+            call skip_test(error, "The processor is not Little-Endian (skipping)")
+            return
+        end if
 
     end subroutine test_little_endian
 
@@ -64,6 +69,15 @@ subroutine test_nmhash32(error)
         integer(int8) :: key_array(size_key_array)
         integer(int32) :: c_hash(0:size_key_array)
 
+        ! The C reference implementation (nmhash.h) does not support
+        ! big-endian. Skip C-comparison on BE; value-correctness is
+        ! verified by the test_nmhash32_kat known-answer test instead.
+        if (.not. little_endian) then
+            call skip_test(error, &
+                "NMHASH32 C-comparison skipped on Big-Endian (see KAT test)")
+            return
+        end if
+
         call read_array("key_array.bin", key_array )
 
         ! Read hash array generated from key array by the C version of nmhash32
@@ -85,6 +99,15 @@ subroutine test_nmhash32x(error)
         integer(int8) :: key_array(size_key_array)
         integer(int32) :: c_hash(0:size_key_array)
 
+        ! The C reference implementation (nmhash.h) does not support
+        ! big-endian. Skip C-comparison on BE; value-correctness is
+        ! verified by the test_nmhash32x_kat known-answer test instead.
+        if (.not. little_endian) then
+            call skip_test(error, &
+                "NMHASH32X C-comparison skipped on Big-Endian (see KAT test)")
+            return
+        end if
+
         call read_array("key_array.bin", key_array )
 
         ! Read hash array generated from key array by the C version of nmhash32x
@@ -162,6 +185,201 @@ subroutine test_spooky_hash(error)
     end subroutine test_spooky_hash
 
 
+    !> Test that all hash functions produce deterministic results
+    !> This test runs on ALL platforms (LE and BE)
+    subroutine test_hash_determinism(error)
+        !> Error handling
+        type(error_type), allocatable, intent(out) :: error
+
+        integer(int8) :: key(8)
+        integer(int32) :: h32_a, h32_b
+        integer(int64) :: h64_a, h64_b
+        integer(int64) :: h128_a(2), h128_b(2)
+
+        key = [1_int8, 2_int8, 3_int8, 4_int8, &
+               5_int8, 6_int8, 7_int8, 8_int8]
+
+        ! nmhash32 determinism
+        h32_a = nmhash32(key, nm_seed)
+        h32_b = nmhash32(key, nm_seed)
+        call check(error, h32_a == h32_b, "NMHASH32 not deterministic")
+        if (allocated(error)) return
+
+        ! nmhash32x determinism
+        h32_a = nmhash32x(key, nm_seed)
+        h32_b = nmhash32x(key, nm_seed)
+        call check(error, h32_a == h32_b, "NMHASH32X not deterministic")
+        if (allocated(error)) return
+
+        ! water_hash determinism
+        h32_a = water_hash(key, water_seed)
+        h32_b = water_hash(key, water_seed)
+        call check(error, h32_a == h32_b, "WATER_HASH not deterministic")
+        if (allocated(error)) return
+
+        ! pengy_hash determinism
+        h64_a = pengy_hash(key, pengy_seed)
+        h64_b = pengy_hash(key, pengy_seed)
+        call check(error, h64_a == h64_b, "PENGY_HASH not deterministic")
+        if (allocated(error)) return
+
+        ! spooky_hash determinism
+        h128_a = spooky_hash(key, spooky_seed)
+        h128_b = spooky_hash(key, spooky_seed)
+        call check(error, all(h128_a == h128_b), &
+            "SPOOKY_HASH not deterministic")
+        if (allocated(error)) return
+
+    end subroutine test_hash_determinism
+
+    !> Collision sanity check: verify distinct inputs produce distinct hashes.
+    !> For these well-tested hash functions with fixed deterministic inputs,
+    !> an accidental collision probability is ~2^-32 (32-bit) or ~2^-64
+    !> (64-bit), making this effectively deterministic.
+    !> This test runs on ALL platforms (LE and BE).
+    subroutine test_hash_distribution(error)
+        !> Error handling
+        type(error_type), allocatable, intent(out) :: error
+
+        integer(int8) :: key_a(8), key_b(8)
+        integer(int32) :: h32_a, h32_b
+        integer(int64) :: h64_a, h64_b
+        integer(int64) :: h128_a(2), h128_b(2)
+
+        key_a = [1_int8, 2_int8, 3_int8, 4_int8, &
+                 5_int8, 6_int8, 7_int8, 8_int8]
+        key_b = [1_int8, 2_int8, 3_int8, 4_int8, &
+                 5_int8, 6_int8, 7_int8, 9_int8]  ! differs in last byte
+
+        ! nmhash32 collision check
+        h32_a = nmhash32(key_a, nm_seed)
+        h32_b = nmhash32(key_b, nm_seed)
+        call check(error, h32_a /= h32_b, &
+            "NMHASH32 same hash for different inputs")
+        if (allocated(error)) return
+
+        ! nmhash32x collision check
+        h32_a = nmhash32x(key_a, nm_seed)
+        h32_b = nmhash32x(key_b, nm_seed)
+        call check(error, h32_a /= h32_b, &
+            "NMHASH32X same hash for different inputs")
+        if (allocated(error)) return
+
+        ! water_hash collision check
+        h32_a = water_hash(key_a, water_seed)
+        h32_b = water_hash(key_b, water_seed)
+        call check(error, h32_a /= h32_b, &
+            "WATER_HASH same hash for different inputs")
+        if (allocated(error)) return
+
+        ! pengy_hash collision check
+        h64_a = pengy_hash(key_a, pengy_seed)
+        h64_b = pengy_hash(key_b, pengy_seed)
+        call check(error, h64_a /= h64_b, &
+            "PENGY_HASH same hash for different inputs")
+        if (allocated(error)) return
+
+        ! spooky_hash collision check
+        h128_a = spooky_hash(key_a, spooky_seed)
+        h128_b = spooky_hash(key_b, spooky_seed)
+        call check(error, any(h128_a /= h128_b), &
+            "SPOOKY_HASH same hash for different inputs")
+        if (allocated(error)) return
+
+    end subroutine test_hash_distribution
+
+
+    !> Known-Answer Test for NMHASH32.
+    !> Verifies the Fortran implementation produces the exact canonical
+    !> LE-normalized hash values across all code paths. Reference values
+    !> were computed on a little-endian platform using the upstream C code.
+    !> This test runs on ALL platforms (LE and BE).
+    subroutine test_nmhash32_kat(error)
+        !> Error handling
+        type(error_type), allocatable, intent(out) :: error
+
+        ! Number of test vectors
+        integer, parameter :: num_kat = 14
+
+        ! Input lengths covering every code path:
+        ! 0=zero, 1/2/3/4=small, 7/8=5-8 path, 9/32=9-32 path,
+        ! 33/100/255=33-255 path, 256/300=long path (256+)
+        integer, parameter :: kat_lengths(num_kat) = [ &
+            0, 1, 2, 3, 4, 7, 8, &
+            9, 32, 33, 100, 255, 256, 300 ]
+
+        ! Reference NMHASH32 values (computed on LE with seed=0xDEADBEEF)
+        integer(int32), parameter :: kat_expected(num_kat) = [ &
+            int(z'B0D9C845', int32), int(z'D52AD23F', int32), &
+            int(z'E909FDFF', int32), int(z'FF1A009C', int32), &
+            int(z'097D4183', int32), int(z'55CC8BBF', int32), &
+            int(z'660D67B4', int32), int(z'CB939B94', int32), &
+            int(z'4CBE45F8', int32), int(z'2FD88BD0', int32), &
+            int(z'83AC6B02', int32), int(z'CC0E4E26', int32), &
+            int(z'567D6B58', int32), int(z'865F0BC9', int32) ]
+
+        ! Deterministic key: key(i) = IAND(i, 255)
+        integer(int8) :: key(300)
+        integer :: i
+        integer(int32) :: got
+
+        do i = 1, 300
+            key(i) = int(iand(i, 255), int8)
+        end do
+
+        do i = 1, num_kat
+            got = nmhash32(key(1:kat_lengths(i)), nm_seed)
+            call check(error, got == kat_expected(i), &
+                "NMHASH32 KAT failed")
+            if (allocated(error)) return
+        end do
+
+    end subroutine test_nmhash32_kat
+
+    !> Known-Answer Test for NMHASH32X.
+    !> Same approach as test_nmhash32_kat but for the NMHASH32X variant.
+    !> This test runs on ALL platforms (LE and BE).
+    subroutine test_nmhash32x_kat(error)
+        !> Error handling
+        type(error_type), allocatable, intent(out) :: error
+
+        ! Number of test vectors
+        integer, parameter :: num_kat = 14
+
+        ! Input lengths covering every code path
+        integer, parameter :: kat_lengths(num_kat) = [ &
+            0, 1, 2, 3, 4, 7, 8, &
+            9, 32, 33, 100, 255, 256, 300 ]
+
+        ! Reference NMHASH32X values (computed on LE with seed=0xDEADBEEF)
+        integer(int32), parameter :: kat_expected(num_kat) = [ &
+            int(z'76844735', int32), int(z'B7AE2C90', int32), &
+            int(z'EE2224FD', int32), int(z'BBE39609', int32), &
+            int(z'08467EE3', int32), int(z'10E572DA', int32), &
+            int(z'2570CFA8', int32), int(z'1A06128A', int32), &
+            int(z'EABBF1B8', int32), int(z'9B1B3428', int32), &
+            int(z'F6F0233D', int32), int(z'7EB7CAFC', int32), &
+            int(z'B34D6C45', int32), int(z'E89BEE9E', int32) ]
+
+        ! Deterministic key: key(i) = IAND(i, 255)
+        integer(int8) :: key(300)
+        integer :: i
+        integer(int32) :: got
+
+        do i = 1, 300
+            key(i) = int(iand(i, 255), int8)
+        end do
+
+        do i = 1, num_kat
+            got = nmhash32x(key(1:kat_lengths(i)), nm_seed)
+            call check(error, got == kat_expected(i), &
+                "NMHASH32X KAT failed")
+            if (allocated(error)) return
+        end do
+
+    end subroutine test_nmhash32x_kat
+
+
     subroutine generate_key_array()
     
         integer        :: i, lun
diff --git a/test/hash_functions/waterhash.h b/test/hash_functions/waterhash.h
@@ -10,6 +10,16 @@
 #include <string.h>
 #include <stdint.h>
 #include <math.h>
+
+/* Endian detection: broaden beyond just __BYTE_ORDER__ to cover all major BE
+ * toolchains (IBM XL, older GCC, etc.) — mirrors nmhash.h's approach. */
+#ifndef WATERHASH_BIG_ENDIAN
+#  if defined(__BIG_ENDIAN__) || (defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
+#    define WATERHASH_BIG_ENDIAN 1
+#  else
+#    define WATERHASH_BIG_ENDIAN 0
+#  endif
+#endif
 const uint64_t _waterp0 = 0xa0761d65ull, _waterp1 = 0xe7037ed1ull, _waterp2 = 0x8ebc6af1ull;
 const uint64_t _waterp3 = 0x589965cdull, _waterp4 = 0x1d8e4e27ull, _waterp5 = 0xeb44accbull;
 
@@ -19,8 +29,21 @@ static inline uint64_t _watermum(const uint64_t A, const uint64_t B) {
 }
 
 static inline uint64_t _waterr08(const uint8_t *p){ uint8_t  v; memcpy(&v, p, 1); return v; }
-static inline uint64_t _waterr16(const uint8_t *p){ uint16_t v; memcpy(&v, p, 2); return v; }
-static inline uint64_t _waterr32(const uint8_t *p){ uint32_t v; memcpy(&v, p, 4); return v; }
+static inline uint64_t _waterr16(const uint8_t *p){
+    uint16_t v; memcpy(&v, p, 2);
+#if WATERHASH_BIG_ENDIAN
+    v = (uint16_t)((v << 8) | (v >> 8));
+#endif
+    return v;
+}
+static inline uint64_t _waterr32(const uint8_t *p){
+    uint32_t v; memcpy(&v, p, 4);
+#if WATERHASH_BIG_ENDIAN
+    v = ((v >> 24) & 0xff) | ((v >> 8) & 0xff00) |
+        ((v << 8) & 0xff0000) | ((v << 24) & 0xff000000u);
+#endif
+    return v;
+}
 static inline uint32_t waterhash(const void* key, uint32_t len, uint64_t seed){
     const uint8_t *p = (const uint8_t*)key;
     uint32_t i;
