Re: [head tinderbox] failure on ia64/ia64

On Mon, Jan 31, 2011 at 04:56:06PM -0800, Marcel Moolenaar wrote:
> 
> On Jan 31, 2011, at 3:51 PM, Pawel Jakub Dawidek wrote:
> 
> > On Mon, Jan 31, 2011 at 10:56:18PM +0000, FreeBSD Tinderbox wrote:
> > [...]
> >> cc -O2 -pipe  -I/src/sbin/hastctl/../hastd -DINET -DINET6 -DYY_NO_UNPUT -DYY_NO_INPUT -DHAVE_CRYPTO -std=gnu99 -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -c /src/sbin/hastctl/../hastd/proto_common.c
> >> cc1: warnings being treated as errors
> >> /src/sbin/hastctl/../hastd/proto_common.c: In function 'proto_common_descriptor_send':
> >> /src/sbin/hastctl/../hastd/proto_common.c:116: warning: cast increases required alignment of target type
> >> /src/sbin/hastctl/../hastd/proto_common.c: In function 'proto_common_descriptor_recv':
> >> /src/sbin/hastctl/../hastd/proto_common.c:146: warning: cast increases required alignment of target type
> >> /src/sbin/hastctl/../hastd/proto_common.c:149: warning: cast increases required alignment of target type
> >> *** Error code 1
> > 
> > Marcel, do you have an idea how one can use CMSG_NXTHDR() on ia64 with
> > high WARNS? With WARNS=6 I get those errors and I've no idea how to fix
> > it properly. If there is a fix, CMSG_NXTHDR() should probably be fixed,
> > but maybe I'm wrong?
> 
> this warning indicates that you're casting from a pointer to type P
> (P having alignment constraints Ap) to a pointer to type Q (Q having
> alignment constraints Aq), and Aq > Ap. The compiler tells you that
> you may end up with misaligned accesses.
> 
> If you know that the pointer satisfies Aq, you can cast through (void *)
> to silence the compiler. If you cannot guarantee that, you have a bigger
> problem. Solutions include "packing" type Q to reduce Aq or to copy the
> data to a local variable.
> 
> Take the statement at line 116 for example:
> 	*((int *)CMSG_DATA(cmsg)) = fd;
> 
> We're effectively casting from a (char *) to a (int *) and then doing
> a 32-bit access (write). The easy fix (casting through (void *) is not
> possible, because you cannot guarantee that the address is properly
> aligned. cmsg points to memory set aside by the following local
> variable:
> 	unsigned char ctrl[CMSG_SPACE(sizeof(fd))];
> 
> There's no guarantee that the compiler will align the character array
> at a 32-bit boundary (though in practice it seems to be). I have seen
> this kind of construct fail on ARM and PowerPC for example.
> 
> In any case: The safest approach here is to use le32enc or be32enc
> rather than casting through (void *). Obviously these function encode
> using a fixed byte order when the original code is using the native
> byte order of the CPU. Having native encoding functions help.
> 
> You could use bcopy as well, but the compiler is typically too smart
> for its own good and it will try to optimize the call away. This
> leaves you with the same misaligned access that you tried to avooid
> by using bcopy(). You need to trick the compiler so that it won't
> optimize the bcopy away, like:
> 	bcopy((void *)&fd, CMSG_DATA(cmsg), sizeof(fd));

Interesting. I did use bcopy() to silence the warning, but the need to
cast to (void *) is surprising.

Still, I'm more concerned with CMSG_NXTHDR() macro, which from what I
see might not be fixed by casting arguments.

-- 
Pawel Jakub Dawidek                       http://www.wheelsystems.com
pjd_at_FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!