Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ set(CMAKE_DISABLE_FIND_PACKAGE_OpenSSL On)
# many tests require handling of signals themselves and even when they don't we'd like for them to generate a core dump, this
# is effectively --catch_system_errors=no broadly across all tests
add_compile_definitions(BOOST_TEST_DEFAULTS_TO_CORE_DUMP)
# softfloat's legacy float32_t/float64_t aliases collide with arm_neon.h, which Boost.Unordered pulls in on arm64;
# nothing here uses them.
add_compile_definitions(SOFTFLOAT_DISABLE_LEGACY_TYPE_ALIASES)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: This definition is directory-scoped, so it does not reach consumers of the generated SysioChain/SysioTester interface. SysioTester.cmake.in links libsoftfloat as a raw archive and exports the SoftFloat include paths without this compile definition. On the documented Apple-Silicon wire-cdt integration path, tester.hpp includes softfloat_types.h before Boost.Unordered pulls in arm_neon.h, reproducing typedef redefinitions for float16_t, float32_t, and float64_t. I reproduced the failure with a Clang 18 AArch64 syntax probe; it passes when this macro is defined. Please export the definition on the generated SysioChain interface, while retaining the in-tree definition as needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exported in 4af9ddb: SysioChain now carries SOFTFLOAT_DISABLE_LEGACY_TYPE_ALIASES and, since it links the archive directly rather than the port's package, also THREAD_LOCAL=__thread, the storage class the archive was built with. Checked with a build-tree find_package(sysio) consumer: both land in INTERFACE_COMPILE_DEFINITIONS. The tree-wide definition stays.


add_definitions(-DSYSIO_SYS_VM_RUNTIME_ENABLED)

Expand Down
7 changes: 7 additions & 0 deletions cmake/SysioTester.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,13 @@ target_include_directories(SysioChain INTERFACE
@CMAKE_INSTALL_FULL_INCLUDEDIR@/wasm-jit
@CMAKE_INSTALL_FULL_INCLUDEDIR@/softfloat)

# softfloat is linked above as a raw archive, so the definitions its CMake package carries are repeated:
# THREAD_LOCAL is the storage class the archive's per-thread state was built with, and the alias disable
# keeps softfloat's legacy float32_t/float64_t typedefs away from arm_neon.h on arm64.
target_compile_definitions(SysioChain INTERFACE
THREAD_LOCAL=__thread
SOFTFLOAT_DISABLE_LEGACY_TYPE_ALIASES)

add_library(SysioTester INTERFACE)

target_link_libraries(SysioTester INTERFACE
Expand Down
62 changes: 62 additions & 0 deletions unittests/softfloat_thread_local_tests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* Berkeley SoftFloat keeps the rounding mode and exception flags in per-thread state (softfloat 3.0.0#3): a
* worker thread starts from the defaults and leaks nothing back to the thread that spawned it. With the
* process-global state of 3.0.0#2 every check below fails.
*/
extern "C" {
#include <softfloat/softfloat.h>
}
#include <softfloat/softfloat.hpp>

#include <boost/test/unit_test.hpp>

#include <cstdint>
#include <thread>

namespace {

const softfloat32_t one = {0x3f800000u};
const softfloat32_t zero = {0u};
const softfloat32_t minus_one = {0xbf800000u};

/// Other suites may leave the calling thread's state behind; start from the defaults and put them back.
struct softfloat_state_guard {
softfloat_state_guard() { reset(); }
~softfloat_state_guard() { reset(); }
static void reset() {
softfloat_roundingMode = softfloat_round_near_even;
softfloat_exceptionFlags = 0;
}
};

} // namespace

BOOST_AUTO_TEST_SUITE(softfloat_thread_local_tests)

BOOST_AUTO_TEST_CASE(rounding_mode_and_exception_flags_are_per_thread) {
softfloat_state_guard guard;

softfloat_roundingMode = softfloat_round_max;
f32_div(one, zero);
BOOST_CHECK_EQUAL(unsigned(softfloat_exceptionFlags), unsigned(softfloat_flag_infinite));

unsigned worker_mode = ~0u;
unsigned worker_flags_before = ~0u;
unsigned worker_flags_after = 0;
std::thread worker([&] {
worker_mode = softfloat_roundingMode;
worker_flags_before = softfloat_exceptionFlags;
softfloat_roundingMode = softfloat_round_minMag;
f32_sqrt(minus_one);
worker_flags_after = softfloat_exceptionFlags;
});
worker.join();

BOOST_CHECK_EQUAL(worker_mode, unsigned(softfloat_round_near_even));
BOOST_CHECK_EQUAL(worker_flags_before, 0u);
BOOST_CHECK_EQUAL(worker_flags_after, unsigned(softfloat_flag_invalid));
BOOST_CHECK_EQUAL(unsigned(softfloat_roundingMode), unsigned(softfloat_round_max));
BOOST_CHECK_EQUAL(unsigned(softfloat_exceptionFlags), unsigned(softfloat_flag_infinite));
}

BOOST_AUTO_TEST_SUITE_END()
4 changes: 2 additions & 2 deletions vcpkg-configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
{
"kind": "git",
"repository": "https://github.com/wire-network/wire-vcpkg-registry",
"baseline": "2806c1bc39dda520c100b5ea75aef8aa2693a85f",
"reference": "2806c1bc39dda520c100b5ea75aef8aa2693a85f",
"baseline": "abed0cc16f9e9514fec4a0b6751bc34ec427ca0d",
"reference": "abed0cc16f9e9514fec4a0b6751bc34ec427ca0d",
"packages": [
"boost",
"softfloat",
Expand Down
2 changes: 1 addition & 1 deletion vcpkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
},
{
"name": "softfloat",
"version": "3.0.0#2"
"version": "3.0.0#3"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium (test coverage): This version bump changes the four SoftFloat state variables from process-global to thread-local, but the current wire-sysio golden tests are serial and never assert rounding or exception-state isolation. The dependency contains a deterministic TLS test, but the registry port configures it with BUILD_TESTING=OFF, so PR CI does not exercise this behavior. Please add a two-thread regression test that assigns distinct rounding modes and exception flags and verifies they do not leak; it should fail with 3.0.0#2 and pass with #3.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unittests/softfloat_thread_local_tests in 4af9ddb: the calling thread sets softfloat_round_max and raises softfloat_flag_infinite, a worker must start from the defaults, set softfloat_round_minMag and raise softfloat_flag_invalid, and neither side may see the other's state. Against the #2 archive (no THREAD_LOCAL, process-global state) all five checks fail at runtime (the worker sees round_max and infinite, the caller ends with minMag and invalid | infinite); against #3 it passes, in this tree under --sys-vm, --sys-vm-jit and --sys-vm-oc.

},
{
"name": "wire-sys-vm",
Expand Down
Loading