diff --git a/.github/actions/build-simulator/action.yml b/.github/actions/build-simulator/action.yml index 80eeb5ae..576d5f02 100644 --- a/.github/actions/build-simulator/action.yml +++ b/.github/actions/build-simulator/action.yml @@ -4,11 +4,8 @@ inputs: os_name: description: A descriptive name for the operating system (e.g. linux, windows) required: true - platform_name: - description: A descriptive name for the target platform (e.g. amd64, aarch64, etc.) - required: true - publish: - description: A boolean that enables publishing of artifacts + architecture: + description: A descriptive name for the target architecture (e.g. x86_64, aarch64, etc.) required: true runs: @@ -48,11 +45,10 @@ runs: run: cmake --build buildsim --target Tactility - name: 'Release' shell: bash - run: Buildscripts/release-simulator.sh buildsim release/Simulator-${{ inputs.os_name }}-${{ inputs.platform_name }} + run: Buildscripts/release-simulator.sh buildsim release/Simulator-${{ inputs.os_name }}-${{ inputs.architecture }} - name: 'Upload Artifact' uses: actions/upload-artifact@v4 - if: ${{ inputs.publish == 'true' }} with: - name: Simulator-${{ inputs.os_name }}-${{ inputs.platform_name }} - path: release/Simulator-${{ inputs.os_name }}-${{ inputs.platform_name }} + name: Simulator-${{ inputs.os_name }}-${{ inputs.architecture }} + path: release/Simulator-${{ inputs.os_name }}-${{ inputs.architecture }} retention-days: 30 diff --git a/.github/workflows/build-simulator.yml b/.github/workflows/build-simulator.yml index 6be91df2..1f0261ab 100644 --- a/.github/workflows/build-simulator.yml +++ b/.github/workflows/build-simulator.yml @@ -17,8 +17,7 @@ jobs: uses: ./.github/actions/build-simulator with: os_name: linux - platform_name: amd64 - publish: true + architecture: x86_64 macOS: runs-on: macos-latest steps: @@ -29,6 +28,4 @@ jobs: uses: ./.github/actions/build-simulator with: os_name: macos - platform_name: aarch64 - # macOS simulator currently fails due to main thread requirement for rendering - publish: false + architecture: aarch64 diff --git a/CMakeLists.txt b/CMakeLists.txt index 1f29fc87..9f123385 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/Devices/simulator/Source/Main.cpp b/Devices/simulator/Source/Main.cpp index 85e834ea..2b9d5dd1 100644 --- a/Devices/simulator/Source/Main.cpp +++ b/Devices/simulator/Source/Main.cpp @@ -10,13 +10,13 @@ constexpr auto* TAG = "FreeRTOS"; namespace simulator { -MainFunction mainFunction = nullptr; +static MainFunction mainFunction = nullptr; void setMain(MainFunction newMainFunction) { mainFunction = newMainFunction; } -static void freertosMainTask(void* parameter) { +static void freertosMainTask(void*) { LOG_I(TAG, "starting app_main()"); assert(simulator::mainFunction); mainFunction(); diff --git a/Devices/simulator/Source/Simulator.h b/Devices/simulator/Source/Simulator.h index 34da7869..d1e900c5 100644 --- a/Devices/simulator/Source/Simulator.h +++ b/Devices/simulator/Source/Simulator.h @@ -1,6 +1,11 @@ #pragma once #include "Main.h" +#include "drivers/sdl_bridge.h" + +#include +#include +#include namespace simulator { /** Set the function pointer of the real app_main() */ @@ -14,8 +19,23 @@ void app_main(); // ESP-IDF's main function, implemented in the application } int main() { - // Actual main function that passes on app_main() (to be executed in a FreeRTOS task) and bootstraps FreeRTOS + // The FreeRTOS POSIX port arms a process-wide SIGALRM timer for its tick and expects every one + // of its task pthreads to have all signals but SIGINT blocked. + // (see prvSetupSignalsAndSchedulerPolicy() in FreeRTOS-Kernel's Posix port.c) + // A signal-generated SIGALRM can land on any thread in the process that doesn't block it. + // This thread stays a plain OS thread (running the SDL loop below, never a FreeRTOS task), + // so without this it's eligible to catch a tick SIGALRM and freeze inside the scheduler's handler. + // Block the same set here, before anything else, so it never can. + sigset_t all_signals_except_sigint; + sigfillset(&all_signals_except_sigint); + sigdelset(&all_signals_except_sigint, SIGINT); + pthread_sigmask(SIG_SETMASK, &all_signals_except_sigint, nullptr); + + // FreeRTOS and app_main() run on a separate thread: macOS requires SDL/Cocoa window creation, + // event pumping and rendering to happen on the real OS main thread, which sdl_bridge_run_main_loop() + // below takes over. freertosMain() never returns, so this thread is detached rather than joined. simulator::setMain(app_main); - simulator::freertosMain(); + std::thread(simulator::freertosMain).detach(); + sdl_bridge_run_main_loop(); return 0; } diff --git a/Devices/simulator/Source/drivers/sdl_bridge.cpp b/Devices/simulator/Source/drivers/sdl_bridge.cpp new file mode 100644 index 00000000..ce4a38dd --- /dev/null +++ b/Devices/simulator/Source/drivers/sdl_bridge.cpp @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: Apache-2.0 +#include "sdl_bridge.h" +#include "sdl_input.h" + +#include + +#include +#include +#include + +namespace { + +struct PresentJob { + Device* device; + void* internal; + int32_t x_start; + int32_t y_start; + int32_t x_end; + int32_t y_end; + const void* color_data; + error_t result; +}; + +std::mutex job_mutex; +std::condition_variable job_ready_cv; +std::condition_variable job_done_cv; +bool job_pending = false; +bool job_done = false; +PresentJob pending_job; + +} + +error_t sdl_bridge_present(Device* device, void* internal, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data) { + std::unique_lock lock(job_mutex); + + pending_job = { device, internal, x_start, y_start, x_end, y_end, color_data, ERROR_NONE }; + job_pending = true; + job_done = false; + job_ready_cv.notify_one(); + + job_done_cv.wait(lock, [] { return job_done; }); + return pending_job.result; +} + +void sdl_bridge_run_main_loop() { + while (true) { + sdl_input_pump(); + + std::unique_lock lock(job_mutex); + if (job_ready_cv.wait_for(lock, std::chrono::milliseconds(1), [] { return job_pending; })) { + PresentJob job = pending_job; + lock.unlock(); + + job.result = sdl_display_execute_draw_bitmap(job.device, job.internal, job.x_start, job.y_start, job.x_end, job.y_end, job.color_data); + + lock.lock(); + pending_job.result = job.result; + job_pending = false; + job_done = true; + lock.unlock(); + job_done_cv.notify_one(); + } + } +} diff --git a/Devices/simulator/Source/drivers/sdl_bridge.h b/Devices/simulator/Source/drivers/sdl_bridge.h new file mode 100644 index 00000000..14b65d8f --- /dev/null +++ b/Devices/simulator/Source/drivers/sdl_bridge.h @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include + +#include + +struct Device; + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Runs forever, pumping SDL input and executing display present jobs submitted via + * sdl_bridge_present(). Must be called exactly once, from the real OS main thread: macOS requires + * SDL/Cocoa window creation, event pumping and rendering to happen there, but FreeRTOS tasks + * (including the lvgl task that owns display flush and indev polling) run on separate pthreads + * spawned by the FreeRTOS POSIX port, not on that thread. + */ +void sdl_bridge_run_main_loop(void); + +/** + * @brief Hands a display flush off to the main thread and blocks until it has finished copying + * the pixel data out (see sdl_display_execute_draw_bitmap() in sdl_display.cpp). Called from the + * lvgl task. Must block: the caller's pixel buffer is single-buffered and gets reused as soon as + * this returns. + */ +error_t sdl_bridge_present(struct Device* device, void* internal, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data); + +/** + * @brief Implemented in sdl_display.cpp: the actual SDL work behind a display flush (lazy window + * init on first call, SDL_UpdateTexture, present). Only ever called from sdl_bridge_run_main_loop() + * on the main thread. + */ +error_t sdl_display_execute_draw_bitmap(struct Device* device, void* internal, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data); + +#ifdef __cplusplus +} +#endif diff --git a/Devices/simulator/Source/drivers/sdl_display.cpp b/Devices/simulator/Source/drivers/sdl_display.cpp index ebe4c5e1..5d9501ac 100644 --- a/Devices/simulator/Source/drivers/sdl_display.cpp +++ b/Devices/simulator/Source/drivers/sdl_display.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "sdl_display.h" +#include "sdl_bridge.h" #include #include @@ -141,8 +142,10 @@ static bool sdl_display_lazy_init(Device* device, SdlDisplayInternal* internal) return true; } -static error_t sdl_display_draw_bitmap(Device* device, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data) { - auto* internal = static_cast(device_get_driver_data(device)); +// Only ever called from sdl_bridge_run_main_loop() on the real main thread - required for +// SDL/Cocoa window creation and rendering on macOS. +error_t sdl_display_execute_draw_bitmap(Device* device, void* internal_ptr, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data) { + auto* internal = static_cast(internal_ptr); if (internal->init_failed) { return ERROR_RESOURCE; @@ -166,6 +169,16 @@ static error_t sdl_display_draw_bitmap(Device* device, int32_t x_start, int32_t return ERROR_NONE; } +static error_t sdl_display_draw_bitmap(Device* device, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data) { + auto* internal = static_cast(device_get_driver_data(device)); + + if (internal->init_failed) { + return ERROR_RESOURCE; + } + + return sdl_bridge_present(device, internal, x_start, y_start, x_end, y_end, color_data); +} + static enum DisplayColorFormat sdl_display_get_color_format(Device*) { return DISPLAY_COLOR_FORMAT_RGB565; } diff --git a/Devices/simulator/Source/drivers/sdl_input.cpp b/Devices/simulator/Source/drivers/sdl_input.cpp index abe45809..9e287c57 100644 --- a/Devices/simulator/Source/drivers/sdl_input.cpp +++ b/Devices/simulator/Source/drivers/sdl_input.cpp @@ -7,11 +7,16 @@ #include #include +#include namespace { constexpr size_t KEY_QUEUE_CAPACITY = 32; +// Written by sdl_input_pump() on the real main thread, read by sdl_input_get_pointer_state()/ +// sdl_input_pop_key()/sdl_input_has_queued_key() on the lvgl task. +std::mutex state_mutex; + SdlPointerState pointer_state = { 0, 0, false }; } // namespace @@ -26,6 +31,7 @@ uint32_t touch_override_until_tick = 0; #define SIM_TOUCH_HOLD_MS 1500 extern "C" void sdl_input_set_touch_override(int32_t x, int32_t y, bool pressed) { + std::lock_guard lock(state_mutex); touch_override.x = x; touch_override.y = y; touch_override.pressed = pressed; @@ -36,6 +42,7 @@ extern "C" void sdl_input_set_touch_override(int32_t x, int32_t y, bool pressed) } extern "C" void sdl_input_clear_touch_override(void) { + std::lock_guard lock(state_mutex); touch_override_active = false; touch_override.pressed = false; } @@ -101,58 +108,73 @@ uint32_t keycode_to_key(SDL_Keycode sdl_key, bool shift) { } // namespace void sdl_input_pump() { - if (!text_input_started) { - SDL_StartTextInput(); - text_input_started = true; + // exit() must run with state_mutex unlocked: it never returns, so a lock_guard held across it + // would never release the mutex, hanging any other thread that later calls into this file's + // other functions (all of which lock state_mutex) while exit() tears the process down. + bool quit_requested = false; + + { + std::lock_guard lock(state_mutex); + + if (!text_input_started) { + SDL_StartTextInput(); + text_input_started = true; + } + + SDL_Event event; + while (SDL_PollEvent(&event)) { + switch (event.type) { + case SDL_MOUSEMOTION: + set_pointer_position(event.motion.x, event.motion.y); + break; + case SDL_MOUSEBUTTONDOWN: + if (event.button.button == SDL_BUTTON_LEFT) { + // event.button.x/y can be stale immediately after a window resize (an + // SDL/X11 event-queue quirk - confirmed by comparing against a live + // SDL_GetWindowSize() at the same instant). SDL_GetMouseState() queries the + // OS for the current pointer position directly, sidestepping that entirely. + int live_x, live_y; + SDL_GetMouseState(&live_x, &live_y); + set_pointer_position(live_x, live_y); + pointer_state.pressed = true; + } + break; + case SDL_MOUSEBUTTONUP: + if (event.button.button == SDL_BUTTON_LEFT) { + pointer_state.pressed = false; + } + break; + case SDL_KEYDOWN: + push_key(keycode_to_key(event.key.keysym.sym, (event.key.keysym.mod & KMOD_SHIFT) != 0)); + break; + case SDL_TEXTINPUT: + // ASCII only (first byte of event.text.text) - sufficient for a simulator keyboard. + push_key(static_cast(event.text.text[0])); + break; + case SDL_WINDOWEVENT: + // Resizing doesn't change what LVGL last rendered, only how large it should + // appear - re-present the existing frame at the new scale immediately, rather + // than leaving stale-looking content on screen until the next LVGL-driven flush. + if (event.window.event == SDL_WINDOWEVENT_SIZE_CHANGED) { + sdl_display_present_now(); + } + break; + case SDL_QUIT: + quit_requested = true; + break; + default: + break; + } + } } - SDL_Event event; - while (SDL_PollEvent(&event)) { - switch (event.type) { - case SDL_MOUSEMOTION: - set_pointer_position(event.motion.x, event.motion.y); - break; - case SDL_MOUSEBUTTONDOWN: - if (event.button.button == SDL_BUTTON_LEFT) { - // event.button.x/y can be stale immediately after a window resize (an - // SDL/X11 event-queue quirk - confirmed by comparing against a live - // SDL_GetWindowSize() at the same instant). SDL_GetMouseState() queries the - // OS for the current pointer position directly, sidestepping that entirely. - int live_x, live_y; - SDL_GetMouseState(&live_x, &live_y); - set_pointer_position(live_x, live_y); - pointer_state.pressed = true; - } - break; - case SDL_MOUSEBUTTONUP: - if (event.button.button == SDL_BUTTON_LEFT) { - pointer_state.pressed = false; - } - break; - case SDL_KEYDOWN: - push_key(keycode_to_key(event.key.keysym.sym, (event.key.keysym.mod & KMOD_SHIFT) != 0)); - break; - case SDL_TEXTINPUT: - // ASCII only (first byte of event.text.text) - sufficient for a simulator keyboard. - push_key(static_cast(event.text.text[0])); - break; - case SDL_WINDOWEVENT: - // Resizing doesn't change what LVGL last rendered, only how large it should - // appear - re-present the existing frame at the new scale immediately, rather - // than leaving stale-looking content on screen until the next LVGL-driven flush. - if (event.window.event == SDL_WINDOWEVENT_SIZE_CHANGED) { - sdl_display_present_now(); - } - break; - case SDL_QUIT: - exit(0); - default: - break; - } + if (quit_requested) { + exit(0); } } void sdl_input_get_pointer_state(SdlPointerState* out_state) { + std::lock_guard lock(state_mutex); if (touch_override_active) { // Auto-release: viewer sends press only; LVGL needs press then release // to register a click. Hold long enough for several indev polls. @@ -168,6 +190,7 @@ void sdl_input_get_pointer_state(SdlPointerState* out_state) { } bool sdl_input_pop_key(uint32_t* out_key) { + std::lock_guard lock(state_mutex); if (key_queue_count == 0) { return false; } @@ -178,5 +201,6 @@ bool sdl_input_pop_key(uint32_t* out_key) { } bool sdl_input_has_queued_key() { + std::lock_guard lock(state_mutex); return key_queue_count > 0; } diff --git a/Devices/simulator/Source/drivers/sdl_input.h b/Devices/simulator/Source/drivers/sdl_input.h index d7bee752..00ac3006 100644 --- a/Devices/simulator/Source/drivers/sdl_input.h +++ b/Devices/simulator/Source/drivers/sdl_input.h @@ -19,9 +19,9 @@ struct SdlPointerState { /** * @brief Drains all pending SDL events exactly once, updating the pointer state and key queue - * below. Safe to call from both the sdl-pointer and sdl-keyboard drivers' polling functions: - * SDL_PollEvent() drains a single global queue, so whichever driver is polled first on a given - * LVGL indev tick pumps events for both. + * below. Must be called only from the real OS main thread (sdl_bridge_run_main_loop()): SDL + * requires event pumping to happen there on macOS. The getters below are safe to call from a + * different thread (the lvgl task, via sdl-pointer/sdl-keyboard's polling functions). */ void sdl_input_pump(void); diff --git a/Devices/simulator/Source/drivers/sdl_keyboard.cpp b/Devices/simulator/Source/drivers/sdl_keyboard.cpp index 09be781c..82e1bbef 100644 --- a/Devices/simulator/Source/drivers/sdl_keyboard.cpp +++ b/Devices/simulator/Source/drivers/sdl_keyboard.cpp @@ -16,8 +16,6 @@ static error_t stop(Device*) { return ERROR_NONE; } // region KeyboardApi static error_t sdl_keyboard_read_key(Device*, KeyboardKeyData* data) { - sdl_input_pump(); - uint32_t key = 0; if (sdl_input_pop_key(&key)) { data->key = key; diff --git a/Devices/simulator/Source/drivers/sdl_pointer.cpp b/Devices/simulator/Source/drivers/sdl_pointer.cpp index 33b48bc1..0ca628c0 100644 --- a/Devices/simulator/Source/drivers/sdl_pointer.cpp +++ b/Devices/simulator/Source/drivers/sdl_pointer.cpp @@ -16,7 +16,6 @@ static error_t stop(Device*) { return ERROR_NONE; } // region PointerApi static error_t sdl_pointer_read_data(Device*, TickType_t) { - sdl_input_pump(); return ERROR_NONE; } diff --git a/Modules/app-module/CMakeLists.txt b/Modules/app-module/CMakeLists.txt index 442d5811..d3b42d1e 100644 --- a/Modules/app-module/CMakeLists.txt +++ b/Modules/app-module/CMakeLists.txt @@ -10,15 +10,29 @@ 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 than calling ::read/::write/::close() directly, on every platform whose build wraps -# those symbols (ESP-IDF always; POSIX except macOS, whose linker doesn't support --wrap) - -# see Tactility/CMakeLists.txt and the top-level CMakeLists.txt for where that's applied. -if (NOT APPLE) +# Tells source/io.cpp its real-syscall fallback must go through __real_read/write/close() rather +# than calling ::read/::write/::close() directly, on every platform where those are wrapped: via +# -Wl,--wrap= on ESP-IDF (see the top-level CMakeLists.txt) and non-Apple POSIX (below), or via +# source/stdio_wrap.cpp's dyld interpose on Apple (whose linker lacks --wrap). +tactility_get_module_name(app-module MODULE_NAME) +target_compile_definitions(${MODULE_NAME} PRIVATE TT_APP_IO_WRAPS_STDIO) + +# Routes every read()/write()/close() and printf-family call to source/stdio_wrap.cpp's __wrap_* +# functions; see that file for what they do and why. ESP-IDF wraps the same symbols itself (see +# the top-level CMakeLists.txt); Apple gets the same routing via dyld interpose instead, self- +# registered in that file. PUBLIC so every consumer of this module (Tactility, and any test target +# that links app-module directly) gets the flags too. +if (NOT ESP_PLATFORM AND NOT APPLE) tactility_get_module_name(app-module MODULE_NAME) - target_compile_definitions(${MODULE_NAME} PRIVATE TT_APP_IO_WRAPS_STDIO) + target_link_options(${MODULE_NAME} PUBLIC + "-Wl,--wrap=read" "-Wl,--wrap=write" "-Wl,--wrap=close" + "-Wl,--wrap=printf" "-Wl,--wrap=fprintf" "-Wl,--wrap=vprintf" "-Wl,--wrap=vfprintf" + "-Wl,--wrap=puts" "-Wl,--wrap=fputs" "-Wl,--wrap=putchar" "-Wl,--wrap=fputc" + "-Wl,--wrap=getchar" "-Wl,--wrap=fgetc" "-Wl,--wrap=fgets" + ) endif () # install.cpp resolves an installed binary's fixed bin//.so path itself, so it diff --git a/Modules/app-module/source/io.cpp b/Modules/app-module/source/io.cpp index 0d7570cf..4830f6a7 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 new file mode 100644 index 00000000..24e926b0 --- /dev/null +++ b/Modules/app-module/source/stdio_wrap.cpp @@ -0,0 +1,327 @@ +// SPDX-License-Identifier: Apache-2.0 + +// 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) { + return app_io_read(fd, buffer, size); +} + +ssize_t __wrap_write(int fd, const void* buffer, size_t size) { + return app_io_write(fd, buffer, size); +} + +int __wrap_close(int fd) { + return app_io_close(fd); +} + +} + +#endif // ESP_PLATFORM + +#ifdef __APPLE__ + +// --wrap also synthesizes __real_read/write/close automatically; dyld interpose doesn't, so +// io.cpp's TT_APP_IO_WRAPS_STDIO fallback needs them defined here. dlsym(RTLD_NEXT, ...) is the +// standard way to reach the true libSystem implementation despite the interpose below: a direct +// call to read/write/close from this file would just recurse into __wrap_read/write/close, since +// interpose rewrites every reference to those symbols in the process, this file included. +#include +#include + +extern "C" { + +ssize_t __real_read(int fd, void* buffer, size_t size) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "read")); + return real(fd, buffer, size); +} + +ssize_t __real_write(int fd, const void* buffer, size_t size) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "write")); + return real(fd, buffer, size); +} + +int __real_close(int fd) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "close")); + return real(fd); +} + +} + +// isn't a public SDK header (it ships with dyld's own source, not +// Xcode/Command Line Tools), so this reimplements its DYLD_INTERPOSE macro locally; reused below +// for the printf-family interposes too. +#define TT_DYLD_INTERPOSE(replacement, replacee) \ + __attribute__((used)) static struct { const void* replacement; const void* replacee; } \ + tt_interpose_##replacee __attribute__((section("__DATA,__interpose"))) = { \ + (const void*)(unsigned long)&(replacement), (const void*)(unsigned long)&(replacee) \ + }; + +TT_DYLD_INTERPOSE(__wrap_read, read) +TT_DYLD_INTERPOSE(__wrap_write, write) +TT_DYLD_INTERPOSE(__wrap_close, close) + +#endif // __APPLE__ + +// region glibc stdio wraps +// +// libc's printf/fprintf/etc are compiled into the C library and call an internal, non-exported +// write() alias, which --wrap=write/the read/write/close interpose above can't reach: only calls +// WE make to the public symbol. These wraps instead redirect calls WE make to printf/fprintf/etc, +// the same trick as read/write/close above. Newlib (ESP-IDF) doesn't have this gap: its stdio does +// call the wrappable syscall stubs, so this block is POSIX-only. +// +// Scoped to the printf/getc families only: fread/fwrite take an arbitrary FILE* and are already +// used sitewide for real file I/O (e.g. File.cpp's readBinaryInternal), so wrapping them would +// route every such call through this file's stdin/stdout check, a correctness risk for unrelated +// code that isn't worth taking here. putc/getc are excluded too since libc defines them as +// macros, not real calls, so wrapping those symbols wouldn't reliably intercept them. + +#if !defined(ESP_PLATFORM) + +#include +#include +#include +#include +#include + +extern "C" { +int __real_vfprintf(FILE* stream, const char* format, va_list args); +int __real_fputs(const char* s, FILE* stream); +int __real_fputc(int c, FILE* stream); +int __real_fgetc(FILE* stream); +char* __real_fgets(char* buffer, int size, FILE* stream); +} + +#ifdef __APPLE__ + +// --wrap synthesizes these automatically elsewhere; on Apple they're defined here via +// dlsym(RTLD_NEXT, ...) instead. See the read/write/close __real_* block above for why. +#include + +extern "C" { + +int __real_vfprintf(FILE* stream, const char* format, va_list args) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "vfprintf")); + return real(stream, format, args); +} + +int __real_fputs(const char* s, FILE* stream) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fputs")); + return real(s, stream); +} + +int __real_fputc(int c, FILE* stream) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fputc")); + return real(c, stream); +} + +int __real_fgetc(FILE* stream) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fgetc")); + return real(stream); +} + +char* __real_fgets(char* buffer, int size, FILE* stream) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fgets")); + return real(buffer, size, stream); +} + +} + +#endif // __APPLE__ + +namespace { + +void writeAllToStdout(const void* data, size_t size) { + const auto* bytes = static_cast(data); + size_t remaining = size; + while (remaining > 0) { + ssize_t written = app_io_write(STDOUT_FILENO, bytes, remaining); + if (written <= 0) { + break; + } + bytes += written; + remaining -= static_cast(written); + } +} + +// Formats into stdout via app_io_write() rather than through a FILE*'s own buffering, since that +// buffering is exactly what glibc's internal write() call sidesteps --wrap for in the first place. +int formatToStdout(const char* format, va_list args) { + char stackBuffer[256]; + va_list argsForStack; + va_copy(argsForStack, args); + int needed = vsnprintf(stackBuffer, sizeof(stackBuffer), format, argsForStack); + va_end(argsForStack); + if (needed < 0) { + return needed; + } + if (static_cast(needed) < sizeof(stackBuffer)) { + writeAllToStdout(stackBuffer, static_cast(needed)); + return needed; + } + auto heapBuffer = std::make_unique(static_cast(needed) + 1); + va_list argsForHeap; + va_copy(argsForHeap, args); + vsnprintf(heapBuffer.get(), static_cast(needed) + 1, format, argsForHeap); + va_end(argsForHeap); + writeAllToStdout(heapBuffer.get(), static_cast(needed)); + return needed; +} + +int readOneFromStdin(char& out) { + return static_cast(app_io_read(STDIN_FILENO, &out, 1)); +} + +} // namespace + +extern "C" { + +int __wrap_vprintf(const char* format, va_list args) { + return formatToStdout(format, args); +} + +int __wrap_printf(const char* format, ...) { + va_list args; + va_start(args, format); + int result = formatToStdout(format, args); + va_end(args); + return result; +} + +int __wrap_vfprintf(FILE* stream, const char* format, va_list args) { + if (stream == stdout) { + return formatToStdout(format, args); + } + return __real_vfprintf(stream, format, args); +} + +int __wrap_fprintf(FILE* stream, const char* format, ...) { + va_list args; + va_start(args, format); + int result = (stream == stdout) ? formatToStdout(format, args) : __real_vfprintf(stream, format, args); + va_end(args); + return result; +} + +int __wrap_puts(const char* s) { + writeAllToStdout(s, strlen(s)); + writeAllToStdout("\n", 1); + return 0; +} + +int __wrap_fputs(const char* s, FILE* stream) { + if (stream == stdout) { + writeAllToStdout(s, strlen(s)); + return 0; + } + return __real_fputs(s, stream); +} + +int __wrap_putchar(int c) { + auto ch = static_cast(c); + writeAllToStdout(&ch, 1); + return c; +} + +int __wrap_fputc(int c, FILE* stream) { + if (stream == stdout) { + return __wrap_putchar(c); + } + return __real_fputc(c, stream); +} + +int __wrap_getchar() { + char c; + return readOneFromStdin(c) == 1 ? static_cast(c) : EOF; +} + +int __wrap_fgetc(FILE* stream) { + if (stream == stdin) { + return __wrap_getchar(); + } + return __real_fgetc(stream); +} + +char* __wrap_fgets(char* buffer, int size, FILE* stream) { + if (stream != stdin) { + return __real_fgets(buffer, size, stream); + } + if (size <= 0) { + return nullptr; + } + int i = 0; + for (; i < size - 1; ++i) { + char c; + if (readOneFromStdin(c) != 1) { + break; + } + buffer[i] = c; + if (c == '\n') { + ++i; + break; + } + } + if (i == 0) { + return nullptr; + } + buffer[i] = '\0'; + return buffer; +} + +} + +#ifdef __APPLE__ +TT_DYLD_INTERPOSE(__wrap_vprintf, vprintf) +TT_DYLD_INTERPOSE(__wrap_printf, printf) +TT_DYLD_INTERPOSE(__wrap_vfprintf, vfprintf) +TT_DYLD_INTERPOSE(__wrap_fprintf, fprintf) +TT_DYLD_INTERPOSE(__wrap_puts, puts) +TT_DYLD_INTERPOSE(__wrap_fputs, fputs) +TT_DYLD_INTERPOSE(__wrap_putchar, putchar) +TT_DYLD_INTERPOSE(__wrap_fputc, fputc) +TT_DYLD_INTERPOSE(__wrap_getchar, getchar) +TT_DYLD_INTERPOSE(__wrap_fgetc, fgetc) +TT_DYLD_INTERPOSE(__wrap_fgets, fgets) +#endif // __APPLE__ + +#endif // !ESP_PLATFORM + +// endregion diff --git a/Modules/app-module/tests/CMakeLists.txt b/Modules/app-module/tests/CMakeLists.txt index d10cf759..f445ec84 100644 --- a/Modules/app-module/tests/CMakeLists.txt +++ b/Modules/app-module/tests/CMakeLists.txt @@ -6,30 +6,12 @@ enable_language(C CXX ASM) file(GLOB_RECURSE TEST_SOURCES CONFIGURE_DEPENDS ${PROJECT_SOURCE_DIR}/source/*.cpp) add_executable(AppModuleTests EXCLUDE_FROM_ALL ${TEST_SOURCES}) -if (NOT APPLE) - # Provides __wrap_read/write/close for the -Wl,--wrap= below (see Tactility/CMakeLists.txt - # for the canonical pairing of this file with those flags). - target_sources(AppModuleTests PRIVATE ${CMAKE_CURRENT_LIST_DIR}/../../../Tactility/Source/AppStdioWrap.cpp) -endif () - target_include_directories(AppModuleTests PRIVATE ${DOCTESTINC} ${CMAKE_CURRENT_LIST_DIR}/../private) add_test(NAME AppModuleTests COMMAND AppModuleTests) -# Matches app-module's own TT_APP_IO_WRAPS_STDIO (see Modules/app-module/CMakeLists.txt): -# io.cpp calls __real_read/write/close() on non-Apple POSIX, so any final link of it needs -# these wraps too, or those symbols go unresolved. The printf-family/getc-family flags are -# needed for the same reason: AppStdioWrap.cpp compiles those __wrap_* functions unconditionally -# on this platform (see its own #if guard), and they reference __real_vfprintf/fputs/fputc/ -# fgetc/fgets - those only exist once the matching --wrap flag is passed. -if (NOT APPLE) - target_link_options(AppModuleTests PRIVATE - "-Wl,--wrap=read" "-Wl,--wrap=write" "-Wl,--wrap=close" - "-Wl,--wrap=printf" "-Wl,--wrap=fprintf" "-Wl,--wrap=vprintf" "-Wl,--wrap=vfprintf" - "-Wl,--wrap=puts" "-Wl,--wrap=fputs" "-Wl,--wrap=putchar" "-Wl,--wrap=fputc" - "-Wl,--wrap=getchar" "-Wl,--wrap=fgetc" "-Wl,--wrap=fgets" - ) -endif () +# No --wrap flags or stdio_wrap.cpp source needed here: linking app-module below already brings +# both along (see its own CMakeLists.txt). target_link_libraries(AppModuleTests PUBLIC TactilityKernel diff --git a/Modules/app-module/tests/source/execute_test.cpp b/Modules/app-module/tests/source/execute_test.cpp index 11a4734a..2bbbf32f 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 400bd8d4..1a1287e5 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); diff --git a/Platforms/platform-posix/CMakeLists.txt b/Platforms/platform-posix/CMakeLists.txt index 35f3136e..4f6c929b 100644 --- a/Platforms/platform-posix/CMakeLists.txt +++ b/Platforms/platform-posix/CMakeLists.txt @@ -6,3 +6,12 @@ add_library(platform-posix OBJECT) target_sources(platform-posix PRIVATE ${SOURCES}) #target_include_directories(platform-posix PUBLIC include/) target_link_libraries(platform-posix PUBLIC TactilityKernel) + +# Routes every pthread_attr_setstack() call to __wrap_pthread_attr_setstack() in source/pthread_stack_wrap.c, +# which no-ops it. see that file for why. +# --wrap is a GNU ld option; Apple's linker doesn't support it, so that file instead self-registers a dyld interpose +# for this symbol on Apple, needing no link flag. PUBLIC so every consumer of this OBJECT library +# (Tactility, and every test target that links platform-posix directly) gets it applied too. +if (NOT APPLE) + target_link_options(platform-posix PUBLIC "-Wl,--wrap=pthread_attr_setstack") +endif () diff --git a/Platforms/platform-posix/source/pthread_stack_wrap.c b/Platforms/platform-posix/source/pthread_stack_wrap.c index 01ff6ca8..ce277b05 100644 --- a/Platforms/platform-posix/source/pthread_stack_wrap.c +++ b/Platforms/platform-posix/source/pthread_stack_wrap.c @@ -2,20 +2,15 @@ #include /** - * Linked in with -Wl,--wrap=pthread_attr_setstack (see Tactility/CMakeLists.txt). + * FreeRTOS's POSIX port hands every task a stack carved from its own heap (pvPortMalloc) via this call. + * pthread_attr_setstack requires page alignment that pvPortMalloc doesn't guarantee. + * When it succeeds anyway, the task's real pthread stack ends up living inside that small FreeRTOS heap region, + * where a thread doing heavier stack work (e.g. Mesa GL shader compilation) can silently corrupt adjacent heap_4 objects. + * No-op'ing the call instead leaves every task's pthread_attr_t at its pthread_attr_init() default, + * so pthread_create() gives it a real, properly sized stack. * - * FreeRTOS's POSIX port (Libraries/FreeRTOS-Kernel/portable/ThirdParty/GCC/Posix/port.c) - * hands every task a stack carved out of its own heap (pvPortMalloc) via this call. - * pthread_attr_setstack requires page alignment, which pvPortMalloc doesn't guarantee; - * when it happens to succeed anyway (allocator alignment can vary run to run), the - * task's real pthread stack ends up living inside that small FreeRTOS heap region - - * fine for typical embedded task code, but a desktop GL driver doing on-the-fly shader - * compilation on that thread (e.g. Mesa on first frame present) can overflow it and - * silently corrupt adjacent heap_4 objects. - * - * Wrapping the call out entirely (rather than patching the vendored port.c) leaves every - * task's pthread_attr_t at its pthread_attr_init() default, so pthread_create() always - * gives it a real, properly allocated default-size stack instead. + * Linked via -Wl,--wrap=pthread_attr_setstack (this module's own CMakeLists.txt) on non-Apple + * platforms; reused as a dyld interpose target below on Apple platforms, whose linker lacks --wrap. */ int __wrap_pthread_attr_setstack(pthread_attr_t* attr, void* stackaddr, size_t stacksize) { (void)attr; @@ -23,3 +18,17 @@ int __wrap_pthread_attr_setstack(pthread_attr_t* attr, void* stackaddr, size_t s (void)stacksize; return 0; } + +#ifdef __APPLE__ + +// isn't a public SDK header (it ships with dyld's own source, not +// Xcode/Command Line Tools), so this reimplements its DYLD_INTERPOSE macro locally. +#define TT_DYLD_INTERPOSE(replacement, replacee) \ + __attribute__((used)) static struct { const void* replacement; const void* replacee; } \ + tt_interpose_##replacee __attribute__((section("__DATA,__interpose"))) = { \ + (const void*)(unsigned long)&(replacement), (const void*)(unsigned long)&(replacee) \ + }; + +TT_DYLD_INTERPOSE(__wrap_pthread_attr_setstack, pthread_attr_setstack) + +#endif diff --git a/Tactility/CMakeLists.txt b/Tactility/CMakeLists.txt index ad3b563f..56bcc830 100644 --- a/Tactility/CMakeLists.txt +++ b/Tactility/CMakeLists.txt @@ -185,31 +185,9 @@ else () # Exports Tactility's own symbols (-rdynamic) so a dlopen()ed app-posix-module app can resolve # calls back into it - the OS-native equivalent of app-esp32-module's custom symbol resolver. set_target_properties(Tactility PROPERTIES ENABLE_EXPORTS ON) - # Routes every pthread_attr_setstack() call (FreeRTOS's POSIX port hands each task a stack - # carved out of its own heap through this) to __wrap_pthread_attr_setstack() in - # Platforms/platform-posix/source/pthread_stack_wrap.c, which no-ops it - see that file for why. - # --wrap is a GNU ld option; Apple's linker doesn't support it. - if (NOT APPLE) - target_link_options(Tactility PRIVATE "-Wl,--wrap=pthread_attr_setstack") - # Routes every read()/write()/close() call to Tactility/Source/AppStdioWrap.cpp's - # __wrap_read/write/close(), which forward into app_io_read/write/close() - the fd-table - # dispatch that lets an app's own stdio (e.g. a fileselection dialog's printf'd result - # path) reach an AppStream a parent bound via app_manager_start_with_streams(). Mirrors - # what ESP-IDF's build already does for the ESP32 target (see top-level CMakeLists.txt). - target_link_options(Tactility PRIVATE "-Wl,--wrap=read" "-Wl,--wrap=write" "-Wl,--wrap=close") - # glibc's printf/fprintf/etc don't call the public write() symbol internally (they're - # already compiled into libc.so, out of --wrap's reach), so the read/write/close wrap - # above can't see them. Newlib (ESP-IDF) doesn't have this gap - its stdio does call the - # wrappable syscall stubs - so this block is POSIX-only. Wrapping these symbols instead - # redirects OUR OWN calls to them (the only ones --wrap can rewrite) through - # AppStdioWrap.cpp's __wrap_* functions, which check the target stream (stdout/stdin) and - # fall back to the real libc function for any other FILE*. - target_link_options(Tactility PRIVATE - "-Wl,--wrap=printf" "-Wl,--wrap=fprintf" "-Wl,--wrap=vprintf" "-Wl,--wrap=vfprintf" - "-Wl,--wrap=puts" "-Wl,--wrap=fputs" "-Wl,--wrap=putchar" "-Wl,--wrap=fputc" - "-Wl,--wrap=getchar" "-Wl,--wrap=fgetc" "-Wl,--wrap=fgets" - ) - endif () + # pthread_attr_setstack and read/write/close/printf-family aren't wrapped here: linking + # platform-posix and app-module already brings those wraps along (see their own + # CMakeLists.txt files). endif () # diff --git a/Tactility/Source/AppStdioWrap.cpp b/Tactility/Source/AppStdioWrap.cpp deleted file mode 100644 index 9937b4b8..00000000 --- a/Tactility/Source/AppStdioWrap.cpp +++ /dev/null @@ -1,200 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -// Paired with -Wl,--wrap=read/write/close - see Tactility/CMakeLists.txt (POSIX) and the -// top-level CMakeLists.txt (ESP32) for where that's applied. On a platform where it isn't -// (currently: macOS, whose linker doesn't support --wrap), these are simply never called - real -// read()/write()/close() calls go straight through unredirected. -#include - -#include - -extern "C" { - -ssize_t __wrap_read(int fd, void* buffer, size_t size) { - return app_io_read(fd, buffer, size); -} - -ssize_t __wrap_write(int fd, const void* buffer, size_t size) { - return app_io_write(fd, buffer, size); -} - -int __wrap_close(int fd) { - return app_io_close(fd); -} - -} - -// region glibc stdio wraps -// -// glibc's printf/fprintf/etc are compiled into libc.so and call an internal, non-exported write() -// alias - --wrap=write (above) can't reach that call, only calls WE make to the public symbol. -// These wraps instead redirect calls WE make to printf/fprintf/etc, the same trick as read/write/ -// close above. Newlib (ESP-IDF) doesn't have this gap - its stdio does call the wrappable syscall -// stubs - so Tactility/CMakeLists.txt only applies the matching -Wl,--wrap= flags on POSIX. -// -// Scoped to the printf/getc families only: fread/fwrite take an arbitrary FILE* and are already -// used sitewide for real file I/O (e.g. File.cpp's readBinaryInternal), so wrapping them would -// route every such call through this file's stdin/stdout check - a correctness risk for unrelated -// code that isn't worth taking here. putc/getc are excluded too since glibc defines them as -// macros, not real calls, so wrapping those symbols wouldn't reliably intercept them. - -#if !defined(ESP_PLATFORM) && !defined(__APPLE__) - -#include -#include -#include -#include -#include - -extern "C" { -int __real_vfprintf(FILE* stream, const char* format, va_list args); -int __real_fputs(const char* s, FILE* stream); -int __real_fputc(int c, FILE* stream); -int __real_fgetc(FILE* stream); -char* __real_fgets(char* buffer, int size, FILE* stream); -} - -namespace { - -void writeAllToStdout(const void* data, size_t size) { - const auto* bytes = static_cast(data); - size_t remaining = size; - while (remaining > 0) { - ssize_t written = app_io_write(STDOUT_FILENO, bytes, remaining); - if (written <= 0) { - break; - } - bytes += written; - remaining -= static_cast(written); - } -} - -// Formats into stdout via app_io_write() rather than through a FILE*'s own buffering, since that -// buffering is exactly what glibc's internal write() call sidesteps --wrap for in the first place. -int formatToStdout(const char* format, va_list args) { - char stackBuffer[256]; - va_list argsForStack; - va_copy(argsForStack, args); - int needed = vsnprintf(stackBuffer, sizeof(stackBuffer), format, argsForStack); - va_end(argsForStack); - if (needed < 0) { - return needed; - } - if (static_cast(needed) < sizeof(stackBuffer)) { - writeAllToStdout(stackBuffer, static_cast(needed)); - return needed; - } - auto heapBuffer = std::make_unique(static_cast(needed) + 1); - va_list argsForHeap; - va_copy(argsForHeap, args); - vsnprintf(heapBuffer.get(), static_cast(needed) + 1, format, argsForHeap); - va_end(argsForHeap); - writeAllToStdout(heapBuffer.get(), static_cast(needed)); - return needed; -} - -int readOneFromStdin(char& out) { - return static_cast(app_io_read(STDIN_FILENO, &out, 1)); -} - -} // namespace - -extern "C" { - -int __wrap_vprintf(const char* format, va_list args) { - return formatToStdout(format, args); -} - -int __wrap_printf(const char* format, ...) { - va_list args; - va_start(args, format); - int result = formatToStdout(format, args); - va_end(args); - return result; -} - -int __wrap_vfprintf(FILE* stream, const char* format, va_list args) { - if (stream == stdout) { - return formatToStdout(format, args); - } - return __real_vfprintf(stream, format, args); -} - -int __wrap_fprintf(FILE* stream, const char* format, ...) { - va_list args; - va_start(args, format); - int result = (stream == stdout) ? formatToStdout(format, args) : __real_vfprintf(stream, format, args); - va_end(args); - return result; -} - -int __wrap_puts(const char* s) { - writeAllToStdout(s, strlen(s)); - writeAllToStdout("\n", 1); - return 0; -} - -int __wrap_fputs(const char* s, FILE* stream) { - if (stream == stdout) { - writeAllToStdout(s, strlen(s)); - return 0; - } - return __real_fputs(s, stream); -} - -int __wrap_putchar(int c) { - auto ch = static_cast(c); - writeAllToStdout(&ch, 1); - return c; -} - -int __wrap_fputc(int c, FILE* stream) { - if (stream == stdout) { - return __wrap_putchar(c); - } - return __real_fputc(c, stream); -} - -int __wrap_getchar() { - char c; - return readOneFromStdin(c) == 1 ? static_cast(c) : EOF; -} - -int __wrap_fgetc(FILE* stream) { - if (stream == stdin) { - return __wrap_getchar(); - } - return __real_fgetc(stream); -} - -char* __wrap_fgets(char* buffer, int size, FILE* stream) { - if (stream != stdin) { - return __real_fgets(buffer, size, stream); - } - if (size <= 0) { - return nullptr; - } - int i = 0; - for (; i < size - 1; ++i) { - char c; - if (readOneFromStdin(c) != 1) { - break; - } - buffer[i] = c; - if (c == '\n') { - ++i; - break; - } - } - if (i == 0) { - return nullptr; - } - buffer[i] = '\0'; - return buffer; -} - -} - -#endif // !ESP_PLATFORM && !__APPLE__ - -// endregion diff --git a/Tactility/Source/app/fileselection/FileSelection.cpp b/Tactility/Source/app/fileselection/FileSelection.cpp index f1cb15c3..7d8cb72c 100644 --- a/Tactility/Source/app/fileselection/FileSelection.cpp +++ b/Tactility/Source/app/fileselection/FileSelection.cpp @@ -85,7 +85,7 @@ int32_t appMain(int argc, char* argv[]) { if (ctx.resultCode == 0) { // The parent captures this via an AppStream bound to our stdout (see startWithMode()) - - // see AppStdioWrap.cpp for how printf() itself gets routed there on POSIX. + // see Modules/app-module/source/stdio_wrap.cpp for how printf() itself gets routed there on POSIX. LOG_I(TAG, "Result: %s", ctx.resultPath.c_str()); printf("%s", ctx.resultPath.c_str()); } diff --git a/Tactility/Source/app/inputdialog/InputDialog.cpp b/Tactility/Source/app/inputdialog/InputDialog.cpp index f8754f65..86f5bbd9 100644 --- a/Tactility/Source/app/inputdialog/InputDialog.cpp +++ b/Tactility/Source/app/inputdialog/InputDialog.cpp @@ -138,7 +138,7 @@ int32_t appMain(int argc, char* argv[]) { if (ctx.resultCode == 0) { // The caller captures this via an AppStream bound to our stdout (see start()); see - // AppStdioWrap.cpp for how printf() itself gets routed there on POSIX. + // Modules/app-module/source/stdio_wrap.cpp for how printf() itself gets routed there on POSIX. printf("%s", ctx.resultText.c_str()); } return ctx.resultCode; diff --git a/Tactility/Tests/CMakeLists.txt b/Tactility/Tests/CMakeLists.txt index 1a060ec2..0f282e2a 100644 --- a/Tactility/Tests/CMakeLists.txt +++ b/Tactility/Tests/CMakeLists.txt @@ -19,18 +19,9 @@ target_include_directories(TactilityTests PRIVATE ${CMAKE_CURRENT_LIST_DIR}/../P add_test(NAME TactilityTests COMMAND TactilityTests) -# Matches Tactility/CMakeLists.txt's own set of --wrap flags: this binary also compiles -# AppStdioWrap.cpp and links app-module (whose io.cpp calls __real_read/write/close() on -# non-Apple POSIX - see TT_APP_IO_WRAPS_STDIO in Modules/app-module/CMakeLists.txt), so it -# needs the same wraps applied or those go unresolved. -if (NOT APPLE) - target_link_options(TactilityTests PRIVATE - "-Wl,--wrap=pthread_attr_setstack" "-Wl,--wrap=read" "-Wl,--wrap=write" "-Wl,--wrap=close" - "-Wl,--wrap=printf" "-Wl,--wrap=fprintf" "-Wl,--wrap=vprintf" "-Wl,--wrap=vfprintf" - "-Wl,--wrap=puts" "-Wl,--wrap=fputs" "-Wl,--wrap=putchar" "-Wl,--wrap=fputc" - "-Wl,--wrap=getchar" "-Wl,--wrap=fgetc" "-Wl,--wrap=fgets" - ) -endif () +# No --wrap flags declared here: linking platform-posix and app-module below already brings all +# of pthread_attr_setstack/read/write/close/the printf family along (see their own CMakeLists.txt +# files). target_link_libraries(TactilityTests PRIVATE TactilityKernel