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.
This commit is contained in:
comex 2023-07-01 15:00:39 -07:00
parent 04868ab9da
commit d7c532d889
11 changed files with 42 additions and 32 deletions

View file

@ -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