[Scummvm-devel] Broken Sword 2 problem

[yotam barnoy]

Haven't tried your test, but I think that even if it seems to solve
the problem, it may not be a real solution but an accidental one.
There's still aliasing going on ie. the struct is in the same memory
space as the uint32 or whatever we're reading/writing, and apparently
that's a bad thing for an optimizing compiler.

I'm currently experimenting with using the __may_alias__ gcc attribute
which one of the gcc guys suggested to me. In other words, using
__attribute__ ((__packed__, __may_alias__)).

I tried another suggestion of his which caused crashing (not sure why):

typedef uint32 __attribute__((aligned(1), __may_alias__)) unaligned_uint32;
return *((unaligned_uint32 *)ptr);

Hopefully __may_alias__ will do the trick.

Yotam


On Wed, Sep 1, 2010 at 2:37 PM, Willem Jan Palenstijn <wjp at usecode.org> wrote:
> On Tue, Aug 31, 2010 at 06:26:41PM +0300, yotam barnoy wrote:
>> For the (hopefully) last episode in this bug saga, see commit 52473.
>
> Wow, very nice catch! Well done!
>
> If I understand your commit message correctly, the issue is that some of the
> READ_*_UINT* functions break strict aliasing rules? It would probably be good
> to try to fix that instead of using -fno-strict-aliasing, if possible.
>
>
> The subtleties of these strict aliasing rules still elude me, but it might be
> the case that the issue is that the two Unaligned32 structs are local to their
> functions, and might be considered different.
>
> I'm trying with this code:
>
> inline __attribute__((__always_inline__)) int read_value(const void* ptr)
> {
>        struct Unaligned32 { int val; } __attribute__ ((__packed__));
>        return ((const Unaligned32 *)ptr)->val;
> }
>
> inline __attribute__((__always_inline__)) void write_value(void* ptr, int value)
> {
>        struct Unaligned32 { int val; } __attribute__ ((__packed__));
>        ((Unaligned32 *)ptr)->val = value;
> }
>
> int test(char* x) {
>        int y = read_value(x);
>        write_value(x, y+1);
>        return read_value(x);
> }
>
> When compiled with g++ 4.5.1 on x86_64 with -O3 and strict aliasing, it returns
> y, and not y+1.
>
> When I move the definition of Unaligned32 out of the functions, it returns y+1:
>
> struct Unaligned32 { int val; } __attribute__ ((__packed__));
> inline __attribute__((__always_inline__)) int read_value(const void* ptr)
> {
>        return ((const Unaligned32 *)ptr)->val;
> }
> inline __attribute__((__always_inline__)) void write_value(void* ptr, int value)
> {
>        ((Unaligned32 *)ptr)->val = value;
> }
>
> int test(char* x) {
>        int y = read_value(x);
>        write_value(x, y+1);
>        return read_value(x);
> }
>
>
> Does it behave the same for you on your platform?
>
> -Willem Jan
>