diff --git a/AGENTS.md b/AGENTS.md index 4a761f7892..ccc5e82c7f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,10 +15,28 @@ Ignore the folder ai_instructions in the root directory, it's old stuff from con `newline=''` silently converts the whole file, turning a two-line addition into a 5000-line diff. Check `git diff --stat` before committing - a whole-file rewrite is obvious there and invisible in the editor. Prefer the Edit tool, which does exact string replacement and can't do this. -5. **Don't feed Python to `bash -c` via a heredoc when the code contains backslashes.** The quoting mangles them, - and an anchor string like `'...MemBlockInfo.cpp \\\r\n'` silently fails to match, so the patch reports - "anchor missing" for reasons that aren't visible. Write the script to a file and run that instead, building - separators with `chr(92)` if need be. +5. **Don't feed Python to `bash -c` via a heredoc when the code contains backslashes.** The Git Bash / MinGW + layer strips one level of backslash escaping on the way in, *even with a quoted delimiter* (`<<'PY'`), which + normally suppresses all substitution. So the script Python receives is not the one you wrote: + + | You write in the heredoc | Python actually sees | Result | + |---|---|---| + | `"\r\n"` | `"\r\n"` | fine - survives, because you *want* Python to interpret it | + | `"\\n"` (intending a literal `\n` in the output) | `"\n"` | **a real newline is written into the file** | + | `'foo \\\r\n'` as a match anchor | `'foo \'` | **anchor silently doesn't match**, reported as "anchor missing" | + + The tell for the first case is a compiler error like `C2001: newline in string literal`; the second case + produces no error at all, just a patch that quietly did nothing. Both are invisible in the heredoc you wrote. + + The rule: **a heredoc is fine as long as every backslash in the script is one you want Python to interpret. + The moment you need a literal backslash in the *output*, stop.** Then either: + - use the Edit tool instead (exact string replacement, no shell in the path - the best option for the common + case of "insert a few lines of C++ that contain `\n`"), or + - write the script to a file with the Write tool and run `python thescript.py`, or + - build the backslash as `chr(92)` so no literal backslash appears in the heredoc at all. + + Note this is about the *Python source*, not the data: reading and rewriting a CRLF file with `newline=''` and + `\r\n` anchors works fine in a heredoc, and is the normal way to patch files here (see rule 4). ## Core Safety Checks diff --git a/Common/MemArenaHorizon.cpp b/Common/MemArenaHorizon.cpp index a57028ffa7..380fc1a8bb 100644 --- a/Common/MemArenaHorizon.cpp +++ b/Common/MemArenaHorizon.cpp @@ -50,11 +50,13 @@ void *MemArena::CreateView(s64 offset, size_t size, void *base) { if (R_FAILED(rc)) { printf("Fatal error creating the view... base: %p offset: %p size: %p src: %p err: %d\n", (void *)base, (void *)offset, (void *)size, (void *)(memoryCodeBase + offset), rc); - } else { - printf("Created the view... base: %p offset: %p size: %p src: %p err: %d\n", - (void *)base, (void *)offset, (void *)size, (void *)(memoryCodeBase + offset), rc); + // Returning base here reports success, so the caller happily uses an unmapped address + // and we take a fault later with nothing pointing back at this. + return nullptr; } + printf("Created the view... base: %p offset: %p size: %p src: %p err: %d\n", + (void *)base, (void *)offset, (void *)size, (void *)(memoryCodeBase + offset), rc); return base; } diff --git a/Common/MemArenaPosix.cpp b/Common/MemArenaPosix.cpp index a413f4eaf2..38598a2d7e 100644 --- a/Common/MemArenaPosix.cpp +++ b/Common/MemArenaPosix.cpp @@ -97,7 +97,11 @@ bool MemArena::GrabMemSpace(size_t size) { } if (ftruncate(fd, size) != 0) { ERROR_LOG(Log::MemMap, "Failed to ftruncate %d (%s) to size %08x", (int)fd, ram_temp_file.c_str(), (int)size); - // Should this be a failure? + // This is a failure: the mmaps below succeed against a short file, and touching a page past + // its end raises SIGBUS - which is only hooked on __APPLE__, so elsewhere it's a bare crash. + close(fd); + fd = -1; + return false; } #endif return true; diff --git a/Common/MemArenaWin32.cpp b/Common/MemArenaWin32.cpp index 4e8d7f139d..70f69d3c1f 100644 --- a/Common/MemArenaWin32.cpp +++ b/Common/MemArenaWin32.cpp @@ -52,6 +52,16 @@ void *MemArena::CreateView(s64 offset, size_t size, void *viewbase) { size = roundup(size); #if PPSSPP_PLATFORM(UWP) // We just grabbed some RAM before using RESERVE. This commits it. + // + // NOTE: This ignores offset, so views don't alias - each gets its own storage. UWP defines + // MASKED_PSP_MEMORY, which folds the uncached and kernel address bits away before they get + // here, so the only casualties are the three VRAM mirrors at 0x04200000/0x04400000/0x04600000, + // which practically nothing depends on. + // + // Don't try to fix this with placeholders: CreateFileMappingFromApp + VirtualAlloc2FromApp + // (MEM_RESERVE_PLACEHOLDER) + MapViewOfFile3FromApp compiles and is available on the targeted + // SDK, but the very first reservation fails at runtime with ERROR_INVALID_ADDRESS (487) - not + // a per-view problem, the app container just won't hand out placeholders. Tried and reverted. void *ptr = VirtualAllocFromApp(viewbase, size, MEM_COMMIT, PAGE_READWRITE); #else void *ptr = MapViewOfFileEx(hMemoryMapping, FILE_MAP_ALL_ACCESS, 0, (DWORD)((u64)offset), size, viewbase); diff --git a/Core/CmdLine.cpp b/Core/CmdLine.cpp index 392bf38ab3..9f5a388265 100644 --- a/Core/CmdLine.cpp +++ b/Core/CmdLine.cpp @@ -72,11 +72,26 @@ ParseParamResult SetValue(CommandLineOptions *options, const CommandLineParam &p break; } case CmdParamType::Int: - *reinterpret_cast *>(optionsPtr + param.offsetInStruct) = std::stoi(value); + { + // Not std::stoi - that throws on junk, which takes the process down with no diagnostic. + int v; + if (sscanf(value.c_str(), "%d", &v) != 1) { + PRINT_STDERR("Error: Invalid value for integer parameter --%s: '%s'.\n", param.longName, value.c_str()); + return ParseParamResult::BadValue; + } + *reinterpret_cast *>(optionsPtr + param.offsetInStruct) = v; break; + } case CmdParamType::Double: - *reinterpret_cast *>(optionsPtr + param.offsetInStruct) = std::stod(value); + { + double v; + if (sscanf(value.c_str(), "%lf", &v) != 1) { + PRINT_STDERR("Error: Invalid value for number parameter --%s: '%s'.\n", param.longName, value.c_str()); + return ParseParamResult::BadValue; + } + *reinterpret_cast *>(optionsPtr + param.offsetInStruct) = v; break; + } case CmdParamType::String: *reinterpret_cast *>(optionsPtr + param.offsetInStruct) = value; break; diff --git a/Core/Compatibility.cpp b/Core/Compatibility.cpp index a526bf8852..4d0cba868f 100644 --- a/Core/Compatibility.cpp +++ b/Core/Compatibility.cpp @@ -213,7 +213,13 @@ void Compatibility::CheckSetting(IniFile &iniFile, const std::string &gameID, co std::string value; Section *section = iniFile.GetSection(option); if (section && section->Get(gameID.c_str(), &value)) { - *flag = stof(value); + // Not stof - it throws on a malformed entry, and compat.ini is user-editable. + float parsed; + if (sscanf(value.c_str(), "%f", &parsed) != 1) { + WARN_LOG(Log::Loader, "compat.ini: [%s] %s is not a number: '%s'", option, gameID.c_str(), value.c_str()); + return; + } + *flag = parsed; if (!activeList_.empty()) { activeList_ += "\n"; @@ -226,7 +232,12 @@ void Compatibility::CheckSetting(IniFile &iniFile, const std::string &gameID, co std::string value; Section *section = iniFile.GetSection(option); if (section && section->Get(gameID.c_str(), &value)) { - *flag = stoi(value); + int parsed; + if (sscanf(value.c_str(), "%d", &parsed) != 1) { + WARN_LOG(Log::Loader, "compat.ini: [%s] %s is not an integer: '%s'", option, gameID.c_str(), value.c_str()); + return; + } + *flag = parsed; if (!activeList_.empty()) { activeList_ += ":" + std::to_string(*flag) + "\n"; diff --git a/GPU/Common/DrawEngineCommon.cpp b/GPU/Common/DrawEngineCommon.cpp index 4ddcf8d326..8c0a6d3bcc 100644 --- a/GPU/Common/DrawEngineCommon.cpp +++ b/GPU/Common/DrawEngineCommon.cpp @@ -228,6 +228,13 @@ bool DrawEngineCommon::TestBoundingBox(const void *vdata, const void *inds, int if (vertexCount > 0 && inds) { GetIndexBounds(inds, vertexCount, vertType, &indexLowerBound, &indexUpperBound); + if (indexUpperBound > 1024) { + // NormalizeVertices below writes indexUpperBound - indexLowerBound + 1 vertices + // into corners, which only has room until the verts region above it. The index + // values are the game's, so the vertexCount cap doesn't bound them. A bbox test + // over this many verts is counter-productive anyway - say it's visible. + return true; + } } // TODO: Avoid normalization if just plain skinning. const u32 vertTypeID = GetVertTypeID(vertType, gstate.getUVGenMode()); diff --git a/GPU/Common/PostShader.cpp b/GPU/Common/PostShader.cpp index 2768cd211d..0603a25d08 100644 --- a/GPU/Common/PostShader.cpp +++ b/GPU/Common/PostShader.cpp @@ -163,6 +163,12 @@ void LoadPostShaderInfo(Draw::DrawContext *draw, const std::vector &direct section.Get("OutputResolution", &info.outputResolution); section.Get("Upscaling", &info.isUpscalingFilter); section.Get("SSAA", &info.SSAAFilterLevel); + if (info.SSAAFilterLevel < 0 || info.SSAAFilterLevel > 8) { + // It multiplies the render resolution, and shader inis come from downloads - + // the neighbouring texture-shader "Scale" is bounded for the same reason. + WARN_LOG(Log::G3D, "Ignoring out-of-range SSAA level %d in shader '%s'", info.SSAAFilterLevel, info.section.c_str()); + info.SSAAFilterLevel = 0; + } section.Get("60fps", &info.requires60fps); section.Get("UsePreviousFrame", &info.usePreviousFrame); diff --git a/GPU/Common/TextureReplacer.cpp b/GPU/Common/TextureReplacer.cpp index e7d035e26a..c6f98e4621 100644 --- a/GPU/Common/TextureReplacer.cpp +++ b/GPU/Common/TextureReplacer.cpp @@ -490,6 +490,12 @@ void TextureReplacer::ParseHashRange(const std::string &key, const std::string & return; } + if (toW == 0 || toH == 0) { + // These end up as desc_.newW/newH, which ReplacedTexture::Prepare divides by. + ERROR_LOG(Log::TexReplacement, "Ignoring invalid hashrange %s = %s, range is empty", key.c_str(), value.c_str()); + return; + } + const u64 rangeKey = ((u64)addr << 32) | ((u64)fromW << 16) | fromH; hashranges_[rangeKey] = WidthHeightPair(toW, toH); } diff --git a/UI/CustomButtonMappingScreen.cpp b/UI/CustomButtonMappingScreen.cpp index 0fc971c36b..f402e03f25 100644 --- a/UI/CustomButtonMappingScreen.cpp +++ b/UI/CustomButtonMappingScreen.cpp @@ -138,6 +138,9 @@ void CustomButtonMappingScreen::CreateDialogViews(UI::ViewGroup *parent) { bool *show = nullptr; memset(array, 0, sizeof(array)); cfg = &g_Config.CustomButton[id_]; + // This screen is reachable from the main menu, so neither GamepadEmu nor the layout screen + // need have sanitized these yet - and everything below indexes the tables with them. + CustomKeyData::Sanitize(*cfg); show = &touch.touchCustom[id_].show; for (int i = 0; i < ARRAY_SIZE(g_customKeyList); i++) array[i] = (0x01 == ((g_Config.CustomButton[id_].key >> i) & 0x01)); diff --git a/UI/GamepadEmu.cpp b/UI/GamepadEmu.cpp index 2ff6505b40..d49edbfdf3 100644 --- a/UI/GamepadEmu.cpp +++ b/UI/GamepadEmu.cpp @@ -1106,12 +1106,7 @@ GamepadEmuView::GamepadEmuView(const TouchControlConfig &config, float xres, flo // Sanitize custom button images, while adding them. for (int i = 0; i < TouchControlConfig::CUSTOM_BUTTON_COUNT; i++) { - if (g_Config.CustomButton[i].shape >= ARRAY_SIZE(CustomKeyData::customKeyShapes)) { - g_Config.CustomButton[i].shape = 0; - } - if (g_Config.CustomButton[i].image >= ARRAY_SIZE(CustomKeyData::customKeyImages)) { - g_Config.CustomButton[i].image = 0; - } + CustomKeyData::Sanitize(g_Config.CustomButton[i]); char temp[64]; snprintf(temp, sizeof(temp), "Custom %d button", i + 1); diff --git a/UI/GamepadEmu.h b/UI/GamepadEmu.h index 6b106f6dae..ce8463c100 100644 --- a/UI/GamepadEmu.h +++ b/UI/GamepadEmu.h @@ -22,6 +22,7 @@ #include "Common/UI/View.h" #include "Common/UI/ViewGroup.h" +#include "Core/ConfigValues.h" #include "Core/CoreParameter.h" #include "Core/HLE/sceCtrl.h" #include "UI/EmuScreen.h" @@ -366,6 +367,17 @@ namespace CustomKeyData { // IMPORTANT: Only add at the end! }; static_assert(ARRAY_SIZE(g_customKeyList) <= 64, "Too many key for a uint64_t bit mask"); + + // image and shape come straight from the ini and index the tables above, so anything that + // reads them has to run this first. + inline void Sanitize(ConfigCustomButton &cfg) { + if (cfg.image < 0 || cfg.image >= (int)ARRAY_SIZE(customKeyImages)) { + cfg.image = 0; + } + if (cfg.shape < 0 || cfg.shape >= (int)ARRAY_SIZE(customKeyShapes)) { + cfg.shape = 0; + } + } }; // Gesture key only have virtual button that can work without constant press diff --git a/UI/TouchControlLayoutScreen.cpp b/UI/TouchControlLayoutScreen.cpp index edf7481d05..b50ad96733 100644 --- a/UI/TouchControlLayoutScreen.cpp +++ b/UI/TouchControlLayoutScreen.cpp @@ -537,12 +537,7 @@ void ControlLayoutView::CreateViews() { for (int i = 0; i < TouchControlConfig::CUSTOM_BUTTON_COUNT; i++) { // Similar to GamepadEmu, we sanitize the images for valid values. - if (g_Config.CustomButton[i].shape >= ARRAY_SIZE(CustomKeyData::customKeyShapes)) { - g_Config.CustomButton[i].shape = 0; - } - if (g_Config.CustomButton[i].image >= ARRAY_SIZE(CustomKeyData::customKeyImages)) { - g_Config.CustomButton[i].image = 0; - } + CustomKeyData::Sanitize(g_Config.CustomButton[i]); char temp[64]; snprintf(temp, sizeof(temp), "Custom %d button", i); diff --git a/libcurl4-openssl-dev_8.20.0-2ubuntu1_arm64.deb b/libcurl4-openssl-dev_8.20.0-2ubuntu1_arm64.deb deleted file mode 100644 index dc31cc5443..0000000000 Binary files a/libcurl4-openssl-dev_8.20.0-2ubuntu1_arm64.deb and /dev/null differ