Don't drop an ISO's trailing partial sector

Not every disc image is a whole number of 2048-byte sectors - tools that build
pre-patched ISOs write images that stop partway through their last one, with a
file legitimately ending there. Two things then conspired to lose that tail.

FileBlockDevice::GetNumBlocks() rounds down, so the partial sector isn't
counted, and the file size clamp in ISOFileSystem measured what the image holds
in whole blocks. A file running to the last byte of such an image got clamped
short - by up to a sector - before anything read it.

FileBlockDevice::ReadBlock() then returned false for a short read of that
sector, and ISOFileSystem::ReadFile substitutes an all-zero sector when a read
fails, so even the bytes that were there came back as zeroes.

Measure the clamp in bytes via GetUncompressedSize() instead of blocks, and
treat a short read at the end of the image as a success with the rest of the
sector zeroed. GetUncompressedSize() defaults to the block-based value and is
only overridden by FileBlockDevice, so nothing else changes behaviour.

Also report why a module was rejected. "Failed to load module" named the file
and nothing else, and the truncation check logged only the byte count, which
points at the executable when the real cause is that the loader was handed
fewer bytes than the file has. ElfReader now keeps the reason for a failed
LoadInto, __KernelLoadELFFromPtr puts it in the error string that reaches the
user, and both messages say which header table overran and by how much.

Co-Authored-By: Claude Opus 5 <[email protected]>

Core/ELF/ElfReader.cpp

@@ -544,8 +544,18 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) {
 	if (header->e_ident[EI_DATA] != ELFDATA2LSB)
 		return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED;
 
-	if (size_ < header->e_phoff + sizeof(Elf32_Phdr) * GetNumSegments() || size_ < header->e_shoff + sizeof(Elf32_Shdr) * GetNumSections()) {
-		ERROR_LOG(Log::Loader, "Truncated ELF, %d bytes with %d sections and %d segments", (int)size_, GetNumSections(), GetNumSegments());
+	const size_t phdrEnd = header->e_phoff + sizeof(Elf32_Phdr) * GetNumSegments();
+	const size_t shdrEnd = header->e_shoff + sizeof(Elf32_Shdr) * GetNumSections();
+	if (size_ < phdrEnd || size_ < shdrEnd) {
+		// Say which table runs off the end and by how much. "Truncated ELF" on its own sends you
+		// looking at the executable, when the usual cause is that we were handed fewer bytes than
+		// the file really has - a short read, or a size clamped somewhere upstream.
+		const char *which = size_ < phdrEnd ? "program header table" : "section header table";
+		const size_t needed = size_ < phdrEnd ? phdrEnd : shdrEnd;
+		loadError_ = StringFromFormat(
+			"Truncated ELF: %s ends at %d but only %d bytes are available (%d short), %d sections, %d segments",
+			which, (int)needed, (int)size_, (int)(needed - size_), GetNumSections(), GetNumSegments());
+		ERROR_LOG(Log::Loader, "%s", loadError_.c_str());
 		// Probably not the right error code.
 		return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED;
 	}

Core/ELF/ElfReader.h

@@ -174,6 +174,13 @@ public:
 	std::vector<SectionID> GetCodeSections() const;
 
 	int LoadInto(u32 vaddr, bool fromTop);
+
+	// Set when LoadInto() rejects the image, so the caller can put the reason in the error it
+	// shows instead of only an error code. Untranslated - this is for developers and bug reports.
+	const std::string &LoadError() const {
+		return loadError_;
+	}
+
 	bool LoadSymbols();
 	bool LoadRelocations(const Elf32_Rel *rels, int numRelocs);
 	void LoadRelocations2(int rel_seg);
@@ -196,6 +203,7 @@ private:
 	std::vector<u32> segmentVAddr;
 	size_t size_ = 0;
 	u32 firstSegAlign = 0;
+	std::string loadError_;
 };
 
 // Homebrew usually ships the unstripped ELF it was built from next to the EBOOT (app.elf beside

Core/FileSystems/BlockDevices.cpp

@@ -306,8 +306,18 @@ FileBlockDevice::~FileBlockDevice() {}
 
 bool FileBlockDevice::ReadBlock(int blockNumber, u8 *outPtr, bool uncached) {
 	FileLoader::Flags flags = uncached ? FileLoader::Flags::HINT_UNCACHED : FileLoader::Flags::NONE;
-	size_t retval = fileLoader_->ReadAt((u64)blockNumber * (u64)GetBlockSize(), 1, 2048, outPtr, flags);
+	const u64 offset = (u64)blockNumber * (u64)GetBlockSize();
+	size_t retval = fileLoader_->ReadAt(offset, 1, 2048, outPtr, flags);
 	if (retval != 2048) {
+		// Not every image is a whole number of sectors. Tools that build pre-patched ISOs do write
+		// images that stop in the middle of their last sector, with a file legitimately ending
+		// there. The bytes that are present are real, so zero the rest of the sector and report
+		// success. Failing instead loses them: callers substitute an all-zero sector, which
+		// quietly corrupts whatever was in that tail.
+		if (retval > 0 && offset < filesize_) {
+			memset(outPtr + retval, 0, 2048 - retval);
+			return true;
+		}
 		DEBUG_LOG(Log::FileSystem, "Could not read 2048 byte block, at block offset %d. Only got %d bytes", blockNumber, (int)retval);
 		return false;
 	}

Core/FileSystems/ISOFileSystem.cpp

@@ -307,10 +307,15 @@ void ISOFileSystem::ReadDirectory(TreeEntry *root) const {
 			// drop the entry - truncated ISOs are common and used to work with just the warning
 			// above, and dropping EBOOT.BIN would turn that into an unbootable game. For a sane
 			// file this is a no-op, since the extent always fits in its sectors.
+			// Measured in bytes, not whole sectors: an image whose length isn't a multiple of the
+			// sector size still contains its final partial sector, and a file is allowed to end
+			// there. Counting blocks discards that tail, which clamped real files short - a dump
+			// with EBOOT.BIN running to the last byte of the image lost the end of it, and the ELF
+			// section headers that live there went with it.
 			if (isFile) {
-				const u64 numBlocks = blockDevice->GetNumBlocks();
-				const u64 firstSector = dir.firstDataSector;
-				const s64 availableBytes = firstSector >= numBlocks ? 0 : (s64)((numBlocks - firstSector) * (u64)sectorSize);
+				const u64 imageBytes = blockDevice->GetUncompressedSize();
+				const u64 firstByte = (u64)dir.firstDataSector * (u64)sectorSize;
+				const s64 availableBytes = firstByte >= imageBytes ? 0 : (s64)(imageBytes - firstByte);
 				if (entry->size > availableBytes) {
 					entry->size = availableBytes;
 				}

Core/HLE/sceKernelModule.cpp

@@ -1468,7 +1468,13 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load
 
 	int result = reader.LoadInto(loadAddress, fromTop);
 	if (result != SCE_KERNEL_ERROR_OK) {
-		ERROR_LOG(Log::sceModule, "LoadInto failed with error %08x",result);
+		// Carry the reader's reason up, so what the user is shown says more than an error code.
+		if (!reader.LoadError().empty()) {
+			*error_string = reader.LoadError();
+		} else {
+			*error_string = StringFromFormat("ELF load failed (%08x)", result);
+		}
+		ERROR_LOG(Log::sceModule, "LoadInto failed with error %08x: %s", result, error_string->c_str());
 		delete [] newptr;
 		module->Cleanup();
 		kernelObjects.Destroy<PSPModule>(module->GetUID());
@@ -2092,7 +2098,8 @@ static bool __KernelLoadExecFromPtr(MIPSState * mips, const u8 *data, size_t siz
 			module->Cleanup();
 			kernelObjects.Destroy<PSPModule>(module->GetUID());
 		}
-		ERROR_LOG(Log::Loader, "Failed to load module %s", filename);
+		ERROR_LOG(Log::Loader, "Failed to load module %s (%d bytes): %s", filename, (int)size,
+			error_string->empty() ? "no reason given" : error_string->c_str());
 		*error_string = "Failed to load executable: " + *error_string;
 		delete[] param_argp;
 		delete[] param_key;