Fix endianness detection that byte-swaps every FP immediate on Windows - #1455
Fix endianness detection that byte-swaps every FP immediate on Windows#1455mahmoudimus wants to merge 2 commits into
Conversation
|
已收到您的邮件,我会尽快回复。
|
The vcpkg workflow has failed on every run since 2026-06-27, on all three
platforms, for any PR that touches the tree:
CMake Error (install-absolute-destination) at src/libtriton/CMakeLists.txt:378 (install):
INSTALL command given absolute DESTINATION path:
/home/runner/work/Triton/Triton/out/install/linux-x64/lib/python3.11/site-packages
Nothing in the repository changed. .github/workflows/vcpkg.yml uses
lukka/get-cmake@latest, which is unpinned, so the runner moved from CMake 3.x
to 4.4.2; CMake 4 promoted install-absolute-destination into the developer
warning category, and CMakePresets.json sets "errors": { "dev": true }.
The prefix was redundant in the first place. In this branch PYTHON_SITE_PACKAGES
is already relative -- the execute_process just above it prints
"lib/pythonX.Y/site-packages" -- and install() resolves a relative DESTINATION
against CMAKE_INSTALL_PREFIX. Prefixing it again only made the path absolute.
Dropping the prefix keeps the destination byte-identical for a normal install
and additionally restores `cmake --install --prefix <other>`, which an absolute
DESTINATION silently ignores.
The other branch of the if() is left alone: it deliberately installs into the
interpreter's real site-packages when no prefix was given, so its absolute path
is intended. It is not reachable from the presets, which always set one.
Verified with CMake 4.4.2 locally: reproduced the error before the change, and
after it the generated cmake_install.cmake records
"${CMAKE_INSTALL_PREFIX}/lib/pythonX.Y/site-packages" -- the same location,
resolved relative to the prefix.
d1e8041 to
84e0c7e
Compare
|
Heads-up on the red checks here: they are not caused by this PR. The failing job is The line number moves between runs only because commits above it shift it: #1457 fixes it, and this branch is now rebased on top of that so it can go green. Note that the workflow runs on these branches are currently held at |
…ndows
Immediate::Immediate(double, size, endianness) byte-swaps when the host and
the emulated platform disagree on byte order. The host side came from:
#ifdef LITTLE_ENDIAN // provided by CMake
CMake does provide it -- but only to one translation unit:
set_source_files_properties(utils/softfloat.cpp PROPERTIES COMPILE_DEFINITIONS
${CMAKE_CXX_BYTE_ORDER}
)
CMAKE_CXX_BYTE_ORDER expands to the literal LITTLE_ENDIAN or BIG_ENDIAN, so
utils/softfloat.cpp gets the macro and arch/immediate.cpp never does. What
immediate.cpp actually sees is the POSIX definition, and that does not mean
what the #ifdef assumes:
* On POSIX, <endian.h> defines LITTLE_ENDIAN and BIG_ENDIAN as named
constants (1234 and 4321), reaching this file transitively. The macro is
therefore always defined regardless of the actual byte order, so the test
is really "am I on POSIX". The correct spelling would have been
BYTE_ORDER == LITTLE_ENDIAN. It selects the right branch today only
because every platform this is built on happens to be little-endian.
* Under MSVC neither header nor per-source definition applies, the macro is
undefined, and Triton concludes it is running big-endian. Every AArch64
floating-point immediate is then byte-swapped on a little-endian machine:
fmov s0, #2.0 -> 0x00000040 (expected 0x40000000)
fmov s0, #1.0 -> 0x0000803f (expected 0x3f800000)
Extend the byte order to the whole library rather than one file, via the
existing triton/config.hpp, as TRITON_BIG_ENDIAN. The namespaced name also
avoids colliding with the POSIX LITTLE_ENDIAN/BIG_ENDIAN constants, which a
translation-unit-wide LITTLE_ENDIAN definition would risk.
Introduced in JonathanSalwan#1359.
84e0c7e to
680eb67
Compare
…t fixes Brings in the three commits proposed upstream as JonathanSalwan#1457, JonathanSalwan#1455 and JonathanSalwan#1456: * absolute install DESTINATION that CMake >= 4 rejects * endianness detection that byte-swapped every AArch64 FP immediate on Windows builds * the platform gate that hid the differential test which found it
Summary
Every AArch64 floating-point immediate is byte-swapped on a Windows build.
fmov s0, #2.0produces0x00000040instead of0x40000000.Reproduced on a VS 2022 build of
dev-v1.0, with macOS/AppleClang as a control:fmov s0, #2.00x400000000x000000400x40000000fmov s0, #1.00x3f8000000x0000803f0x3f800000Both Windows values are exact 32-bit byte swaps of the correct answer.
Root cause
Immediate::Immediate(double, size, endianness)(src/libtriton/arch/immediate.cpp) byte-swaps when the host and the emulated platform disagree on byte order. The host side comes from:CMake does provide it -- but only to one translation unit:
CMAKE_CXX_BYTE_ORDERexpands to the literalLITTLE_ENDIANorBIG_ENDIAN, soutils/softfloat.cppgets the macro andarch/immediate.cppnever does. Whatimmediate.cppactually sees is the POSIX definition, which does not mean what the#ifdefassumes.The check also does not work as an endianness test, independently of that:
On POSIX
<endian.h>definesLITTLE_ENDIANandBIG_ENDIANas named constants — 1234 and 4321 — so the macro is always defined regardless of the machine's actual byte order. It reaches this translation unit transitively via<sys/types.h>. The correct spelling would have been#if BYTE_ORDER == LITTLE_ENDIAN. It selects the right branch today only because every platform Triton is built on happens to be little-endian; a big-endian POSIX host would fail the same way Windows does.Under MSVC neither the header nor the per-source definition applies, the macro is undefined, and Triton concludes it is running big-endian — so it swaps every FP immediate on a little-endian machine.
Introduced in #1359. The constructor has exactly one caller, the AArch64 FP-immediate decode at
aarch64Cpu.cpp:539, so the blast radius isfmov <Sd|Dd|Hd>, #immon Windows builds.Fix
Extend the byte order to the whole library rather than a single file, reusing the
CMAKE_CXX_BYTE_ORDERthis file already relies on, and expose it through the existingtriton/config.hppasTRITON_BIG_ENDIAN. The namespaced name also avoids colliding with the POSIXLITTLE_ENDIAN/BIG_ENDIANconstants, which a global-DLITTLE_ENDIANwould have done.Absence means little-endian, which is correct for every platform Triton currently targets.
Testing
/* #undef TRITON_BIG_ENDIAN */; bothfmovcases correct;ctest47/47, no regression.fmovcases correct after the fix.ctest47/47.This has no test covering it in-tree, because
UnicornAArch64Semantics— the test that catches it — is currently gated off on Windows. A follow-up PR removes that gate.