Re: asr(4) error with new clang/llvm
On 2 January 2015 at 04:07, David Chisnall wrote: > > The correct solution is to declare the array to have 0 elements (although > this will break C++ code). A zero-length array at the end of a structure is > specifically defined by the C standard (since C99) to be a variable-length > array. A length-one array was used in C89 prior to this for this purpose. > Using a 1-element array in C is undefined behaviour. > > Note that this change will also require fixing code that allocates it to > allocate space for n elements not n-1. I was thinking of making that change, but the driver was not particularly straightforward. In addition to your point about allocation I noticed that it used sizeof() the union containing these variable-length-array structs. I wouldn't want to try to fix it without hardware to test. ___ 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: asr(4) error with new clang/llvm
On 2 Jan 2015, at 05:00, Ed Maste wrote: > > It's a variable length array in a struct / union. Other than being > confusing and now triggering a warning after the clang update it > should be fine. > > Most likely we need to build asr with -Werror disabled for that > warning, perhaps -Wno-error-array-bounds. I'll take a look tomorrow > morning if nobody else gets to it first. The correct solution is to declare the array to have 0 elements (although this will break C++ code). A zero-length array at the end of a structure is specifically defined by the C standard (since C99) to be a variable-length array. A length-one array was used in C89 prior to this for this purpose. Using a 1-element array in C is undefined behaviour. Note that this change will also require fixing code that allocates it to allocate space for n elements not n-1. David ___ 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: asr(4) error with new clang/llvm
> On Jan 2, 2015, at 12:00 AM, Ed Maste wrote: > > On 1 January 2015 at 23:13, Steven Hartland wrote: >> >> On 02/01/2015 01:23, Bjoern A. Zeeb wrote: >>> >>> Hi, >>> >>> you need the next line of source to see that while the union only defines >>> Simple[1], the comparison goes up to SG_LIST (or something) which is indeed >>> defined as 58. Cn someone fix this? This makes i386 compiles failing >>> currently. >>> >>> /scratch/tmp/bz/head.svn/sys/modules/asr/../../dev/asr/asr.c:1849:29: >>> error: array index 58 is past the end of the array (which contains 1 >>> element) [-Werror,-Warray-bounds] >>> while ((len > 0) && (sg < &((PPRIVATE_SCSI_SCB_EXECUTE_MESSAGE) >>>^ >>> /scratch/tmp/bz/head.svn/sys/dev/asr/i2omsg.h:934:8: note: array 'Simple' >>> declared here >>>I2O_SGE_SIMPLE_ELEMENT Simple[1]; >>>^ >> >> If that's wrong it looks like there's also a number of calls to the macro >> SG(SGL,Index,Flags,Buffer,Size) which are also wrong as Index is used in >> the same way: >> &(((PI2O_SG_ELEMENT)(SGL))->u.Simple[Index] >> >> There appears to be two calls to SG where Index is 1. >> >> I'm afraid I have no idea what the fix would be as the entire driver is very >> voodoo like to me :( > > It's a variable length array in a struct / union. Other than being > confusing and now triggering a warning after the clang update it > should be fine. > > Most likely we need to build asr with -Werror disabled for that > warning, perhaps -Wno-error-array-bounds. I'll take a look tomorrow > morning if nobody else gets to it first. > I got to it first ;-) Scott ___ 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: asr(4) error with new clang/llvm
On Jan 1, 2015, at 21:00, Ed Maste wrote:
> On 1 January 2015 at 23:13, Steven Hartland wrote:
>>
>> On 02/01/2015 01:23, Bjoern A. Zeeb wrote:
>>>
>>> Hi,
>>>
>>> you need the next line of source to see that while the union only defines
>>> Simple[1], the comparison goes up to SG_LIST (or something) which is indeed
>>> defined as 58. Cn someone fix this? This makes i386 compiles failing
>>> currently.
>>>
>>> /scratch/tmp/bz/head.svn/sys/modules/asr/../../dev/asr/asr.c:1849:29:
>>> error: array index 58 is past the end of the array (which contains 1
>>> element) [-Werror,-Warray-bounds]
>>> while ((len > 0) && (sg < &((PPRIVATE_SCSI_SCB_EXECUTE_MESSAGE)
>>>^
>>> /scratch/tmp/bz/head.svn/sys/dev/asr/i2omsg.h:934:8: note: array 'Simple'
>>> declared here
>>>I2O_SGE_SIMPLE_ELEMENT Simple[1];
>>>^
>>
>> If that's wrong it looks like there's also a number of calls to the macro
>> SG(SGL,Index,Flags,Buffer,Size) which are also wrong as Index is used in
>> the same way:
>> &(((PI2O_SG_ELEMENT)(SGL))->u.Simple[Index]
>>
>> There appears to be two calls to SG where Index is 1.
>>
>> I'm afraid I have no idea what the fix would be as the entire driver is very
>> voodoo like to me :(
>
> It's a variable length array in a struct / union. Other than being
> confusing and now triggering a warning after the clang update it
> should be fine.
>
> Most likely we need to build asr with -Werror disabled for that
> warning, perhaps -Wno-error-array-bounds. I'll take a look tomorrow
> morning if nobody else gets to it first.
It looks like more fallout from bsd.sys.mk being removed from bad.kmod.mk
(NO_WARRAY_BOUNDS should be defined to the appropriate compiler flag):
1 # $FreeBSD: projects/building-blocks/sys/modules/asr/Makefile 228865
2011-12-24 17:54:58Z dim $
2
3 .PATH: ${.CURDIR}/../../dev/asr
4
5 KMOD= asr
6 SRCS= asr.c
7 SRCS+= opt_scsi.h opt_cam.h
8 SRCS+= device_if.h bus_if.h pci_if.h
9
10 .if ${MACHINE_CPUARCH} == "i386"
11 SRCS+= opt_asr.h
12 .endif
13
14 .include
15
16 CWARNFLAGS.asr.c= ${NO_WARRAY_BOUNDS}
17 CWARNFLAGS+=${CWARNFLAGS.${.IMPSRC:T}}
signature.asc
Description: Message signed with OpenPGP using GPGMail
Re: asr(4) error with new clang/llvm
On 1 January 2015 at 23:13, Steven Hartland wrote: > > On 02/01/2015 01:23, Bjoern A. Zeeb wrote: >> >> Hi, >> >> you need the next line of source to see that while the union only defines >> Simple[1], the comparison goes up to SG_LIST (or something) which is indeed >> defined as 58. Cn someone fix this? This makes i386 compiles failing >> currently. >> >> /scratch/tmp/bz/head.svn/sys/modules/asr/../../dev/asr/asr.c:1849:29: >> error: array index 58 is past the end of the array (which contains 1 >> element) [-Werror,-Warray-bounds] >> while ((len > 0) && (sg < &((PPRIVATE_SCSI_SCB_EXECUTE_MESSAGE) >> ^ >> /scratch/tmp/bz/head.svn/sys/dev/asr/i2omsg.h:934:8: note: array 'Simple' >> declared here >> I2O_SGE_SIMPLE_ELEMENT Simple[1]; >> ^ > > If that's wrong it looks like there's also a number of calls to the macro > SG(SGL,Index,Flags,Buffer,Size) which are also wrong as Index is used in > the same way: > &(((PI2O_SG_ELEMENT)(SGL))->u.Simple[Index] > > There appears to be two calls to SG where Index is 1. > > I'm afraid I have no idea what the fix would be as the entire driver is very > voodoo like to me :( It's a variable length array in a struct / union. Other than being confusing and now triggering a warning after the clang update it should be fine. Most likely we need to build asr with -Werror disabled for that warning, perhaps -Wno-error-array-bounds. I'll take a look tomorrow morning if nobody else gets to it first. ___ 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: asr(4) error with new clang/llvm
On 02/01/2015 01:23, Bjoern A. Zeeb wrote: Hi, you need the next line of source to see that while the union only defines Simple[1], the comparison goes up to SG_LIST (or something) which is indeed defined as 58. Cn someone fix this? This makes i386 compiles failing currently. /scratch/tmp/bz/head.svn/sys/modules/asr/../../dev/asr/asr.c:1849:29: error: array index 58 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds] while ((len > 0) && (sg < &((PPRIVATE_SCSI_SCB_EXECUTE_MESSAGE) ^ /scratch/tmp/bz/head.svn/sys/dev/asr/i2omsg.h:934:8: note: array 'Simple' declared here I2O_SGE_SIMPLE_ELEMENT Simple[1]; ^ If that's wrong it looks like there's also a number of calls to the macro SG(SGL,Index,Flags,Buffer,Size) which are also wrong as Index is used in the same way: &(((PI2O_SG_ELEMENT)(SGL))->u.Simple[Index] There appears to be two calls to SG where Index is 1. I'm afraid I have no idea what the fix would be as the entire driver is very voodoo like to me :( Regards Steve ___ 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"
