type of crash: alignment exeption, how to fix ?

Hi all !

While working on porting all irrlicht-engine's file-loaders to big_endian, found that one of them while have big-endian ifdefs inside still crashes.

That is "milkshape 3d" (ms3d) file loader. There is source of that loader:

https://sourceforge.net/p/irrlicht/cod ... t/CMS3DMeshFileLoader.cpp


As can be seen it have big_endian ifdefs, but something still wrong. Once i tried to load any .ms3d file over that loader on amigaos4 (on win32 and linux on little-endian all fine), then i crashes with some strange crash called "alignment exeption", with point out on some strange line in that code (line 383 , when doing "animation time":

f32 framesPerSecond = *(float*)pPtr;


Maybe GR on our side is borken (which is happens offten when heavy C++ usage is happens), but what for sure is that it crashes on loading for sure.

Any ideas/help very welcome, thanks !

@kas1e

No, the GR is likely correct.

While all PPC CPUs should handle unaligned integer accesses the same cannot be said of floating point. Some other OSes may use exception handling to handle such accesses without crashing but this comes at a high cost to performance, so it is better to fix those programs.

Instead of reading the floating point value from an unaligned pointer you should do something like:

@Salas00

Oh, thanks a bunch ! That for sure i can't know about at all.

And yes, it fixed things when i build that code with:

"-Wall -fno-exceptions -fno-rtti -fstrict-aliasing -gstabs -D_DEBUG"

But when i build it with:

"-Wall -fno-exceptions -fno-rtti -fstrict-aliasing -fexpensive-optimizations -O3"

it then crashes the same.

It's probably or "-fexpensive-optimizations" or "-O3" which still somehow "optimize" that code in the way we don't need ..

Is there any way how to keep both -fexpensive-optimizations and -O3, but make it works in that case ?

Thanks again

@kas1e

Assuming that gcc doesn't try to optimize inline assembler code you could use something like:



and then do "f32 framesPerSecond = get_unaligned_float(pPtr);"

Note that this only works for PPC CPUs so some "#ifdef __PPC__" would be in order.

@Frederik
Sadly but nope, still crashes :(

I even tried to replace that:

__asm__ ("lwz %0,0(%1)" : "=r" (tmp.u) : "r" (ptr));

on that :

__asm__ __volatile__("lwz %0,0(%1)" : "=r" (tmp.u) : "r" (ptr));

but still the same.

@kas1e

Which version of gcc are you using?

I tried compiling my code here with "-O4 -fexpensive-optimizations" and it did generate the correct assembler instructions (I'm using gcc 8.2.0 from adtools).

@Salas00
8.2.0 from adtools as well. Will made some more tests to see what flags exactly cause that

@Salas00
Found that the flag which start causing issues are -O2 or -O3 or -O4. -fexpensive-optimizations make no differences, and -O1 works too. Once anything more than -O1, then issues there.

Maybe its combo issue when "-fno-exceptions -fno-rtti -fstrict-aliasing" is in use as well ?

@kas1e

What if you try using this version?



This one should also do the byteswap operation basically for free (gcc 8.2.0 generates a lwbrx instruction).

@kas1e

Quote:

Out of those -fstrict-aliasing seems to be the most likely to be the culprit (the others are just disabling some C++ language features), but gcc still generates the correct code for me even when I add it to the command line.

That being said, I would say that strict aliasing is the default and that if you don't want it you have to disable it with -fno-strict-aliasing.

@Salas00
The same :( And i start to fear its just another issue comeup about alignment somewhere else in the code. Just without optimisation we was lucky enough to bypass that.

I build with -O2 and gstabs (and with our new code about that floats unalignment), and stack trace now point out on that part:



It point out on that first line "for (u16 tmp=0; tmp<numVertices; ++tmp)".

Its line 215 in src:
https://sourceforge.net/p/irrlicht/cod ... t/CMS3DMeshFileLoader.cpp

Type of crash also alignment, and maybe its just another issue which come up after we fix float one.

And that "numVertices" are "u16 numVertices = *(u16*)pPtr;". And that u16 are defined as :




In the stack trace, before pointing out on that line, it also point out on line 67, which is PACK_STRUCT;

In other words probabaly it another alignment issue come up ?

@kas1e

You could reduce the optimization level for a specific function using __attribute__((optimize("O2"))) (assuming that -O2 works correctly).

The vertices structure looks like it's also unaligned so some kind of fix is needed there as well.

@salas00
Quote:

Do you mean initial MS3DVertex struct ? It declared like this:




Maybe that float Vertex[3]; are one which cause those unaligned issues again ?

And PACK_STRUCT are :




I also asked one of Irrlicht's developers if he can deal with it all, that what he say:

Quote:

@kas1e

Quote:

No, that is not what I meant because I had not even looked at that part yet, but that makes it even worse.

This code is clearly not written to be portable to different CPU architectures (not ones that don't allow unaligned accesses anyway).

The best solution that wouldn't require any hacks would be to remove __attribute__((packed)) from all the structures and then rewrite the loader so that it reads one field at a time (this could be done with helper functions that could also handle the endian conversion). Then no unaligned memory accesses would be necessary.

@Salas00

Check PM plz.

@All (but probably mostly question to Frederik again)

Can someone explain plz in human's words, what is that problem with our CPU about " don't allow unaligned accesses " ? I mean, did i understand right, that PPC32 does need the gaps inside the struct so it can align certain variables to certain memory addresses ?

Like for example a pointer might not follow directly behind a single char, but only can be placed after 4 chars in a row so it's memory address is again dividable by 4 (or whatever alignment it needs) ? So if a struct isn't doing that then the compiler must insert empty buffers in between variables to ensure pointer variables end up like that.

And it can't just be avoided by forcing it to use packing without gaps like in that irrlicht's loader they do currently for other platforms ?

I.e. in short - there is simply no way to have sizeof (some_struct) work ? I mean not in a platform independent way. Using the PACK_STRUCT macro works only if the platform actually supports packed structs ?

And probabaly c/c++ have no sizeof alternative which sums up the sizes of all members of a struct ?

And what _exact_ alignment rules we need to follow ? Like is it possible to use strict packing (without gaps) when there are no pointers inside the struct and when the size of the whole struct ends up being dividable by 4 (in which case all structs which are like that are already no problem)

Thanks!

Access with floating point registers to memory need to be either on a 4 byte boundary in the early PPC models and maybe even on an 8 byte boundary for 440 fpu/apu if I remember correctly.

Normally the kernel has code built-in to handle these exceptions, I thought.

Edit:

From the IBM docs:

On the PPC440, any floating point load or store whose
operands cross a cache line boundary (32 bytes) will take an alignment trap. Normally floating point operands are aligned on an 8 byte boundary, so no alignment trap will
occur using a single floating point load or store.

I think the G3/G4 CPUs are less strict.

Btw I thought that a 4 byte alignment for each struct member was default. I am not sure on which PPC you are coding on.

It even becomes more murky if for example the 440 has a double FPU attached. Then it must be on a 16 byte boundary i read.

@Hedeon
Quote:

Only with floating point ? Nothing about integers/shorts/bytes whatever ?

Because in the code above, once Salas00 fix the floating-point part which crashes, i start to crash on the line "for (u16 tmp=0; tmp<numVertices; ++tmp)", so in for() loop when i tried to fill the "verticies" structure which are:



So it crashes again because of float Vertex[3] then, right ?

Is it mean that PACK_STRUCT didn't works for CPU which didn't handle unalignmed memeory access , i.e. our cpus don't actually supports packed structs then ?

Quote:

Imho not amigaos4 kernel. I.e. it handle exeption by generating GR :) but nothing like handle it in kernel to make it work, as it will cause slowdown in the kernel.

Quote:

I think too, but even if not, there we use that PACK_STRUCT define, which mean __attribute__((packed)) , which mean that things got packet and aligned. That why i don't get why we still have alignment expections in that code. Like things still not aligned.


Quote:

That one which in x5000/020.

Quote:

Isn't there some general alignment rules which can cover all PPC32 cpus ?

@kas1e

I don't recommend to hack playing with optimization level: even if it stops crashing, that would be due to a side effect.

The problem is that the structure is directly loaded from the binary file. As salass0 said, the loader have to be modified.

I thought about this solution: The idea is to access float values from the loaded binary only with byte access.

- Declare a union to manage access with float and bytes, rename the struct MS3DVertex into MS3DVertexPacked and define a struct MS3DVertex without the PACK_STRUCT keyword:



- Modify the code (lines 206 to 225) by something like that (sorry, not able to compile and test here):



Additionally, about gcc options, avoid optimization options except -OX ones. And about -fexpensive-optimizations, the doc says it is activated with -O2 and stronger levels.

Ok, the WarpOS kernel had auto correct.

Anyway, the most common alignment errors are:

• The operand of a floating-point load or store instruction is not word-aligned.
• The operand of an integer double-word load or store instruction is not word-aligned.

But like I said, there are exceptions (like the 440) where it is even more strict (64 bit or even 128 bit).

As far as I know integer single word load or store can be done from 1 byte aligned addresses.