From d7c532d8894ce806c9af13b8dd3eec975642b348 Mon Sep 17 00:00:00 2001
From: comex <comexk@gmail.com>
Date: Sat, 1 Jul 2023 15:00:39 -0700
Subject: 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.
---
 src/common/page_table.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'src/common')

diff --git a/src/common/page_table.h b/src/common/page_table.h
index fec8378f3..e653d52ad 100644
--- a/src/common/page_table.h
+++ b/src/common/page_table.h
@@ -51,7 +51,7 @@ struct PageTable {
     class PageInfo {
     public:
         /// Returns the page pointer
-        [[nodiscard]] u8* Pointer() const noexcept {
+        [[nodiscard]] uintptr_t Pointer() const noexcept {
             return ExtractPointer(raw.load(std::memory_order_relaxed));
         }
 
@@ -61,7 +61,7 @@ struct PageTable {
         }
 
         /// Returns the page pointer and attribute pair, extracted from the same atomic read
-        [[nodiscard]] std::pair<u8*, PageType> PointerType() const noexcept {
+        [[nodiscard]] std::pair<uintptr_t, PageType> PointerType() const noexcept {
             const uintptr_t non_atomic_raw = raw.load(std::memory_order_relaxed);
             return {ExtractPointer(non_atomic_raw), ExtractType(non_atomic_raw)};
         }
@@ -73,13 +73,13 @@ struct PageTable {
         }
 
         /// Write a page pointer and type pair atomically
-        void Store(u8* pointer, PageType type) noexcept {
-            raw.store(reinterpret_cast<uintptr_t>(pointer) | static_cast<uintptr_t>(type));
+        void Store(uintptr_t pointer, PageType type) noexcept {
+            raw.store(pointer | static_cast<uintptr_t>(type));
         }
 
         /// Unpack a pointer from a page info raw representation
-        [[nodiscard]] static u8* ExtractPointer(uintptr_t raw) noexcept {
-            return reinterpret_cast<u8*>(raw & (~uintptr_t{0} << ATTRIBUTE_BITS));
+        [[nodiscard]] static uintptr_t ExtractPointer(uintptr_t raw) noexcept {
+            return raw & (~uintptr_t{0} << ATTRIBUTE_BITS);
         }
 
         /// Unpack a page type from a page info raw representation
-- 
cgit v1.2.3