Re: A suspicious warning in sys/boot/zfs/zfsimpl.c

2012-12-02 Thread Dimitry Andric

On 2012-12-02 01:37, Garrett Cooper wrote:

On Sat, Dec 1, 2012 at 3:05 AM, Andriy Gapon  wrote:

I believe that there is no actual problem there.


 It's probably bugs with clang dealing with alignment problems.


Which bugs?



These warnings (and others) go largely unnoticed because of the fact
that -Werror isn't on on sys/boot. I filed kern/173932 for that and
have been grinding away on warnings for the past couple days in my
spare time -- with my local modifications sys/boot compiles with
-Werror now with gcc, but not clang.
Thanks,
-Garrett

/store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:101:24: error:
tentative definition of variable with internal linkage has incomplete
non-array type 'struct zfsmount'
[-Werror,-Wtentative-definition-incomplete-type]
static struct zfsmount zfsmount;
^
/store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:101:15: note:
forward declaration of 'struct zfsmount'
static struct zfsmount zfsmount;
   ^


This is just a programming error: struct zfsmount is declared in
zfsimpl.c, but that file is included on line 151, so 50 lines after the
static variable definition.

Either the include should be moved up, or the definition moved down.



In file included from /store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:151:
In file included from
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:38:
/store/freebsd/head/sys/boot/i386/zfsboot/../../../cddl/boot/zfs/zfssubr.c:207:9:
error: cast from 'char *' to 'zio_eck_t *' (aka 'struct zio_eck *')
increases required alignment from 1 to 4 [-Werror,-Wcast-align]
 eck = (zio_eck_t *)((char *)data + size) - 1;
   ^~


Clang is right here, the alignment *is* being increased.  However, on
x86, there is no problem with it, so the warning can be ignored.  On the
other hand, if the code should be portable to alignment-sensitive
arches, the code should be fixed.  The same holds for all the other
alignment warnings.

The difference between gcc and clang is that gcc never seems to warn
about alignment issues on x86.  You must compile the code with a gcc
targeting an alignment-sensitive arch, to get any warnings.  Clang
always warns, when using -Wcast-align.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: A suspicious warning in sys/boot/zfs/zfsimpl.c

2012-12-02 Thread Dimitry Andric

On 2012-12-01 12:05, Andriy Gapon wrote:

on 01/12/2012 12:59 Garrett Cooper said the following:

On Mon, Jul 2, 2012 at 6:21 PM, Taku YAMAMOTO  wrote:

When I built the world as of r237813, clang reported a warning which
caught my attention.

===> sys/boot/zfs (all)
clang  -O2 -pipe -march=pentium4 -DBOOTPROG=\"zfsloader\" 
-I/usr/src/sys/boot/zfs/../common -I/usr/src/sys/boot/zfs/../.. -I. 
-I/usr/src/sys/boot/zfs/../../../lib/libstand -I/usr/src/sys/boot/zfs/../../cddl/boot/zfs 
-ffreestanding -mpreferred-stack-boundary=2 -mno-mmx -mno-3dnow -mno-sse -mno-sse2 
-mno-sse3 -msoft-float -Wformat -Wall -DNDEBUG -std=gnu99 -Qunused-arguments  -c 
/usr/src/sys/boot/zfs/zfs.c -o zfs.o
In file included from /usr/src/sys/boot/zfs/zfs.c:48:
/usr/src/sys/boot/zfs/zfsimpl.c:2033:19: warning: array index 264 is past the 
end of the array (which contains 192 elements) [-Warray-bounds]
 memcpy(path, 
&dn.dn_bonus[sizeof(znode_phys_t)],
   ^   
/usr/src/sys/boot/zfs/../../cddl/boot/zfs/zfsimpl.h:788:2: note: array 
'dn_bonus' declared here
 uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)];
 ^

I don't have a zfs-powered machine, so I'm not sure whether this
warning is false-positive or not.


 I'm seeing the same warnings trying to build HEAD r242903 with
clang on amd64. Andriy CCed.


I believe that there is no actual problem there.


Indeed.  The ZFS code seems to be using the dnode_phys_t::dn_bonus field
as a sort of flexible struct member.  These specific warnings can be
ignored, or turned off.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: A suspicious warning in sys/boot/zfs/zfsimpl.c

2012-12-01 Thread Garrett Cooper
On Sat, Dec 1, 2012 at 3:05 AM, Andriy Gapon  wrote:

> I believe that there is no actual problem there.

It's probably bugs with clang dealing with alignment problems.
These warnings (and others) go largely unnoticed because of the fact
that -Werror isn't on on sys/boot. I filed kern/173932 for that and
have been grinding away on warnings for the past couple days in my
spare time -- with my local modifications sys/boot compiles with
-Werror now with gcc, but not clang.
Thanks,
-Garrett

/store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:101:24: error:
tentative definition of variable with internal linkage has incomplete
non-array type 'struct zfsmount'
[-Werror,-Wtentative-definition-incomplete-type]
static struct zfsmount zfsmount;
   ^
/store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:101:15: note:
forward declaration of 'struct zfsmount'
static struct zfsmount zfsmount;
  ^
In file included from /store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:151:
In file included from
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:38:
/store/freebsd/head/sys/boot/i386/zfsboot/../../../cddl/boot/zfs/zfssubr.c:207:9:
error: cast from 'char *' to 'zio_eck_t *' (aka 'struct zio_eck *')
increases required alignment from 1 to 4 [-Werror,-Wcast-align]
eck = (zio_eck_t *)((char *)data + size) - 1;
  ^~
===> lib/libpam/modules/pam_rhosts (all)
In file included from /store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:151:
===> usr.sbin/devinfo (all)
`kldstat.o' is up to date.
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:876:28:
error: cast from 'char *' to 'vdev_phys_t *' (aka 'struct vdev_phys
*') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
vdev_phys_t *vdev_label = (vdev_phys_t *) zap_scratch;
  ^~~
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1043:7:
error: cast from 'char *' to 'const struct uberblock *' increases
required alignment from 1 to 4 [-Werror,-Wcast-align]
up = (const struct uberblock *)upbuf;
 ^~~
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1223:12:
error: cast from 'char *' to 'const blkptr_t *' (aka 'const struct
blkptr *') increases required alignment from 1 to 4
[-Werror,-Wcast-align]
indbp = (const blkptr_t *) dnode_cache_buf;
^~
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1262:7:
error: cast from 'char *' to 'const mzap_phys_t *' (aka 'const struct
mzap_phys *') increases required alignment from 1 to 4
[-Werror,-Wcast-align]
mz = (const mzap_phys_t *) zap_scratch;
 ^
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1289:8:
error: cast from 'uint16_t *' (aka 'unsigned short *') to
'zap_leaf_chunk_t *' (aka 'union zap_leaf_chunk *') increases required
alignment from 2 to 4 [-Werror,-Wcast-align]
nc = &ZAP_LEAF_CHUNK(zl, zc->l_entry.le_name_chunk);
  ^
/store/freebsd/head/sys/boot/i386/zfsboot/../../../cddl/boot/zfs/zfsimpl.h:1139:3:
note: expanded from macro 'ZAP_LEAF_CHUNK'
((zap_leaf_chunk_t *) \
 ^~
In file included from /store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:151:
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1300:9:
error: cast from 'uint16_t *' (aka 'unsigned short *') to
'zap_leaf_chunk_t *' (aka 'union zap_leaf_chunk *') increases required
alignment from 2 to 4 [-Werror,-Wcast-align]
nc = &ZAP_LEAF_CHUNK(zl, nc->l_array.la_next);
  ^~~
/store/freebsd/head/sys/boot/i386/zfsboot/../../../cddl/boot/zfs/zfsimpl.h:1139:3:
note: expanded from macro 'ZAP_LEAF_CHUNK'
((zap_leaf_chunk_t *) \
 ^~
In file included from /store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:151:
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1317:8:
error: cast from 'uint16_t *' (aka 'unsigned short *') to
'zap_leaf_chunk_t *' (aka 'union zap_leaf_chunk *') increases required
alignment from 2 to 4 [-Werror,-Wcast-align]
vc = &ZAP_LEAF_CHUNK(zl, zc->l_entry.le_value_chunk);
  ^~
/store/freebsd/head/sys/boot/i386/zfsboot/../../../cddl/boot/zfs/zfsimpl.h:1139:3:
note: expanded from macro 'ZAP_LEAF_CHUNK'
((zap_leaf_chunk_t *) \
 ^~
In file included from /store/freebsd/head/sys/boot/i386/zfsboot/zfsboot.c:151:
/store/freebsd/head/sys/boot/i386/zfsboot/../../zfs/zfsimpl.c:1333:19:
error: cast from 'char *' to 'zap_phys_t *' (aka 'struct zap_phys *')
increases required alignment from 1 to 4 [-Werror,-Wcast-align]

Re: A suspicious warning in sys/boot/zfs/zfsimpl.c

2012-12-01 Thread Garrett Cooper
On Mon, Jul 2, 2012 at 6:21 PM, Taku YAMAMOTO  wrote:
> When I built the world as of r237813, clang reported a warning which
> caught my attention.
>
> ===> sys/boot/zfs (all)
> clang  -O2 -pipe -march=pentium4 -DBOOTPROG=\"zfsloader\" 
> -I/usr/src/sys/boot/zfs/../common -I/usr/src/sys/boot/zfs/../.. -I. 
> -I/usr/src/sys/boot/zfs/../../../lib/libstand 
> -I/usr/src/sys/boot/zfs/../../cddl/boot/zfs -ffreestanding 
> -mpreferred-stack-boundary=2 -mno-mmx -mno-3dnow -mno-sse -mno-sse2 -mno-sse3 
> -msoft-float -Wformat -Wall -DNDEBUG -std=gnu99 -Qunused-arguments  -c 
> /usr/src/sys/boot/zfs/zfs.c -o zfs.o
> In file included from /usr/src/sys/boot/zfs/zfs.c:48:
> /usr/src/sys/boot/zfs/zfsimpl.c:2033:19: warning: array index 264 is past the 
> end of the array (which contains 192 elements) [-Warray-bounds]
> memcpy(path, 
> &dn.dn_bonus[sizeof(znode_phys_t)],
>   ^   
> /usr/src/sys/boot/zfs/../../cddl/boot/zfs/zfsimpl.h:788:2: note: array 
> 'dn_bonus' declared here
> uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)];
> ^
>
> I don't have a zfs-powered machine, so I'm not sure whether this
> warning is false-positive or not.

I'm seeing the same warnings trying to build HEAD r242903 with
clang on amd64. Andriy CCed.
Thanks,
-Garrett
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: A suspicious warning in sys/boot/zfs/zfsimpl.c

2012-12-01 Thread Andriy Gapon
on 01/12/2012 12:59 Garrett Cooper said the following:
> On Mon, Jul 2, 2012 at 6:21 PM, Taku YAMAMOTO  wrote:
>> When I built the world as of r237813, clang reported a warning which
>> caught my attention.
>>
>> ===> sys/boot/zfs (all)
>> clang  -O2 -pipe -march=pentium4 -DBOOTPROG=\"zfsloader\" 
>> -I/usr/src/sys/boot/zfs/../common -I/usr/src/sys/boot/zfs/../.. -I. 
>> -I/usr/src/sys/boot/zfs/../../../lib/libstand 
>> -I/usr/src/sys/boot/zfs/../../cddl/boot/zfs -ffreestanding 
>> -mpreferred-stack-boundary=2 -mno-mmx -mno-3dnow -mno-sse -mno-sse2 
>> -mno-sse3 -msoft-float -Wformat -Wall -DNDEBUG -std=gnu99 -Qunused-arguments 
>>  -c /usr/src/sys/boot/zfs/zfs.c -o zfs.o
>> In file included from /usr/src/sys/boot/zfs/zfs.c:48:
>> /usr/src/sys/boot/zfs/zfsimpl.c:2033:19: warning: array index 264 is past 
>> the end of the array (which contains 192 elements) [-Warray-bounds]
>> memcpy(path, 
>> &dn.dn_bonus[sizeof(znode_phys_t)],
>>   ^   
>> 
>> /usr/src/sys/boot/zfs/../../cddl/boot/zfs/zfsimpl.h:788:2: note: array 
>> 'dn_bonus' declared here
>> uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)];
>> ^
>>
>> I don't have a zfs-powered machine, so I'm not sure whether this
>> warning is false-positive or not.
> 
> I'm seeing the same warnings trying to build HEAD r242903 with
> clang on amd64. Andriy CCed.

I believe that there is no actual problem there.

-- 
Andriy Gapon
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


A suspicious warning in sys/boot/zfs/zfsimpl.c

2012-07-02 Thread Taku YAMAMOTO
When I built the world as of r237813, clang reported a warning which
caught my attention.

===> sys/boot/zfs (all)
clang  -O2 -pipe -march=pentium4 -DBOOTPROG=\"zfsloader\" 
-I/usr/src/sys/boot/zfs/../common -I/usr/src/sys/boot/zfs/../.. -I. 
-I/usr/src/sys/boot/zfs/../../../lib/libstand 
-I/usr/src/sys/boot/zfs/../../cddl/boot/zfs -ffreestanding 
-mpreferred-stack-boundary=2 -mno-mmx -mno-3dnow -mno-sse -mno-sse2 -mno-sse3 
-msoft-float -Wformat -Wall -DNDEBUG -std=gnu99 -Qunused-arguments  -c 
/usr/src/sys/boot/zfs/zfs.c -o zfs.o
In file included from /usr/src/sys/boot/zfs/zfs.c:48:
/usr/src/sys/boot/zfs/zfsimpl.c:2033:19: warning: array index 264 is past the 
end of the array (which contains 192 elements) [-Warray-bounds]
memcpy(path, &dn.dn_bonus[sizeof(znode_phys_t)],
  ^   
/usr/src/sys/boot/zfs/../../cddl/boot/zfs/zfsimpl.h:788:2: note: array 
'dn_bonus' declared here
uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)];
^

I don't have a zfs-powered machine, so I'm not sure whether this
warning is false-positive or not.

-- 
-|-__   YAMAMOTO, Taku
 | __ < 

  - A chicken is an egg's way of producing more eggs. -
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"