summaryrefslogtreecommitdiff
path: root/src/video_core
diff options
context:
space:
mode:
authorGravatar comex2023-07-01 15:00:39 -0700
committerGravatar comex2023-07-15 12:00:28 -0700
commitd7c532d8894ce806c9af13b8dd3eec975642b348 (patch)
tree1353e6d1776384575019db28987e23237552a9de /src/video_core
parentfile_sys/content_archive: Detect compressed NCAs (#11047) (diff)
downloadyuzu-d7c532d8894ce806c9af13b8dd3eec975642b348.tar.gz
yuzu-d7c532d8894ce806c9af13b8dd3eec975642b348.tar.xz
yuzu-d7c532d8894ce806c9af13b8dd3eec975642b348.zip
Fixes and workarounds to make UBSan happier on macOS
There are still some other issues not addressed here, but it's a start. Workarounds for false-positive reports: - `RasterizerAccelerated`: Put a gigantic array behind a `unique_ptr`, because UBSan has a [hardcoded limit](https://stackoverflow.com/questions/64531383/c-runtime-error-using-fsanitize-undefined-object-has-a-possibly-invalid-vp) of how big it thinks objects can be, specifically when dealing with offset-to-top values used with multiple inheritance. Hopefully this doesn't have a performance impact. - `QueryCacheBase::QueryCacheBase`: Avoid an operation that UBSan thinks is UB even though it at least arguably isn't. See the link in the comment for more information. Fixes for correct reports: - `PageTable`, `Memory`: Use `uintptr_t` values instead of pointers to avoid UB from pointer overflow (when pointer arithmetic wraps around the address space). - `KScheduler::Reload`: `thread->GetOwnerProcess()` can be `nullptr`; avoid calling methods on it in this case. (The existing code returns a garbage reference to a field, which is then passed into `LoadWatchpointArray`, and apparently it's never used, so it's harmless in practice but still triggers UBSan.) - `KAutoObject::Close`: This function calls `this->Destroy()`, which overwrites the beginning of the object with junk (specifically a free list pointer). Then it calls `this->UnregisterWithKernel()`. UBSan complains about a type mismatch because the vtable has been overwritten, and I believe this is indeed UB. `UnregisterWithKernel` also loads `m_kernel` from the 'freed' object, which seems to be technically safe (the overwriting doesn't extend as far as that field), but seems dubious. Switch to a `static` method and load `m_kernel` in advance.
Diffstat (limited to 'src/video_core')
-rw-r--r--src/video_core/query_cache.h4
-rw-r--r--src/video_core/rasterizer_accelerated.cpp5
-rw-r--r--src/video_core/rasterizer_accelerated.h3
3 files changed, 8 insertions, 4 deletions
diff --git a/src/video_core/query_cache.h b/src/video_core/query_cache.h
index 1528cc1dd..7047e2e63 100644
--- a/src/video_core/query_cache.h
+++ b/src/video_core/query_cache.h
@@ -103,7 +103,9 @@ public:
103 explicit QueryCacheBase(VideoCore::RasterizerInterface& rasterizer_, 103 explicit QueryCacheBase(VideoCore::RasterizerInterface& rasterizer_,
104 Core::Memory::Memory& cpu_memory_) 104 Core::Memory::Memory& cpu_memory_)
105 : rasterizer{rasterizer_}, 105 : rasterizer{rasterizer_},
106 cpu_memory{cpu_memory_}, streams{{CounterStream{static_cast<QueryCache&>(*this), 106 // Use reinterpret_cast instead of static_cast as workaround for
107 // UBSan bug (https://github.com/llvm/llvm-project/issues/59060)
108 cpu_memory{cpu_memory_}, streams{{CounterStream{reinterpret_cast<QueryCache&>(*this),
107 VideoCore::QueryType::SamplesPassed}}} { 109 VideoCore::QueryType::SamplesPassed}}} {
108 (void)slot_async_jobs.insert(); // Null value 110 (void)slot_async_jobs.insert(); // Null value
109 } 111 }
diff --git a/src/video_core/rasterizer_accelerated.cpp b/src/video_core/rasterizer_accelerated.cpp
index 4a197d65d..f200a650f 100644
--- a/src/video_core/rasterizer_accelerated.cpp
+++ b/src/video_core/rasterizer_accelerated.cpp
@@ -13,7 +13,8 @@ namespace VideoCore {
13 13
14using namespace Core::Memory; 14using namespace Core::Memory;
15 15
16RasterizerAccelerated::RasterizerAccelerated(Memory& cpu_memory_) : cpu_memory{cpu_memory_} {} 16RasterizerAccelerated::RasterizerAccelerated(Memory& cpu_memory_)
17 : cached_pages(std::make_unique<CachedPages>()), cpu_memory{cpu_memory_} {}
17 18
18RasterizerAccelerated::~RasterizerAccelerated() = default; 19RasterizerAccelerated::~RasterizerAccelerated() = default;
19 20
@@ -26,7 +27,7 @@ void RasterizerAccelerated::UpdatePagesCachedCount(VAddr addr, u64 size, int del
26 std::atomic_thread_fence(std::memory_order_acquire); 27 std::atomic_thread_fence(std::memory_order_acquire);
27 const u64 page_end = Common::DivCeil(addr + size, YUZU_PAGESIZE); 28 const u64 page_end = Common::DivCeil(addr + size, YUZU_PAGESIZE);
28 for (u64 page = addr >> YUZU_PAGEBITS; page != page_end; ++page) { 29 for (u64 page = addr >> YUZU_PAGEBITS; page != page_end; ++page) {
29 std::atomic_uint16_t& count = cached_pages.at(page >> 2).Count(page); 30 std::atomic_uint16_t& count = cached_pages->at(page >> 2).Count(page);
30 31
31 if (delta > 0) { 32 if (delta > 0) {
32 ASSERT_MSG(count.load(std::memory_order::relaxed) < UINT16_MAX, "Count may overflow!"); 33 ASSERT_MSG(count.load(std::memory_order::relaxed) < UINT16_MAX, "Count may overflow!");
diff --git a/src/video_core/rasterizer_accelerated.h b/src/video_core/rasterizer_accelerated.h
index 7118b8aff..e6c0ea87a 100644
--- a/src/video_core/rasterizer_accelerated.h
+++ b/src/video_core/rasterizer_accelerated.h
@@ -41,7 +41,8 @@ private:
41 }; 41 };
42 static_assert(sizeof(CacheEntry) == 8, "CacheEntry should be 8 bytes!"); 42 static_assert(sizeof(CacheEntry) == 8, "CacheEntry should be 8 bytes!");
43 43
44 std::array<CacheEntry, 0x2000000> cached_pages; 44 using CachedPages = std::array<CacheEntry, 0x2000000>;
45 std::unique_ptr<CachedPages> cached_pages;
45 Core::Memory::Memory& cpu_memory; 46 Core::Memory::Memory& cpu_memory;
46}; 47};
47 48