dereferencing type-punned pointer will break strict-aliasing rules

[Thomas Moestl]

On Mon, 2003/07/28 at 09:30:08 +0900, Jun Kuriyama wrote:
> 
> Is this caused by -oS option?
> 
> ----- in making BOOTMFS in make release
> cc -c -Os -pipe -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -fformat-extensions -std=c99  -nostdinc -I-  -I. -I/usr/src/sys -I/usr/src/sys/dev -I/usr/src/sys/contrib/dev/acpica -I/usr/src/sys/contrib/ipfilter -I/usr/src/sys/contrib/dev/ath -I/usr/src/sys/contrib/dev/ath/freebsd -D_KERNEL -include opt_global.h -fno-common -finline-limit=15000  -mno-align-long-strings -mpreferred-stack-boundary=2 -ffreestanding -Werror  /usr/src/sys/geom/geom_dev.c
> /usr/src/sys/geom/geom_dev.c: In function `g_dev_open':
> /usr/src/sys/geom/geom_dev.c:198: warning: dereferencing type-punned pointer will break strict-aliasing rules
> [...]

Yes, by implying -fstrict-aliasing, so using -fno-strict-aliasing is a
workaround. The problem is caused by the i386 PCPU_GET/PCPU_SET
implementation:

	#define	__PCPU_GET(name) ({						\
		__pcpu_type(name) __result;					\
										\
	[...]
		} else if (sizeof(__result) == 4) {				\
			u_int __i;						\
			__asm __volatile("movl %%fs:%1,%0"			\
			    : "=r" (__i)					\
			    : "m" (*(u_int *)(__pcpu_offset(name))));		\
			__result = *(__pcpu_type(name) *)&__i;			\
	[...]

In this case, the PCPU_GET is used to retrieve curthread, causing
sizeof(__result) to be 4, so the cast at the end of the code snippet
is from a u_int * to struct thread *, and __i is accessed through the
casted pointer, which violates the C99 aliasing rules.
An alternative is to type-pun via a union, which is also a bit ugly,
but explicitly allowed by C99. Patch attached (but only superficially
tested).

	- Thomas

-- 
Thomas Moestl <t.moestl at tu-bs.de>	http://www.tu-bs.de/~y0015675/
              <tmm at FreeBSD.org>		http://people.FreeBSD.org/~tmm/
PGP fingerprint: 1C97 A604 2BD0 E492 51D0  9C0F 1FE6 4F1D 419C 776C
-------------- next part --------------
Index: pcpu.h
===================================================================
RCS file: /vol/ncvs/src/sys/i386/include/pcpu.h,v
retrieving revision 1.36
diff -u -r1.36 pcpu.h
--- pcpu.h	27 Jun 2003 21:50:52 -0000	1.36
+++ pcpu.h	28 Jul 2003 01:37:57 -0000
@@ -96,23 +96,32 @@
 	__pcpu_type(name) __result;					\
 									\
 	if (sizeof(__result) == 1) {					\
-		u_char __b;						\
+		union {							\
+			u_char __b;					\
+			__pcpu_type(name) __r;				\
+		} __u;							\
 		__asm __volatile("movb %%fs:%1,%0"			\
-		    : "=r" (__b)					\
+		    : "=r" (__u.__b)					\
 		    : "m" (*(u_char *)(__pcpu_offset(name))));		\
-		__result = *(__pcpu_type(name) *)&__b;			\
+		__result = __u.__r;					\
 	} else if (sizeof(__result) == 2) {				\
-		u_short __w;						\
+		union {							\
+			u_short __w;					\
+			__pcpu_type(name) __r;				\
+		} __u;							\
 		__asm __volatile("movw %%fs:%1,%0"			\
-		    : "=r" (__w)					\
+		    : "=r" (__u.__w)					\
 		    : "m" (*(u_short *)(__pcpu_offset(name))));		\
-		__result = *(__pcpu_type(name) *)&__w;			\
+		__result = __u.__r;					\
 	} else if (sizeof(__result) == 4) {				\
-		u_int __i;						\
+		union {							\
+			u_int __i;					\
+			__pcpu_type(name) __r;				\
+		} __u;							\
 		__asm __volatile("movl %%fs:%1,%0"			\
-		    : "=r" (__i)					\
+		    : "=r" (__u.__i)					\
 		    : "m" (*(u_int *)(__pcpu_offset(name))));		\
-		__result = *(__pcpu_type(name) *)&__i;			\
+		__result = __u.__r;					\
 	} else {							\
 		__result = *__PCPU_PTR(name);				\
 	}								\
@@ -127,23 +136,32 @@
 	__pcpu_type(name) __val = (val);				\
 									\
 	if (sizeof(__val) == 1) {					\
-		u_char __b;						\
-		__b = *(u_char *)&__val;				\
+		union {							\
+			u_char __b;					\
+			__pcpu_type(name) __v;				\
+		} __u;							\
+		__u.__v = __val;					\
 		__asm __volatile("movb %1,%%fs:%0"			\
 		    : "=m" (*(u_char *)(__pcpu_offset(name)))		\
-		    : "r" (__b));					\
+		    : "r" (__u.__b));					\
 	} else if (sizeof(__val) == 2) {				\
-		u_short __w;						\
-		__w = *(u_short *)&__val;				\
+		union {							\
+			u_short __w;					\
+			__pcpu_type(name) __v;				\
+		} __u;							\
+		__u.__v = __val;					\
 		__asm __volatile("movw %1,%%fs:%0"			\
 		    : "=m" (*(u_short *)(__pcpu_offset(name)))		\
-		    : "r" (__w));					\
+		    : "r" (__u.__w));					\
 	} else if (sizeof(__val) == 4) {				\
-		u_int __i;						\
-		__i = *(u_int *)&__val;					\
+		union {							\
+			u_int __i;					\
+			__pcpu_type(name) __v;				\
+		} __u;							\
+		__u.__v = __val;					\
 		__asm __volatile("movl %1,%%fs:%0"			\
 		    : "=m" (*(u_int *)(__pcpu_offset(name)))		\
-		    : "r" (__i));					\
+		    : "r" (__u.__i));					\
 	} else {							\
 		*__PCPU_PTR(name) = __val;				\
 	}								\