CDVD: Ignore version information when loading ELF

pcsx2/Achievements.cpp

@@ -18,6 +18,7 @@
 #ifdef ENABLE_ACHIEVEMENTS
 
 #include "Achievements.h"
+#include "CDVD/CDVD.h"
 #include "CDVD/IsoFS/IsoFS.h"
 #include "CDVD/IsoFS/IsoFSCDVD.h"
 #include "Elfheader.h"
@@ -121,7 +122,6 @@ namespace Achievements
 	static void GetLbInfoCallback(s32 status_code, const std::string& content_type, Common::HTTPDownloader::Request::Data data);
 	static void GetPatches(u32 game_id);
 	static std::string_view GetELFNameForHash(const std::string& elf_path);
-	static std::optional<std::vector<u8>> ReadELFFromCurrentDisc(const std::string& elf_path);
 	static std::string GetGameHash();
 	static void SetChallengeMode(bool enabled);
 	static void SendGetGameId();
@@ -1356,47 +1356,6 @@ std::string_view Achievements::GetELFNameForHash(const std::string& elf_path)
 	return std::string_view(elf_path).substr(start, end - start);
 }
 
-std::optional<std::vector<u8>> Achievements::ReadELFFromCurrentDisc(const std::string& elf_path)
-{
-	std::optional<std::vector<u8>> ret;
-
-	// ELF suffix hack. Taken from CDVD::loadElf
-	// MLB2k6 has an elf whose suffix is actually ;2
-	std::string filepath(elf_path);
-	const std::string::size_type semi_pos = filepath.rfind(';');
-	if (semi_pos != std::string::npos && std::string_view(filepath).substr(semi_pos) != ";1")
-	{
-		Console.Warning(fmt::format("(Achievements) Non-conforming version suffix ({}) detected and replaced.", elf_path));
-		filepath.erase(semi_pos);
-		filepath += ";1";
-	}
-
-	IsoFSCDVD isofs;
-	IsoFile file(isofs);
-	Error error;
-	if (!file.open(filepath, &error))
-	{
-		Console.Error(fmt::format("(Achievements) Failed to open ELF '{}' on disc: {}", elf_path, error.GetDescription()));
-		return ret;
-	}
-
-	const u32 size = file.getLength();
-	ret = std::vector<u8>();
-	ret->resize(size);
-
-	if (size > 0)
-	{
-		const s32 bytes_read = file.read(ret->data(), static_cast<s32>(size));
-		if (bytes_read != static_cast<s32>(size))
-		{
-			Console.Error(fmt::format("(Achievements) Only read {} of {} bytes of ELF '{}'", bytes_read, size, elf_path));
-			ret.reset();
-		}
-	}
-
-	return ret;
-}
-
 std::string Achievements::GetGameHash()
 {
 	const std::string elf_path = VMManager::GetDiscELF();
@@ -1404,34 +1363,37 @@ std::string Achievements::GetGameHash()
 		return {};
 
 	// this.. really shouldn't be invalid
-	const std::string_view name_for_hash(GetELFNameForHash(elf_path));
+	const std::string_view name_for_hash = GetELFNameForHash(elf_path);
 	if (name_for_hash.empty())
 		return {};
 
-	std::optional<std::vector<u8>> elf_data(ReadELFFromCurrentDisc(elf_path));
+	ElfObject elfo;
-	if (!elf_data.has_value())
+	Error error;
+	if (!cdvdLoadElf(&elfo, elf_path, false, &error))
+	{
+		Console.Error(fmt::format("(Achievements) Failed to read ELF '{}' on disc: {}", elf_path, error.GetDescription()));
 		return {};
+	}
 
 	// See rcheevos hash.c - rc_hash_ps2().
 	const u32 MAX_HASH_SIZE = 64 * 1024 * 1024;
-	const u32 hash_size = std::min<u32>(static_cast<u32>(elf_data->size()), MAX_HASH_SIZE);
+	const u32 hash_size = std::min<u32>(elfo.GetSize(), MAX_HASH_SIZE);
-	pxAssert(hash_size <= elf_data->size());
+	pxAssert(hash_size <= elfo.GetSize());
 
 	MD5Digest digest;
 	if (!name_for_hash.empty())
 		digest.Update(name_for_hash.data(), static_cast<u32>(name_for_hash.size()));
 	if (hash_size > 0)
-		digest.Update(elf_data->data(), hash_size);
+		digest.Update(elfo.GetData().data(), hash_size);
 
 	u8 hash[16];
 	digest.Final(hash);
 
-	std::string hash_str(
+	const std::string hash_str =
 		StringUtil::StdStringFromFormat("%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x", hash[0], hash[1], hash[2],
-			hash[3], hash[4], hash[5], hash[6], hash[7], hash[8], hash[9], hash[10], hash[11], hash[12], hash[13], hash[14], hash[15]));
+			hash[3], hash[4], hash[5], hash[6], hash[7], hash[8], hash[9], hash[10], hash[11], hash[12], hash[13], hash[14], hash[15]);
 
-	Console.WriteLn("Hash for '%.*s' (%zu bytes, %u bytes hashed): %s", static_cast<int>(name_for_hash.size()), name_for_hash.data(),
+	Console.WriteLn(fmt::format("Hash for '{}' ({} bytes, {} bytes hashed): {}", name_for_hash, elfo.GetSize(), hash_size, hash_str));
-		elf_data->size(), hash_size, hash_str.c_str());
 	return hash_str;
 }
 

pcsx2/CDVD/CDVD.cpp

@@ -383,46 +383,58 @@ s32 cdvdWriteConfig(const u8* config)
 	return 0;
 }
 
-bool cdvdLoadElf(ElfObject* elfo, std::string elfpath, bool isPSXElf, Error* error)
+bool cdvdLoadElf(ElfObject* elfo, const std::string_view& elfpath, bool isPSXElf, Error* error)
 {
 	if (StringUtil::StartsWith(elfpath, "host:"))
 	{
 		std::string host_filename(elfpath.substr(5));
-		if (!elfo->OpenFile(host_filename, isPSXElf, error))
+		return elfo->OpenFile(host_filename, isPSXElf, error);
-			return false;
 	}
-	else
+	else if (StringUtil::StartsWith(elfpath, "cdrom:") || StringUtil::StartsWith(elfpath, "cdrom0:"))
 	{
-		// Mimic PS2 behavior!
+		// Strip out cdrom: prefix, and any leading slashes.
-		// Much trial-and-error with changing the ISOFS and BOOT2 contents of an image have shown that
+		size_t start_pos = (elfpath[5] == '0') ? 7 : 6;
-		// the PS2 BIOS performs the peculiar task of *ignoring* the version info from the parsed BOOT2
+		while (start_pos < elfpath.size() && (elfpath[start_pos] == '\\' || elfpath[start_pos] == '/'))
-		// filename *and* the ISOFS, when loading the game's ELF image.  What this means is:
+			start_pos++;
-		//
-		//   1. a valid PS2 ELF can have any version (ISOFS), and the version need not match the one in SYSTEM.CNF.
-		//   2. the version info on the file in the BOOT2 parameter of SYSTEM.CNF can be missing, 10 chars long,
-		//      or anything else.  Its all ignored.
-		//   3. Games loading their own files do *not* exhibit this behavior; likely due to using newer IOP modules
-		//      or lower level filesystem APIs (fortunately that doesn't affect us).
-		//
-		// FIXME: Properly mimicing this behavior is troublesome since we need to add support for "ignoring"
-		// version information when doing file searches.  I'll add this later.  For now, assuming a ;1 should
-		// be sufficient (no known games have their ELF binary as anything but version ;1)
-		const std::string::size_type semi_pos = elfpath.rfind(';');
-		if (semi_pos != std::string::npos && std::string_view(elfpath).substr(semi_pos) != ";1")
-		{
-			Console.WriteLn(Color_Blue, "(LoadELF) Non-conforming version suffix (%s) detected and replaced.", elfpath.c_str());
-			elfpath.erase(semi_pos);
-			elfpath += ";1";
-		}
 
-		// Fix cdrom:path, the iso reader doesn't like it.
+		// Strip out any version information. Some games use ;2 (MLB2k6), others put multiple versions in
-		if (StringUtil::StartsWith(elfpath, "cdrom:") && elfpath[6] != '\\' && elfpath[6] != '/')
+		// (Syphon Filter Omega Strain). The PS2 BIOS appears to ignore the suffix entirely, so we'll do
-			elfpath.insert(6, 1, '\\');
+		// the same, and hope that no games actually have multiple ELFs with different versions.
+		// Previous notes:
+		//   Mimic PS2 behavior!
+		//   Much trial-and-error with changing the ISOFS and BOOT2 contents of an image have shown that
+		//   the PS2 BIOS performs the peculiar task of *ignoring* the version info from the parsed BOOT2
+		//   filename *and* the ISOFS, when loading the game's ELF image.  What this means is:
+		//
+		//     1. a valid PS2 ELF can have any version (ISOFS), and the version need not match the one in SYSTEM.CNF.
+		//     2. the version info on the file in the BOOT2 parameter of SYSTEM.CNF can be missing, 10 chars long,
+		//        or anything else.  Its all ignored.
+		//     3. Games loading their own files do *not* exhibit this behavior; likely due to using newer IOP modules
+		//        or lower level filesystem APIs (fortunately that doesn't affect us).
+		//
+		size_t length = elfpath.length() - start_pos;
+		const size_t semi_pos = elfpath.find(';', start_pos);
+		if (semi_pos != std::string::npos)
+			length = semi_pos - start_pos;
+
+		std::string iso_filename(elfpath.substr(start_pos, length));
+		DevCon.WriteLn(fmt::format("cvdLoadElf(): '{}' -> '{}' in ISO.", elfpath, iso_filename));
+		if (iso_filename.empty())
+		{
+			Error::SetString(error, "ISO filename is empty.");
+			return false;
+		}
 
 		IsoFSCDVD isofs;
 		IsoFile file(isofs);
-		if (!file.open(elfpath, error) || !elfo->OpenIsoFile(elfpath, file, isPSXElf, error))
+		if (!file.open(iso_filename, error))
 			return false;
+
+		return elfo->OpenIsoFile(std::move(iso_filename), file, isPSXElf, error);
+	}
+	else
+	{
+		Console.Error(fmt::format("cdvdLoadElf(): Unknown device in ELF path '{}'", elfpath));
 	}
 
 	return true;
@@ -446,7 +458,7 @@ static CDVDDiscType GetPS2ElfName(std::string* name, std::string* version)
 	Error error;
 	IsoFSCDVD isofs;
 	IsoFile file(isofs);
-	if (!file.open("SYSTEM.CNF;1", &error))
+	if (!file.open("SYSTEM.CNF", &error))
 	{
 		Console.Error(fmt::format("(GetElfName) Failed to open SYSTEM.CNF: {}", error.GetDescription()));
 		return CDVDDiscType::Other;

pcsx2/CDVD/CDVD.h

@@ -190,7 +190,7 @@ extern void cdvdWrite(u8 key, u8 rt);
 extern void cdvdGetDiscInfo(std::string* out_serial, std::string* out_elf_path, std::string* out_version, u32* out_crc,
 	CDVDDiscType* out_disc_type);
 extern u32 cdvdGetElfCRC(const std::string& path);
-extern bool cdvdLoadElf(ElfObject* elfo, std::string filename, bool isPSXElf, Error* error);
+extern bool cdvdLoadElf(ElfObject* elfo, const std::string_view& filename, bool isPSXElf, Error* error);
 
 extern s32 cdvdCtrlTrayOpen();
 extern s32 cdvdCtrlTrayClose();

pcsx2/CDVD/CDVDcommon.cpp

@@ -75,7 +75,7 @@ static int CheckDiskTypeFS(int baseType)
 	IsoDirectory rootdir(isofs);
 	if (rootdir.OpenRootDirectory())
 	{
-		if (IsoFile file(isofs); file.open(rootdir, "SYSTEM.CNF;1"))
+		if (IsoFile file(isofs); file.open(rootdir, "SYSTEM.CNF"))
 		{
 			const int size = file.getLength();
 			const std::unique_ptr<char[]> buffer = std::make_unique<char[]>(size + 1);
@@ -95,13 +95,13 @@ static int CheckDiskTypeFS(int baseType)
 		}
 
 		// PS2 Linux disc 2, doesn't have a System.CNF or a normal ELF
-		if (rootdir.Exists("P2L_0100.02;1"))
+		if (rootdir.Exists("P2L_0100.02"))
 			return CDVD_TYPE_PS2DVD;
 
-		if (rootdir.Exists("PSX.EXE;1"))
+		if (rootdir.Exists("PSX.EXE"))
 			return CDVD_TYPE_PSCD;
 
-		if (rootdir.Exists("VIDEO_TS/VIDEO_TS.IFO;1"))
+		if (rootdir.Exists("VIDEO_TS/VIDEO_TS.IFO"))
 			return CDVD_TYPE_DVDV;
 	}
 

pcsx2/CDVD/IsoFS/IsoFS.cpp

@@ -130,7 +130,7 @@ bool IsoDirectory::Open(const IsoFileDescriptor& directoryEntry, Error* error /*
 
 	files.clear();
 
-	uint remainingSize = directoryEntry.size;
+	u32 remainingSize = directoryEntry.size;
 
 	u8 b[257];
 
@@ -145,25 +145,12 @@ bool IsoDirectory::Open(const IsoFileDescriptor& directoryEntry, Error* error /*
 
 		remainingSize -= b[0];
 
-		dataStream.read(b + 1, b[0] - 1);
+		if (dataStream.read(b + 1, static_cast<s32>(b[0] - 1)) != static_cast<s32>(b[0] - 1))
+			break;
 
-		auto isoFile = IsoFileDescriptor(b, b[0]);
+		files.emplace_back(b, b[0]);
-
-		files.push_back(isoFile);
-
-		const std::string::size_type semi_pos = isoFile.name.rfind(';');
-		if (semi_pos != std::string::npos && std::string_view(isoFile.name).substr(semi_pos) != ";1")
-		{
-			const std::string origName = isoFile.name;
-			isoFile.name.erase(semi_pos);
-			isoFile.name += ";1";
-			Console.WriteLn("(IsoFS) Non-conforming version suffix (%s) detected. Creating 'hard-linked' entry (%s)",origName.c_str(), isoFile.name.c_str());
-
-			files.push_back(isoFile);
-		}
 	}
 
-	b[0] = 0;
 	return true;
 }
 
@@ -194,19 +181,13 @@ std::optional<IsoFileDescriptor> IsoDirectory::FindFile(const std::string_view&
 	const IsoDirectory* dir = this;
 	IsoDirectory subdir(internalReader);
 
-	// walk through path ("." and ".." entries are in the directories themselves, so even if the
+	for (size_t index = 0; index < (parts.size() - 1); index++)
-	// path included . and/or .., it still works)
-
-	// ignore the device (cdrom0:\)
-	const bool has_device = (parts.front().back() == ':');
-
-	for (size_t index = has_device ? 1 : 0; index < (parts.size() - 1); index++)
 	{
-		const int subdir_index = GetIndexOf(parts[index]);
+		const int subdir_index = dir->GetIndexOf(parts[index]);
 		if (subdir_index < 0)
 			return std::nullopt;
 
-		const IsoFileDescriptor& subdir_entry = GetEntry(static_cast<size_t>(index));
+		const IsoFileDescriptor& subdir_entry = GetEntry(static_cast<size_t>(subdir_index));
 		if (subdir_entry.IsFile() || !subdir.Open(subdir_entry, nullptr))
 			return std::nullopt;
 
@@ -217,7 +198,7 @@ std::optional<IsoFileDescriptor> IsoDirectory::FindFile(const std::string_view&
 	if (file_index < 0)
 		return std::nullopt;
 
-	return GetEntry(static_cast<size_t>(file_index));
+	return dir->GetEntry(static_cast<size_t>(file_index));
 }
 
 bool IsoDirectory::Exists(const std::string_view& filePath) const
@@ -293,9 +274,9 @@ void IsoFileDescriptor::Load(const u8* data, int length)
 
 	flags = data[25];
 
-	int fileNameLength = data[32];
+	int file_name_length = data[32];
 
-	if (fileNameLength == 1)
+	if (file_name_length == 1)
 	{
 		u8 c = data[33];
 
@@ -314,9 +295,16 @@ void IsoFileDescriptor::Load(const u8* data, int length)
 	}
 	else
 	{
-		// copy string and up-convert from ascii to wxChar
-
 		const u8* fnsrc = data + 33;
-		name.assign(reinterpret_cast<const char*>(fnsrc), fileNameLength);
+
+		// Strip any version information like the PS2 BIOS does.
+		int length_without_version = 0;
+		for (; length_without_version < file_name_length; length_without_version++)
+		{
+			if (fnsrc[length_without_version] == ';' || fnsrc[length_without_version] == '\0')
+				break;
+		}
+
+		name.assign(reinterpret_cast<const char*>(fnsrc), length_without_version);
 	}
 }

pcsx2/Elfheader.h

@@ -125,6 +125,7 @@ public:
 	ElfObject(const ElfObject&) = delete;
 	~ElfObject();
 
+	__fi const std::vector<u8>& GetData() const { return data; }
 	__fi const ELF_HEADER& GetHeader() const { return *reinterpret_cast<const ELF_HEADER*>(data.data()); }
 	__fi u32 GetSize() const { return static_cast<u32>(data.size()); }