diff --git a/Common/ExceptionHandlerSetup.cpp b/Common/ExceptionHandlerSetup.cpp index e26b9c7d85..2cdb36a87f 100644 --- a/Common/ExceptionHandlerSetup.cpp +++ b/Common/ExceptionHandlerSetup.cpp @@ -312,6 +312,28 @@ static struct sigaction old_sa_bus; static stack_t old_signal_stack{}; static bool old_signal_stack_valid = false; +// Hand the signal on to whatever was installed before us. Returning from a fault handler just +// re-runs the faulting instruction, so for anything we can't deal with this is the only exit +// that isn't an infinite loop. +static void ChainToPreviousHandler(int sig, siginfo_t *info, void *raw_context) { + struct sigaction *old_sa = sig == SIGSEGV ? &old_sa_segv : &old_sa_bus; + // Per the sigaction man page: with SA_SIGINFO it's sa_sigaction, otherwise sa_handler is + // SIG_DFL, SIG_IGN, or a handler pointer. + if (old_sa->sa_flags & SA_SIGINFO) { + old_sa->sa_sigaction(sig, info, raw_context); + return; + } + if (old_sa->sa_handler == SIG_DFL) { + signal(sig, SIG_DFL); + return; + } + if (old_sa->sa_handler == SIG_IGN) { + // Ignore signal + return; + } + old_sa->sa_handler(sig); +} + static void sigsegv_handler(int sig, siginfo_t* info, void* raw_context) { if (sig != SIGSEGV && sig != SIGBUS) { // We are not interested in other signals - handle it as usual. @@ -320,7 +342,11 @@ static void sigsegv_handler(int sig, siginfo_t* info, void* raw_context) { ucontext_t* context = (ucontext_t*)raw_context; int sicode = info->si_code; if (sicode != SEGV_MAPERR && sicode != SEGV_ACCERR) { - // Huh? Return. + // Not an address fault we can do anything with - an MTE, protection-key or shadow + // stack fault, or a signal sent with kill(). Returning here would re-run the + // faulting instruction forever at 100% CPU, and would also swallow the signal from + // whatever was installed before us (a crash reporter, say). + ChainToPreviousHandler(sig, info, raw_context); return; } uintptr_t bad_address = (uintptr_t)info->si_addr; @@ -346,26 +372,7 @@ static void sigsegv_handler(int sig, siginfo_t* info, void* raw_context) { // SIG_IGN: The signal is ignored // Any other value is a function pointer to a signal handler - struct sigaction* old_sa; - if (sig == SIGSEGV) { - old_sa = &old_sa_segv; - } else { - old_sa = &old_sa_bus; - } - - if (old_sa->sa_flags & SA_SIGINFO) { - old_sa->sa_sigaction(sig, info, raw_context); - return; - } - if (old_sa->sa_handler == SIG_DFL) { - signal(sig, SIG_DFL); - return; - } - if (old_sa->sa_handler == SIG_IGN) { - // Ignore signal - return; - } - old_sa->sa_handler(sig); + ChainToPreviousHandler(sig, info, raw_context); } } diff --git a/Common/MemoryUtil.cpp b/Common/MemoryUtil.cpp index be2a14ddbe..d4de2cf131 100644 --- a/Common/MemoryUtil.cpp +++ b/Common/MemoryUtil.cpp @@ -145,8 +145,17 @@ void *AllocateExecutableMemory(size_t size) { #endif if (ptr) { ptr = VirtualAlloc(ptr, aligned_size, MEM_RESERVE | MEM_COMMIT, prot); + if (!ptr) { + // Finding a free region isn't the same as being able to reserve it: VirtualAlloc + // rounds a non-null address down to the 64K allocation granularity, and + // SearchForFreeMem only returns page alignment, so the rounded-down base can land + // back inside something committed. Another thread allocating in between does it too. + WARN_LOG(Log::Common, "Could not reserve the nearby executable memory found for jit. Proceeding with far memory."); + } } else { WARN_LOG(Log::Common, "Unable to find nearby executable memory for jit. Proceeding with far memory."); + } + if (!ptr) { // Can still run, thanks to "RipAccessible". ptr = VirtualAlloc(nullptr, aligned_size, MEM_RESERVE | MEM_COMMIT, prot); } diff --git a/Common/TimeUtil.cpp b/Common/TimeUtil.cpp index e01c3108e8..b508a30c92 100644 --- a/Common/TimeUtil.cpp +++ b/Common/TimeUtil.cpp @@ -238,12 +238,12 @@ double time_now_d() { } uint64_t time_now_raw() { - struct timeval tv; - gettimeofday(&tv, nullptr); - if (start == 0) { - start = tv.tv_sec; - } - return (double)tv.tv_sec + (double)tv.tv_usec * (1.0 / micros); + // Nanoseconds, like the other platforms - from_time_raw() scales by 1/nanos. This used to + // build a double of seconds and return it through the uint64_t, so it both lost the fraction + // and was off by a factor of a billion. + struct timespec tp; + clock_gettime(CLOCK_MONOTONIC, &tp); + return (uint64_t)tp.tv_sec * 1000000000ULL + tp.tv_nsec; } double from_time_raw(uint64_t raw_time) { @@ -257,7 +257,10 @@ double from_time_raw_relative(uint64_t raw_time) { void yield() {} double time_now_unix_utc() { - return time_now_raw(); + // Not time_now_raw() - that's a monotonic clock with no relation to the epoch. + struct timeval tv; + gettimeofday(&tv, nullptr); + return (double)tv.tv_sec + (double)tv.tv_usec * (1.0 / micros); } double time_to_unix_utc(double t) { @@ -267,10 +270,12 @@ double time_to_unix_utc(double t) { } Instant::Instant() { - struct timeval tv; - gettimeofday(&tv, nullptr); - nativeStart_ = tv.tv_sec; - nsecs_ = tv.tv_usec; + // Has to be the same clock, and the same unit, as ElapsedNanos() below: this took the wall + // clock in microseconds while that one subtracts it from a monotonic clock in nanoseconds. + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + nativeStart_ = ts.tv_sec; + nsecs_ = ts.tv_nsec; } int64_t Instant::ElapsedNanos() const { @@ -278,12 +283,12 @@ int64_t Instant::ElapsedNanos() const { clock_gettime(CLOCK_MONOTONIC, &ts); int64_t secs = ts.tv_sec - nativeStart_; - int64_t usecs = ts.tv_nsec - nsecs_; - if (usecs < 0) { + int64_t nsecs = ts.tv_nsec - nsecs_; + if (nsecs < 0) { secs--; - usecs += 1000000; + nsecs += 1000000000; } - return secs * 1000000000 + usecs * 1000; + return secs * 1000000000 + nsecs; } double Instant::ElapsedSeconds() const { diff --git a/Core/Core.cpp b/Core/Core.cpp index 8c5e8c5156..bee40b41f8 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -913,7 +913,7 @@ void Core_MemoryExceptionHLE(MIPSState *mips, u32 address, u32 accessSize, Memor // We try to derive the reason here, though maybe it should be passed in explicitly? // TODO: This check should probably be added to regular memory accesses too. if (Memory::IsValidAddress(address)) { - if (accessSize == 2 || accessSize == 4 || accessSize == 8 || (address & (accessSize - 1))) { + if ((accessSize == 2 || accessSize == 4 || accessSize == 8) && (address & (accessSize - 1))) { extra = " (unaligned)"; } else if (accessSize > 8 && (accessSize & 3)) { extra = " (unaligned struct)"; diff --git a/Core/Instance.cpp b/Core/Instance.cpp index fcb350f82e..09cba0c3f5 100644 --- a/Core/Instance.cpp +++ b/Core/Instance.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #endif @@ -96,10 +97,16 @@ static bool UpdateInstanceCounter(void (*callback)(volatile InstanceInfo *)) { } bool result = false; - if (mlock(buf, BUF_SIZE) == 0) { + // An actual advisory lock on the shm object. This used to call mlock(), which only pins + // pages in RAM and provides no mutual exclusion whatsoever - two instances starting at the + // same moment could both read the counter and come away with the same PPSSPP_ID, then both + // believe they were the first instance and write the config over each other. + if (flock(hIDMapFile, LOCK_EX) == 0) { callback(buf); - munlock(buf, BUF_SIZE); + flock(hIDMapFile, LOCK_UN); result = true; + } else { + ERROR_LOG(Log::sceNet, "flock(%s) failure: %s", ID_SHM_NAME, GetLastErrorMsg().c_str()); } munmap(buf, BUF_SIZE); @@ -157,7 +164,14 @@ void InitInstanceCounter() { #endif bool success = UpdateInstanceCounter([](volatile InstanceInfo *buf) { - PPSSPP_ID = ++buf->next; + // The shared segment outlives the processes that used it (see the shm_unlink comment), + // so next keeps climbing across runs and eventually wraps this uint8_t. ID 0 is not a + // valid instance - it fails the IsFirstInstance() check, which quietly disables config + // saving - so skip past it. + if (++buf->next == 0) { + buf->next = 1; + } + PPSSPP_ID = buf->next; buf->total++; }); if (!success) { diff --git a/Core/PSPLoaders.cpp b/Core/PSPLoaders.cpp index f6d0a5090a..72fe344601 100644 --- a/Core/PSPLoaders.cpp +++ b/Core/PSPLoaders.cpp @@ -160,11 +160,21 @@ void InitMemorySizeForGame() { } if (umdData.empty()) { - std::vector umdDataBin; - // .data() rather than &umdDataBin[0] - the file can legitimately be empty, and - // indexing an empty vector is undefined. - if (pspFileSystem.ReadEntireFile("disc0:/UMD_DATA.BIN", umdDataBin) >= 0) { - umdData = std::string((const char *)umdDataBin.data(), umdDataBin.size()); + // A real UMD_DATA.BIN is a few dozen bytes - it's just the disc ID line matched below. + // Check the size first: this comes off the disc image, which is not something we trust, + // and ReadEntireFile has no limit of its own - it would resize a vector to whatever the + // image claims, and the string copy after it doubles that. + const s64 MAX_UMD_DATA_SIZE = 4096; + const PSPFileInfo info = pspFileSystem.GetFileInfo("disc0:/UMD_DATA.BIN"); + if (info.exists && info.size > MAX_UMD_DATA_SIZE) { + WARN_LOG(Log::Loader, "Ignoring implausibly large UMD_DATA.BIN (%lld bytes)", (long long)info.size); + } else if (info.exists) { + std::vector umdDataBin; + // .data() rather than &umdDataBin[0] - the file can legitimately be empty, and + // indexing an empty vector is undefined. + if (pspFileSystem.ReadEntireFile("disc0:/UMD_DATA.BIN", umdDataBin) >= 0) { + umdData = std::string((const char *)umdDataBin.data(), umdDataBin.size()); + } } } diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index cc921feb6d..05ce10c43d 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -2627,7 +2627,7 @@ bool FramebufferManagerCommon::NotifyBlockTransferBefore(u32 dstBasePtr, int dst if (srcRect.channel == RASTER_DEPTH) { // Ignore the found buffer if it's not 16-bit - we create a new more suitable one instead. - if (dstRect.channel == RASTER_COLOR && dstRect.vfb->fb_format == GE_FORMAT_8888) { + if (dstBuffer && dstRect.channel == RASTER_COLOR && dstRect.vfb->fb_format == GE_FORMAT_8888) { dstBuffer = false; } } diff --git a/GPU/Common/ReplacedTexture.cpp b/GPU/Common/ReplacedTexture.cpp index 6da8465662..0b8b7f2d00 100644 --- a/GPU/Common/ReplacedTexture.cpp +++ b/GPU/Common/ReplacedTexture.cpp @@ -106,7 +106,10 @@ ReplacedTexture::~ReplacedTexture() { } for (auto &level : levels_) { - vfs_->ReleaseFile(level.fileRef); + // Null when replacement was switched off after we were cached - see NotifyConfigChanged. + if (vfs_) { + vfs_->ReleaseFile(level.fileRef); + } level.fileRef = nullptr; } } @@ -201,7 +204,11 @@ inline uint32_t RoundUpTo4(uint32_t value) { } void ReplacedTexture::Prepare(VFSBackend *vfs) { - _assert_(vfs != nullptr); + if (!vfs) { + // Replacement was switched off while this was queued. Nothing to load from any more. + SetState(ReplacementState::NOT_FOUND); + return; + } this->vfs_ = vfs; diff --git a/GPU/Common/TextureReplacer.cpp b/GPU/Common/TextureReplacer.cpp index c170ae96d1..e7d035e26a 100644 --- a/GPU/Common/TextureReplacer.cpp +++ b/GPU/Common/TextureReplacer.cpp @@ -109,6 +109,12 @@ void TextureReplacer::NotifyConfigChanged() { } if (!replaceEnabled_ && wasReplaceEnabled) { + // Everything in levelCache_ holds this pointer - LoadIni fixes them up when it swaps the + // VFS, and the same has to happen here or they're left dangling. Decimate(ALL) below only + // frees their data, it doesn't erase the entries. + for (auto &repl : levelCache_) { + repl.second->vfs_ = nullptr; + } delete vfs_; vfs_ = nullptr; Decimate(ReplacerDecimateMode::ALL); @@ -178,6 +184,14 @@ bool TextureReplacer::LoadIni(std::string *error, bool notify) { if (ini.GetOrCreateSection("games")->Get(gameID_.c_str(), &overrideFilename)) { if (overrideFilename == "true") { // Ignore it + } else if (HasParentDirComponent(overrideFilename)) { + // Same reason the [hashes] filenames are checked: for a directory-backed pack, + // DirectoryReader resolves this against the pack directory, so "../.." reaches + // anything on disk. Third-party packs are just downloads. + *error = "Override ini name must stay inside the pack: '" + overrideFilename + "'"; + ERROR_LOG(Log::TexReplacement, "%s", error->c_str()); + delete dir; + return false; } else if (!overrideFilename.empty() && overrideFilename != INI_FILENAME) { IniFile overrideIni; iniLoaded = overrideIni.LoadFromVFS(*dir, overrideFilename);