Fix endianness detection when __BYTE_ORDER__ is undefined (MSVC)

[MrWater00]

Problem

plat_local.h selects the byte-swap helpers with

#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__

without checking that the macros exist. MSVC defines neither, so the preprocessor replaces both with 0 (warning C4668), the condition is true, and rev8_be32 / rev8_be64 become no-ops on little-endian MSVC targets. The SHA2 core and the address helpers in slh_adrs.h then work with the wrong byte order, and the library produces non-standard results.

Change

Guard both tests with defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__), so the byte-swapping versions are used when the macros are missing. No other lines change.

Testing

• MSVC 19.44 (x64), building xfips205 from the root sources plus test/xfips205.c with cl /O2, running the existing ACVP keyGen vectors (v1.1.0.40) through test/acvp_client.py with the binary path redirected:
  • before: 0/120 pass (SHA2 0/60, SHAKE 0/60)
  • after: 120/120 pass (60/60 each)
  • the C4668 warnings for plat_local.h lines 110 and 123 disappear
• Linux, gcc: make (with the project's -Werror -pedantic flags) and make test: 1248/1248 PASS.

I only ran keyGen on MSVC: acvp_client.py passes sigGen/sigVer data on the command line, which exceeds the cmd.exe length limit on Windows for sigVer. I did not test 32-bit MSVC.

Notes

• This was found downstream: liboqs vendors this file, and SLH_DSA_PURE_SHA2_* signatures failed to verify on MSVC builds (SLH-DSA SHA2 parameter sets fail signature verification on Windows/MSVC build (SHAKE variants unaffected) open-quantum-safe/liboqs#2516). The liboqs maintainers asked for the fix to land here first.
• plat_local.h(64) also triggers C4668 for __WORDSIZE on MSVC. It falls through to the 64-bit default, which is right for x64, so I left it alone.
• I did not find a Windows/MSVC job in .github/workflows. I'm happy to add one in a separate PR if you want it.
• If you prefer the explicit style used in mlkem-native/mldsa-native (a defined(__BYTE_ORDER__) check plus an MSVC little-endian fallback for x86/x64/ARM64), I can rework the change that way.

[dstebila]

If you prefer the explicit style used in mlkem-native/mldsa-native (a defined(BYTE_ORDER) check plus an MSVC little-endian fallback for x86/x64/ARM64), I can rework the change that way.

I think that would be a good idea: if a compiler lacks the GCC macros, the code now assumes little-endian with no warning.

[dstebila]

I did not find a Windows/MSVC job in .github/workflows. I'm happy to add one in a separate PR if you want it.

That would be great to have too!

[dstebila]

Do you think we should add -Wundef to the Makefile, which on gcc/clang would catch undefined macros?

[mkannwischer]

Thanks @MrWater00. Do you mind aligning this with mlkem-native?

There should be centralized macro for detecting endianness. If endianness cannot be detected, we should use portable code.

[MrWater00]

Thanks for the reviews, and agreed. I'll rework this:

• Centralize the endianness detection in slh_sys.h, modeled on sys.h in
  mlkem-native / mldsa-native (use __BYTE_ORDER__ when present; treat MSVC on
  x86/x64/ARM64 as little-endian).
• If endianness cannot be detected, plat_local.h will use portable code
  instead of assuming little-endian.
• I'll test -Wundef locally and report back before proposing it.
• I'll open the Windows/MSVC CI job as a separate PR after this one.

[MrWater00]

Thanks for the reviews. I reworked the change and pushed it as a second commit:

• Endianness detection is now centralized in slh_sys.h
  (SLH_SYS_LITTLE_ENDIAN / SLH_SYS_BIG_ENDIAN), following mlkem-native and
  mldsa-native: __BYTE_ORDER__ when the compiler provides it, and MSVC on
  x86/x64/ARM64 is treated as little-endian.
• plat_local.h uses those macros. If neither is defined, rev8_be32/64 fall
  back to portable code that checks the byte order at run time instead of
  assuming little-endian.

Testing:

• MSVC 19.44 x64: ACVP keyGen vectors 120/120 (SHA2 60/60, SHAKE 60/60). The
  only remaining C4668 in this file is the existing __WORDSIZE one.
• Linux gcc 15.2: make test 1248/1248.
• Linux gcc with -U__BYTE_ORDER__ (so the portable fallback is the code
  being compiled): make test 1248/1248.
• -Werror -Wundef builds cleanly with gcc 15.2 and clang on x86_64/glibc.
• Negative control: forcing -DSLH_SYS_BIG_ENDIAN on x86_64 makes make test
  fail 816/1248, so the macro does drive the swaps and ACVP detects a wrong
  choice.

On -Wundef: it would not have caught this bug, since gcc and clang define
__BYTE_ORDER__, but it flags undefined macros in #if on those compilers.
I only tested x86_64/glibc. A target that reaches the __WORDSIZE branch in
plat_local.h without __WORDSIZE being defined would warn, and the Makefile
uses -Werror. I'm happy to add it if you want, otherwise I'd leave it out of
this PR.

I'll open the Windows/MSVC CI job as a separate PR after this one.

[mkannwischer]

If I see correctly, we are currently not testing on a big endian platform. Could you please port the big endian tests from mlkem-native? https://github.com/pq-code-package/mlkem-native/blob/c2827a8e8be4cd09b20dfe3544fe854777bcaefb/.github/workflows/ci.yml#L108

[mkannwischer]

Instead of detecting endianness at runtime, I'd prefer a portable version:

static SLH_INLINE uint32_t rev8_be32(uint32_t x)
{
  const uint8_t *p = (const uint8_t *)&x;
  return ((uint32_t)p[0] << 24) | ((uint32_t)p[1] << 16) |
         ((uint32_t)p[2] << 8) | (uint32_t)p[3];
}

[MrWater00]

Done in 63ed897: replaced the run-time check with your portable version, and
used the same pattern for rev8_be64.

[mkannwischer]

I believe introducing 3 code paths here is a risk that's not really justified.
Can we have two implementations: one little endian and one portable, please?

[MrWater00]

Done in 63ed897: there are two implementations now. The little-endian
shift/mask version is selected by SLH_SYS_LITTLE_ENDIAN, and the portable
version is used otherwise. SLH_SYS_BIG_ENDIAN is still defined in slh_sys.h,
as in mlkem-native, but plat_local.h no longer uses it, so big-endian hosts
take the portable path.

[mkannwischer]

If I see correctly, we are currently not testing on a big endian platform. Could you please port the big endian tests from mlkem-native? https://github.com/pq-code-package/mlkem-native/blob/c2827a8e8be4cd09b20dfe3544fe854777bcaefb/.github/workflows/ci.yml#L108

[mkannwischer]

If I see correctly, we are currently not testing on a big endian platform. Could you please port the big endian tests from mlkem-native? https://github.com/pq-code-package/mlkem-native/blob/c2827a8e8be4cd09b20dfe3544fe854777bcaefb/.github/workflows/ci.yml#L108

I'll do that as a follow-up. I see that there are more big endian issues.

[MrWater00]

Re-tested after the rework:

• MSVC 19.44 x64: ACVP keyGen vectors 120/120 (SHA2 60/60, SHAKE 60/60).
• Linux gcc 15.2: make test 1248/1248.
• Linux gcc with -U__BYTE_ORDER__ (so the portable version is the code being
  compiled): make test 1248/1248.
• -Werror -Wundef builds cleanly with gcc and clang.

I have not tested on a big-endian machine. Thanks for taking the big-endian CI
in #55.