From 679d31b8e2829628d2344bac1658b8f8eac59149 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Fri, 18 Sep 2026 21:03:31 +0200 Subject: [PATCH] Fix for io wrapping --- CMakeLists.txt | 10 +++- Modules/app-module/CMakeLists.txt | 1 + Modules/app-module/source/io.cpp | 53 +++++++++++------- Modules/app-module/source/stdio_wrap.cpp | 39 ++++++++++++- .../app-module/tests/source/execute_test.cpp | 56 +++++++++++++++++++ Modules/app-module/tests/source/io_test.cpp | 41 ++++++++++++++ 6 files changed, 174 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1f7fb145e..fa57eb66a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -62,9 +62,13 @@ if (DEFINED ENV{ESP_IDF_VERSION}) # to every target; PanicHandler.cpp branches internally per architecture. idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=esp_panic_handler" APPEND) - idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=read" APPEND) - idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=write" APPEND) - idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=close" APPEND) + # Wraps newlib's reentrant syscall stubs, not the plain read()/write()/close() newlib itself + # implements as thin wrappers around them. newlib's own stdio (fflush()'s buffer-flush path in particular) + # calls these _r stubs directly, bypassing the plain names entirely. + # See Modules/app-module/source/stdio_wrap.cpp's own comment for the exact call chain. + idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=_read_r" APPEND) + idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=_write_r" APPEND) + idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=_close_r" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=lv_button_create" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=lv_dropdown_create" APPEND) diff --git a/Modules/app-module/CMakeLists.txt b/Modules/app-module/CMakeLists.txt index 007bac6f3..d3b42d1ee 100644 --- a/Modules/app-module/CMakeLists.txt +++ b/Modules/app-module/CMakeLists.txt @@ -10,6 +10,7 @@ tactility_add_module(app-module INCLUDE_DIRS include/ REQUIRES TactilityKernel service-module minitar PRIV_REQUIRES TactilityKernelCpp + WHOLE_ARCHIVE ) # Tells source/io.cpp its real-syscall fallback must go through __real_read/write/close() rather diff --git a/Modules/app-module/source/io.cpp b/Modules/app-module/source/io.cpp index 0d7570cfc..4830f6a75 100644 --- a/Modules/app-module/source/io.cpp +++ b/Modules/app-module/source/io.cpp @@ -8,13 +8,42 @@ #include #if defined(TT_APP_IO_WRAPS_STDIO) +#ifdef ESP_PLATFORM +// Newlib's stdio (fflush()'s buffer-flush path, in particular) calls the reentrant _read_r/ +// _write_r/_close_r syscall stubs directly. The plain read()/write()/close() newlib provides are +// just thin wrappers around them (esp-idf's components/newlib/src/syscalls.c: `write(fd, dst, +// size) { return _write_r(__getreent(), fd, dst, size); }`). Wrapping the _r stubs catches both; +// wrapping the plain names would only catch direct write()-style callers. +#include +extern "C" { +ssize_t __real__read_r(struct _reent* r, int fd, void* buffer, size_t size); +ssize_t __real__write_r(struct _reent* r, int fd, const void* buffer, size_t size); +int __real__close_r(struct _reent* r, int fd); +} +namespace { +ssize_t real_read(int fd, void* buffer, size_t size) { return __real__read_r(__getreent(), fd, buffer, size); } +ssize_t real_write(int fd, const void* buffer, size_t size) { return __real__write_r(__getreent(), fd, buffer, size); } +int real_close(int fd) { return __real__close_r(__getreent(), fd); } +} // namespace +#else extern "C" { ssize_t __real_read(int fd, void* buffer, size_t size); ssize_t __real_write(int fd, const void* buffer, size_t size); int __real_close(int fd); } +namespace { +ssize_t real_read(int fd, void* buffer, size_t size) { return __real_read(fd, buffer, size); } +ssize_t real_write(int fd, const void* buffer, size_t size) { return __real_write(fd, buffer, size); } +int real_close(int fd) { return __real_close(fd); } +} // namespace +#endif #else #include +namespace { +ssize_t real_read(int fd, void* buffer, size_t size) { return ::read(fd, buffer, size); } +ssize_t real_write(int fd, const void* buffer, size_t size) { return ::write(fd, buffer, size); } +int real_close(int fd) { return ::close(fd); } +} // namespace #endif namespace { @@ -56,11 +85,7 @@ ssize_t app_io_read(int fd, void* buffer, size_t size) { errno = EBADF; return -1; } -#if defined(TT_APP_IO_WRAPS_STDIO) - return __real_read(fd, buffer, size); -#else - return ::read(fd, buffer, size); -#endif + return real_read(fd, buffer, size); } ssize_t app_io_write(int fd, const void* buffer, size_t size) { @@ -78,22 +103,14 @@ ssize_t app_io_write(int fd, const void* buffer, size_t size) { // output is currently being intercepted. Without this, a log line emitted while any app // instance has its stdout captured would vanish from the console entirely instead of // just also being visible to the capturing parent. -#if defined(TT_APP_IO_WRAPS_STDIO) - __real_write(fd, buffer, size); -#else - ::write(fd, buffer, size); -#endif + real_write(fd, buffer, size); return result; } if (table != nullptr && app_fd_table_is_app_owned(table, fd)) { errno = EBADF; return -1; } -#if defined(TT_APP_IO_WRAPS_STDIO) - return __real_write(fd, buffer, size); -#else - return ::write(fd, buffer, size); -#endif + return real_write(fd, buffer, size); } int app_io_close(int fd) { @@ -108,11 +125,7 @@ int app_io_close(int fd) { return -1; } } -#if defined(TT_APP_IO_WRAPS_STDIO) - return __real_close(fd); -#else - return ::close(fd); -#endif + return real_close(fd); } } // extern "C" diff --git a/Modules/app-module/source/stdio_wrap.cpp b/Modules/app-module/source/stdio_wrap.cpp index 50bf3bf5a..24e926b05 100644 --- a/Modules/app-module/source/stdio_wrap.cpp +++ b/Modules/app-module/source/stdio_wrap.cpp @@ -1,12 +1,43 @@ // SPDX-License-Identifier: Apache-2.0 -// Paired with -Wl,--wrap=read/write/close on POSIX (this module's own CMakeLists.txt) and ESP32 -// (top-level CMakeLists.txt); self-registered via dyld interpose below on Apple, whose linker -// doesn't support --wrap. +// Paired with -Wl,--wrap= on POSIX (this module's own CMakeLists.txt) and ESP32 (top-level +// CMakeLists.txt); self-registered via dyld interpose below on Apple, whose linker doesn't +// support --wrap. #include #include +#ifdef ESP_PLATFORM + +// Newlib's own stdio (fflush()'s buffer-flush path, in particular) calls the reentrant +// _read_r/_write_r/_close_r syscall stubs directly, not the plain read()/write()/close() newlib +// itself provides as thin wrappers around them (see e.g. esp-idf's components/newlib/src/ +// syscalls.c: `write(fd, dst, size) { return _write_r(__getreent(), fd, dst, size); }`) +// Wrapping the plain names only catches direct write()-style callers, not newlib's own internal +// stdio calls, so the _r stubs are wrapped here instead. +#include + +extern "C" { + +ssize_t __wrap__read_r(struct _reent* r, int fd, void* buffer, size_t size) { + (void)r; + return app_io_read(fd, buffer, size); +} + +ssize_t __wrap__write_r(struct _reent* r, int fd, const void* buffer, size_t size) { + (void)r; + return app_io_write(fd, buffer, size); +} + +int __wrap__close_r(struct _reent* r, int fd) { + (void)r; + return app_io_close(fd); +} + +} + +#else + extern "C" { ssize_t __wrap_read(int fd, void* buffer, size_t size) { @@ -23,6 +54,8 @@ int __wrap_close(int fd) { } +#endif // ESP_PLATFORM + #ifdef __APPLE__ // --wrap also synthesizes __real_read/write/close automatically; dyld interpose doesn't, so diff --git a/Modules/app-module/tests/source/execute_test.cpp b/Modules/app-module/tests/source/execute_test.cpp index 11a4734a3..2bbbf32fd 100644 --- a/Modules/app-module/tests/source/execute_test.cpp +++ b/Modules/app-module/tests/source/execute_test.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -110,6 +111,11 @@ int32_t stream_writer_app_main(int, char*[]) { return 7; } +int32_t printf_stream_writer_app_main(int, char*[]) { + printf("loc"); + return 7; +} + // Writes argv[0] to its own stdout, for the app_execute_with_streams() argv-delivery test. int32_t argv_echo_app_main(int argc, char* argv[]) { if (argc < 1) { @@ -321,3 +327,53 @@ TEST_CASE("app_execute_for_result_with_streams delivers both the stream data and app_manager_stop(parent_id); app_manager_remove("test.app.execute.parent_streams"); } + +TEST_CASE("app_execute_for_result_with_streams pipes a child's plain printf() calls too") { + ensure_memory_loader_registered(); + + AppManifest parent_manifest { "test.app.execute.printf_parent", "Parent", APP_CATEGORY_USER, { APP_LOCATION_MEMORY, reinterpret_cast(location_app_main) } }; + REQUIRE_EQ(app_manager_add(&parent_manifest), ERROR_NONE); + + uint32_t parent_id = 0; + REQUIRE_EQ(app_start("test.app.execute.printf_parent", 0, nullptr, &parent_id), ERROR_NONE); + CHECK(wait_for_state(parent_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + TaskEventGroup parent_event_group {}; + task_event_group_construct(&parent_event_group); + AppEventSubscription parent_sub {}; + REQUIRE_EQ(app_event_subscribe_with_app_id(&parent_sub, &parent_event_group, parent_id), ERROR_NONE); + + uint8_t storage[64]; + AppStream child_stdout {}; + AppStreamBinding binding { STDOUT_FILENO, &child_stdout, storage, sizeof(storage), &parent_event_group }; + + AppLocation location { APP_LOCATION_MEMORY, reinterpret_cast(printf_stream_writer_app_main) }; + uint32_t child_id = 0; + REQUIRE_EQ(app_execute_for_result_with_streams(location, AppStackConfig {}, 0, nullptr, &binding, 1, parent_id, &child_id), ERROR_NONE); + + std::vector received; + while (app_stream_await(&child_stdout, APP_FILE_WAIT_READABLE, pdMS_TO_TICKS(1000)) == ERROR_NONE) { + uint8_t chunk[16]; + size_t n = app_stream_read(&child_stdout, chunk, sizeof(chunk)); + if (n == 0) { + break; // EOF + } + received.insert(received.end(), chunk, chunk + n); + } + REQUIRE_EQ(received.size(), 3u); + CHECK_EQ(std::memcmp(received.data(), "loc", 3), 0); + + REQUIRE_EQ(task_event_group_wait(&parent_event_group, parent_sub.bit, false, nullptr, pdMS_TO_TICKS(2000)), ERROR_NONE); + AppEvent event {}; + REQUIRE_EQ(app_event_poll(&parent_sub, &event), ERROR_NONE); + CHECK_EQ(event.type, APP_EVENT_RESULT); + CHECK_EQ(event.result.launch_id, child_id); + CHECK_EQ(event.result.result, 7); + + app_stream_unsubscribe(&child_stdout); + app_event_unsubscribe(&parent_sub); + task_event_group_destruct(&parent_event_group); + app_manager_stop(child_id); + app_manager_stop(parent_id); + app_manager_remove("test.app.execute.printf_parent"); +} diff --git a/Modules/app-module/tests/source/io_test.cpp b/Modules/app-module/tests/source/io_test.cpp index 400bd8d43..1a1287e54 100644 --- a/Modules/app-module/tests/source/io_test.cpp +++ b/Modules/app-module/tests/source/io_test.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -66,6 +67,11 @@ int32_t stdout_writer_app_main(int, char*[]) { return 0; } +int32_t stdout_printf_app_main(int, char*[]) { + printf("hello"); + return 0; +} + std::atomic g_blocked_writer_saw_error { false }; std::atomic g_blocked_writer_done { false }; @@ -180,6 +186,41 @@ TEST_CASE("app_start_with_streams pipes a child's app_io_write() calls into a pa app_manager_remove("test.io.writer"); } +TEST_CASE("app_start_with_streams pipes a child's plain printf() calls into a parent-owned AppStream") { + ensure_memory_loader_registered(); + + AppManifest manifest { "test.io.printf_writer", "PrintfWriter", APP_CATEGORY_USER, { APP_LOCATION_MEMORY, reinterpret_cast(stdout_printf_app_main) } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + TaskEventGroup event_group {}; + task_event_group_construct(&event_group); + + uint8_t storage[64]; + AppStream child_stdout {}; + + AppStreamBinding binding { STDOUT_FILENO, &child_stdout, storage, sizeof(storage), &event_group }; + AppInstanceId child_id = 0; + REQUIRE_EQ(app_start_with_streams("test.io.printf_writer", &binding, 1, &child_id), ERROR_NONE); + + std::vector received; + while (app_stream_await(&child_stdout, APP_FILE_WAIT_READABLE, pdMS_TO_TICKS(1000)) == ERROR_NONE) { + uint8_t chunk[16]; + size_t n = app_stream_read(&child_stdout, chunk, sizeof(chunk)); + if (n == 0) { + break; // EOF + } + received.insert(received.end(), chunk, chunk + n); + } + + REQUIRE_EQ(received.size(), 5u); + CHECK_EQ(std::memcmp(received.data(), "hello", 5), 0); + + REQUIRE(wait_for_state(child_id, APP_INSTANCE_STATE_STOPPED, 1000)); + app_stream_unsubscribe(&child_stdout); + task_event_group_destruct(&event_group); + app_manager_remove("test.io.printf_writer"); +} + TEST_CASE("a write blocked on a full stream wakes with an error once the consumer closes it") { ensure_memory_loader_registered(); g_blocked_writer_saw_error.store(false, std::memory_order_relaxed);