Re: CVS commit: src/external/bsd/file

[David Laight <david%l8s.co.uk@localhost>]

On Sat, Jan 18, 2014 at 11:53:09AM +0100, Martin Husemann wrote:
> On Sat, Jan 18, 2014 at 10:46:50AM +0000, Justin Cormack wrote:
> > I believe gcc is correct here, you are only allowed to alias via a
> > char pointer or the original type. Because you put a void pointer in
> > instead gcc is right to complain.

You can also copy to 'void *' and then back to the original type.
In fact that is the only way you are allowed to use 'void *'.

> Right, but actually the complaint does not go away if we make the ? operator
> into an if () with two separate memcpy() calls without any casts.

I wouldn't expect it to.

There are two possible things gcc is complaining about:
1) It thinks you are accessing nh32 when only nh64 has been initialised
   (or v.v.).
2) It doesn't think the memcpy() calls actually writes the structures.

The first can be eliminated by unconditionally compiling both memcpy() calls.

If gcc doesn't think the memcpy() is writing to the structures then
it might completely mis-optimise the code.

For instance: if memcpy() is (modulo typing bugs):
#define memcpy(d, s, len) \
        if (__builtin_constant(len) && (len) == 8) \
                *(uint64_t *)(d) = *(uint64_t *)(s); \
        else \
                __memcpy(d, s, len) \

And you have:
        struct { int32_t a; int32_t b } x, y = {1, 2};
then:
        memcpy(&x, &y, 8);
        printf("%u", x.a + x.b);
gcc can optimise away 'y' and the memcpy() completely, leaving 'x'
undefined.

While this might not affect the code in file, it might cause obscure
effects elsewhere.

The builtin memcpy() should have appropriate constraints to indicate
that it reads and writes the memory.
I think it should be possible the use asm statements that generate
comments but have "m" constraints (to a correctly sized char [])
top & tail in the memcpy() expansion so that the old buffer is read
before the copy and the new one afterwards.

Maybe:
#define reference_memory(ptr, len) \
        asm volatile ( :: "m" ( *(struct {char x[len]; } *)(ptr)) )
#define memcpy(d, s, len) \
        if (__builtin_constant(len) && (len) == 8) { \
                reference_memory(s, 8); \
                *(uint64_t *)(d) = *(uint64_t *)(s); \
                reference_memory(d, 8); \
        } else \
                __memcpy(d, s, len) \

But my gnu asm is a bit weak!

        David

-- 
David Laight: david%l8s.co.uk@localhost