From 2e9d7bd208c9dec152ce86bb4ebb2a5bab0b1289 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Thu, 16 May 2024 00:17:25 +0200 Subject: [PATCH] Revert "Artic Base: Fix out of bounds cache reads (#127)" (#129) This reverts commit 05cccb585d7c248784e547ae65ca595823c385c7. --- src/common/static_lru_cache.h | 8 ----- src/core/file_sys/artic_cache.cpp | 59 ++++++------------------------- 2 files changed, 11 insertions(+), 56 deletions(-) diff --git a/src/common/static_lru_cache.h b/src/common/static_lru_cache.h index 46c3ce4de..bd692e94e 100644 --- a/src/common/static_lru_cache.h +++ b/src/common/static_lru_cache.h @@ -88,16 +88,8 @@ public: } } - void invalidate(const key_type& key) { - auto i = find(key); - if (i != m_list.cend()) { - m_list.erase(i); - } - } - void clear() { m_list.clear(); - m_array.fill(value_type{}); } private: diff --git a/src/core/file_sys/artic_cache.cpp b/src/core/file_sys/artic_cache.cpp index 7ce2f0346..b5c963495 100644 --- a/src/core/file_sys/artic_cache.cpp +++ b/src/core/file_sys/artic_cache.cpp @@ -25,20 +25,13 @@ ResultVal ArticCache::Read(s32 file_handle, std::size_t offset, std auto res = ReadFromArtic(file_handle, reinterpret_cast(big_cache_entry.second.data()), length, offset); - if (res.Failed()) { - big_cache.invalidate(std::make_pair(offset, length)); + if (res.Failed()) return res; - } - read_progress = res.Unwrap(); + length = res.Unwrap(); } else { - LOG_TRACE(Service_FS, "ArticCache BHIT: offset={}, length={}", offset, - read_progress); - } - memcpy(buffer, big_cache_entry.second.data(), read_progress); - if (read_progress < length) { - // Invalidate the entry as it is not fully read - big_cache.invalidate(std::make_pair(offset, length)); + LOG_TRACE(Service_FS, "ArticCache BHIT: offset={}, length={}", offset, length); } + memcpy(buffer, big_cache_entry.second.data(), length); } else { if (segments[0].second < very_big_cache_skip) { std::unique_lock very_big_read_guard(very_big_cache_mutex); @@ -51,39 +44,28 @@ ResultVal ArticCache::Read(s32 file_handle, std::size_t offset, std auto res = ReadFromArtic( file_handle, reinterpret_cast(very_big_cache_entry.second.data()), length, offset); - if (res.Failed()) { - very_big_cache.invalidate(std::make_pair(offset, length)); + if (res.Failed()) return res; - } - read_progress = res.Unwrap(); + length = res.Unwrap(); } else { - LOG_TRACE(Service_FS, "ArticCache VBHIT: offset={}, length={}", offset, - read_progress); - } - memcpy(buffer, very_big_cache_entry.second.data(), read_progress); - if (read_progress < length) { - // Invalidate the entry as it is not fully read - very_big_cache.invalidate(std::make_pair(offset, length)); + LOG_TRACE(Service_FS, "ArticCache VBHIT: offset={}, length={}", offset, length); } + memcpy(buffer, very_big_cache_entry.second.data(), length); } else { LOG_TRACE(Service_FS, "ArticCache SKIP: offset={}, length={}", offset, length); auto res = ReadFromArtic(file_handle, buffer, length, offset); if (res.Failed()) return res; - read_progress = res.Unwrap(); + length = res.Unwrap(); } } - return read_progress; + return length; } // TODO(PabloMK7): Make cache thread safe, read the comment in CacheReady function. std::unique_lock read_guard(cache_mutex); - bool read_past_end = false; for (const auto& seg : segments) { - if (read_past_end) { - break; - } std::size_t read_size = cache_line_size; std::size_t page = OffsetToPage(seg.first); // Check if segment is in cache @@ -91,28 +73,9 @@ ResultVal ArticCache::Read(s32 file_handle, std::size_t offset, std if (!cache_entry.first) { // If not found, read from artic and cache the data auto res = ReadFromArtic(file_handle, cache_entry.second.data(), read_size, page); - if (res.Failed()) { - // Invalidate the requested entry as it is not populated - cache.invalidate(page); - - // In the very unlikely case the file size is a multiple of the cache size, - // and the game request more data than the file size, this will save us from - // returning an incorrect out of bounds error caused by reading at just the very end - // of the file. - constexpr u32 out_of_bounds_read = 714; - if (res.Code().description == out_of_bounds_read) { - return read_progress; - } + if (res.Failed()) return res; - } - size_t expected_read_size = read_size; read_size = res.Unwrap(); - // - if (read_size < expected_read_size) { - // Invalidate the requested entry as it is not fully read - cache.invalidate(page); - read_past_end = true; - } LOG_TRACE(Service_FS, "ArticCache MISS: page={}, length={}, into={}", page, seg.second, (seg.first - page)); } else {