diff --git a/src/audio_core/opus/decoder.cpp b/src/audio_core/opus/decoder.cpp index 1c13cb60e9..d12db9e792 100644 --- a/src/audio_core/opus/decoder.cpp +++ b/src/audio_core/opus/decoder.cpp @@ -4,6 +4,8 @@ // SPDX-FileCopyrightText: Copyright 2023 yuzu Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later +#include + #include "audio_core/opus/decoder.h" #include "audio_core/opus/hardware_opus.h" #include "audio_core/opus/parameters.h" @@ -117,8 +119,10 @@ Result OpusDecoder::DecodeInterleaved(u32* out_data_size, u64* out_time_taken, R_UNLESS(input_data.size_bytes() > sizeof(OpusPacketHeader), ResultInputDataTooSmall); - auto* header_p{reinterpret_cast(input_data.data())}; - OpusPacketHeader header{ReverseHeader(*header_p)}; + // Guest input has no alignment guarantee. + OpusPacketHeader header_raw; + std::memcpy(&header_raw, input_data.data(), sizeof(header_raw)); + OpusPacketHeader header{ReverseHeader(header_raw)}; R_UNLESS(in_data.size_bytes() >= header.size && header.size + sizeof(OpusPacketHeader) <= input_data.size_bytes(), @@ -160,8 +164,10 @@ Result OpusDecoder::DecodeInterleavedForMultiStream(u32* out_data_size, u64* out R_UNLESS(input_data.size_bytes() > sizeof(OpusPacketHeader), ResultInputDataTooSmall); - auto* header_p{reinterpret_cast(input_data.data())}; - OpusPacketHeader header{ReverseHeader(*header_p)}; + // Guest input has no alignment guarantee. + OpusPacketHeader header_raw; + std::memcpy(&header_raw, input_data.data(), sizeof(header_raw)); + OpusPacketHeader header{ReverseHeader(header_raw)}; LOG_TRACE(Service_Audio, "header size {:#x} input data size {:#x} in_data size {:#x}", header.size, input_data.size_bytes(), in_data.size_bytes()); diff --git a/src/audio_core/renderer/system_manager.cpp b/src/audio_core/renderer/system_manager.cpp index d2c924fdf2..9da660d2d3 100644 --- a/src/audio_core/renderer/system_manager.cpp +++ b/src/audio_core/renderer/system_manager.cpp @@ -57,11 +57,9 @@ bool SystemManager::Add(System& system_) { LOG_ERROR(Service_Audio, "Maximum AudioRenderer Systems active, cannot add more!"); return false; } - { - std::scoped_lock l{mutex1}; - if (systems.empty()) - InitializeUnsafe(); - } + std::scoped_lock l{mutex1}; + if (systems.empty()) + InitializeUnsafe(); systems.push_back(&system_); return true; } diff --git a/src/common/logging.cpp b/src/common/logging.cpp index 6815c77807..0ecb6bef4c 100644 --- a/src/common/logging.cpp +++ b/src/common/logging.cpp @@ -275,7 +275,10 @@ struct FileBackend final : public Backend { boost::replace_all(message, username, "user"); } #endif - bytes_written += file->WriteSpan(std::span{message.begin(), message.end()}); + // Threads log without a shared lock; fwrite locks the stream, these counters are atomic. + const auto written = file->WriteSpan(std::span{message.begin(), message.end()}); + const auto total_written = + bytes_written.fetch_add(written, std::memory_order_relaxed) + written; // Option to log each line rather than 4k buffers if (Settings::values.log_flush_line.GetValue()) @@ -284,7 +287,7 @@ struct FileBackend final : public Backend { using namespace Common::Literals; // Prevent logs from exceeding a set maximum size in the event that log entries are spammed. const auto write_limit = Settings::values.extended_logging.GetValue() ? 1_GiB : 100_MiB; - const bool write_limit_exceeded = bytes_written > write_limit; + const bool write_limit_exceeded = total_written > write_limit; if (entry.log_level >= Level::Error || write_limit_exceeded) { // Stop writing after the write limit is exceeded. // Don't close the file so we can print a stacktrace if necessary @@ -299,8 +302,8 @@ struct FileBackend final : public Backend { } private: std::optional file; - std::size_t bytes_written = 0; - bool enabled = true; + std::atomic bytes_written = 0; + std::atomic_bool enabled = true; }; #endif /// @brief Backend that keeps the last lines in memory, off unless the debug server turns it on diff --git a/src/common/scratch_buffer.h b/src/common/scratch_buffer.h index cad1506922..5ef52ea69b 100644 --- a/src/common/scratch_buffer.h +++ b/src/common/scratch_buffer.h @@ -65,7 +65,9 @@ class ScratchBuffer { void resize(size_type size) { if (size > buffer_capacity) { auto new_buffer = std::make_unique_for_overwrite(size); - std::memcpy(new_buffer.get(), buffer.get(), buffer_capacity * sizeof(T)); + if (buffer_capacity > 0) { + std::memcpy(new_buffer.get(), buffer.get(), buffer_capacity * sizeof(T)); + } buffer = std::move(new_buffer); buffer_capacity = size; } diff --git a/src/common/zbic_compression.cpp b/src/common/zbic_compression.cpp index 5e0e4d947b..9653e97e24 100644 --- a/src/common/zbic_compression.cpp +++ b/src/common/zbic_compression.cpp @@ -17,6 +17,15 @@ #endif #include "zstd.h" +// zstd.c declares the ASan poisoning functions itself, which gives them C++ linkage here and +// breaks ASan links. Declaring them first through the sanitizer header keeps C linkage. +#if defined(__SANITIZE_ADDRESS__) +#include +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) +#include +#endif +#endif #define g_ZSTD_threading_useless_symbol g_ZSTD_zbic_threading_useless_symbol #include "zstd.c" #undef g_ZSTD_threading_useless_symbol diff --git a/src/core/core.cpp b/src/core/core.cpp index b9b1247567..a0ab1833b2 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -461,10 +461,14 @@ struct System::Impl { } } - perf_stats.emplace(params.program_id); - // Reset counters and set time origin to current frame - GetAndResetPerfStats(); - perf_stats->BeginSystemFrame(); + { + // Frontend timers poll GetAndResetPerfStats from their own threads. + std::scoped_lock lk{perf_stats_mutex}; + perf_stats.emplace(params.program_id); + // Reset counters and set time origin to current frame + perf_stats->GetAndResetStats(core_timing.GetGlobalTimeUs()); + perf_stats->BeginSystemFrame(); + } const FileSys::PatchManager pm(params.program_id, system.GetFileSystemController(), system.GetContentProvider()); auto const metadata = pm.GetControlMetadata(); @@ -518,7 +522,10 @@ struct System::Impl { audio_core.reset(); gpu_core.reset(); host1x_core.reset(); - perf_stats.reset(); + { + std::scoped_lock lk{perf_stats_mutex}; + perf_stats.reset(); + } cpu_manager.Shutdown(); debugger.reset(); kernel.Shutdown(); @@ -556,6 +563,7 @@ struct System::Impl { } PerfStatsResults GetAndResetPerfStats() { + std::scoped_lock lk{perf_stats_mutex}; if (!perf_stats) { return {}; } @@ -590,6 +598,7 @@ struct System::Impl { std::optional general_channel_context; std::optional general_channel_event; std::optional perf_stats; + std::mutex perf_stats_mutex; std::optional host1x_core; std::optional device_memory; std::optional audio_core; @@ -622,9 +631,11 @@ struct System::Impl { std::atomic_bool is_powered_on{}; bool is_multicore : 1 = false; bool extended_memory_layout : 1 = false; - bool exit_locked : 1 = false; - bool exit_requested : 1 = false; - bool nvdec_active : 1 = false; + // Separate atomics: these are set from service, NVDEC and frontend threads, and bit-fields + // sharing a byte lost concurrent updates. + std::atomic_bool exit_locked{}; + std::atomic_bool exit_requested{}; + std::atomic_bool nvdec_active{}; /// EDEN_NX2_DEBUG_SERVER; the last member, so it stops before anything it reads goes away. std::unique_ptr debug_service; diff --git a/src/core/core_timing.cpp b/src/core/core_timing.cpp index dfc3f6f2a6..1f677c15c7 100644 --- a/src/core/core_timing.cpp +++ b/src/core/core_timing.cpp @@ -110,12 +110,13 @@ void CoreTiming::ClearPendingEvents() { } void CoreTiming::Pause(bool is_paused) { - paused = is_paused; - pause_event.Set(); - if (!is_paused) { - pause_end_time = GetGlobalTimeNs().count(); + // Store the boundary before clearing paused: the timer thread runs Advance as soon as + // it reads paused == false, and that (sequentially consistent) load makes it visible. + pause_end_time.store(GetGlobalTimeNs().count(), std::memory_order_relaxed); } + paused = is_paused; + pause_event.Set(); } void CoreTiming::SyncPause(bool is_paused) { @@ -132,10 +133,6 @@ void CoreTiming::SyncPause(bool is_paused) { while (paused_set != is_paused) ; } - - if (!is_paused) { - pause_end_time = GetGlobalTimeNs().count(); - } } bool CoreTiming::IsRunning() const { @@ -283,8 +280,9 @@ std::optional CoreTiming::Advance() { // If this event was scheduled into a pause, its time now is going to be way // behind. Re-set this event to continue from the end of the pause. auto next_time{evt.time + next_schedule_time}; - if (evt.time < pause_end_time) { - next_time = pause_end_time + next_schedule_time; + const s64 last_pause_end = pause_end_time.load(std::memory_order_relaxed); + if (evt.time < last_pause_end) { + next_time = last_pause_end + next_schedule_time; } event_queue.update(evt.handle, Event{next_time, event_fifo_id++, evt.type, diff --git a/src/core/core_timing.h b/src/core/core_timing.h index 298fc9595d..13d9d492d8 100644 --- a/src/core/core_timing.h +++ b/src/core/core_timing.h @@ -149,7 +149,7 @@ class CoreTiming { s64 timer_resolution_ns; #endif u64 event_fifo_id = 0; - s64 pause_end_time{}; + std::atomic pause_end_time{}; /// Cycle timing u64 cpu_ticks{}; s64 downcount{}; diff --git a/src/core/file_sys/vfs/vfs_vector.cpp b/src/core/file_sys/vfs/vfs_vector.cpp index 4d4a90f640..dfb35ecc09 100644 --- a/src/core/file_sys/vfs/vfs_vector.cpp +++ b/src/core/file_sys/vfs/vfs_vector.cpp @@ -40,6 +40,9 @@ bool VectorVfsFile::IsReadable() const { } std::size_t VectorVfsFile::Read(u8* data_, std::size_t length, std::size_t offset) const { + if (offset >= data.size()) { + return 0; + } const auto read = (std::min)(length, data.size() - offset); std::memcpy(data_, data.data() + offset, read); return read; diff --git a/src/hid_core/resources/abstracted_pad/abstract_button_handler.cpp b/src/hid_core/resources/abstracted_pad/abstract_button_handler.cpp index 239fc7656e..2e4eea1659 100644 --- a/src/hid_core/resources/abstracted_pad/abstract_button_handler.cpp +++ b/src/hid_core/resources/abstracted_pad/abstract_button_handler.cpp @@ -79,6 +79,9 @@ void NpadAbstractButtonHandler::UpdateAllButtonLifo() { Core::HID::NpadIdType npad_id = properties_handler->GetNpadId(); for (std::size_t i = 0; i < AruidIndexMax; i++) { auto* data = applet_resource_holder->applet_resource->GetAruidDataByIndex(i); + if (data == nullptr || !data->flag.is_assigned) { + continue; + } auto& npad_entry = data->shared_memory_format->npad.npad_entry[NpadIdTypeToIndex(npad_id)]; UpdateButtonLifo(npad_entry, data->aruid); } @@ -88,6 +91,9 @@ void NpadAbstractButtonHandler::UpdateCoreBatteryState() { Core::HID::NpadIdType npad_id = properties_handler->GetNpadId(); for (std::size_t i = 0; i < AruidIndexMax; i++) { auto* data = applet_resource_holder->applet_resource->GetAruidDataByIndex(i); + if (data == nullptr || !data->flag.is_assigned) { + continue; + } auto& npad_entry = data->shared_memory_format->npad.npad_entry[NpadIdTypeToIndex(npad_id)]; UpdateButtonLifo(npad_entry, data->aruid); } diff --git a/src/input_common/drivers/sdl_driver.cpp b/src/input_common/drivers/sdl_driver.cpp index a308554071..a1092e7e38 100644 --- a/src/input_common/drivers/sdl_driver.cpp +++ b/src/input_common/drivers/sdl_driver.cpp @@ -9,6 +9,7 @@ #include "common/logging.h" #include "common/math_util.h" #include "common/param_package.h" +#include "common/scope_exit.h" #include "common/settings.h" #include "common/thread.h" #include "common/vector_math.h" @@ -538,6 +539,12 @@ void SDLDriver::InitJoystick(SDL_JoystickID joystick_id) { } } + // SDL calls its event watcher with the joystick lock held, and the watcher then takes + // joystick_map_mutex. Take SDL's lock first here, since SDLJoystick calls into SDL. + SDL_LockJoysticks(); + SCOPE_EXIT { + SDL_UnlockJoysticks(); + }; std::scoped_lock lock{joystick_map_mutex}; if (joystick_map.find(guid) == joystick_map.end()) { auto joystick = std::make_shared(guid, 0, sdl_joystick, sdl_gamecontroller); @@ -656,6 +663,11 @@ void SDLDriver::HandleGameControllerEvent(const SDL_Event& event) { } void SDLDriver::CloseJoysticks() { + // Same lock order as InitJoystick: clearing the map closes SDL handles. + SDL_LockJoysticks(); + SCOPE_EXIT { + SDL_UnlockJoysticks(); + }; std::scoped_lock lock{joystick_map_mutex}; joystick_map.clear(); } diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index d38658d6a9..355e3413f2 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -28,6 +28,7 @@ add_executable(tests core/nx2_debug_server.cpp core/nx2_movie_pacing.cpp core/priority_queue.cpp + core/vfs_vector.cpp core/internal_network/network.cpp video_core/frame_dump_schedule.cpp video_core/memory_tracker.cpp @@ -50,6 +51,7 @@ add_executable(tests video_core/uniform_stream_reuse.cpp video_core/shader_invalidation.cpp video_core/descriptor_table.cpp + video_core/usage_tracker.cpp video_core/global_address_tracking.cpp video_core/global_atomic_routing.cpp video_core/nvn_storage_descriptor.cpp diff --git a/src/tests/core/vfs_vector.cpp b/src/tests/core/vfs_vector.cpp new file mode 100644 index 0000000000..c51a0360eb --- /dev/null +++ b/src/tests/core/vfs_vector.cpp @@ -0,0 +1,25 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include + +#include + +#include "core/file_sys/vfs/vfs_vector.h" + +namespace { +TEST_CASE("VectorVfsFile reads stop at the end of the data", "[core][vfs]") { + const FileSys::VectorVfsFile file(std::vector{1, 2, 3, 4}); + std::array out{}; + + REQUIRE(file.Read(out.data(), out.size(), 2) == 2); + REQUIRE(out[0] == 3); + REQUIRE(out[1] == 4); + REQUIRE(file.Read(out.data(), out.size(), 4) == 0); + REQUIRE(file.Read(out.data(), out.size(), 100) == 0); + + const FileSys::VectorVfsFile empty; + REQUIRE(empty.Read(out.data(), out.size(), 0) == 0); +} +} // namespace diff --git a/src/tests/video_core/usage_tracker.cpp b/src/tests/video_core/usage_tracker.cpp new file mode 100644 index 0000000000..bff1b57f17 --- /dev/null +++ b/src/tests/video_core/usage_tracker.cpp @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + +#include + +#include "video_core/buffer_cache/usage_tracker.h" + +namespace { +using VideoCommon::UsageTracker; + +TEST_CASE("UsageTracker covers partly used 64-byte units", "[video_core][usage_tracker]") { + UsageTracker tracker(1 << 20); + tracker.Track(0, 100); + REQUIRE(tracker.IsUsed(0, 1)); + REQUIRE(tracker.IsUsed(64, 16)); + REQUIRE(tracker.IsUsed(96, 4)); + REQUIRE_FALSE(tracker.IsUsed(128, 64)); + + tracker.Reset(); + tracker.Track(60, 8); + REQUIRE(tracker.IsUsed(0, 4)); + REQUIRE(tracker.IsUsed(64, 4)); + REQUIRE_FALSE(tracker.IsUsed(128, 4)); +} + +TEST_CASE("UsageTracker does not mark a whole page for small ranges", "[video_core][usage_tracker]") { + UsageTracker tracker(1 << 20); + tracker.Track(4096 + 128, 16); + REQUIRE(tracker.IsUsed(4096 + 128, 16)); + REQUIRE_FALSE(tracker.IsUsed(4096 + 256, 64)); + REQUIRE_FALSE(tracker.IsUsed(4096, 64)); + + // A range ending on a page boundary marks nothing in the next page. + tracker.Reset(); + tracker.Track(4096 - 64, 64); + REQUIRE(tracker.IsUsed(4096 - 64, 64)); + REQUIRE_FALSE(tracker.IsUsed(4096, 64)); + REQUIRE_FALSE(tracker.IsUsed(4096 + 2048, 64)); +} + +TEST_CASE("UsageTracker spans pages", "[video_core][usage_tracker]") { + UsageTracker tracker(1 << 20); + tracker.Track(4096 - 32, 4096 + 64); + REQUIRE(tracker.IsUsed(4096 - 32, 1)); + REQUIRE(tracker.IsUsed(4096 + 1024, 1)); + REQUIRE(tracker.IsUsed(2 * 4096, 16)); + REQUIRE_FALSE(tracker.IsUsed(2 * 4096 + 64, 64)); + REQUIRE_FALSE(tracker.IsUsed(0, 64)); +} +} // namespace diff --git a/src/video_core/buffer_cache/usage_tracker.h b/src/video_core/buffer_cache/usage_tracker.h index 5dc58e4063..781486478a 100644 --- a/src/video_core/buffer_cache/usage_tracker.h +++ b/src/video_core/buffer_cache/usage_tracker.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -72,6 +73,9 @@ class UsageTracker { if (IsPageUsed(page, offset, size)) { return true; } + if (page == page_end) { + return false; + } for (size_t i = page + 1; i < page_end; i++) { if (pages[i] != 0) { return true; diff --git a/src/video_core/gpu.cpp b/src/video_core/gpu.cpp index 48cf6abce8..a834502181 100644 --- a/src/video_core/gpu.cpp +++ b/src/video_core/gpu.cpp @@ -66,7 +66,11 @@ struct GPU::Impl { , is_async{is_async_} {} - ~Impl() = default; + ~Impl() { + // The renderer's teardown still flushes work that touches the bound channel's state, + // and `channels` is declared after `renderer`, so it would be freed first. + renderer.reset(); + } std::shared_ptr CreateChannel(s32 channel_id) { auto channel_state = std::make_shared(channel_id);