diff --git a/CMakeLists.txt b/CMakeLists.txt index 7c0a581f2..daec1f6c4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -94,17 +94,17 @@ if (DEFINED ENV{ESP_IDF_VERSION}) # opendir()/readdir()/closedir() also define the plain names directly (vfs_calls.c). # Wrapped so opendir("/") synthesizes a listing of own registered filesystems instead of ENOENT. - # See Platforms/platform-esp32/source/root_dir.cpp. + # See Modules/app-esp32-module/source/root_dir.cpp. idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=opendir" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=readdir" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=closedir" APPEND) # mkdir() is an alias in vfs_calls.c. Wrapped so an existing mount root reports EEXIST. - # See Platforms/platform-esp32/source/mkdir.cpp. + # See Modules/app-esp32-module/source/path_wrap.cpp. idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=mkdir" APPEND) - # Path-based VFS calls are aliases in vfs_calls.c. Wrapped so a NULL path fails with EFAULT instead of crashing. - # See Platforms/platform-esp32/source/vfs_null_path.cpp. + # Path-based VFS calls are aliases in vfs_calls.c. Wrapped so an app's relative path is resolved against its cwd, + # and a NULL path fails with EFAULT instead of crashing. See Modules/app-esp32-module/source/path_wrap.cpp. idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=_open_r" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=_stat_r" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=_link_r" APPEND) @@ -200,6 +200,13 @@ if (NOT DEFINED ENV{ESP_IDF_VERSION}) set(FREERTOS_PORT GCC_POSIX CACHE STRING "") add_subdirectory(Libraries/FreeRTOS-Kernel) target_compile_definitions(freertos_kernel PUBLIC "projCOVERAGE_TEST=0") + # xTaskCreate() and vTaskDelete() are defined by freertos_task_hooks.c instead, see its header + set_source_files_properties(Libraries/FreeRTOS-Kernel/tasks.c + TARGET_DIRECTORY freertos_kernel + PROPERTIES COMPILE_DEFINITIONS "xTaskCreate=freertos_real_xTaskCreate;vTaskDelete=freertos_real_vTaskDelete" + ) + target_sources(freertos_kernel PRIVATE Platforms/platform-posix/freertos/freertos_task_hooks.c) + target_include_directories(freertos_kernel PUBLIC Platforms/platform-posix/freertos) # EmbedTLS set(ENABLE_TESTING OFF) diff --git a/Documentation/ideas.md b/Documentation/ideas.md index 28f10e2e2..89017ee85 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -7,6 +7,8 @@ ## Higher Priority +- Terminal app should always run in non-LVGL mode if no PSRAM is present. +- Crash app should work without LVGL, like Boot app. It should wait for any input (keyboard or pointer) to continue. - NimBLE looses pairing key after reboot - CrashDiagnostics shouldn't show a QR when there's no callstack - Move USB host task stacks to SPIRAM when available: esp32_usbhost*.cpp diff --git a/Modules/app-esp32-module/source/app_esp32_loader_service.cpp b/Modules/app-esp32-module/source/app_esp32_loader_service.cpp index 0c17a3827..69bdfd9d8 100644 --- a/Modules/app-esp32-module/source/app_esp32_loader_service.cpp +++ b/Modules/app-esp32-module/source/app_esp32_loader_service.cpp @@ -87,7 +87,12 @@ std::string resolve_elf_path(const std::string& path) { constexpr ElfRequirements EXECUTABLE_REQUIREMENTS = { .elf_class = ELF_CLASS_32, .data = ELF_DATA_2LSB, - .type = ELF_TYPE_DYN, +#if CONFIG_ELF_LOADER_BUS_ADDRESS_MIRROR + // Loaded by section rather than by program header, so a relocatable object without any (e.g. xcc700's output) loads too + .types = ELF_TYPE_MASK(ELF_TYPE_DYN) | ELF_TYPE_MASK(ELF_TYPE_REL), +#else + .types = ELF_TYPE_MASK(ELF_TYPE_DYN), +#endif #if defined(__XTENSA__) .machine = ELF_MACHINE_XTENSA, #elif defined(__riscv) @@ -107,7 +112,7 @@ bool is_executable_file(const std::string& resolved_path) { error_t api_load(AppLocation location, AppRuntime* out_runtime) { if (location.type != APP_LOCATION_PATH) { - LOG_E(TAG, "Out of memory"); + LOG_E(TAG, "Unsupported location type"); return ERROR_NOT_SUPPORTED; } diff --git a/Modules/app-esp32-module/source/app_symbols.cpp b/Modules/app-esp32-module/source/app_symbols.cpp index d258a9b76..9ba453e5f 100644 --- a/Modules/app-esp32-module/source/app_symbols.cpp +++ b/Modules/app-esp32-module/source/app_symbols.cpp @@ -5,22 +5,35 @@ // Pending signals are delivered here, at the entry and again when a call was interrupted by one, // since the app holds no lock of the system itself there. // Allocations are counted per app instance (see app/memory.h). +// Memory, files and tasks are tracked for apps that request cleanup (see app/resources.h). #include #include +#include #include #include +#include +#include +#include + #include +#include +#include +#include #include #include #include #include +#include +#include #include #include +#include #include +#include namespace { @@ -91,6 +104,7 @@ pid_t app_getppid() { void* record_alloc(void* ptr) { if (ptr != nullptr) { app_memory_record_alloc(heap_caps_get_allocated_size(ptr)); + app_resources_track_alloc(ptr); } return ptr; } @@ -98,6 +112,7 @@ void* record_alloc(void* ptr) { void record_free(void* ptr) { if (ptr != nullptr) { app_memory_record_free(heap_caps_get_allocated_size(ptr)); + app_resources_untrack_alloc(ptr); } } @@ -111,6 +126,8 @@ void* app_calloc(size_t count, size_t size) { void* app_realloc(void* ptr, size_t size) { const size_t old_size = (ptr != nullptr) ? heap_caps_get_allocated_size(ptr) : 0; + // Untracked before ptr may be freed + app_resources_untrack_alloc(ptr); void* result = realloc(ptr, size); // A failed realloc() leaves the old block allocated if (result != nullptr || size == 0) { @@ -118,6 +135,8 @@ void* app_realloc(void* ptr, size_t size) { app_memory_record_free(old_size); } record_alloc(result); + } else if (ptr != nullptr) { + app_resources_track_alloc(ptr); } return result; } @@ -161,6 +180,291 @@ void app_operator_delete_array_sized(void* ptr, size_t) { app_operator_delete_array(ptr); } +// region Files + +int app_open(const char* path, int flags, ...) { + va_list args; + va_start(args, flags); + const mode_t mode = (flags & O_CREAT) ? static_cast(va_arg(args, int)) : 0; + va_end(args); + const int fd = open(path, flags, mode); + if (fd >= 0) { + app_resources_track_fd(fd); + } + return fd; +} + +int app_close(int fd) { + app_resources_untrack_fd(fd); + return close(fd); +} + +FILE* app_fopen(const char* path, const char* mode) { + FILE* file = fopen(path, mode); + if (file != nullptr) { + app_resources_track_file(file); + } + return file; +} + +FILE* app_fdopen(int fd, const char* mode) { + FILE* file = fdopen(fd, mode); + if (file != nullptr) { + // Closed through the FILE from now on + app_resources_untrack_fd(fd); + app_resources_track_file(file); + } + return file; +} + +int app_fclose(FILE* file) { + app_resources_untrack_file(file); + return fclose(file); +} + +DIR* app_opendir(const char* path) { + DIR* dir = opendir(path); + if (dir != nullptr) { + app_resources_track_dir(dir); + } + return dir; +} + +int app_closedir(DIR* dir) { + app_resources_untrack_dir(dir); + return closedir(dir); +} + +// endregion + +// region FreeRTOS tasks + +struct TaskStart { + TaskFunction_t function; + void* parameter; + AppInstanceId app_instance_id; +}; + +// Also tracked as an allocation of the app, so it's freed when the task is deleted before it ever ran +TaskStart* task_start_create(TaskFunction_t function, void* parameter) { + auto* start = static_cast(malloc(sizeof(TaskStart))); + if (start != nullptr) { + *start = { function, parameter, app_resources_current_app() }; + app_resources_track_alloc(start); + } + return start; +} + +void task_trampoline(void* context) { + auto* start = static_cast(context); + // Only fails when app_resources_release() is about to delete this task + if (!app_resources_enter_task(start->app_instance_id)) { + vTaskSuspend(nullptr); + } + const TaskStart copy = *start; + app_resources_untrack_alloc(start); + free(start); + copy.function(copy.parameter); +} + +void delete_task(void* handle) { + vTaskDelete(static_cast(handle)); +} + +void delete_task_with_caps(void* handle) { + vTaskDeleteWithCaps(static_cast(handle)); +} + +struct TaskCreate { + TaskStart* start; + const char* name; + uint32_t stack_depth; + UBaseType_t priority; + BaseType_t core; + // xTaskCreateStaticPinnedToCore() only + StackType_t* stack_buffer; + StaticTask_t* task_buffer; + // xTaskCreatePinnedToCoreWithCaps() only + bool with_caps; + UBaseType_t memory_caps; + TaskHandle_t created; +}; + +bool create_task(void* context, void** out_handle) { + auto* create = static_cast(context); + TaskHandle_t handle = nullptr; + if (create->stack_buffer != nullptr) { + handle = xTaskCreateStaticPinnedToCore(task_trampoline, create->name, create->stack_depth, create->start, create->priority, create->stack_buffer, create->task_buffer, create->core); + } else if (create->with_caps) { + if (xTaskCreatePinnedToCoreWithCaps(task_trampoline, create->name, create->stack_depth, create->start, create->priority, &handle, create->core, create->memory_caps) != pdPASS) { + handle = nullptr; + } + } else if (xTaskCreatePinnedToCore(task_trampoline, create->name, create->stack_depth, create->start, create->priority, &handle, create->core) != pdPASS) { + handle = nullptr; + } + create->created = handle; + *out_handle = handle; + return handle != nullptr; +} + +/** @return the created task, or nullptr */ +TaskHandle_t create_tracked_task(TaskFunction_t function, void* parameter, TaskCreate create) { + create.start = task_start_create(function, parameter); + if (create.start == nullptr) { + return nullptr; + } + if (!app_resources_create_task(create_task, &create, create.with_caps ? delete_task_with_caps : delete_task, nullptr)) { + app_resources_untrack_alloc(create.start); + free(create.start); + return nullptr; + } + return create.created; +} + +BaseType_t app_xTaskCreatePinnedToCore(TaskFunction_t function, const char* name, uint32_t stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle, BaseType_t core) { + if (app_resources_current_app() == 0) { + return xTaskCreatePinnedToCore(function, name, stack_depth, parameter, priority, out_handle, core); + } + TaskHandle_t handle = create_tracked_task(function, parameter, { nullptr, name, stack_depth, priority, core, nullptr, nullptr, false, 0, nullptr }); + if (out_handle != nullptr) { + *out_handle = handle; + } + return handle != nullptr ? pdPASS : errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY; +} + +BaseType_t app_xTaskCreate(TaskFunction_t function, const char* name, configSTACK_DEPTH_TYPE stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle) { + return app_xTaskCreatePinnedToCore(function, name, stack_depth, parameter, priority, out_handle, tskNO_AFFINITY); +} + +TaskHandle_t app_xTaskCreateStaticPinnedToCore(TaskFunction_t function, const char* name, uint32_t stack_depth, void* parameter, UBaseType_t priority, StackType_t* stack_buffer, StaticTask_t* task_buffer, BaseType_t core) { + if (app_resources_current_app() == 0) { + return xTaskCreateStaticPinnedToCore(function, name, stack_depth, parameter, priority, stack_buffer, task_buffer, core); + } + return create_tracked_task(function, parameter, { nullptr, name, stack_depth, priority, core, stack_buffer, task_buffer, false, 0, nullptr }); +} + +TaskHandle_t app_xTaskCreateStatic(TaskFunction_t function, const char* name, uint32_t stack_depth, void* parameter, UBaseType_t priority, StackType_t* stack_buffer, StaticTask_t* task_buffer) { + return app_xTaskCreateStaticPinnedToCore(function, name, stack_depth, parameter, priority, stack_buffer, task_buffer, tskNO_AFFINITY); +} + +BaseType_t app_xTaskCreatePinnedToCoreWithCaps(TaskFunction_t function, const char* name, configSTACK_DEPTH_TYPE stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle, BaseType_t core, UBaseType_t memory_caps) { + if (app_resources_current_app() == 0) { + return xTaskCreatePinnedToCoreWithCaps(function, name, stack_depth, parameter, priority, out_handle, core, memory_caps); + } + TaskHandle_t handle = create_tracked_task(function, parameter, { nullptr, name, stack_depth, priority, core, nullptr, nullptr, true, memory_caps, nullptr }); + if (out_handle != nullptr) { + *out_handle = handle; + } + return handle != nullptr ? pdPASS : errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY; +} + +BaseType_t app_xTaskCreateWithCaps(TaskFunction_t function, const char* name, configSTACK_DEPTH_TYPE stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle, UBaseType_t memory_caps) { + return app_xTaskCreatePinnedToCoreWithCaps(function, name, stack_depth, parameter, priority, out_handle, tskNO_AFFINITY, memory_caps); +} + +void app_vTaskDelete(TaskHandle_t handle) { + app_resources_untrack_task(handle != nullptr ? handle : xTaskGetCurrentTaskHandle()); + vTaskDelete(handle); +} + +void app_vTaskDeleteWithCaps(TaskHandle_t handle) { + app_resources_untrack_task(handle != nullptr ? handle : xTaskGetCurrentTaskHandle()); + vTaskDeleteWithCaps(handle); +} + +// endregion + +// region pthreads + +struct ThreadStart { + void* (*function)(void*); + void* argument; + AppInstanceId app_instance_id; +}; + +// A pthread is tracked by its pthread_t, but deleted through the FreeRTOS task that runs it +std::mutex thread_tasks_mutex; +std::unordered_map thread_tasks; + +void* thread_key(pthread_t thread) { + return reinterpret_cast(static_cast(thread)); +} + +void thread_ended() { + const pthread_t self = pthread_self(); + app_resources_untrack_task(thread_key(self)); + std::lock_guard lock(thread_tasks_mutex); + thread_tasks.erase(self); +} + +void* thread_trampoline(void* context) { + auto* start = static_cast(context); + if (!app_resources_enter_task(start->app_instance_id)) { + return nullptr; + } + const ThreadStart copy = *start; + app_resources_untrack_alloc(start); + free(start); + { + std::lock_guard lock(thread_tasks_mutex); + thread_tasks[pthread_self()] = xTaskGetCurrentTaskHandle(); + } + void* result = copy.function(copy.argument); + thread_ended(); + return result; +} + +// ESP-IDF's own bookkeeping of the thread leaks +void delete_thread(void* handle) { + std::lock_guard lock(thread_tasks_mutex); + auto iterator = thread_tasks.find(static_cast(reinterpret_cast(handle))); + if (iterator != thread_tasks.end()) { + vTaskDelete(iterator->second); + thread_tasks.erase(iterator); + } +} + +struct ThreadCreate { + pthread_t* thread; + const pthread_attr_t* attributes; + ThreadStart* start; +}; + +bool create_thread(void* context, void** out_handle) { + const auto* create = static_cast(context); + if (pthread_create(create->thread, create->attributes, thread_trampoline, create->start) != 0) { + return false; + } + *out_handle = thread_key(*create->thread); + return true; +} + +int app_pthread_create(pthread_t* thread, const pthread_attr_t* attributes, void* (*function)(void*), void* argument) { + if (app_resources_current_app() == 0) { + return pthread_create(thread, attributes, function, argument); + } + auto* start = static_cast(malloc(sizeof(ThreadStart))); + if (start == nullptr) { + return ENOMEM; + } + *start = { function, argument, app_resources_current_app() }; + app_resources_track_alloc(start); + ThreadCreate create { thread, attributes, start }; + if (!app_resources_create_task(create_thread, &create, delete_thread, nullptr)) { + app_resources_untrack_alloc(start); + free(start); + return EAGAIN; + } + return 0; +} + +void app_pthread_exit(void* result) { + thread_ended(); + pthread_exit(result); +} + +// endregion + } // namespace extern "C" { @@ -186,6 +490,23 @@ extern const ModuleSymbol app_esp32_symbols[] = { { "_ZdaPv", reinterpret_cast(app_operator_delete_array) }, // operator delete[](void*) { "_ZdlPvj", reinterpret_cast(app_operator_delete_sized) }, // operator delete(void*, unsigned int) { "_ZdaPvj", reinterpret_cast(app_operator_delete_array_sized) }, // operator delete[](void*, unsigned int) + { "open", reinterpret_cast(app_open) }, + { "close", reinterpret_cast(app_close) }, + { "fopen", reinterpret_cast(app_fopen) }, + { "fdopen", reinterpret_cast(app_fdopen) }, + { "fclose", reinterpret_cast(app_fclose) }, + { "opendir", reinterpret_cast(app_opendir) }, + { "closedir", reinterpret_cast(app_closedir) }, + { "xTaskCreate", reinterpret_cast(app_xTaskCreate) }, + { "xTaskCreatePinnedToCore", reinterpret_cast(app_xTaskCreatePinnedToCore) }, + { "xTaskCreateStatic", reinterpret_cast(app_xTaskCreateStatic) }, + { "xTaskCreateStaticPinnedToCore", reinterpret_cast(app_xTaskCreateStaticPinnedToCore) }, + { "xTaskCreateWithCaps", reinterpret_cast(app_xTaskCreateWithCaps) }, + { "xTaskCreatePinnedToCoreWithCaps", reinterpret_cast(app_xTaskCreatePinnedToCoreWithCaps) }, + { "vTaskDelete", reinterpret_cast(app_vTaskDelete) }, + { "vTaskDeleteWithCaps", reinterpret_cast(app_vTaskDeleteWithCaps) }, + { "pthread_create", reinterpret_cast(app_pthread_create) }, + { "pthread_exit", reinterpret_cast(app_pthread_exit) }, MODULE_SYMBOL_TERMINATOR }; diff --git a/Modules/app-esp32-module/source/path_wrap.cpp b/Modules/app-esp32-module/source/path_wrap.cpp new file mode 100644 index 000000000..61e592100 --- /dev/null +++ b/Modules/app-esp32-module/source/path_wrap.cpp @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: Apache-2.0 +#include + +#include + +#include +#include +#include +#include + +#include + +// Wraps of the path-based VFS calls (-Wl,--wrap=, see the top-level CMakeLists.txt): +// - ESP-IDF's VFS has no cwd: an app instance's relative paths are resolved against its own (see app/libc.h). +// - ESP-IDF's VFS dereferences a NULL path. These fail with EFAULT instead, like other POSIX systems do. +// opendir() gets the same treatment in its own wrap (root_dir.cpp). + +namespace { + +/** + * @return the path to pass on to the VFS: resolved for an app instance, as-is otherwise. + * NULL when the resolved path doesn't fit (errno is set to ENAMETOOLONG). + */ +const char* resolve(const char* path, char (&buffer)[FILE_MAX_PATH_STRING_LENGTH]) { + const char* resolved; + return app_libc_try_resolve_path(path, buffer, sizeof(buffer), &resolved) ? resolved : path; +} + +} // namespace + +extern "C" { + +int __real__open_r(struct _reent* r, const char* path, int flags, int mode); +int __real__stat_r(struct _reent* r, const char* path, struct stat* st); +int __real__link_r(struct _reent* r, const char* n1, const char* n2); +int __real__unlink_r(struct _reent* r, const char* path); +int __real__rename_r(struct _reent* r, const char* src, const char* dst); +int __real_truncate(const char* path, off_t length); +int __real_access(const char* path, int amode); +int __real_utime(const char* path, const struct utimbuf* times); +int __real_rmdir(const char* name); +int __real_mkdir(const char* path, mode_t mode); + +// open() and fopen() both go through _open_r +int __wrap__open_r(struct _reent* r, const char* path, int flags, int mode) { + if (path == nullptr) { + r->_errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + r->_errno = ENAMETOOLONG; + return -1; + } + return __real__open_r(r, resolved, flags, mode); +} + +int __wrap__stat_r(struct _reent* r, const char* path, struct stat* st) { + if (path == nullptr) { + r->_errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + r->_errno = ENAMETOOLONG; + return -1; + } + return __real__stat_r(r, resolved, st); +} + +int __wrap__link_r(struct _reent* r, const char* n1, const char* n2) { + if (n1 == nullptr || n2 == nullptr) { + r->_errno = EFAULT; + return -1; + } + char buffer1[FILE_MAX_PATH_STRING_LENGTH]; + char buffer2[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved1 = resolve(n1, buffer1); + const char* resolved2 = resolve(n2, buffer2); + if (resolved1 == nullptr || resolved2 == nullptr) { + r->_errno = ENAMETOOLONG; + return -1; + } + return __real__link_r(r, resolved1, resolved2); +} + +// unlink() and remove() both go through _unlink_r +int __wrap__unlink_r(struct _reent* r, const char* path) { + if (path == nullptr) { + r->_errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + r->_errno = ENAMETOOLONG; + return -1; + } + return __real__unlink_r(r, resolved); +} + +int __wrap__rename_r(struct _reent* r, const char* src, const char* dst) { + if (src == nullptr || dst == nullptr) { + r->_errno = EFAULT; + return -1; + } + char src_buffer[FILE_MAX_PATH_STRING_LENGTH]; + char dst_buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved_src = resolve(src, src_buffer); + const char* resolved_dst = resolve(dst, dst_buffer); + if (resolved_src == nullptr || resolved_dst == nullptr) { + r->_errno = ENAMETOOLONG; + return -1; + } + return __real__rename_r(r, resolved_src, resolved_dst); +} + +int __wrap_truncate(const char* path, off_t length) { + if (path == nullptr) { + errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_truncate(resolved, length); +} + +int __wrap_access(const char* path, int amode) { + if (path == nullptr) { + errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_access(resolved, amode); +} + +int __wrap_utime(const char* path, const struct utimbuf* times) { + if (path == nullptr) { + errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_utime(resolved, times); +} + +int __wrap_rmdir(const char* name) { + if (name == nullptr) { + errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(name, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_rmdir(resolved); +} + +// mkdir() on an existing FATFS mount root (e.g. "/sdcard") fails without setting EEXIST, +// which breaks the common "mkdir() then accept EEXIST" pattern. Any existing path reports EEXIST. +int __wrap_mkdir(const char* path, mode_t mode) { + if (path == nullptr) { + errno = EFAULT; + return -1; + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolve(path, buffer); + if (resolved == nullptr) { + return -1; + } + struct stat info; + if (stat(resolved, &info) == 0) { + errno = EEXIST; + return -1; + } + return __real_mkdir(resolved, mode); +} + +} diff --git a/Platforms/platform-esp32/source/root_dir.cpp b/Modules/app-esp32-module/source/root_dir.cpp similarity index 88% rename from Platforms/platform-esp32/source/root_dir.cpp rename to Modules/app-esp32-module/source/root_dir.cpp index 09305011a..ddd38e46e 100644 --- a/Platforms/platform-esp32/source/root_dir.cpp +++ b/Modules/app-esp32-module/source/root_dir.cpp @@ -11,7 +11,10 @@ // `dd_vfs_idx` is never a real VFS's table offset here and `dd_rsv` is left untouched (0) by every real VFS implementation, // so tagging it with a nonzero magic value reliably tells our handles apart from real ones. +#include + #include +#include #include @@ -71,13 +74,20 @@ extern "C" int __real_closedir(DIR* pdir); extern "C" { DIR* __wrap_opendir(const char* name) { - // The VFS dereferences a NULL path (see vfs_null_path.cpp) + // The VFS dereferences a NULL path and has no cwd (see path_wrap.cpp) if (name == nullptr) { errno = EFAULT; return nullptr; } - if (strcmp(name, "/") != 0) { - return __real_opendir(name); + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved; + if (!app_libc_try_resolve_path(name, buffer, sizeof(buffer), &resolved)) { + resolved = name; + } else if (resolved == nullptr) { + return nullptr; + } + if (strcmp(resolved, "/") != 0) { + return __real_opendir(resolved); } auto* dir = static_cast(calloc(1, sizeof(RootDir))); diff --git a/Modules/app-module/include/app/elf_check.h b/Modules/app-module/include/app/elf_check.h index 1c23ba0cc..170c8a025 100644 --- a/Modules/app-module/include/app/elf_check.h +++ b/Modules/app-module/include/app/elf_check.h @@ -12,7 +12,9 @@ extern "C" { #define ELF_CLASS_32 1 #define ELF_CLASS_64 2 #define ELF_DATA_2LSB 1 +#define ELF_TYPE_REL 1 #define ELF_TYPE_DYN 3 +#define ELF_TYPE_MASK(type) (1U << (type)) #define ELF_MACHINE_XTENSA 94 #define ELF_MACHINE_RISCV 243 #define ELF_MACHINE_X86_64 62 @@ -22,17 +24,23 @@ extern "C" { struct ElfRequirements { uint8_t elf_class; /**< ELF_CLASS_32 / ELF_CLASS_64 */ uint8_t data; /**< ELF_DATA_2LSB */ - uint16_t type; /**< ELF_TYPE_DYN */ + uint32_t types; /**< ELF_TYPE_MASK() of every accepted e_type, e.g. ELF_TYPE_MASK(ELF_TYPE_DYN) */ uint16_t machine; /**< ELF_MACHINE_XTENSA / _RISCV / _X86_64 / _AARCH64 */ }; /** * Reads the first 20 bytes of @a path (e_ident, e_type, e_machine; identical offsets for * ELF32 and ELF64) and checks the magic number and every field in @a requirements match. + * The e_type matches when it's one of @a requirements' types. * @return false if @a path can't be opened, is too short, or doesn't match */ bool elf_check_file(const char* path, const struct ElfRequirements* requirements); +/** + * @return true if @a path starts with the ELF magic number, whether or not it's loadable on this platform + */ +bool elf_has_magic(const char* path); + #ifdef __cplusplus } #endif diff --git a/Modules/app-module/include/app/libc.h b/Modules/app-module/include/app/libc.h index 5710df409..d0605bc8a 100644 --- a/Modules/app-module/include/app/libc.h +++ b/Modules/app-module/include/app/libc.h @@ -30,6 +30,17 @@ bool app_libc_try_getcwd(char* buf, size_t size, char** out_result); bool app_libc_try_chdir(const char* path, int* out_result); +/** + * Resolves @a path against the calling app instance's cwd, for the path-based libc calls: the platform's + * libc has no per-app cwd. + * On ESP32, ".", ".." and empty segments are collapsed too, also in an absolute path, since FATFS doesn't support them. + * On POSIX, the path is kept as written, so trailing separators and ".." through symlinks keep their meaning. + * Returns false for a NULL or empty @a path, which the caller then passes on as-is. + * @param[out] buf receives the resolved path + * @param[out] out_path @a buf on success, NULL when the resolved path doesn't fit (errno is set to ENAMETOOLONG) + */ +bool app_libc_try_resolve_path(const char* path, char* buf, size_t size, const char** out_path); + /** App fds report as character devices, which also makes isatty() true for them. */ bool app_libc_try_fstat(int fd, struct stat* st, int* out_result); diff --git a/Modules/app-module/include/app/manifest.h b/Modules/app-module/include/app/manifest.h index 601f23414..0b81bb733 100644 --- a/Modules/app-module/include/app/manifest.h +++ b/Modules/app-module/include/app/manifest.h @@ -32,6 +32,13 @@ enum AppManifestFlags { APP_MANIFEST_FLAG_HIDDEN = 1 << 0, /** No window-manager dependency. Safe to start from a context with no GUI available. */ APP_MANIFEST_FLAG_HEADLESS = 1 << 1, + /** + * When the app's main task ends, release what its binary leaked: tasks are deleted after a grace period, + * then files and directories are closed and memory is freed. + * Only covers calls made by a loaded binary (not APP_LOCATION_MEMORY apps), and not sockets, dup() or LVGL objects. + * Must not be used by an app that hands ownership of memory, files or tasks to the system, as they would be released twice. + */ + APP_MANIFEST_FLAG_CLEANUP = 1 << 2, }; /** Largest stack depth (in words) an app may request. Keeps `depth * sizeof(StackType_t)` safely diff --git a/Modules/app-module/include/app/resources.h b/Modules/app-module/include/app/resources.h new file mode 100644 index 000000000..5e4775b34 --- /dev/null +++ b/Modules/app-module/include/app/resources.h @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * Resource tracking for apps whose manifest sets APP_MANIFEST_FLAG_CLEANUP. + * The platform's hooks for an app binary's own calls report what it acquires and releases here. + * Every function does nothing when the calling task's app instance doesn't track its resources. + */ + +/** Ends a task that was still running when its app's main task ended. */ +typedef void (*AppResourceTaskDelete)(void* handle); + +/** Waits for a task that AppResourceTaskDelete only asked to end, so it no longer runs app code. */ +typedef void (*AppResourceTaskJoin)(void* handle); + +/** + * Creates a task. + * @param[in] context as passed to app_resources_create_task() + * @param[out] out_handle identifies the task, also when untracking it + * @return true on success + */ +typedef bool (*AppResourceTaskCreate)(void* context, void** out_handle); + +/** @return the calling task's app instance if it tracks its resources, 0 otherwise */ +AppInstanceId app_resources_current_app(void); + +/** + * Makes the calling task part of @a app_instance_id, so its own calls are tracked for that instance too. + * Called first thing by a task an app created, with the id from app_resources_current_app() of the creating task. + * @return false when @a app_instance_id no longer tracks its resources: its binary may be unloaded, so the task must end without running app code + */ +bool app_resources_enter_task(AppInstanceId app_instance_id); + +/** + * Like app_resources_enter_task(), but the calling thread's calls are only tracked for @a app_instance_id: + * it doesn't become part of the instance otherwise. For a thread that isn't a FreeRTOS task. + */ +bool app_resources_enter_thread(AppInstanceId app_instance_id); + +void app_resources_track_alloc(void* ptr); +void app_resources_untrack_alloc(void* ptr); + +void app_resources_track_fd(int fd); +void app_resources_untrack_fd(int fd); + +void app_resources_track_file(FILE* file); +void app_resources_untrack_file(FILE* file); + +void app_resources_track_dir(DIR* dir); +void app_resources_untrack_dir(DIR* dir); + +/** + * Calls @a create and tracks the task it created, before that task can untrack itself. + * @param[in] delete_task ends the task if it's still running after the grace period + * @param[in] join_task nullable, called after @a delete_task, when the task deletion is asynchronous + * @return the result of @a create + */ +bool app_resources_create_task(AppResourceTaskCreate create, void* context, AppResourceTaskDelete delete_task, AppResourceTaskJoin join_task); + +/** Called by a task that is ending by itself, or by whoever ends it. */ +void app_resources_untrack_task(void* handle); + +#ifdef __cplusplus +} +#endif diff --git a/Modules/app-module/private/app/private/resources.h b/Modules/app-module/private/app/private/resources.h new file mode 100644 index 000000000..8222bb21a --- /dev/null +++ b/Modules/app-module/private/app/private/resources.h @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include + +#include + +#include + +/** How long an app's own tasks get to end by themselves after its main task ended, before they're deleted. */ +constexpr uint32_t APP_CLEANUP_TASK_GRACE_MS = 1000; + +/** Starts tracking the resources of @a app_instance_id (see app/resources.h). */ +error_t app_resources_register(AppInstanceId app_instance_id); + +/** + * First phase of releasing what @a app_instance_id left behind: its tasks are deleted after APP_CLEANUP_TASK_GRACE_MS, + * and no new ones can start. Must run before the app's binary is unloaded, as they run its code. + * Until app_resources_release(), the calling task's own calls are tracked for the instance, + * so what the binary's static destructors free or close while it's unloaded is no longer tracked. + * Called by the instance's own task, after it left the instance. + */ +void app_resources_release_tasks(AppInstanceId app_instance_id); + +/** + * Second phase: stops tracking @a app_instance_id, closes its files, directories and fds and frees its memory. + * Must run after the app's binary is unloaded, on the task that called app_resources_release_tasks(). + */ +void app_resources_release(AppInstanceId app_instance_id); diff --git a/Modules/app-module/private/app/private/scheduler.h b/Modules/app-module/private/app/private/scheduler.h index df8687739..b4b69e99e 100644 --- a/Modules/app-module/private/app/private/scheduler.h +++ b/Modules/app-module/private/app/private/scheduler.h @@ -35,6 +35,9 @@ error_t app_scheduler_stop(AppInstanceId app_instance_id, TickType_t join_timeou // app_scheduler_current_app_id() is public - see app/scheduler.h. +/** Makes the calling task part of @a app_instance_id, for a task that an app instance created (see app/resources.h). */ +void app_scheduler_set_current_app_id(AppInstanceId app_instance_id); + #ifdef __cplusplus } #endif diff --git a/Modules/app-module/source/elf_check.cpp b/Modules/app-module/source/elf_check.cpp index b08594da3..bc0d8b77c 100644 --- a/Modules/app-module/source/elf_check.cpp +++ b/Modules/app-module/source/elf_check.cpp @@ -40,6 +40,20 @@ bool elf_check_file(const char* path, const struct ElfRequirements* requirements return elf_class == requirements->elf_class && data == requirements->data - && type == requirements->type + && type < 32 + && (requirements->types & ELF_TYPE_MASK(type)) != 0 && machine == requirements->machine; } + +bool elf_has_magic(const char* path) { + FILE* file = fopen(path, "rb"); + if (file == nullptr) { + return false; + } + + uint8_t magic[sizeof(ELF_MAGIC)]; + size_t read = fread(magic, 1, sizeof(magic), file); + fclose(file); + + return read == sizeof(magic) && memcmp(magic, ELF_MAGIC, sizeof(ELF_MAGIC)) == 0; +} diff --git a/Modules/app-module/source/libc.cpp b/Modules/app-module/source/libc.cpp index 9025ac885..9a3ed36a0 100644 --- a/Modules/app-module/source/libc.cpp +++ b/Modules/app-module/source/libc.cpp @@ -19,6 +19,7 @@ #include #include +#include #include namespace { @@ -68,6 +69,44 @@ bool sleep_unless_signalled(TickType_t ticks, TickType_t* out_remaining) { } } +#ifdef ESP_PLATFORM +/** + * Collapses the ".", ".." and empty segments of an absolute path, in place. + * Every segment written is preceded by a '/' that was read, so writing never overtakes reading. + */ +void normalize_path(char* path) { + size_t length = 0; + const char* segment = path; + while (*segment != '\0') { + while (*segment == '/') { + segment++; + } + const char* end = segment; + while (*end != '\0' && *end != '/') { + end++; + } + const size_t segment_length = end - segment; + if (segment_length == 2 && segment[0] == '.' && segment[1] == '.') { + while (length > 0 && path[length - 1] != '/') { + length--; + } + if (length > 0) { + length--; // the '/' before the removed segment + } + } else if (segment_length > 0 && !(segment_length == 1 && segment[0] == '.')) { + path[length++] = '/'; + memmove(path + length, segment, segment_length); + length += segment_length; + } + segment = end; + } + if (length == 0) { + path[length++] = '/'; + } + path[length] = '\0'; +} +#endif + } // namespace extern "C" { @@ -117,34 +156,86 @@ bool app_libc_try_getcwd(char* buf, size_t size, char** out_result) { return false; // ERROR_NOT_FOUND: not an app instance. } -// app_dir_set_cwd() requires an already-absolute path and reports both "not an app instance" and -// "no such directory" as ERROR_NOT_FOUND, so app_dir_get_cwd() is used first as an unambiguous -// "is this an app instance" probe (it's needed anyway, to resolve a relative path). -bool app_libc_try_chdir(const char* path, int* out_result) { - if (path == nullptr || path[0] == '\0') { +// The cwd is written to buf directly and the path appended to it, so a relative path needs no second buffer. +bool app_libc_try_resolve_path(const char* path, char* buf, size_t size, const char** out_path) { + // Checked before app_dir_get_cwd() takes the ledger's lock: every path-based libc call in the + // process ends up here, including the simulator's foreign threads (e.g. SDL's) and early boot. + if (path == nullptr || path[0] == '\0' || app_scheduler_current_app_id() == 0) { return false; } - char cwd[FILE_MAX_PATH_STRING_LENGTH]; - if (app_dir_get_cwd(cwd, sizeof(cwd)) != ERROR_NONE) { + const error_t cwd_result = app_dir_get_cwd(buf, size); + if (cwd_result == ERROR_NOT_FOUND) { return false; // not an app instance } + const size_t path_length = strlen(path); + if (path[0] == '/') { + if (path_length >= size) { + errno = ENAMETOOLONG; + *out_path = nullptr; + return true; + } + memcpy(buf, path, path_length + 1); + } else { + const size_t cwd_length = (cwd_result == ERROR_NONE) ? strlen(buf) : size; + // No separator after the root, which already ends with one + const size_t separator_length = (cwd_length > 0 && cwd_length < size && buf[cwd_length - 1] == '/') ? 0 : 1; + if (cwd_length + separator_length + path_length >= size) { + errno = ENAMETOOLONG; + *out_path = nullptr; + return true; + } + if (separator_length != 0) { + buf[cwd_length] = '/'; + } + memcpy(buf + cwd_length + separator_length, path, path_length + 1); + } + +#ifdef ESP_PLATFORM + normalize_path(buf); +#endif + *out_path = buf; + return true; +} + +// app_dir_set_cwd() requires an already-absolute path and reports both "not an app instance" and +// "no such directory" as ERROR_NOT_FOUND, so the path is resolved first, which also tells the two apart. +bool app_libc_try_chdir(const char* path, int* out_result) { char resolved[FILE_MAX_PATH_STRING_LENGTH]; - const int written = (path[0] == '/') ? snprintf(resolved, sizeof(resolved), "%s", path) - : (strcmp(cwd, "/") == 0) ? snprintf(resolved, sizeof(resolved), "/%s", path) - : snprintf(resolved, sizeof(resolved), "%s/%s", cwd, path); - if (written < 0 || static_cast(written) >= sizeof(resolved)) { - errno = ENAMETOOLONG; + const char* resolved_path; + if (!app_libc_try_resolve_path(path, resolved, sizeof(resolved), &resolved_path)) { + return false; + } + if (resolved_path == nullptr) { *out_result = -1; return true; } - if (app_dir_set_cwd(resolved) == ERROR_NONE) { +#ifdef ESP_PLATFORM + if (app_dir_set_cwd(resolved_path) == ERROR_NONE) { *out_result = 0; } else { errno = ENOENT; *out_result = -1; } +#else + // The cwd is kept physical, like the kernel's own: ".." in the path follows symlinks + char* canonical = realpath(resolved_path, nullptr); + if (canonical == nullptr) { + *out_result = -1; + return true; + } + if (strlen(canonical) > FILE_MAX_PATH_LENGTH) { + errno = ENAMETOOLONG; + *out_result = -1; + } else if (app_dir_set_cwd(canonical) == ERROR_NONE) { + *out_result = 0; + } else { + errno = ENOTDIR; + *out_result = -1; + } + free(canonical); +#endif return true; } diff --git a/Modules/app-module/source/package_manifest_parsing_v3.cpp b/Modules/app-module/source/package_manifest_parsing_v3.cpp index 80a316af4..d301a07bb 100644 --- a/Modules/app-module/source/package_manifest_parsing_v3.cpp +++ b/Modules/app-module/source/package_manifest_parsing_v3.cpp @@ -134,6 +134,18 @@ error_t parse_app_manifest(const std::map& properties, } } + // .cleanup (optional; defaults to false) + auto cleanup_iterator = properties.find(prefix + "cleanup"); + if (cleanup_iterator != properties.end()) { + if (!app_package_manifest_is_valid_bool(cleanup_iterator->second)) { + LOG_E(TAG, "Invalid %scleanup", prefix.c_str()); + return ERROR_INVALID_ARGUMENT; + } + if (cleanup_iterator->second == "true") { + out_manifest.flags |= APP_MANIFEST_FLAG_CLEANUP; + } + } + out_manifest.category = APP_CATEGORY_USER; return ERROR_NONE; diff --git a/Modules/app-module/source/resources.cpp b/Modules/app-module/source/resources.cpp new file mode 100644 index 000000000..b783aa983 --- /dev/null +++ b/Modules/app-module/source/resources.cpp @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: Apache-2.0 +#include +#include +#include +#include + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +constexpr auto* TAG = "app_resources"; + +namespace { + +constexpr uint32_t TASK_POLL_INTERVAL_MS = 10; + +struct Tracker { + std::unordered_set allocations; + std::unordered_set fds; + std::unordered_set files; + std::unordered_set dirs; + struct Task { + AppResourceTaskDelete delete_task; + AppResourceTaskJoin join_task; + }; + std::unordered_map tasks; + // Set by app_resources_release_tasks(): no task can start or be created anymore + bool tasks_released = false; + size_t deleted_task_count = 0; +}; + +// std rather than the kernel's Mutex: tasks an app created with pthread_create() aren't FreeRTOS tasks on the simulator. +// Recursive, since app_resources_create_task() holds it while the platform creates a task. +std::recursive_mutex registry_mutex; +std::unordered_map registry; +// Lets every hook return without locking while no app tracks its resources +std::atomic registry_size { 0 }; + +// Set by app_resources_enter_thread(). Only read once registry_size is non-zero, as thread_local can't be read before the scheduler starts on ESP32. +thread_local AppInstanceId thread_owner = 0; + +AppInstanceId current_owner() { + return thread_owner != 0 ? thread_owner : app_scheduler_current_app_id(); +} + +/** @return the tracker of @a app_instance_id if it still accepts tasks, nullptr otherwise. Requires registry_mutex. */ +Tracker* find_tracker_accepting_tasks(AppInstanceId app_instance_id) { + auto iterator = registry.find(app_instance_id); + return iterator != registry.end() && !iterator->second->tasks_released ? iterator->second : nullptr; +} + +/** Runs @a action on the calling task's tracker, under registry_mutex. */ +template +void with_current_tracker(Action action) { + if (registry_size.load(std::memory_order_acquire) == 0) { + return; + } + const AppInstanceId app_instance_id = current_owner(); + if (app_instance_id == 0) { + return; + } + std::lock_guard lock(registry_mutex); + auto iterator = registry.find(app_instance_id); + if (iterator != registry.end()) { + action(*iterator->second); + } +} + +size_t task_count(AppInstanceId app_instance_id) { + std::lock_guard lock(registry_mutex); + auto iterator = registry.find(app_instance_id); + return iterator != registry.end() ? iterator->second->tasks.size() : 0; +} + +} // namespace + +error_t app_resources_register(AppInstanceId app_instance_id) { + auto* tracker = new (std::nothrow) Tracker(); + if (tracker == nullptr) { + return ERROR_OUT_OF_MEMORY; + } + std::lock_guard lock(registry_mutex); + registry[app_instance_id] = tracker; + registry_size.fetch_add(1, std::memory_order_release); + return ERROR_NONE; +} + +void app_resources_release_tasks(AppInstanceId app_instance_id) { + for (uint32_t waited = 0; waited < APP_CLEANUP_TASK_GRACE_MS && task_count(app_instance_id) > 0; waited += TASK_POLL_INTERVAL_MS) { + delay_millis(TASK_POLL_INTERVAL_MS); + } + + std::vector> joins; + { + std::lock_guard lock(registry_mutex); + auto iterator = registry.find(app_instance_id); + if (iterator == registry.end()) { + return; + } + Tracker* tracker = iterator->second; + tracker->tasks_released = true; + // Deleted while holding the lock, so none of them can be stopped halfway through a tracker call + for (auto& [handle, task] : tracker->tasks) { + task.delete_task(handle); + if (task.join_task != nullptr) { + joins.emplace_back(handle, task.join_task); + } + } + tracker->deleted_task_count = tracker->tasks.size(); + tracker->tasks.clear(); + } + + // Outside the lock: a task that is ending may still make tracker calls + for (auto& [handle, join_task] : joins) { + join_task(handle); + } + + thread_owner = app_instance_id; +} + +void app_resources_release(AppInstanceId app_instance_id) { + Tracker* tracker; + { + std::lock_guard lock(registry_mutex); + auto iterator = registry.find(app_instance_id); + if (iterator == registry.end()) { + return; + } + tracker = iterator->second; + registry.erase(iterator); + registry_size.fetch_sub(1, std::memory_order_release); + } + thread_owner = 0; + + for (FILE* file : tracker->files) { + fclose(file); + } + for (DIR* dir : tracker->dirs) { + closedir(dir); + } + for (int fd : tracker->fds) { + close(fd); + } + for (void* ptr : tracker->allocations) { + free(ptr); + } + + if (tracker->deleted_task_count != 0 || !tracker->files.empty() || !tracker->dirs.empty() || !tracker->fds.empty() || !tracker->allocations.empty()) { + LOG_W(TAG, "[instance %lu] Released %u tasks, %u files, %u directories, %u fds and %u allocations", + static_cast(app_instance_id), + static_cast(tracker->deleted_task_count), + static_cast(tracker->files.size()), + static_cast(tracker->dirs.size()), + static_cast(tracker->fds.size()), + static_cast(tracker->allocations.size())); + } + + delete tracker; +} + +extern "C" { + +AppInstanceId app_resources_current_app(void) { + if (registry_size.load(std::memory_order_acquire) == 0) { + return 0; + } + const AppInstanceId app_instance_id = current_owner(); + if (app_instance_id == 0) { + return 0; + } + std::lock_guard lock(registry_mutex); + return registry.contains(app_instance_id) ? app_instance_id : 0; +} + +bool app_resources_enter_task(AppInstanceId app_instance_id) { + std::lock_guard lock(registry_mutex); + if (find_tracker_accepting_tasks(app_instance_id) == nullptr) { + return false; + } + app_scheduler_set_current_app_id(app_instance_id); + return true; +} + +bool app_resources_enter_thread(AppInstanceId app_instance_id) { + std::lock_guard lock(registry_mutex); + if (find_tracker_accepting_tasks(app_instance_id) == nullptr) { + return false; + } + thread_owner = app_instance_id; + return true; +} + +void app_resources_track_alloc(void* ptr) { + with_current_tracker([ptr](Tracker& tracker) { tracker.allocations.insert(ptr); }); +} + +void app_resources_untrack_alloc(void* ptr) { + with_current_tracker([ptr](Tracker& tracker) { tracker.allocations.erase(ptr); }); +} + +void app_resources_track_fd(int fd) { + with_current_tracker([fd](Tracker& tracker) { tracker.fds.insert(fd); }); +} + +void app_resources_untrack_fd(int fd) { + with_current_tracker([fd](Tracker& tracker) { tracker.fds.erase(fd); }); +} + +void app_resources_track_file(FILE* file) { + with_current_tracker([file](Tracker& tracker) { tracker.files.insert(file); }); +} + +void app_resources_untrack_file(FILE* file) { + with_current_tracker([file](Tracker& tracker) { tracker.files.erase(file); }); +} + +void app_resources_track_dir(DIR* dir) { + with_current_tracker([dir](Tracker& tracker) { tracker.dirs.insert(dir); }); +} + +void app_resources_untrack_dir(DIR* dir) { + with_current_tracker([dir](Tracker& tracker) { tracker.dirs.erase(dir); }); +} + +bool app_resources_create_task(AppResourceTaskCreate create, void* context, AppResourceTaskDelete delete_task, AppResourceTaskJoin join_task) { + std::lock_guard lock(registry_mutex); + const AppInstanceId app_instance_id = current_owner(); + if (app_instance_id != 0 && registry.contains(app_instance_id) && find_tracker_accepting_tasks(app_instance_id) == nullptr) { + return false; + } + void* handle = nullptr; + if (!create(context, &handle)) { + return false; + } + with_current_tracker([handle, delete_task, join_task](Tracker& tracker) { tracker.tasks[handle] = { delete_task, join_task }; }); + return true; +} + +void app_resources_untrack_task(void* handle) { + with_current_tracker([handle](Tracker& tracker) { tracker.tasks.erase(handle); }); +} + +} // extern "C" diff --git a/Modules/app-module/source/scheduler.cpp b/Modules/app-module/source/scheduler.cpp index 4e05d5ecd..5f12c6cb3 100644 --- a/Modules/app-module/source/scheduler.cpp +++ b/Modules/app-module/source/scheduler.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -88,6 +89,8 @@ struct TaskContext { // Live allocations of the app's own code, see app/memory.h. Only updated by the app's own task. int32_t allocCount; int64_t allocBytes; + // APP_MANIFEST_FLAG_CLEANUP: the instance's resources are tracked (see app/private/resources.h) + bool cleanup; }; struct ReaperContext { @@ -142,10 +145,15 @@ thread_local TaskContext* current_task_context = nullptr; struct DeferredUnload { const AppLoaderApi* loader = nullptr; void* runtime = nullptr; + // Non-zero when the instance's resources are released after unloading (see app/private/resources.h) + AppInstanceId cleanup_instance_id = 0; ~DeferredUnload() { if (loader != nullptr) { loader->unload(runtime); + if (cleanup_instance_id != 0) { + app_resources_release(cleanup_instance_id); + } pending_deferred_unloads.fetch_sub(1, std::memory_order_release); } } @@ -299,20 +307,33 @@ void finish_app_task(TaskContext* ctx, int32_t result, bool exiting) { set_current_app_id(0); + // Tasks the app created may still be running its code. Its memory and files are only released after unloading, + // since its static destructors can still free and close them. + if (ctx->cleanup) { + app_resources_release_tasks(ctx->app_instance_id); + } + #ifndef ESP_PLATFORM if (exiting) { current_deferred_unload->loader = ctx->loader; current_deferred_unload->runtime = ctx->runtime; + current_deferred_unload->cleanup_instance_id = ctx->cleanup ? ctx->app_instance_id : 0; DeferredUnload::pending_deferred_unloads.fetch_add(1, std::memory_order_release); } else { ctx->loader->unload(ctx->runtime); + if (ctx->cleanup) { + app_resources_release(ctx->app_instance_id); + } } #else (void)exiting; ctx->loader->unload(ctx->runtime); + if (ctx->cleanup) { + app_resources_release(ctx->app_instance_id); + } #endif - if (ctx->allocCount > 0 || ctx->allocBytes > 0) { + if (!ctx->cleanup && (ctx->allocCount > 0 || ctx->allocBytes > 0)) { LOG_W(TAG, "[instance %lu] %ld allocations (%lld bytes) not freed", ctx->app_instance_id, static_cast(ctx->allocCount), static_cast(ctx->allocBytes)); } @@ -366,6 +387,18 @@ void finish_app_task(TaskContext* ctx, int32_t result, bool exiting) { #endif } +bool has_cleanup_flag(const char* manifest_id) { + if (manifest_id[0] == '\0') { + return false; + } + auto& ledger = app_ledger(); + mutex_lock(&ledger.mutex); + auto iterator = ledger.manifests.find(manifest_id); + const bool cleanup = iterator != ledger.manifests.end() && (iterator->second->flags & APP_MANIFEST_FLAG_CLEANUP) != 0; + mutex_unlock(&ledger.mutex); + return cleanup; +} + } // namespace extern "C" { @@ -466,6 +499,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, const AppStartContext .taskTcb = task_tcb, .allocCount = 0, .allocBytes = 0, + .cleanup = false, }; if (context == nullptr) { @@ -481,6 +515,13 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, const AppStartContext return ERROR_OUT_OF_MEMORY; } + if (has_cleanup_flag(start_context->id)) { + context->cleanup = app_resources_register(app_instance_id) == ERROR_NONE; + if (!context->cleanup) { + LOG_W(TAG, "[instance %lu] Failed to track resources", app_instance_id); + } + } + char task_name[16]; snprintf(task_name, sizeof(task_name), "app_%lu", static_cast(app_instance_id)); @@ -496,6 +537,10 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, const AppStartContext } #endif if (task_handle == nullptr) { + if (context->cleanup) { + app_resources_release_tasks(app_instance_id); + app_resources_release(app_instance_id); + } delete context; #ifdef ESP_PLATFORM memory_free(task_tcb); @@ -561,6 +606,10 @@ AppInstanceId app_scheduler_current_app_id(void) { return get_current_app_id(); } +void app_scheduler_set_current_app_id(AppInstanceId app_instance_id) { + set_current_app_id(app_instance_id); +} + void app_memory_record_alloc(size_t size) { // Checked first: thread_local can't be read before the scheduler starts on ESP32 if (get_current_app_id() == 0) { diff --git a/Modules/app-module/tests/source/elf_check_test.cpp b/Modules/app-module/tests/source/elf_check_test.cpp new file mode 100644 index 000000000..4411ea7f0 --- /dev/null +++ b/Modules/app-module/tests/source/elf_check_test.cpp @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: Apache-2.0 +#include "doctest.h" + +#include + +#include +#include +#include +#include + +namespace { + +constexpr ElfRequirements REQUIREMENTS = { + .elf_class = ELF_CLASS_32, + .data = ELF_DATA_2LSB, + .types = ELF_TYPE_MASK(ELF_TYPE_DYN) | ELF_TYPE_MASK(ELF_TYPE_REL), + .machine = ELF_MACHINE_XTENSA, +}; + +class TempFile { + std::string path; + +public: + explicit TempFile(const uint8_t* data, size_t size) { + char name[] = "/tmp/elf_check_test_XXXXXX"; + const int fd = mkstemp(name); + REQUIRE_NE(fd, -1); + REQUIRE_EQ(write(fd, data, size), static_cast(size)); + close(fd); + path = name; + } + + ~TempFile() { unlink(path.c_str()); } + + const char* get() const { return path.c_str(); } +}; + +TempFile makeElf(uint8_t elf_class, uint16_t type, uint16_t machine) { + uint8_t header[20] = { 0x7f, 'E', 'L', 'F', elf_class, ELF_DATA_2LSB }; + header[16] = type & 0xFF; + header[17] = type >> 8; + header[18] = machine & 0xFF; + header[19] = machine >> 8; + return TempFile(header, sizeof(header)); +} + +} // namespace + +TEST_CASE("elf_check_file accepts every type in the mask") { + CHECK(elf_check_file(makeElf(ELF_CLASS_32, ELF_TYPE_DYN, ELF_MACHINE_XTENSA).get(), &REQUIREMENTS)); + CHECK(elf_check_file(makeElf(ELF_CLASS_32, ELF_TYPE_REL, ELF_MACHINE_XTENSA).get(), &REQUIREMENTS)); +} + +TEST_CASE("elf_check_file rejects a type outside the mask") { + CHECK_FALSE(elf_check_file(makeElf(ELF_CLASS_32, 2, ELF_MACHINE_XTENSA).get(), &REQUIREMENTS)); + CHECK_FALSE(elf_check_file(makeElf(ELF_CLASS_32, 0xFE00, ELF_MACHINE_XTENSA).get(), &REQUIREMENTS)); +} + +TEST_CASE("elf_check_file rejects a mismatched class or machine") { + CHECK_FALSE(elf_check_file(makeElf(ELF_CLASS_64, ELF_TYPE_DYN, ELF_MACHINE_XTENSA).get(), &REQUIREMENTS)); + CHECK_FALSE(elf_check_file(makeElf(ELF_CLASS_32, ELF_TYPE_DYN, ELF_MACHINE_RISCV).get(), &REQUIREMENTS)); +} + +TEST_CASE("elf_has_magic is true for any ELF and false otherwise") { + CHECK(elf_has_magic(makeElf(ELF_CLASS_32, 2, ELF_MACHINE_RISCV).get())); + + const uint8_t script[] = "echo hello\n"; + CHECK_FALSE(elf_has_magic(TempFile(script, sizeof(script) - 1).get())); + + const uint8_t truncated[] = { 0x7f, 'E', 'L' }; + CHECK_FALSE(elf_has_magic(TempFile(truncated, sizeof(truncated)).get())); + + CHECK_FALSE(elf_has_magic("/nonexistent/elf_check_test")); +} diff --git a/Modules/app-module/tests/source/package_manifest_test.cpp b/Modules/app-module/tests/source/package_manifest_test.cpp index e49bed908..3c44b7fed 100644 --- a/Modules/app-module/tests/source/package_manifest_test.cpp +++ b/Modules/app-module/tests/source/package_manifest_test.cpp @@ -28,6 +28,14 @@ error_t parse(const std::map& properties, PackageManif return package_manifest_parse_v3(properties, package, bindings, 1); } +error_t parse_app_flags(const std::map& properties, uint8_t& out_flags) { + PackageManifest package {}; + AppManifestBinding bindings[1] {}; + const error_t result = package_manifest_parse_v3(properties, package, bindings, 1); + out_flags = bindings[0].manifest.flags; + return result; +} + } // namespace TEST_CASE("package manifest v3: requires.ram defaults to 0") { @@ -54,6 +62,31 @@ TEST_CASE("package manifest v3: an invalid requires.ram rejects the manifest") { } } +TEST_CASE("package manifest v3: cleanup defaults to disabled") { + uint8_t flags = 0xFF; + REQUIRE_EQ(parse_app_flags(minimal_v3_properties(), flags), ERROR_NONE); + CHECK_EQ(flags & APP_MANIFEST_FLAG_CLEANUP, 0); +} + +TEST_CASE("package manifest v3: cleanup=true sets the cleanup flag") { + auto properties = minimal_v3_properties(); + properties["app.0.cleanup"] = "true"; + uint8_t flags = 0; + REQUIRE_EQ(parse_app_flags(properties, flags), ERROR_NONE); + CHECK_NE(flags & APP_MANIFEST_FLAG_CLEANUP, 0); + + properties["app.0.cleanup"] = "false"; + REQUIRE_EQ(parse_app_flags(properties, flags), ERROR_NONE); + CHECK_EQ(flags & APP_MANIFEST_FLAG_CLEANUP, 0); +} + +TEST_CASE("package manifest v3: an invalid cleanup rejects the manifest") { + auto properties = minimal_v3_properties(); + properties["app.0.cleanup"] = "yes"; + uint8_t flags = 0; + CHECK_EQ(parse_app_flags(properties, flags), ERROR_INVALID_ARGUMENT); +} + TEST_CASE("package manifest compatibility: requires_device_id must list this device, if set") { PackageManifest package {}; CHECK(app_package_manifest_is_compatible(&package)); diff --git a/Modules/app-module/tests/source/resources_test.cpp b/Modules/app-module/tests/source/resources_test.cpp new file mode 100644 index 000000000..30c8a273a --- /dev/null +++ b/Modules/app-module/tests/source/resources_test.cpp @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: Apache-2.0 +#include "doctest.h" + +#include +#include +#include +#include + +#include + +#include "FreeRTOS.h" +#include "task.h" + +#include +#include + +#include +#include +#include + +namespace { + +// Far above any id the ledger hands out during these tests +constexpr AppInstanceId TRACKED_ID = 0x7FFF0001; + +std::atomic deleted_tasks { 0 }; + +void delete_task(void* handle) { + deleted_tasks++; + vTaskDelete(static_cast(handle)); +} + +bool is_open(int fd) { + return fcntl(fd, F_GETFD) != -1; +} + +struct TaskParameters { + AppInstanceId app_instance_id; + bool ends_by_itself; +}; + +void task_main(void* context) { + auto* parameters = static_cast(context); + app_resources_enter_task(parameters->app_instance_id); + if (parameters->ends_by_itself) { + app_resources_untrack_task(xTaskGetCurrentTaskHandle()); + vTaskDelete(nullptr); + } + while (true) { + delay_millis(10); + } +} + +bool create_task(void* context, void** out_handle) { + TaskHandle_t handle = nullptr; + if (xTaskCreate(task_main, "resources_test", 4096, context, 1, &handle) != pdPASS) { + return false; + } + *out_handle = handle; + return true; +} + +} // namespace + +TEST_CASE("app resources: nothing is tracked without a registered instance") { + app_scheduler_set_current_app_id(TRACKED_ID); + CHECK_EQ(app_resources_current_app(), 0); + const int fd = open("/dev/null", O_RDONLY); + app_resources_track_fd(fd); + app_scheduler_set_current_app_id(0); + + REQUIRE_EQ(app_resources_register(TRACKED_ID), ERROR_NONE); + app_resources_release_tasks(TRACKED_ID); + app_resources_release(TRACKED_ID); + CHECK(is_open(fd)); + close(fd); +} + +TEST_CASE("app resources: release closes and frees what is still tracked") { + REQUIRE_EQ(app_resources_register(TRACKED_ID), ERROR_NONE); + app_scheduler_set_current_app_id(TRACKED_ID); + CHECK_EQ(app_resources_current_app(), TRACKED_ID); + + const int leaked_fd = open("/dev/null", O_RDONLY); + const int closed_fd = open("/dev/null", O_RDONLY); + REQUIRE_NE(leaked_fd, -1); + REQUIRE_NE(closed_fd, -1); + app_resources_track_fd(leaked_fd); + app_resources_track_fd(closed_fd); + app_resources_untrack_fd(closed_fd); + + FILE* file = fopen("/dev/null", "r"); + REQUIRE_NE(file, nullptr); + app_resources_track_file(file); + const int file_fd = fileno(file); + + DIR* dir = opendir("/"); + REQUIRE_NE(dir, nullptr); + app_resources_track_dir(dir); + const int dir_fd = dirfd(dir); + + app_resources_track_alloc(malloc(16)); + + app_scheduler_set_current_app_id(0); + app_resources_release_tasks(TRACKED_ID); + app_resources_release(TRACKED_ID); + + CHECK_FALSE(is_open(leaked_fd)); + CHECK_FALSE(is_open(file_fd)); + CHECK_FALSE(is_open(dir_fd)); + CHECK(is_open(closed_fd)); + close(closed_fd); + + CHECK_FALSE(app_resources_enter_task(TRACKED_ID)); +} + +TEST_CASE("app resources: a task that ends within the grace period is not deleted") { + REQUIRE_EQ(app_resources_register(TRACKED_ID), ERROR_NONE); + app_scheduler_set_current_app_id(TRACKED_ID); + deleted_tasks = 0; + TaskParameters parameters { TRACKED_ID, true }; + CHECK(app_resources_create_task(create_task, ¶meters, delete_task, nullptr)); + app_scheduler_set_current_app_id(0); + + app_resources_release_tasks(TRACKED_ID); + app_resources_release(TRACKED_ID); + CHECK_EQ(deleted_tasks.load(), 0); +} + +TEST_CASE("app resources: a task still running after the grace period is deleted") { + REQUIRE_EQ(app_resources_register(TRACKED_ID), ERROR_NONE); + app_scheduler_set_current_app_id(TRACKED_ID); + deleted_tasks = 0; + TaskParameters parameters { TRACKED_ID, false }; + CHECK(app_resources_create_task(create_task, ¶meters, delete_task, nullptr)); + app_scheduler_set_current_app_id(0); + + app_resources_release_tasks(TRACKED_ID); + app_resources_release(TRACKED_ID); + CHECK_EQ(deleted_tasks.load(), 1); +} + +TEST_CASE("app resources: what is freed and closed between the two phases is not released again") { + REQUIRE_EQ(app_resources_register(TRACKED_ID), ERROR_NONE); + app_scheduler_set_current_app_id(TRACKED_ID); + void* memory = malloc(16); + app_resources_track_alloc(memory); + const int fd = open("/dev/null", O_RDONLY); + app_resources_track_fd(fd); + app_scheduler_set_current_app_id(0); + + app_resources_release_tasks(TRACKED_ID); + CHECK_FALSE(app_resources_enter_task(TRACKED_ID)); + + // As the binary's static destructors do while it's unloaded + app_resources_untrack_alloc(memory); + free(memory); + app_resources_untrack_fd(fd); + close(fd); + const int reused_fd = open("/dev/null", O_RDONLY); + REQUIRE_EQ(reused_fd, fd); + + app_resources_release(TRACKED_ID); + CHECK(is_open(reused_fd)); + close(reused_fd); +} diff --git a/Modules/app-posix-module/private/app_posix/malloc_wrap.h b/Modules/app-posix-module/private/app_posix/malloc_wrap.h index 68d685bfa..cc3737dfa 100644 --- a/Modules/app-posix-module/private/app_posix/malloc_wrap.h +++ b/Modules/app-posix-module/private/app_posix/malloc_wrap.h @@ -11,9 +11,22 @@ */ void app_posix_set_current_image(uintptr_t start, uintptr_t end); +/** Gets the range set by app_posix_set_current_image() on the calling thread, for a thread the app creates. */ +void app_posix_get_current_image(uintptr_t* out_start, uintptr_t* out_end); + +/** @return true if @a caller lies inside the image of the app running on the calling thread */ +bool app_posix_is_app_caller(const void* caller); + #else -// Allocations aren't counted on Apple platforms +// Allocations aren't counted, and resources aren't tracked, on Apple platforms inline void app_posix_set_current_image(uintptr_t, uintptr_t) {} +inline void app_posix_get_current_image(uintptr_t* out_start, uintptr_t* out_end) { + *out_start = 0; + *out_end = 0; +} + +inline bool app_posix_is_app_caller(const void*) { return false; } + #endif diff --git a/Modules/app-posix-module/private/app_posix/stdio_wrap.h b/Modules/app-posix-module/private/app_posix/stdio_wrap.h index 32997e9a8..b1fc1d006 100644 --- a/Modules/app-posix-module/private/app_posix/stdio_wrap.h +++ b/Modules/app-posix-module/private/app_posix/stdio_wrap.h @@ -4,6 +4,8 @@ #include #include #include +#include +#include #include #include #include @@ -14,6 +16,21 @@ #include +/** + * While alive, the path-based wraps called on this thread treat the call as made by the app's own code + * when @a caller lies inside its image, and resolve its relative paths against the app's cwd (see app/libc.h). + * Code built into the simulator keeps the process's cwd, which its relative mount points (e.g. "system") rely on. + */ +class AppPathCallScope { + bool previous; + +public: + explicit AppPathCallScope(const void* caller); + ~AppPathCallScope(); + AppPathCallScope(const AppPathCallScope&) = delete; + AppPathCallScope& operator=(const AppPathCallScope&) = delete; +}; + // Implemented in stdio_wrap.cpp, installed under the real names by stdio_wrap_elf.cpp or stdio_wrap_apple.cpp. extern "C" { ssize_t __wrap_read(int fd, void* buffer, size_t size); @@ -34,6 +51,22 @@ int __wrap_usleep(useconds_t usec); unsigned int __wrap_sleep(unsigned int seconds); [[noreturn]] void __wrap_exit(int status); +int __wrap_open(const char* path, int flags, ...); +FILE* __wrap_fopen(const char* path, const char* mode); +int __wrap_stat(const char* path, struct stat* st); +int __wrap_lstat(const char* path, struct stat* st); +int __wrap_access(const char* path, int mode); +int __wrap_unlink(const char* path); +int __wrap_remove(const char* path); +int __wrap_rename(const char* src, const char* dst); +int __wrap_mkdir(const char* path, mode_t mode); +int __wrap_rmdir(const char* path); +DIR* __wrap_opendir(const char* path); +int __real_fclose(FILE* file); +FILE* __real_fdopen(int fd, const char* mode); +int __real_closedir(DIR* dir); +int __wrap_truncate(const char* path, off_t length); + int __wrap_vprintf(const char* format, va_list args); int __wrap_printf(const char* format, ...); int __wrap_vfprintf(FILE* stream, const char* format, va_list args); diff --git a/Modules/app-posix-module/private/app_posix/task_wrap.h b/Modules/app-posix-module/private/app_posix/task_wrap.h new file mode 100644 index 000000000..837a25c97 --- /dev/null +++ b/Modules/app-posix-module/private/app_posix/task_wrap.h @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#ifndef __APPLE__ + +/** Routes the tasks and threads an app creates through resource tracking (see app/resources.h). Idempotent. */ +void app_posix_install_task_hooks(); + +#else + +// Resources aren't tracked on Apple platforms +inline void app_posix_install_task_hooks() {} + +#endif diff --git a/Modules/app-posix-module/source/app_posix_loader_service.cpp b/Modules/app-posix-module/source/app_posix_loader_service.cpp index 7fe7e0a11..fccc45df6 100644 --- a/Modules/app-posix-module/source/app_posix_loader_service.cpp +++ b/Modules/app-posix-module/source/app_posix_loader_service.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include @@ -82,7 +83,7 @@ bool is_regular_file(const std::string& path) { constexpr ElfRequirements EXECUTABLE_REQUIREMENTS = { .elf_class = ELF_CLASS_64, .data = ELF_DATA_2LSB, - .type = ELF_TYPE_DYN, + .types = ELF_TYPE_MASK(ELF_TYPE_DYN), #if defined(__x86_64__) .machine = ELF_MACHINE_X86_64, #elif defined(__aarch64__) @@ -146,6 +147,8 @@ error_t api_load(AppLocation location, AppRuntime* out_runtime) { LOG_I(TAG, "Loading %s", app_path.c_str()); + app_posix_install_task_hooks(); + void* handle = dlopen(app_path.c_str(), RTLD_NOW | RTLD_LOCAL); if (handle == nullptr) { LOG_E(TAG, "dlopen(%s) failed: %s", app_path.c_str(), dlerror()); @@ -188,7 +191,10 @@ int32_t api_run(AppRuntime runtime_ptr, uint32_t /*app_instance_id*/, int argc, void api_unload(AppRuntime runtime_ptr) { auto* runtime = static_cast(runtime_ptr); + // The image's static destructors run here, and what they free is the app's own (see app/memory.h) + app_posix_set_current_image(runtime->image_start, runtime->image_end); dlclose(runtime->handle); + app_posix_set_current_image(0, 0); delete runtime; } diff --git a/Modules/app-posix-module/source/malloc_wrap.cpp b/Modules/app-posix-module/source/malloc_wrap.cpp index ab1e34ec3..0901d70bf 100644 --- a/Modules/app-posix-module/source/malloc_wrap.cpp +++ b/Modules/app-posix-module/source/malloc_wrap.cpp @@ -8,6 +8,7 @@ #include #include +#include #include @@ -36,6 +37,7 @@ inline bool is_app_caller(void* caller) { inline void* record_alloc(void* ptr, void* caller) { if (ptr != nullptr && is_app_caller(caller)) { app_memory_record_alloc(malloc_usable_size(ptr)); + app_resources_track_alloc(ptr); } return ptr; } @@ -43,6 +45,7 @@ inline void* record_alloc(void* ptr, void* caller) { inline void record_free(void* ptr, void* caller) { if (ptr != nullptr && is_app_caller(caller)) { app_memory_record_free(malloc_usable_size(ptr)); + app_resources_untrack_alloc(ptr); } } @@ -72,6 +75,15 @@ void app_posix_set_current_image(uintptr_t start, uintptr_t end) { image_end = end; } +void app_posix_get_current_image(uintptr_t* out_start, uintptr_t* out_end) { + *out_start = image_start; + *out_end = image_end; +} + +bool app_posix_is_app_caller(const void* caller) { + return is_app_caller(const_cast(caller)); +} + extern "C" { void* malloc(size_t size) { @@ -88,6 +100,8 @@ void* realloc(void* ptr, size_t size) { return __libc_realloc(ptr, size); } const size_t old_size = (ptr != nullptr) ? malloc_usable_size(ptr) : 0; + // Untracked before ptr may be freed + app_resources_untrack_alloc(ptr); void* result = __libc_realloc(ptr, size); // A failed realloc() leaves the old block allocated if (result != nullptr || size == 0) { @@ -96,7 +110,10 @@ void* realloc(void* ptr, size_t size) { } if (result != nullptr) { app_memory_record_alloc(malloc_usable_size(result)); + app_resources_track_alloc(result); } + } else if (ptr != nullptr) { + app_resources_track_alloc(ptr); } return result; } diff --git a/Modules/app-posix-module/source/stdio_wrap.cpp b/Modules/app-posix-module/source/stdio_wrap.cpp index 4a294fc6f..d076af162 100644 --- a/Modules/app-posix-module/source/stdio_wrap.cpp +++ b/Modules/app-posix-module/source/stdio_wrap.cpp @@ -5,6 +5,7 @@ // a dlopen()ed app's own printf/write calls, so these wraps are installed under their real names instead // - dyld interpose on Apple (stdio_wrap_apple.cpp), plain strong definitions elsewhere (stdio_wrap_elf.cpp). // app-module's io.cpp falls through to the __real_read/__real_write/__real_close defined here. +#include #include #include @@ -12,6 +13,8 @@ #include #include +#include + #include #include @@ -266,6 +269,247 @@ void __wrap_exit(int status) { // endregion +// region path wraps +// +// The process has a single cwd, so the relative paths of an app binary's own calls are resolved against +// the app instance's cwd first (see app/libc.h). Every other caller's paths are passed on as-is. + +namespace { + +// Set by AppPathCallScope. Never set on Apple platforms, where the caller can't be identified. +thread_local bool app_path_call = false; + +/** + * @return the path to pass on: resolved for an app binary's own call, as-is otherwise. + * NULL when the resolved path doesn't fit (errno is set to ENAMETOOLONG). + */ +const char* resolvePath(const char* path, char (&buffer)[FILE_MAX_PATH_STRING_LENGTH]) { + if (!app_path_call) { + return path; + } + const char* resolved; + return app_libc_try_resolve_path(path, buffer, sizeof(buffer), &resolved) ? resolved : path; +} + +/** open() only reads its mode argument when it can create a file */ +bool openNeedsMode(int flags) { +#ifdef O_TMPFILE + if ((flags & O_TMPFILE) == O_TMPFILE) { + return true; + } +#endif + return (flags & O_CREAT) != 0; +} + +} // namespace + +AppPathCallScope::AppPathCallScope(const void* caller) : previous(app_path_call) { + app_path_call = app_posix_is_app_caller(caller); +} + +AppPathCallScope::~AppPathCallScope() { + app_path_call = previous; +} + +extern "C" { + +int __real_open(const char* path, int flags, mode_t mode) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "open")); + return real(path, flags, mode); +} + +FILE* __real_fopen(const char* path, const char* mode) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fopen")); + return real(path, mode); +} + +int __real_stat(const char* path, struct stat* st) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "stat")); + return real(path, st); +} + +int __real_lstat(const char* path, struct stat* st) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "lstat")); + return real(path, st); +} + +int __real_access(const char* path, int mode) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "access")); + return real(path, mode); +} + +int __real_unlink(const char* path) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "unlink")); + return real(path); +} + +int __real_remove(const char* path) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "remove")); + return real(path); +} + +int __real_rename(const char* src, const char* dst) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "rename")); + return real(src, dst); +} + +int __real_mkdir(const char* path, mode_t mode) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "mkdir")); + return real(path, mode); +} + +int __real_rmdir(const char* path) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "rmdir")); + return real(path); +} + +DIR* __real_opendir(const char* path) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "opendir")); + return real(path); +} + +int __real_fclose(FILE* file) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fclose")); + return real(file); +} + +FILE* __real_fdopen(int fd, const char* mode) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "fdopen")); + return real(fd, mode); +} + +int __real_closedir(DIR* dir) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "closedir")); + return real(dir); +} + +int __real_truncate(const char* path, off_t length) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "truncate")); + return real(path, length); +} + +int __wrap_open(const char* path, int flags, ...) { + mode_t mode = 0; + if (openNeedsMode(flags)) { + va_list args; + va_start(args, flags); + mode = static_cast(va_arg(args, int)); + va_end(args); + } + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_open(resolved, flags, mode); +} + +// libc's own fopen() calls an internal open() alias, which the open() wrap can't reach +FILE* __wrap_fopen(const char* path, const char* mode) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return nullptr; + } + return __real_fopen(resolved, mode); +} + +int __wrap_stat(const char* path, struct stat* st) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_stat(resolved, st); +} + +int __wrap_lstat(const char* path, struct stat* st) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_lstat(resolved, st); +} + +int __wrap_access(const char* path, int mode) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_access(resolved, mode); +} + +int __wrap_unlink(const char* path) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_unlink(resolved); +} + +int __wrap_remove(const char* path) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_remove(resolved); +} + +int __wrap_rename(const char* src, const char* dst) { + char src_buffer[FILE_MAX_PATH_STRING_LENGTH]; + char dst_buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved_src = resolvePath(src, src_buffer); + const char* resolved_dst = resolvePath(dst, dst_buffer); + if (resolved_src == nullptr || resolved_dst == nullptr) { + return -1; + } + return __real_rename(resolved_src, resolved_dst); +} + +int __wrap_mkdir(const char* path, mode_t mode) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_mkdir(resolved, mode); +} + +int __wrap_rmdir(const char* path) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_rmdir(resolved); +} + +DIR* __wrap_opendir(const char* path) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return nullptr; + } + return __real_opendir(resolved); +} + +int __wrap_truncate(const char* path, off_t length) { + char buffer[FILE_MAX_PATH_STRING_LENGTH]; + const char* resolved = resolvePath(path, buffer); + if (resolved == nullptr) { + return -1; + } + return __real_truncate(resolved, length); +} + +} + +// endregion + // region stdio wraps // // libc's printf/fprintf/etc call an internal, non-exported write() alias that the read/write/close diff --git a/Modules/app-posix-module/source/stdio_wrap_apple.cpp b/Modules/app-posix-module/source/stdio_wrap_apple.cpp index 048e203cd..e60692eec 100644 --- a/Modules/app-posix-module/source/stdio_wrap_apple.cpp +++ b/Modules/app-posix-module/source/stdio_wrap_apple.cpp @@ -32,6 +32,19 @@ TT_DYLD_INTERPOSE(__wrap_getppid, getppid) TT_DYLD_INTERPOSE(__wrap_usleep, usleep) TT_DYLD_INTERPOSE(__wrap_sleep, sleep) +TT_DYLD_INTERPOSE(__wrap_open, open) +TT_DYLD_INTERPOSE(__wrap_fopen, fopen) +TT_DYLD_INTERPOSE(__wrap_stat, stat) +TT_DYLD_INTERPOSE(__wrap_lstat, lstat) +TT_DYLD_INTERPOSE(__wrap_access, access) +TT_DYLD_INTERPOSE(__wrap_unlink, unlink) +TT_DYLD_INTERPOSE(__wrap_remove, remove) +TT_DYLD_INTERPOSE(__wrap_rename, rename) +TT_DYLD_INTERPOSE(__wrap_mkdir, mkdir) +TT_DYLD_INTERPOSE(__wrap_rmdir, rmdir) +TT_DYLD_INTERPOSE(__wrap_opendir, opendir) +TT_DYLD_INTERPOSE(__wrap_truncate, truncate) + TT_DYLD_INTERPOSE(__wrap_vprintf, vprintf) TT_DYLD_INTERPOSE(__wrap_printf, printf) TT_DYLD_INTERPOSE(__wrap_vfprintf, vfprintf) diff --git a/Modules/app-posix-module/source/stdio_wrap_elf.cpp b/Modules/app-posix-module/source/stdio_wrap_elf.cpp index e2b70a1b3..1a7eac6a0 100644 --- a/Modules/app-posix-module/source/stdio_wrap_elf.cpp +++ b/Modules/app-posix-module/source/stdio_wrap_elf.cpp @@ -3,8 +3,12 @@ // Plain strong definitions: ELF gives the main executable's symbols priority process-wide, // including for a dlopen()ed app's own calls. +// Files opened by an app's own code are tracked here, where the caller is still known (see app/resources.h). +#include #include +#include + extern "C" { ssize_t read(int fd, void* buffer, size_t size) { @@ -16,6 +20,9 @@ ssize_t write(int fd, const void* buffer, size_t size) { } int close(int fd) { + if (app_posix_is_app_caller(__builtin_return_address(0))) { + app_resources_untrack_fd(fd); + } return __wrap_close(fd); } @@ -79,6 +86,114 @@ void exit(int status) { __wrap_exit(status); } +int open(const char* path, int flags, ...) { + // The mode is only passed (and read) when the file can be created + bool has_mode = (flags & O_CREAT) != 0; +#ifdef O_TMPFILE + has_mode = has_mode || (flags & O_TMPFILE) == O_TMPFILE; +#endif + mode_t mode = 0; + if (has_mode) { + va_list args; + va_start(args, flags); + mode = static_cast(va_arg(args, int)); + va_end(args); + } + AppPathCallScope scope(__builtin_return_address(0)); + const int fd = __wrap_open(path, flags, mode); + if (fd >= 0 && app_posix_is_app_caller(__builtin_return_address(0))) { + app_resources_track_fd(fd); + } + return fd; +} + +FILE* fopen(const char* path, const char* mode) { + AppPathCallScope scope(__builtin_return_address(0)); + FILE* file = __wrap_fopen(path, mode); + if (file != nullptr && app_posix_is_app_caller(__builtin_return_address(0))) { + app_resources_track_file(file); + } + return file; +} + +FILE* fdopen(int fd, const char* mode) { + FILE* file = __real_fdopen(fd, mode); + if (file != nullptr && app_posix_is_app_caller(__builtin_return_address(0))) { + // Closed through the FILE from now on + app_resources_untrack_fd(fd); + app_resources_track_file(file); + } + return file; +} + +int fclose(FILE* file) { + if (app_posix_is_app_caller(__builtin_return_address(0))) { + app_resources_untrack_file(file); + } + return __real_fclose(file); +} + +int stat(const char* path, struct stat* st) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_stat(path, st); +} + +int lstat(const char* path, struct stat* st) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_lstat(path, st); +} + +int access(const char* path, int mode) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_access(path, mode); +} + +int unlink(const char* path) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_unlink(path); +} + +int remove(const char* path) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_remove(path); +} + +int rename(const char* src, const char* dst) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_rename(src, dst); +} + +int mkdir(const char* path, mode_t mode) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_mkdir(path, mode); +} + +int rmdir(const char* path) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_rmdir(path); +} + +DIR* opendir(const char* path) { + AppPathCallScope scope(__builtin_return_address(0)); + DIR* dir = __wrap_opendir(path); + if (dir != nullptr && app_posix_is_app_caller(__builtin_return_address(0))) { + app_resources_track_dir(dir); + } + return dir; +} + +int closedir(DIR* dir) { + if (app_posix_is_app_caller(__builtin_return_address(0))) { + app_resources_untrack_dir(dir); + } + return __real_closedir(dir); +} + +int truncate(const char* path, off_t length) { + AppPathCallScope scope(__builtin_return_address(0)); + return __wrap_truncate(path, length); +} + int vprintf(const char* format, va_list args) { return __wrap_vprintf(format, args); } diff --git a/Modules/app-posix-module/source/task_wrap.cpp b/Modules/app-posix-module/source/task_wrap.cpp new file mode 100644 index 000000000..e506d1c62 --- /dev/null +++ b/Modules/app-posix-module/source/task_wrap.cpp @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 +#ifndef __APPLE__ + +// Tasks created by an app's own code are tracked (see app/resources.h). +// FreeRTOS tasks go through the hooks of freertos_task_hooks.h, pthreads through a plain strong definition, +// like the ones in stdio_wrap_elf.cpp. A task the app creates runs as part of that app, within its image. +#include +#include + +#include + +#include + +#include +#include + +#include +#include +#include +#include + +namespace { + +struct TaskStart { + AppInstanceId app_instance_id; + uintptr_t image_start; + uintptr_t image_end; +}; + +/** Allocates the parameters for a trampoline, also tracked as an allocation of the app, so it's freed when the task never ran. */ +template +Start* start_create() { + auto* start = static_cast(malloc(sizeof(Start))); + if (start != nullptr) { + start->task.app_instance_id = app_resources_current_app(); + app_posix_get_current_image(&start->task.image_start, &start->task.image_end); + app_resources_track_alloc(start); + } + return start; +} + +template +void start_free(Start* start) { + app_resources_untrack_alloc(start); + free(start); +} + +/** + * @param[in] freertos_task false for a plain pthread, which can't take part in the app-libc paths that use FreeRTOS + * @return false when the task must end without running app code + */ +bool enter_task(const TaskStart& start, bool freertos_task) { + const bool entered = freertos_task ? app_resources_enter_task(start.app_instance_id) : app_resources_enter_thread(start.app_instance_id); + if (!entered) { + return false; + } + app_posix_set_current_image(start.image_start, start.image_end); + return true; +} + +// region FreeRTOS tasks + +struct FreeRtosTaskStart { + TaskStart task; + TaskFunction_t function; + void* parameter; +}; + +void freertos_task_trampoline(void* context) { + auto* start = static_cast(context); + // Only fails when app_resources_release() is about to delete this task + if (!enter_task(start->task, true)) { + vTaskSuspend(nullptr); + } + const TaskFunction_t function = start->function; + void* parameter = start->parameter; + start_free(start); + function(parameter); +} + +void freertos_task_delete(void* handle) { + freertos_real_vTaskDelete(static_cast(handle)); +} + +struct FreeRtosTaskCreate { + FreeRtosTaskStart* start; + const char* name; + configSTACK_DEPTH_TYPE stack_depth; + UBaseType_t priority; + TaskHandle_t created; +}; + +bool freertos_task_create(void* context, void** out_handle) { + auto* create = static_cast(context); + if (freertos_real_xTaskCreate(freertos_task_trampoline, create->name, create->stack_depth, create->start, create->priority, &create->created) != pdPASS) { + return false; + } + *out_handle = create->created; + return true; +} + +BaseType_t create_hook(const void* caller, TaskFunction_t function, const char* name, configSTACK_DEPTH_TYPE stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle) { + if (!app_posix_is_app_caller(caller) || app_resources_current_app() == 0) { + return freertos_real_xTaskCreate(function, name, stack_depth, parameter, priority, out_handle); + } + auto* start = start_create(); + if (start == nullptr) { + return errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY; + } + start->function = function; + start->parameter = parameter; + FreeRtosTaskCreate create { start, name, stack_depth, priority, nullptr }; + if (!app_resources_create_task(freertos_task_create, &create, freertos_task_delete, nullptr)) { + start_free(start); + return errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY; + } + if (out_handle != nullptr) { + *out_handle = create.created; + } + return pdPASS; +} + +void delete_hook(const void* caller, TaskHandle_t handle) { + if (app_posix_is_app_caller(caller)) { + app_resources_untrack_task(handle != nullptr ? handle : xTaskGetCurrentTaskHandle()); + } + freertos_real_vTaskDelete(handle); +} + +// endregion + +// region pthreads + +struct ThreadStart { + TaskStart task; + void* (*function)(void*); + void* argument; +}; + +void* thread_key(pthread_t thread) { + return reinterpret_cast(static_cast(thread)); +} + +int real_pthread_create(pthread_t* thread, const pthread_attr_t* attributes, void* (*function)(void*), void* argument) { + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "pthread_create")); + return real(thread, attributes, function, argument); +} + +void* thread_trampoline(void* context) { + auto* start = static_cast(context); + if (!enter_task(start->task, false)) { + return nullptr; + } + void* (*function)(void*) = start->function; + void* argument = start->argument; + start_free(start); + void* result = function(argument); + app_resources_untrack_task(thread_key(pthread_self())); + return result; +} + +constexpr long THREAD_JOIN_TIMEOUT_MS = 500; + +pthread_t thread_of(void* handle) { + return static_cast(reinterpret_cast(handle)); +} + +// Only takes effect at the thread's next cancellation point +void thread_delete(void* handle) { + pthread_cancel(thread_of(handle)); +} + +// A detached thread can't be joined, and might still be running app code when the app is unloaded +void thread_join(void* handle) { + timespec deadline {}; + clock_gettime(CLOCK_REALTIME, &deadline); + deadline.tv_nsec += THREAD_JOIN_TIMEOUT_MS * 1000000L; + deadline.tv_sec += deadline.tv_nsec / 1000000000L; + deadline.tv_nsec %= 1000000000L; + pthread_timedjoin_np(thread_of(handle), nullptr, &deadline); +} + +struct ThreadCreate { + pthread_t* thread; + const pthread_attr_t* attributes; + ThreadStart* start; +}; + +bool thread_create(void* context, void** out_handle) { + const auto* create = static_cast(context); + if (real_pthread_create(create->thread, create->attributes, thread_trampoline, create->start) != 0) { + return false; + } + *out_handle = thread_key(*create->thread); + return true; +} + +// endregion + +} // namespace + +// Never uninstalled: the hooks pass every call through unless an app tracks its resources +void app_posix_install_task_hooks() { + freertos_set_task_hooks(create_hook, delete_hook); +} + +extern "C" { + +int pthread_create(pthread_t* thread, const pthread_attr_t* attributes, void* (*function)(void*), void* argument) { + if (!app_posix_is_app_caller(__builtin_return_address(0)) || app_resources_current_app() == 0) { + return real_pthread_create(thread, attributes, function, argument); + } + auto* start = start_create(); + if (start == nullptr) { + return ENOMEM; + } + start->function = function; + start->argument = argument; + ThreadCreate create { thread, attributes, start }; + if (!app_resources_create_task(thread_create, &create, thread_delete, thread_join)) { + start_free(start); + return EAGAIN; + } + return 0; +} + +void pthread_exit(void* result) { + app_resources_untrack_task(thread_key(pthread_self())); + static auto real = reinterpret_cast(dlsym(RTLD_NEXT, "pthread_exit")); + real(result); + __builtin_unreachable(); +} + +} // extern "C" + +#endif diff --git a/Modules/app-posix-module/tests/CMakeLists.txt b/Modules/app-posix-module/tests/CMakeLists.txt index ae0559c87..fbf8f7bdf 100644 --- a/Modules/app-posix-module/tests/CMakeLists.txt +++ b/Modules/app-posix-module/tests/CMakeLists.txt @@ -22,6 +22,19 @@ set_target_properties(printf_fixture PROPERTIES POSITION_INDEPENDENT_CODE ON) add_library(exit_fixture SHARED EXCLUDE_FROM_ALL ${CMAKE_CURRENT_LIST_DIR}/fixtures/exit_fixture.cpp) set_target_properties(exit_fixture PROPERTIES POSITION_INDEPENDENT_CODE ON) +# Fixture: leaks files, memory and optionally a thread, for APP_MANIFEST_FLAG_CLEANUP tests. +add_library(leak_fixture SHARED EXCLUDE_FROM_ALL ${CMAKE_CURRENT_LIST_DIR}/fixtures/leak_fixture.cpp) +set_target_properties(leak_fixture PROPERTIES POSITION_INDEPENDENT_CODE ON) + +# Fixture: static objects whose destructors free and close what the app acquired, when it's unloaded. +add_library(static_fixture SHARED EXCLUDE_FROM_ALL ${CMAKE_CURRENT_LIST_DIR}/fixtures/static_fixture.cpp) +set_target_properties(static_fixture PROPERTIES POSITION_INDEPENDENT_CODE ON) + +# Fixture: makes path-based calls with relative paths, which must resolve against the app's own cwd. +add_library(paths_fixture SHARED EXCLUDE_FROM_ALL ${CMAKE_CURRENT_LIST_DIR}/fixtures/paths_fixture.cpp) +target_include_directories(paths_fixture PRIVATE ${CMAKE_SOURCE_DIR}/TactilityKernel/include) +set_target_properties(paths_fixture PROPERTIES POSITION_INDEPENDENT_CODE ON) + # A file with a ".so" extension but no ELF header, for is_executable() rejection tests. set(NON_ELF_FIXTURE_PATH "${CMAKE_CURRENT_BINARY_DIR}/not-elf.so") file(WRITE "${NON_ELF_FIXTURE_PATH}" "not an elf file") @@ -40,13 +53,16 @@ add_custom_target(install_dir_fixture DEPENDS "${INSTALL_DIR_FIXTURE_BIN_DIR}/ap file(GLOB_RECURSE TEST_SOURCES CONFIGURE_DEPENDS ${PROJECT_SOURCE_DIR}/source/*.cpp) add_executable(AppPosixModuleTests EXCLUDE_FROM_ALL ${TEST_SOURCES}) -add_dependencies(AppPosixModuleTests app_posix_module_test_fixture printf_fixture exit_fixture install_dir_fixture) +add_dependencies(AppPosixModuleTests app_posix_module_test_fixture printf_fixture exit_fixture leak_fixture paths_fixture static_fixture install_dir_fixture) target_include_directories(AppPosixModuleTests PRIVATE ${DOCTESTINC}) target_compile_definitions(AppPosixModuleTests PRIVATE FIXTURE_APP_PATH="$" PRINTF_FIXTURE_APP_PATH="$" EXIT_FIXTURE_APP_PATH="$" + LEAK_FIXTURE_APP_PATH="$" + PATHS_FIXTURE_APP_PATH="$" + STATIC_FIXTURE_APP_PATH="$" FIXTURE_NON_ELF_PATH="${NON_ELF_FIXTURE_PATH}" FIXTURE_INSTALL_DIR_PATH="${INSTALL_DIR_FIXTURE_PATH}" ) diff --git a/Modules/app-posix-module/tests/fixtures/leak_fixture.cpp b/Modules/app-posix-module/tests/fixtures/leak_fixture.cpp new file mode 100644 index 000000000..b69df16e7 --- /dev/null +++ b/Modules/app-posix-module/tests/fixtures/leak_fixture.cpp @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace { + +void* run_forever(void*) { + while (true) { + sleep(1); + } + return nullptr; +} + +} // namespace + +// Leaks an fd, a FILE, a DIR and memory, and writes their fds to argv[1]. +// With "thread" as argv[2], it also leaves a thread running. +extern "C" int32_t main(int argc, char* argv[]) { + if (argc < 2) { + return 1; + } + const int fd = open("/dev/null", O_RDONLY); + FILE* file = fopen("/dev/null", "r"); + DIR* dir = opendir("/"); + void* memory = malloc(64); + if (fd < 0 || file == nullptr || dir == nullptr || memory == nullptr) { + return 2; + } + if (argc > 2 && strcmp(argv[2], "thread") == 0) { + pthread_t thread; + if (pthread_create(&thread, nullptr, run_forever, nullptr) != 0) { + return 3; + } + } + FILE* output = fopen(argv[1], "w"); + if (output == nullptr) { + return 4; + } + fprintf(output, "%d %d %d", fd, fileno(file), dirfd(dir)); + fclose(output); + return 0; +} diff --git a/Modules/app-posix-module/tests/fixtures/paths_fixture.cpp b/Modules/app-posix-module/tests/fixtures/paths_fixture.cpp new file mode 100644 index 000000000..10d229d5c --- /dev/null +++ b/Modules/app-posix-module/tests/fixtures/paths_fixture.cpp @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: Apache-2.0 +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace { + +/** @return the number of the first check that failed, 0 when all passed */ +int run_relative_path_checks(const std::string& directory) { + if (chdir(directory.c_str()) != 0) { + return 1; + } + + int fd = open("file.txt", O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (fd < 0) { + return 2; + } + if (write(fd, "abc", 3) != 3) { + return 3; + } + close(fd); + + if (mkdir("sub", 0755) != 0) { + return 4; + } + struct stat st {}; + if (stat("./sub", &st) != 0 || !S_ISDIR(st.st_mode)) { + return 5; + } + DIR* dir = opendir("sub"); + if (dir == nullptr) { + return 6; + } + closedir(dir); + + // "." and ".." segments are collapsed + FILE* file = fopen("sub/.././file.txt", "r"); + if (file == nullptr) { + return 7; + } + char content[4] = {}; + const size_t read_count = fread(content, 1, 3, file); + fclose(file); + if (read_count != 3 || strcmp(content, "abc") != 0) { + return 8; + } + + if (rename("file.txt", "sub/moved.txt") != 0) { + return 9; + } + if (access("sub/moved.txt", F_OK) != 0 || access("file.txt", F_OK) == 0) { + return 10; + } + if (truncate("sub/moved.txt", 1) != 0 || stat("sub/moved.txt", &st) != 0 || st.st_size != 1) { + return 11; + } + if (unlink("sub/moved.txt") != 0) { + return 12; + } + if (rmdir("sub") != 0) { + return 13; + } + + // Left behind for the test to find in the app's cwd, rather than in the process's + fd = open("kept.txt", O_WRONLY | O_CREAT, 0644); + if (fd < 0) { + return 14; + } + close(fd); + + // chdir() collapses ".." too + if (chdir("..") != 0) { + return 15; + } + char cwd[FILE_MAX_PATH_STRING_LENGTH]; + const std::string parent = directory.substr(0, directory.rfind('/')); + if (getcwd(cwd, sizeof(cwd)) == nullptr || parent != cwd) { + return 16; + } + + const std::string too_long(FILE_MAX_PATH_STRING_LENGTH, 'a'); + if (open(too_long.c_str(), O_RDONLY) != -1 || errno != ENAMETOOLONG) { + return 17; + } + + if (chdir(directory.c_str()) != 0) { + return 18; + } + + // A trailing separator requires a directory + fd = open("plain", O_WRONLY | O_CREAT, 0644); + if (fd < 0) { + return 19; + } + close(fd); + if (unlink("plain/") != -1 || errno != ENOTDIR || access("plain", F_OK) != 0) { + return 20; + } + unlink("plain"); + + // ".." after a symlink goes to the parent of its target + // symlink() isn't resolved against the app's cwd, so it gets absolute paths + const std::string link_target = directory + "/real/inner"; + const std::string link_path = directory + "/link"; + if (mkdir("real", 0755) != 0 || mkdir("real/inner", 0755) != 0 || symlink(link_target.c_str(), link_path.c_str()) != 0) { + return 21; + } + fd = open("real/marker", O_WRONLY | O_CREAT, 0644); + if (fd < 0) { + return 22; + } + close(fd); + if (access("link/../marker", F_OK) != 0) { + return 23; + } + unlink("real/marker"); + unlink("link"); + rmdir("real/inner"); + rmdir("real"); + + return 0; +} + +} // namespace + +// Runs the checks in the directory argv[1] and writes the number of the first one that failed to argv[2] (absolute) +extern "C" int32_t main(int argc, char* argv[]) { + if (argc < 3) { + return 1; + } + const int failed_check = run_relative_path_checks(argv[1]); + FILE* output = fopen(argv[2], "w"); + if (output == nullptr) { + return 2; + } + fprintf(output, "%d", failed_check); + fclose(output); + return 0; +} diff --git a/Modules/app-posix-module/tests/fixtures/static_fixture.cpp b/Modules/app-posix-module/tests/fixtures/static_fixture.cpp new file mode 100644 index 000000000..9e6e567ca --- /dev/null +++ b/Modules/app-posix-module/tests/fixtures/static_fixture.cpp @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: Apache-2.0 +#include +#include +#include +#include +#include + +namespace { + +struct FileHolder { + FILE* file = nullptr; + + ~FileHolder() { + if (file != nullptr) { + fclose(file); + } + } +}; + +// Destroyed when the binary is unloaded, after the app ended +std::vector buffer; +FileHolder holder; + +} // namespace + +// Fills static objects with memory and a file their destructors release. Ends with exit() when argv[1] is "exit". +extern "C" int32_t main(int argc, char* argv[]) { + buffer.resize(4096); + holder.file = fopen("/dev/null", "r"); + if (holder.file == nullptr) { + return 1; + } + if (argc > 1 && strcmp(argv[1], "exit") == 0) { + exit(0); + } + return 0; +} diff --git a/Modules/app-posix-module/tests/source/cleanup_test.cpp b/Modules/app-posix-module/tests/source/cleanup_test.cpp new file mode 100644 index 000000000..623efdbbc --- /dev/null +++ b/Modules/app-posix-module/tests/source/cleanup_test.cpp @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: Apache-2.0 +#include "doctest.h" + +#include +#include +#include + +#include + +#include + +#include +#include + +#include +#include + +extern ServiceManifest loader_service_manifest; + +namespace { + +struct LeakedFds { + int fd = -1; + int file_fd = -1; + int dir_fd = -1; +}; + +bool is_open(int fd) { + return fcntl(fd, F_GETFD) != -1; +} + +void ensure_path_loader_registered() { + if (service_manager_find_instance(APP_LOADER_PATH_SERVICE_ID) == nullptr) { + service_manager_add(&loader_service_manifest, /*auto_start=*/true); + } +} + +/** Runs the leak fixture as a top-level app and reads back the fds it leaked. */ +bool run_leak_fixture(uint8_t flags, bool with_thread, LeakedFds& out_fds) { + ensure_path_loader_registered(); + + char output_path[] = "/tmp/cleanup_test_XXXXXX"; + const int output_fd = mkstemp(output_path); + if (output_fd == -1) { + return false; + } + close(output_fd); + + AppManifest manifest { "test.posix.leak", "Leak", APP_CATEGORY_USER, { APP_LOCATION_PATH, const_cast(LEAK_FIXTURE_APP_PATH) }, flags }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + const char* argv[] = { LEAK_FIXTURE_APP_PATH, output_path, with_thread ? "thread" : "" }; + AppStartContext context; + REQUIRE_EQ(app_start_context_from_id("test.posix.leak", &context), ERROR_NONE); + app_start_context_set_arguments_ext(&context, 3, argv); + AppInstanceId app_instance_id = 0; + REQUIRE_EQ(app_start_with_context(&context, &app_instance_id), ERROR_NONE); + + // Beyond the task grace period and thread join timeout + for (int waited = 0; waited < 5000 && app_manager_get_state(app_instance_id) != APP_INSTANCE_STATE_STOPPED; waited += 10) { + delay_millis(10); + } + const bool stopped = app_manager_get_state(app_instance_id) == APP_INSTANCE_STATE_STOPPED; + app_manager_remove("test.posix.leak"); + + FILE* output = fopen(output_path, "r"); + const bool read = output != nullptr && fscanf(output, "%d %d %d", &out_fds.fd, &out_fds.file_fd, &out_fds.dir_fd) == 3; + if (output != nullptr) { + fclose(output); + } + unlink(output_path); + return stopped && read; +} + +/** Runs the static fixture with cleanup, which crashes on a double free or double close if releasing happens too early. */ +bool run_static_fixture(bool exit) { + ensure_path_loader_registered(); + AppManifest manifest { "test.posix.static", "Static", APP_CATEGORY_USER, { APP_LOCATION_PATH, const_cast(STATIC_FIXTURE_APP_PATH) }, APP_MANIFEST_FLAG_CLEANUP }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + const char* argv[] = { STATIC_FIXTURE_APP_PATH, exit ? "exit" : "" }; + AppStartContext context; + REQUIRE_EQ(app_start_context_from_id("test.posix.static", &context), ERROR_NONE); + app_start_context_set_arguments_ext(&context, 2, argv); + AppInstanceId app_instance_id = 0; + REQUIRE_EQ(app_start_with_context(&context, &app_instance_id), ERROR_NONE); + for (int waited = 0; waited < 5000 && app_manager_get_state(app_instance_id) != APP_INSTANCE_STATE_STOPPED; waited += 10) { + delay_millis(10); + } + const bool stopped = app_manager_get_state(app_instance_id) == APP_INSTANCE_STATE_STOPPED; + // Waits out a deferred unload, which releases the resources + delay_millis(100); + app_manager_remove("test.posix.static"); + return stopped; +} + +} // namespace + +TEST_CASE("an app with APP_MANIFEST_FLAG_CLEANUP has what its static destructors release left alone") { + CHECK(run_static_fixture(false)); +} + +TEST_CASE("an app with APP_MANIFEST_FLAG_CLEANUP that calls exit() has what its static destructors release left alone") { + CHECK(run_static_fixture(true)); +} + +TEST_CASE("an app with APP_MANIFEST_FLAG_CLEANUP has its leaked files closed when it ends") { + LeakedFds fds; + REQUIRE(run_leak_fixture(APP_MANIFEST_FLAG_CLEANUP, false, fds)); + CHECK_FALSE(is_open(fds.fd)); + CHECK_FALSE(is_open(fds.file_fd)); + CHECK_FALSE(is_open(fds.dir_fd)); +} + +TEST_CASE("an app with APP_MANIFEST_FLAG_CLEANUP has its leftover thread ended before it's unloaded") { + LeakedFds fds; + REQUIRE(run_leak_fixture(APP_MANIFEST_FLAG_CLEANUP, true, fds)); + CHECK_FALSE(is_open(fds.fd)); +} + +TEST_CASE("an app without APP_MANIFEST_FLAG_CLEANUP keeps leaking") { + LeakedFds fds; + REQUIRE(run_leak_fixture(0, false, fds)); + CHECK(is_open(fds.fd)); + CHECK(is_open(fds.file_fd)); + CHECK(is_open(fds.dir_fd)); + close(fds.fd); + close(fds.file_fd); + close(fds.dir_fd); +} diff --git a/Modules/app-posix-module/tests/source/libc_test.cpp b/Modules/app-posix-module/tests/source/libc_test.cpp index 87c07fd2b..8a6563c37 100644 --- a/Modules/app-posix-module/tests/source/libc_test.cpp +++ b/Modules/app-posix-module/tests/source/libc_test.cpp @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 -// libc calls made by an app instance (fstat, termios, poll, printf, exit), routed by this module's -// libc wraps (../source/stdio_wrap.cpp) to the app instance's own fds and lifecycle. +// libc calls made by an app instance (fstat, termios, poll, printf, exit, path-based calls), routed by this +// module's libc wraps (../source/stdio_wrap.cpp) to the app instance's own fds, cwd and lifecycle. #include "doctest.h" #include @@ -15,7 +15,10 @@ #include #include +#include +#include +#include #include #include #include @@ -28,9 +31,11 @@ #include #include #include +#include #include extern ServiceManifest app_internal_loader_service_manifest; +extern ServiceManifest loader_service_manifest; namespace { @@ -207,6 +212,19 @@ int32_t icrnl_app_main(int, char*[]) { return 0; } +// Set by the app below: whether a relative path opened by code built into the simulator resolved against the process's cwd +std::atomic g_builtin_relative_open { -1 }; +const char* g_builtin_relative_path = nullptr; + +int32_t builtin_relative_path_app_main(int, char*[]) { + FILE* file = fopen(g_builtin_relative_path, "r"); + g_builtin_relative_open.store(file != nullptr ? 1 : 0, std::memory_order_release); + if (file != nullptr) { + fclose(file); + } + return 0; +} + } // namespace TEST_CASE("ICRNL is on by default and makes stdin read the Enter key's \\r as \\n, until an app clears it") { @@ -692,3 +710,59 @@ TEST_CASE("app_signal_send() rejects an out-of-range signal and an unknown app") CHECK_EQ(app_signal_send(1, 32), ERROR_INVALID_ARGUMENT); CHECK_EQ(app_signal_send(0x7FFFFFF0, SIGTERM), ERROR_NOT_FOUND); } + +TEST_CASE("Path-based calls of an app binary resolve relative paths against the app's own cwd") { + if (service_manager_find_instance(APP_LOADER_PATH_SERVICE_ID) == nullptr) { + service_manager_add(&loader_service_manifest, /*auto_start=*/true); + } + char directory_template[] = "/tmp/tactility-paths-XXXXXX"; + REQUIRE(mkdtemp(directory_template) != nullptr); + const std::string result_path = std::string(directory_template) + ".result"; + + const char* argv[] = { PATHS_FIXTURE_APP_PATH, directory_template, result_path.c_str() }; + AppLocation location { APP_LOCATION_PATH, const_cast(PATHS_FIXTURE_APP_PATH) }; + AppStartContext context = app_start_context_for_location(location); + app_start_context_set_arguments_ext(&context, 3, argv); + AppInstanceId instance_id = 0; + REQUIRE_EQ(app_start_with_context(&context, &instance_id), ERROR_NONE); + REQUIRE(wait_for_state(instance_id, APP_INSTANCE_STATE_STOPPED, 2000)); + + int failed_check = -1; + FILE* result = fopen(result_path.c_str(), "r"); + REQUIRE_NE(result, nullptr); + CHECK_EQ(fscanf(result, "%d", &failed_check), 1); + fclose(result); + CHECK_EQ(failed_check, 0); + + const std::string kept_path = std::string(directory_template) + "/kept.txt"; + CHECK_EQ(access(kept_path.c_str(), F_OK), 0); + // Outside an app, relative paths still resolve against the process's own cwd + CHECK_NE(access("kept.txt", F_OK), 0); + + unlink(kept_path.c_str()); + unlink(result_path.c_str()); + rmdir(directory_template); +} + +TEST_CASE("Path-based calls of code built into the simulator keep resolving against the process's cwd") { + ensure_memory_loader_registered(); + char path_template[] = "tactility-builtin-path-XXXXXX"; + const int fd = mkstemp(path_template); + REQUIRE_NE(fd, -1); + close(fd); + g_builtin_relative_path = path_template; + g_builtin_relative_open.store(-1, std::memory_order_relaxed); + + AppManifest manifest { "test.libc.builtin_paths", "Paths", APP_CATEGORY_USER, { APP_LOCATION_MEMORY, reinterpret_cast(builtin_relative_path_app_main) } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + AppInstanceId instance_id = 0; + AppStartContext context; + REQUIRE_EQ(app_start_context_from_id("test.libc.builtin_paths", &context), ERROR_NONE); + REQUIRE_EQ(app_start_with_context(&context, &instance_id), ERROR_NONE); + REQUIRE(wait_for_state(instance_id, APP_INSTANCE_STATE_STOPPED, 2000)); + + CHECK_EQ(g_builtin_relative_open.load(std::memory_order_acquire), 1); + + unlink(path_template); + app_manager_remove("test.libc.builtin_paths"); +} diff --git a/Modules/c-symbols-module/source/module.cpp b/Modules/c-symbols-module/source/module.cpp index 63f7c07c6..6098f8741 100644 --- a/Modules/c-symbols-module/source/module.cpp +++ b/Modules/c-symbols-module/source/module.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -35,7 +36,13 @@ static const ModuleSymbol SYMBOLS[] = { DEFINE_MODULE_SYMBOL(system), DEFINE_MODULE_SYMBOL(getenv), DEFINE_MODULE_SYMBOL(qsort), + // errno.h +#ifdef ESP_PLATFORM + // Newlib's errno is a macro for *__errno() + DEFINE_MODULE_SYMBOL(__errno), +#endif // time.h + DEFINE_MODULE_SYMBOL(clock), DEFINE_MODULE_SYMBOL(strftime), DEFINE_MODULE_SYMBOL(time), DEFINE_MODULE_SYMBOL(difftime), diff --git a/Modules/lvgl-module/include/lvgl/fonts.h b/Modules/lvgl-module/include/lvgl/fonts.h index b22cf77a5..333a959c3 100644 --- a/Modules/lvgl-module/include/lvgl/fonts.h +++ b/Modules/lvgl-module/include/lvgl/fonts.h @@ -14,11 +14,13 @@ enum LvglFontSize { FONT_SIZE_LARGE, }; +#define LVGL_ICON_FONT_SHARED LVGL_ICON_FONT_SHARED_DEFAULT + enum LvglIconFont { LVGL_ICON_FONT_STATUSBAR, LVGL_ICON_FONT_LAUNCHER, - LVGL_ICON_FONT_SHARED, - LVGL_ICON_FONT_SHARED_2X, + LVGL_ICON_FONT_SHARED_DEFAULT, + LVGL_ICON_FONT_SHARED_LARGE, }; /** @@ -39,11 +41,11 @@ void lvgl_set_text_font(enum LvglFontSize font_size, const lv_font_t* font, uint */ void lvgl_set_icon_font(enum LvglIconFont icon_font, const lv_font_t* font, uint32_t height); -const lv_font_t* lvgl_get_shared_icon_font(void); -uint32_t lvgl_get_shared_icon_font_height(void); +const lv_font_t* lvgl_get_shared_icon_default_font(void); +uint32_t lvgl_get_shared_icon_default_font_height(void); -const lv_font_t* lvgl_get_shared_icon_font_2x(void); -uint32_t lvgl_get_shared_icon_font_2x_height(void); +const lv_font_t* lvgl_get_shared_icon_large_font(void); +uint32_t lvgl_get_shared_icon_large_font_height(void); const lv_font_t* lvgl_get_text_font(enum LvglFontSize font_size); uint32_t lvgl_get_text_font_height(enum LvglFontSize font_size); diff --git a/Modules/lvgl-module/source/fonts.c b/Modules/lvgl-module/source/fonts.c index c6c432c49..c244364cc 100644 --- a/Modules/lvgl-module/source/fonts.c +++ b/Modules/lvgl-module/source/fonts.c @@ -8,7 +8,7 @@ struct RegisteredFont { }; static struct RegisteredFont text_fonts[FONT_SIZE_LARGE + 1]; -static struct RegisteredFont icon_fonts[LVGL_ICON_FONT_SHARED_2X + 1]; +static struct RegisteredFont icon_fonts[LVGL_ICON_FONT_SHARED_LARGE + 1]; void lvgl_set_text_font(enum LvglFontSize font_size, const lv_font_t* font, uint32_t height) { text_fonts[font_size] = (struct RegisteredFont) { .font = font, .height = height }; @@ -30,13 +30,13 @@ const lv_font_t* lvgl_get_text_font(enum LvglFontSize font_size) { return get_fo uint32_t lvgl_get_text_font_height(enum LvglFontSize font_size) { return get_height(&text_fonts[font_size]); } -const lv_font_t* lvgl_get_shared_icon_font() { return get_font(&icon_fonts[LVGL_ICON_FONT_SHARED]); } +const lv_font_t* lvgl_get_shared_icon_default_font() { return get_font(&icon_fonts[LVGL_ICON_FONT_SHARED_DEFAULT]); } -uint32_t lvgl_get_shared_icon_font_height() { return get_height(&icon_fonts[LVGL_ICON_FONT_SHARED]); } +uint32_t lvgl_get_shared_icon_default_font_height() { return get_height(&icon_fonts[LVGL_ICON_FONT_SHARED_DEFAULT]); } -const lv_font_t* lvgl_get_shared_icon_font_2x() { return get_font(&icon_fonts[LVGL_ICON_FONT_SHARED_2X]); } +const lv_font_t* lvgl_get_shared_icon_large_font() { return get_font(&icon_fonts[LVGL_ICON_FONT_SHARED_LARGE]); } -uint32_t lvgl_get_shared_icon_font_2x_height() { return get_height(&icon_fonts[LVGL_ICON_FONT_SHARED_2X]); } +uint32_t lvgl_get_shared_icon_large_font_height() { return get_height(&icon_fonts[LVGL_ICON_FONT_SHARED_LARGE]); } const lv_font_t* lvgl_get_launcher_icon_font() { return get_font(&icon_fonts[LVGL_ICON_FONT_LAUNCHER]); } diff --git a/Modules/lvgl-module/source/symbols.c b/Modules/lvgl-module/source/symbols.c index e5c02078d..e9e5b8199 100644 --- a/Modules/lvgl-module/source/symbols.c +++ b/Modules/lvgl-module/source/symbols.c @@ -31,8 +31,10 @@ const struct ModuleSymbol lvgl_module_symbols[] = { DEFINE_MODULE_SYMBOL(lvgl_binfont_create), DEFINE_MODULE_SYMBOL(lvgl_binfont_destroy), // lvgl_fonts - DEFINE_MODULE_SYMBOL(lvgl_get_shared_icon_font), - DEFINE_MODULE_SYMBOL(lvgl_get_shared_icon_font_height), + DEFINE_MODULE_SYMBOL(lvgl_get_shared_icon_default_font), + DEFINE_MODULE_SYMBOL(lvgl_get_shared_icon_default_font_height), + DEFINE_MODULE_SYMBOL(lvgl_get_shared_icon_large_font), + DEFINE_MODULE_SYMBOL(lvgl_get_shared_icon_large_font_height), DEFINE_MODULE_SYMBOL(lvgl_get_text_font), DEFINE_MODULE_SYMBOL(lvgl_get_text_font_height), DEFINE_MODULE_SYMBOL(lvgl_get_launcher_icon_font), diff --git a/Modules/lvgl-module/source/widgets/toolbar.cpp b/Modules/lvgl-module/source/widgets/toolbar.cpp index cac2e1167..99cef708d 100644 --- a/Modules/lvgl-module/source/widgets/toolbar.cpp +++ b/Modules/lvgl-module/source/widgets/toolbar.cpp @@ -29,7 +29,31 @@ static const _lv_font_t* getToolbarFont(UiDensity uiDensity) { static uint32_t getActionIconPadding(UiDensity uiDensity) { auto toolbar_height = getToolbarHeight(uiDensity); // Minimal 8 pixels total padding for selection/animation (4+4 pixels) - return (uiDensity != LVGL_UI_DENSITY_COMPACT) ? (uint32_t)(toolbar_height * 0.2f) : 10; + return (uiDensity != LVGL_UI_DENSITY_COMPACT) ? (uint32_t)(toolbar_height * 0.2f) : 8; +} + +static bool is_monochrome(lv_obj_t* obj) { + return lv_display_get_color_format(lv_obj_get_display(obj)) == LV_COLOR_FORMAT_I1; +} + +/** + * Makes a toolbar button transparent. Its icon or text is drawn in the accent colour when it's + * selected or pressed, instead of showing an outline. + */ +static void apply_button_style(lv_obj_t* button) { + lv_obj_set_style_bg_opa(button, LV_OPA_TRANSP, LV_STATE_DEFAULT); + lv_obj_set_style_shadow_width(button, 0, LV_STATE_DEFAULT); + // Colours can't show the selection on monochrome displays, so those keep the theme's outline + if (is_monochrome(button)) { + return; + } + const lv_color_t accent = lv_theme_get_color_primary(button); + const lv_state_t states[] = { LV_STATE_FOCUSED, LV_STATE_FOCUS_KEY, LV_STATE_PRESSED }; + for (const lv_state_t state : states) { + lv_obj_set_style_outline_width(button, 0, state); + // Inherited by the button's label or image, including symbol images + lv_obj_set_style_text_color(button, accent, state); + } } /** @@ -119,9 +143,7 @@ lv_obj_t* lvgl_toolbar_create(lv_obj_t* parent, const char* title) { auto* close_button_wrapper = create_action_wrapper(obj, ui_density); toolbar->close_button = lv_button_create(close_button_wrapper); - if (ui_density == LVGL_UI_DENSITY_COMPACT) { - lv_obj_set_style_bg_opa(toolbar->close_button, LV_OPA_TRANSP, LV_STATE_DEFAULT); - } + apply_button_style(toolbar->close_button); lv_obj_set_size(toolbar->close_button, toolbar_height - icon_padding, toolbar_height - icon_padding); @@ -204,9 +226,7 @@ static lv_obj_t* toolbar_add_button_action(lv_obj_t* obj, const char* imageOrBut lv_obj_set_size(action_button, toolbar_height - padding, toolbar_height - padding); lv_obj_set_style_pad_all(action_button, 0, LV_STATE_DEFAULT); lv_obj_align(action_button, LV_ALIGN_CENTER, 0, 0); - if (ui_density == LVGL_UI_DENSITY_COMPACT) { - lv_obj_set_style_bg_opa(action_button, LV_OPA_TRANSP, LV_STATE_DEFAULT); - } + apply_button_style(action_button); lv_obj_add_event_cb(action_button, callback, LV_EVENT_SHORT_CLICKED, user_data); lv_obj_t* button_content; diff --git a/Platforms/platform-esp32/source/mkdir.cpp b/Platforms/platform-esp32/source/mkdir.cpp deleted file mode 100644 index 0a382800e..000000000 --- a/Platforms/platform-esp32/source/mkdir.cpp +++ /dev/null @@ -1,26 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include - -#include - -// mkdir() on an existing FATFS mount root (e.g. "/sdcard") fails without setting EEXIST, -// which breaks the common "mkdir() then accept EEXIST" pattern. Any existing path reports EEXIST. -// A NULL path fails with EFAULT instead of crashing in the VFS (see vfs_null_path.cpp). -extern "C" { - -int __real_mkdir(const char* path, mode_t mode); - -int __wrap_mkdir(const char* path, mode_t mode) { - if (path == nullptr) { - errno = EFAULT; - return -1; - } - struct stat info; - if (stat(path, &info) == 0) { - errno = EEXIST; - return -1; - } - return __real_mkdir(path, mode); -} - -} diff --git a/Platforms/platform-esp32/source/vfs_null_path.cpp b/Platforms/platform-esp32/source/vfs_null_path.cpp deleted file mode 100644 index 179ba388c..000000000 --- a/Platforms/platform-esp32/source/vfs_null_path.cpp +++ /dev/null @@ -1,96 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include -#include -#include -#include - -#include - -// ESP-IDF's VFS dereferences a NULL path. These fail with EFAULT instead, like other POSIX systems do. -// opendir() and mkdir() get the same check in their own wraps (root_dir.cpp, mkdir.cpp). -extern "C" { - -int __real__open_r(struct _reent* r, const char* path, int flags, int mode); -int __real__stat_r(struct _reent* r, const char* path, struct stat* st); -int __real__link_r(struct _reent* r, const char* n1, const char* n2); -int __real__unlink_r(struct _reent* r, const char* path); -int __real__rename_r(struct _reent* r, const char* src, const char* dst); -int __real_truncate(const char* path, off_t length); -int __real_access(const char* path, int amode); -int __real_utime(const char* path, const struct utimbuf* times); -int __real_rmdir(const char* name); - -// open() and fopen() both go through _open_r -int __wrap__open_r(struct _reent* r, const char* path, int flags, int mode) { - if (path == nullptr) { - r->_errno = EFAULT; - return -1; - } - return __real__open_r(r, path, flags, mode); -} - -int __wrap__stat_r(struct _reent* r, const char* path, struct stat* st) { - if (path == nullptr) { - r->_errno = EFAULT; - return -1; - } - return __real__stat_r(r, path, st); -} - -int __wrap__link_r(struct _reent* r, const char* n1, const char* n2) { - if (n1 == nullptr || n2 == nullptr) { - r->_errno = EFAULT; - return -1; - } - return __real__link_r(r, n1, n2); -} - -int __wrap__unlink_r(struct _reent* r, const char* path) { - if (path == nullptr) { - r->_errno = EFAULT; - return -1; - } - return __real__unlink_r(r, path); -} - -int __wrap__rename_r(struct _reent* r, const char* src, const char* dst) { - if (src == nullptr || dst == nullptr) { - r->_errno = EFAULT; - return -1; - } - return __real__rename_r(r, src, dst); -} - -int __wrap_truncate(const char* path, off_t length) { - if (path == nullptr) { - errno = EFAULT; - return -1; - } - return __real_truncate(path, length); -} - -int __wrap_access(const char* path, int amode) { - if (path == nullptr) { - errno = EFAULT; - return -1; - } - return __real_access(path, amode); -} - -int __wrap_utime(const char* path, const struct utimbuf* times) { - if (path == nullptr) { - errno = EFAULT; - return -1; - } - return __real_utime(path, times); -} - -int __wrap_rmdir(const char* name) { - if (name == nullptr) { - errno = EFAULT; - return -1; - } - return __real_rmdir(name); -} - -} diff --git a/Platforms/platform-posix/freertos/freertos_task_hooks.c b/Platforms/platform-posix/freertos/freertos_task_hooks.c new file mode 100644 index 000000000..ddc7ae04e --- /dev/null +++ b/Platforms/platform-posix/freertos/freertos_task_hooks.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 +#include "freertos_task_hooks.h" + +static FreeRtosTaskCreateHook create_hook = NULL; +static FreeRtosTaskDeleteHook delete_hook = NULL; + +void freertos_set_task_hooks(FreeRtosTaskCreateHook create, FreeRtosTaskDeleteHook delete_task) { + create_hook = create; + delete_hook = delete_task; +} + +BaseType_t xTaskCreate(TaskFunction_t function, const char* const name, const configSTACK_DEPTH_TYPE stack_depth, void* const parameter, UBaseType_t priority, TaskHandle_t* const out_handle) { + if (create_hook != NULL) { + return create_hook(__builtin_return_address(0), function, name, stack_depth, parameter, priority, out_handle); + } + return freertos_real_xTaskCreate(function, name, stack_depth, parameter, priority, out_handle); +} + +void vTaskDelete(TaskHandle_t handle) { + if (delete_hook != NULL) { + delete_hook(__builtin_return_address(0), handle); + return; + } + freertos_real_vTaskDelete(handle); +} diff --git a/Platforms/platform-posix/freertos/freertos_task_hooks.h b/Platforms/platform-posix/freertos/freertos_task_hooks.h new file mode 100644 index 000000000..4cb3a2439 --- /dev/null +++ b/Platforms/platform-posix/freertos/freertos_task_hooks.h @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include "FreeRTOS.h" +#include "task.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * On the simulator, an app binary binds to xTaskCreate() and vTaskDelete() through the dynamic linker, + * so these are the simulator's own definitions, built into FreeRTOS-Kernel in place of its own. + * They call the hooks when set, passing the address they were called from. + */ + +typedef BaseType_t (*FreeRtosTaskCreateHook)(const void* caller, TaskFunction_t function, const char* name, configSTACK_DEPTH_TYPE stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle); +typedef void (*FreeRtosTaskDeleteHook)(const void* caller, TaskHandle_t handle); + +void freertos_set_task_hooks(FreeRtosTaskCreateHook create, FreeRtosTaskDeleteHook delete_task); + +/** FreeRTOS-Kernel's own xTaskCreate() */ +BaseType_t freertos_real_xTaskCreate(TaskFunction_t function, const char* name, configSTACK_DEPTH_TYPE stack_depth, void* parameter, UBaseType_t priority, TaskHandle_t* out_handle); + +/** FreeRTOS-Kernel's own vTaskDelete() */ +void freertos_real_vTaskDelete(TaskHandle_t handle); + +#ifdef __cplusplus +} +#endif diff --git a/Tactility/Private/Tactility/app/terminal/TerminalRenderer.h b/Tactility/Private/Tactility/app/terminal/TerminalRenderer.h index b6e6ee297..6083a7b62 100644 --- a/Tactility/Private/Tactility/app/terminal/TerminalRenderer.h +++ b/Tactility/Private/Tactility/app/terminal/TerminalRenderer.h @@ -120,8 +120,7 @@ class TerminalRenderer { Device* display = nullptr; - // True for a monochrome display. frameBuffer/hwFrameBuffers/fullFrameBuffer format-handling - // all lives in graphics-module now (see paintCell()/presentRegion()). + // True for a monochrome display. paintCell() then draws every non-black colour as white. bool monochrome = false; // Scratch buffer glyphs are painted into before being pushed. Sized for one text row, not the diff --git a/Tactility/Source/app/AppGrid.cpp b/Tactility/Source/app/AppGrid.cpp index f81a38fc8..4af67ffa9 100644 --- a/Tactility/Source/app/AppGrid.cpp +++ b/Tactility/Source/app/AppGrid.cpp @@ -27,7 +27,7 @@ struct PageLayout { }; PageLayout computePageLayout(lv_obj_t* grid) { - const auto icon_size = static_cast(lvgl_get_shared_icon_font_2x_height()); + const auto icon_size = static_cast(lvgl_get_shared_icon_large_font_height()); const auto pad = icon_size / 16; const auto gap = TILE_GAP; const auto text_height = lv_font_get_line_height(lvgl_get_text_font(FONT_SIZE_SMALL)); @@ -186,7 +186,7 @@ void AppGrid::populate() { lv_obj_set_user_data(tile, const_cast(&item)); lv_obj_t* icon = lv_label_create(tile); - lv_obj_set_style_text_font(icon, lvgl_get_shared_icon_font_2x(), LV_STATE_DEFAULT); + lv_obj_set_style_text_font(icon, lvgl_get_shared_icon_large_font(), LV_STATE_DEFAULT); lv_obj_set_style_text_align(icon, LV_TEXT_ALIGN_CENTER, LV_STATE_DEFAULT); lv_obj_set_size(icon, layout.iconSize, layout.iconSize); if (!monochrome) { diff --git a/Tactility/Source/app/applist/AppList.cpp b/Tactility/Source/app/applist/AppList.cpp index 4a962babe..007097b4b 100644 --- a/Tactility/Source/app/applist/AppList.cpp +++ b/Tactility/Source/app/applist/AppList.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -161,7 +162,8 @@ void createWidgets(lv_obj_t* parent, void* userData) { auto* toolbar = lvgl_toolbar_create(parent, "Apps"); lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, ctx); ctx->grid.createWidgets(parent, toolbar); - lvgl_toolbar_add_text_button_action(toolbar, "?", onHelpPressed, ctx); + auto* help_button = lvgl_toolbar_add_text_button_action(toolbar, LVGL_ICON_SHARED_HELP, onHelpPressed, ctx); + lv_obj_set_style_text_font(help_button, lvgl_get_shared_icon_default_font(), LV_STATE_DEFAULT); } int32_t appMain(int argc, char* argv[]) { diff --git a/Tactility/Source/app/apppackagelist/AppPackageList.cpp b/Tactility/Source/app/apppackagelist/AppPackageList.cpp index 07de5951f..3a5d05a3f 100644 --- a/Tactility/Source/app/apppackagelist/AppPackageList.cpp +++ b/Tactility/Source/app/apppackagelist/AppPackageList.cpp @@ -52,7 +52,7 @@ void createPackageWidget(const PackageManifest* package, lv_obj_t* list) { const char* label = (app_manager_find_manifest(package->id, &appManifest) == ERROR_NONE) ? appManifest.name : package->id; lv_obj_t* btn = lv_list_add_button(list, LVGL_ICON_SHARED_DEPLOYED_CODE, label); lv_obj_t* image = lv_obj_get_child(btn, 0); - lv_obj_set_style_text_font(image, lvgl_get_shared_icon_font(), LV_PART_MAIN); + lv_obj_set_style_text_font(image, lvgl_get_shared_icon_default_font(), LV_PART_MAIN); lv_obj_add_event_cb(btn, &onPackagePressed, LV_EVENT_SHORT_CLICKED, const_cast(package)); } diff --git a/Tactility/Source/app/gpssettings/GpsSettings.cpp b/Tactility/Source/app/gpssettings/GpsSettings.cpp index 4497ff1be..df6c658b5 100644 --- a/Tactility/Source/app/gpssettings/GpsSettings.cpp +++ b/Tactility/Source/app/gpssettings/GpsSettings.cpp @@ -62,6 +62,7 @@ struct Context { void rebuildDeviceList(Context* ctx); void updateDeviceStates(Context* ctx); void createWidgets(lv_obj_t* parent, void* userData); +void destroyWidgets(void* userData); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); @@ -187,8 +188,12 @@ void createDeviceRow(Context* ctx, Device* device) { // Rebuilds the device list. Only needs to run when the set of devices could've changed (on // creation, and after returning from AddGps) - button state itself is refreshed by the timer. void rebuildDeviceList(Context* ctx) { - lv_obj_clean(ctx->deviceListWrapper); ctx->deviceRows.clear(); + // Rebuilt by createWidgets() when the window resurfaces + if (ctx->deviceListWrapper == nullptr) { + return; + } + lv_obj_clean(ctx->deviceListWrapper); device_for_each_of_type(&GPS_TYPE, ctx, [](Device* device, void* context) { createDeviceRow(static_cast(context), device); @@ -256,6 +261,13 @@ void createWidgets(lv_obj_t* parent, void* userData) { updateDeviceStates(ctx); } +// The window's widgets are deleted while another window is on top, so the timer must not reach them +void destroyWidgets(void* userData) { + auto* ctx = static_cast(userData); + ctx->deviceListWrapper = nullptr; + ctx->deviceRows.clear(); +} + int32_t appMain(int argc, char* argv[]) { uint32_t appInstanceId = app_scheduler_current_app_id(); Context ctx {}; @@ -273,7 +285,7 @@ int32_t appMain(int argc, char* argv[]) { AppEventSubscription sub {}; check(app_event_subscribe(&sub, &event_group) == ERROR_NONE); - WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx); + WindowId window = window_manager_create_ext(appInstanceId, createWidgets, destroyWidgets, &ctx); ctx.timer->start(); bool shouldClose = false; diff --git a/Tactility/Source/app/shell/Shell.cpp b/Tactility/Source/app/shell/Shell.cpp index 850d7fea1..ed1ca4108 100644 --- a/Tactility/Source/app/shell/Shell.cpp +++ b/Tactility/Source/app/shell/Shell.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -322,6 +323,10 @@ int runCommand(int argc, char** argv, int* found) { if (app_is_executable_path(resolved)) { return runElf(resolved, argc, argv); } + if (elf_has_magic(resolved)) { + printf("%s: cannot execute binary file\n", argv[0]); + return 126; + } // A script runs in its own `sh` app instance, so it gets its own task and stack rather // than nesting another interpreter on this one's. std::vector shArgv; diff --git a/Tactility/Source/app/shell/main.cpp b/Tactility/Source/app/shell/main.cpp index fa79b9e83..b05967f5f 100644 --- a/Tactility/Source/app/shell/main.cpp +++ b/Tactility/Source/app/shell/main.cpp @@ -93,7 +93,7 @@ extern const ::AppManifest manifest = { .category = APP_CATEGORY_SYSTEM, .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(main) }, .flags = APP_MANIFEST_FLAG_HIDDEN | APP_MANIFEST_FLAG_HEADLESS, - .stack = { .depth = 6144, .desired_memory_capability = 0 }, + .stack = { .depth = 8192, .desired_memory_capability = 0 }, }; static int32_t shMain(int argc, char* argv[]) { diff --git a/Tactility/Source/app/terminal/TerminalRenderer.cpp b/Tactility/Source/app/terminal/TerminalRenderer.cpp index 7701aa1b4..f95cc0806 100644 --- a/Tactility/Source/app/terminal/TerminalRenderer.cpp +++ b/Tactility/Source/app/terminal/TerminalRenderer.cpp @@ -288,7 +288,11 @@ void TerminalRenderer::paintCell(int row, int col, char ch, uint8_t attr) { for (int y = 0; y < cellHeight; y++) { for (int x = 0; x < cellWidth; x++) { const uint8_t alpha = mask[y * cellWidth + x]; - const uint16_t colour = alpha == 0 ? bg : (alpha == 255 ? fg : blendRgb565(fg, bg, alpha)); + uint16_t colour = alpha == 0 ? bg : (alpha == 255 ? fg : blendRgb565(fg, bg, alpha)); + // Any non-black colour is ink-on, also when the frame buffer is RGB565 for LVGL to convert by luminance + if (monochrome && colour != 0x0000) { + colour = 0xFFFF; + } pixel_buffer_set_pixel_rgb565(frameBuffer, pixelX + x, pixelY + y, colour, PIXEL_BUFFER_CONVERSION_EXACT_BLACK); } } diff --git a/Tactility/Source/app/timezone/TimeZone.cpp b/Tactility/Source/app/timezone/TimeZone.cpp index df8803232..2bbd94805 100644 --- a/Tactility/Source/app/timezone/TimeZone.cpp +++ b/Tactility/Source/app/timezone/TimeZone.cpp @@ -209,7 +209,7 @@ void createWidgets(lv_obj_t* parent, void* userData) { lv_obj_set_style_margin_left(icon, 8, 0); lv_obj_set_style_image_recolor_opa(icon, 255, 0); lv_obj_set_style_image_recolor(icon, lv_theme_get_color_primary(parent), 0); - lv_obj_set_style_text_font(icon, lvgl_get_shared_icon_font(), LV_STATE_DEFAULT); + lv_obj_set_style_text_font(icon, lvgl_get_shared_icon_default_font(), LV_STATE_DEFAULT); lv_image_set_src(icon, LVGL_ICON_SHARED_SEARCH); auto* textarea = lv_textarea_create(search_wrapper); diff --git a/Tactility/Source/lvgl/FontSizes.cpp b/Tactility/Source/lvgl/FontSizes.cpp index ecf38cda1..b1aba59c8 100644 --- a/Tactility/Source/lvgl/FontSizes.cpp +++ b/Tactility/Source/lvgl/FontSizes.cpp @@ -20,9 +20,9 @@ uint16_t getTextFontSize(uint16_t defaultSize, LvglFontSize fontSize) { uint16_t getIconFontSize(uint16_t defaultSize, LvglIconFont iconFont) { switch (iconFont) { case LVGL_ICON_FONT_LAUNCHER: return scale(defaultSize, 2.6f); - case LVGL_ICON_FONT_SHARED_2X: return getIconFontSize(defaultSize, LVGL_ICON_FONT_SHARED) * 2; + case LVGL_ICON_FONT_SHARED_DEFAULT: return scale(defaultSize, 1.25); + case LVGL_ICON_FONT_SHARED_LARGE: return getIconFontSize(defaultSize, LVGL_ICON_FONT_SHARED) * 2; case LVGL_ICON_FONT_STATUSBAR: - case LVGL_ICON_FONT_SHARED: default: return scale(defaultSize, 1.15f); } } diff --git a/Tactility/Source/lvgl/Fonts.cpp b/Tactility/Source/lvgl/Fonts.cpp index 163bf0ce1..ec05cda49 100644 --- a/Tactility/Source/lvgl/Fonts.cpp +++ b/Tactility/Source/lvgl/Fonts.cpp @@ -70,8 +70,8 @@ constexpr TextFontDefinition TEXT_FONTS[] = { static const IconFontDefinition ICON_FONTS[] = { { LVGL_ICON_FONT_STATUSBAR, "statusbar", lvgl_icon_statusbar_names, lvgl_icon_statusbar_name_count }, { LVGL_ICON_FONT_LAUNCHER, "launcher", lvgl_icon_launcher_names, lvgl_icon_launcher_name_count }, - { LVGL_ICON_FONT_SHARED, "shared", lvgl_icon_shared_names, lvgl_icon_shared_name_count }, - { LVGL_ICON_FONT_SHARED_2X, "shared2x", lvgl_icon_shared_names, lvgl_icon_shared_name_count }, + { LVGL_ICON_FONT_SHARED_DEFAULT, "shared_default", lvgl_icon_shared_names, lvgl_icon_shared_name_count }, + { LVGL_ICON_FONT_SHARED_LARGE, "shared_large", lvgl_icon_shared_names, lvgl_icon_shared_name_count }, }; struct LoadedFont { @@ -95,7 +95,7 @@ static bool isTextFontGenerated(LvglFontSize fontSize) { /** The shared icon font replaces the 2x one when it isn't generated */ static bool isIconFontGenerated(LvglIconFont iconFont) { - return iconFont != LVGL_ICON_FONT_SHARED_2X || !hasLimitedMemory(); + return iconFont != LVGL_ICON_FONT_SHARED_LARGE || !hasLimitedMemory(); } static std::vector loadedFonts; @@ -313,8 +313,8 @@ void loadFonts(const FontConfiguration& configuration) { } } - if (!isIconFontGenerated(LVGL_ICON_FONT_SHARED_2X)) { - lvgl_set_icon_font(LVGL_ICON_FONT_SHARED_2X, lvgl_get_shared_icon_font(), lvgl_get_shared_icon_font_height()); + if (!isIconFontGenerated(LVGL_ICON_FONT_SHARED_LARGE)) { + lvgl_set_icon_font(LVGL_ICON_FONT_SHARED_LARGE, lvgl_get_shared_icon_default_font(), lvgl_get_shared_icon_default_font_height()); } deleteStaleCachedFonts(TEXT_CACHE_PREFIX, text_file_names); diff --git a/Tactility/Tests/Source/FontsTest.cpp b/Tactility/Tests/Source/FontsTest.cpp index 8ae8e7f0c..e515d0de3 100644 --- a/Tactility/Tests/Source/FontsTest.cpp +++ b/Tactility/Tests/Source/FontsTest.cpp @@ -67,8 +67,8 @@ TEST_CASE("updateFontCache and loadFonts use a custom regular font and size") { CHECK_EQ(lvgl_get_text_font_height(FONT_SIZE_DEFAULT), 16); CHECK_EQ(lvgl_get_text_font_height(FONT_SIZE_LARGE), 20); CHECK(lvgl_get_text_font(FONT_SIZE_DEFAULT) != LV_FONT_DEFAULT); - CHECK_EQ(lvgl_get_shared_icon_font_height(), 18); - CHECK(lvgl_get_shared_icon_font() != LV_FONT_DEFAULT); + CHECK_EQ(lvgl_get_shared_icon_default_font_height(), 20); + CHECK(lvgl_get_shared_icon_default_font() != LV_FONT_DEFAULT); // All characters of a custom TTF are rasterized, not just the ones of the system font lv_font_glyph_dsc_t glyph;