[UBSAN] Core/FileSystems/ISOFileSystem.cpp:66 - constructor call on misaligned address

[glebm]

Happens on startup:

../Core/FileSystems/ISOFileSystem.cpp:66:8: runtime error: constructor call on misaligned address 0x7f9928c7f742 for type 'struct u32_be', which requires 4 byte alignment
0x7f9928c7f742: note: pointer points here
 00 00  c0 a4 a1 4d 99 7f 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  e0 6e 9f 4d 99 7f
              ^ 

At a690127

[glebm]

Update: Analysis below is wrong, actual fix in #14008

---

An example of problematic code:

ppsspp/Core/FileSystems/ISOFileSystem.cpp

Lines 206 to 216 in 5484907

	u8 theSector[2048];
	if (!blockDevice->ReadBlock(secnum, theSector)) {
	blockDevice->NotifyReadError();
	ERROR_LOG(FILESYS, "Error reading block for directory %s - skipping", root->name.c_str());
	root->valid = true; // Prevents re-reading
	return;
	}
	lastReadBlock_ = secnum; // Hm, this could affect timing... but lazy loading is probably more realistic.
	for (int offset = 0; offset < 2048; ) {
	DirectoryEntry &dir = *(DirectoryEntry *)&theSector[offset];

Here, u8 theSector[2048]; is not aligned correctly.

One way to define it with the correct alignment is:

alignas(alignof(DirectoryEntry)) u8 theSector[2048];

This may not fix the issue entirely, because theSector is cast with an offset which I think may not be aligned after the first loop iteration. One correct way to fix that is to memcpy the data instead of casting it.

[unknownbrackets]

We definitely won't work properly in many places on a platform that doesn't like unaligned 32 bit reads, including in the jit x64 instructions we produce (which the checks are probably not seeing.)

In fact, PSP games cannot perform unaligned 32-bit memory access (the MIPS CPU doesn't allow it), and we explicitly combine the code sequences they use to read or write an unaligned value into a direct unaligned memory access.

-[Unknown]

[glebm]

Is this something you consider a bug?

[unknownbrackets]

Well, it's a correctness bug. I'm just warning that we probably have more where that came from.

-[Unknown]

[hrydgard]

This is of no consequence for any platform that we support running on (and yes, that includes ARMv7).

I guess still nice to be "correct".