netlink.c:97: error: cast increases required alignment of target type

netlink.c:97: error: cast increases required alignment of target type

[Timo Juhani Lindfors]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

Hi,

git clone git://git.kernel.org/pub/scm/network/ofono/ofono.git
cd ofono
./bootstrap
./configure --enable-debug --prefix=/home/lindi/install --enable-maintainer-mode --mandir=/home/lindi/install/usr/share/man --sysconfdir=/home/lindi/install/etc --localstatedir=/home/lindi/install/var --disable-datafiles
make

fails with

>   CC     gdbus/watch.o
>   CC     gisi/netlink.o
> cc1: warnings being treated as errors
> gisi/netlink.c: In function 'g_pn_nl_process':
> gisi/netlink.c:97: error: cast increases required alignment of target type
> gisi/netlink.c:129: error: cast increases required alignment of target type
> gisi/netlink.c:130: error: cast increases required alignment of target type

on armel (openmoko gta02) debian unstable system with gcc 4:4.3.3-9
apparently since in mainstainer mode -Wcast-align complains about

#define NLMSG_NEXT(nlh,len)      ((len) -= NLMSG_ALIGN((nlh)->nlmsg_len), \
                                  (struct nlmsghdr*)(((char*)(nlh)) + NLMSG_ALIGN((nlh)->nlmsg_len)))

which is defined in /usr/include/linux/netlink. I don't know how to
best fix this. The options that I see are

1) Add

pragma GCC diagnostic warning "-Wcast-align"

to ignore the issue in the four affected files: gisi/client.c gisi/iter.c gisi/netlink.c gisi/socket.c

2) Disable -Wcast-alignt checks completely even in maintainer mode (not very nice)

3) Include a fixed version of NLMSG_ALIGN (and also send it usptream of course).


best regards,
Timo Lindfors

Re: netlink.c:97: error: cast increases required alignment of target type

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

Hi,

2009/9/15 Timo Juhani Lindfors <timo.lindfors@iki.fi>:
>>   CC     gdbus/watch.o
>>   CC     gisi/netlink.o
>> cc1: warnings being treated as errors
>> gisi/netlink.c: In function 'g_pn_nl_process':
>> gisi/netlink.c:97: error: cast increases required alignment of target type
>> gisi/netlink.c:129: error: cast increases required alignment of target type
>> gisi/netlink.c:130: error: cast increases required alignment of target type

This was discussed some weeks ago, and that warning is bogus in this
case, but there wasn't a clean workaround for it.

> on armel (openmoko gta02) debian unstable system with gcc 4:4.3.3-9
> apparently since in mainstainer mode -Wcast-align complains about

First of all, you probably should configure --disable-isimodem, which
excludes gisi as well, since the openmoko surely doesn't have a Nokia
modem in it. ;)

Also, distributors really should build without maintainer mode, which
makes the problem go away. It does suck for us, though, when building
in scratchbox armel target.

Cheers,
Aki

Re: netlink.c:97: error: cast increases required alignment of target type

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

Le lundi 9 novembre 2009 19:57:07 andrzej zaborowski, vous avez écrit :
> 2009/11/9 Rémi Denis-Courmont <remi@remlab.net>:
> > As far as I know NO extra alignement is needed. The kernel will anyway
> > memory copy from/to the sockaddr_pn buffer as part of the socket calls,
> > so it couldn't care less about alignment.
> 
> Practically I'm sure you're right and maybe we should stick a (void *)
> cast in between the casts or disable the warning.  Formally that
> memcpy is an implementation detail and could be true in one kernel
> version and false in the next version, so when you pass a struct
> sockaddr * to a function it should, in theory be aligned as such.

Well no. The only thing you're allowed to read from someone else's sockaddr is 
the family field, which does have the same alignment in sockaddr_pn as in 
sockaddr (practically, two bytes). After that, the reader is responsible for 
casting back to the correct type.

So this warning is _bogus_.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

Re: netlink.c:97: error: cast increases required alignment of target type

[andrzej zaborowski]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

2009/11/9 Rémi Denis-Courmont <remi@remlab.net>:
> As far as I know NO extra alignement is needed. The kernel will anyway memory
> copy from/to the sockaddr_pn buffer as part of the socket calls, so it
> couldn't care less about alignment.

Practically I'm sure you're right and maybe we should stick a (void *)
cast in between the casts or disable the warning.  Formally that
memcpy is an implementation detail and could be true in one kernel
version and false in the next version, so when you pass a struct
sockaddr * to a function it should, in theory be aligned as such.

Regards

Re: netlink.c:97: error: cast increases required alignment of target type

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

Le lundi 9 novembre 2009 19:34:36 andrzej zaborowski, vous avez écrit :
> 2009/11/9 Rémi Denis-Courmont <remi@remlab.net>:
> > Le lundi 9 novembre 2009 17:45:25 andrzej zaborowski, vous avez écrit :
> >> Hi,
> >> resurrecting this thread because I think there's no reason ofono
> >> should not build on cpus with strict alignment rules, the netlink code
> >> is actually aligned correctly.  These warnings are a result of
> >> -Wcast-align, the compiler can't know that the casts in the macros are
> >> safe, but they obviously are if you look at the arithmetics there.  In
> >> the case of the netlink warnings they can be turned off for all users
> >> of <linux/netlink.h> using a pragma or only for this cast by casting
> >> to void * first (the only way according to gcc people), the first
> >> attached file adds them (against linux), I'll send it to linux if
> >> there's no better idea.  There are also similar warnings when casting
> >> struct sockaddr_pn * to struct sockaddr * and these can be worked
> >> around in ofono, see second patch, but probably should also be in
> >> linux's include/linux/phonet.h instead.
> >
> > Why do you change the sockaddr_pn type name?!
> 
> Because it's been defined without explicit alignment and I don't think
> you can redefine it with the same name?

As far as I know NO extra alignement is needed. The kernel will anyway memory 
copy from/to the sockaddr_pn buffer as part of the socket calls, so it 
couldn't care less about alignment.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

Re: netlink.c:97: error: cast increases required alignment of target type

[andrzej zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

2009/11/9 Rémi Denis-Courmont <remi@remlab.net>:
> Le lundi 9 novembre 2009 17:45:25 andrzej zaborowski, vous avez écrit :
>> Hi,
>> resurrecting this thread because I think there's no reason ofono
>> should not build on cpus with strict alignment rules, the netlink code
>> is actually aligned correctly.  These warnings are a result of
>> -Wcast-align, the compiler can't know that the casts in the macros are
>> safe, but they obviously are if you look at the arithmetics there.  In
>> the case of the netlink warnings they can be turned off for all users
>> of <linux/netlink.h> using a pragma or only for this cast by casting
>> to void * first (the only way according to gcc people), the first
>> attached file adds them (against linux), I'll send it to linux if
>> there's no better idea.  There are also similar warnings when casting
>> struct sockaddr_pn * to struct sockaddr * and these can be worked
>> around in ofono, see second patch, but probably should also be in
>> linux's include/linux/phonet.h instead.
>
> Why do you change the sockaddr_pn type name?!

Because it's been defined without explicit alignment and I don't think
you can redefine it with the same name?

Regards

Re: netlink.c:97: error: cast increases required alignment of target type

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

Le lundi 9 novembre 2009 17:45:25 andrzej zaborowski, vous avez écrit :
> Hi,
> resurrecting this thread because I think there's no reason ofono
> should not build on cpus with strict alignment rules, the netlink code
> is actually aligned correctly.  These warnings are a result of
> -Wcast-align, the compiler can't know that the casts in the macros are
> safe, but they obviously are if you look at the arithmetics there.  In
> the case of the netlink warnings they can be turned off for all users
> of <linux/netlink.h> using a pragma or only for this cast by casting
> to void * first (the only way according to gcc people), the first
> attached file adds them (against linux), I'll send it to linux if
> there's no better idea.  There are also similar warnings when casting
> struct sockaddr_pn * to struct sockaddr * and these can be worked
> around in ofono, see second patch, but probably should also be in
> linux's include/linux/phonet.h instead.

Why do you change the sockaddr_pn type name?!

-- 
Rémi Denis-Courmont
http://www.remlab.net/

Re: netlink.c:97: error: cast increases required alignment of target type

[andrzej zaborowski]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

Hi,
resurrecting this thread because I think there's no reason ofono
should not build on cpus with strict alignment rules, the netlink code
is actually aligned correctly.  These warnings are a result of
-Wcast-align, the compiler can't know that the casts in the macros are
safe, but they obviously are if you look at the arithmetics there.  In
the case of the netlink warnings they can be turned off for all users
of <linux/netlink.h> using a pragma or only for this cast by casting
to void * first (the only way according to gcc people), the first
attached file adds them (against linux), I'll send it to linux if
there's no better idea.  There are also similar warnings when casting
struct sockaddr_pn * to struct sockaddr * and these can be worked
around in ofono, see second patch, but probably should also be in
linux's include/linux/phonet.h instead.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: linux-netlink-alignment-no-warn.patch --]
[-- Type: text/x-patch, Size: 1911 bytes --]

diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h
index a60c821..d9e7d2f 100644
--- a/include/linux/if_addr.h
+++ b/include/linux/if_addr.h
@@ -56,7 +56,7 @@ struct ifa_cacheinfo
 
 /* backwards compatibility for userspace */
 #ifndef __KERNEL__
-#define IFA_RTA(r)  ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ifaddrmsg))))
+#define IFA_RTA(r)  ((struct rtattr*)(void*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ifaddrmsg))))
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 5ba398e..8f0f76d 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -81,7 +81,7 @@ struct nlmsghdr
 #define NLMSG_SPACE(len) NLMSG_ALIGN(NLMSG_LENGTH(len))
 #define NLMSG_DATA(nlh)  ((void*)(((char*)nlh) + NLMSG_LENGTH(0)))
 #define NLMSG_NEXT(nlh,len)	 ((len) -= NLMSG_ALIGN((nlh)->nlmsg_len), \
-				  (struct nlmsghdr*)(((char*)(nlh)) + NLMSG_ALIGN((nlh)->nlmsg_len)))
+				  (struct nlmsghdr*)(void*)(((char*)(nlh)) + NLMSG_ALIGN((nlh)->nlmsg_len)))
 #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
 			   (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
 			   (nlh)->nlmsg_len <= (len))
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index ba3254e..a8fa2ae 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -141,7 +141,7 @@ struct rtattr
 			 (rta)->rta_len >= sizeof(struct rtattr) && \
 			 (rta)->rta_len <= (len))
 #define RTA_NEXT(rta,attrlen)	((attrlen) -= RTA_ALIGN((rta)->rta_len), \
-				 (struct rtattr*)(((char*)(rta)) + RTA_ALIGN((rta)->rta_len)))
+				 (struct rtattr*)(void*)(((char*)(rta)) + RTA_ALIGN((rta)->rta_len)))
 #define RTA_LENGTH(len)	(RTA_ALIGN(sizeof(struct rtattr)) + (len))
 #define RTA_SPACE(len)	RTA_ALIGN(RTA_LENGTH(len))
 #define RTA_DATA(rta)   ((void*)(((char*)(rta)) + RTA_LENGTH(0)))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: ofono-sockaddr_pn-alignment-no-warn.patch --]
[-- Type: text/x-patch, Size: 1771 bytes --]

diff --git a/gisi/client.c b/gisi/client.c
index f0db24e..26ace15 100644
--- a/gisi/client.c
+++ b/gisi/client.c
@@ -69,6 +69,9 @@ struct _GIsiClient {
 	void *debug_data;
 };
 
+typedef struct sockaddr_pn __attribute__ ((aligned (__alignof(struct sockaddr))))
+	sockaddr_pn_align;
+
 static gboolean g_isi_callback(GIOChannel *channel, GIOCondition cond,
 				gpointer data);
 static gboolean g_isi_timeout(gpointer data);
@@ -233,7 +236,7 @@ GIsiRequest *g_isi_request_make(GIsiClient *cl,	const void *__restrict buf,
 				GIsiResponseFunc cb, void *opaque)
 {
 	struct iovec iov[2];
-	const struct sockaddr_pn dst = {
+	const sockaddr_pn_align dst = {
 		.spn_family = AF_PHONET,
 		.spn_resource = cl->resource,
 	};
@@ -324,7 +327,7 @@ void g_isi_request_cancel(GIsiRequest *req)
 #define PN_COMMGR 0x10
 #define PNS_SUBSCRIBED_RESOURCES_IND	0x10
 
-static const struct sockaddr_pn commgr = {
+static const sockaddr_pn_align commgr = {
 	.spn_family = AF_PHONET,
 	.spn_resource = PN_COMMGR,
 };
diff --git a/gisi/socket.c b/gisi/socket.c
index 4373e9d..883aa14 100644
--- a/gisi/socket.c
+++ b/gisi/socket.c
@@ -38,10 +38,13 @@
 
 #include "socket.h"
 
+typedef struct sockaddr_pn __attribute__ ((aligned (__alignof(struct sockaddr))))
+	sockaddr_pn_align;
+
 GIOChannel *phonet_new(GIsiModem *modem, uint8_t resource)
 {
 	GIOChannel *channel;
-	struct sockaddr_pn addr = {
+	sockaddr_pn_align addr = {
 		.spn_family = AF_PHONET,
 		.spn_resource = resource,
 	};
@@ -83,7 +86,7 @@ size_t phonet_peek_length(GIOChannel *channel)
 ssize_t phonet_read(GIOChannel *channel, void *restrict buf, size_t len,
 			uint16_t *restrict obj, uint8_t *restrict res)
 {
-	struct sockaddr_pn addr;
+	sockaddr_pn_align addr;
 	socklen_t addrlen = sizeof(addr);
 	ssize_t ret;