From owner-freebsd-toolchain@FreeBSD.ORG Mon Dec 26 10:10:40 2011 Return-Path: Delivered-To: freebsd-toolchain@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1233) id 981ED1065673; Mon, 26 Dec 2011 10:10:40 +0000 (UTC) Date: Mon, 26 Dec 2011 10:10:40 +0000 From: Alexander Best To: current@freebsd.org Message-ID: <20111226101040.GA6361@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="W/nzBZO5zC0uMSeA" Content-Disposition: inline Cc: freebsd-toolchain@freebsd.org, freebsd-arch@freebsd.org Subject: [rfc] removing/conditionalising WERROR= in Makefiles X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Dec 2011 10:10:40 -0000 --W/nzBZO5zC0uMSeA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline hi there, i grep'ed through src/sys and found several places where WERROR= was set in order to get rid of the default -Werror setting. i tried to remove those WERROR= overrides from any Makefile, where doing so did not break tinderbox. in those cases, where it couldn't be completely removed, i added conditions to only set WERROR= for the particular achitecture or compiler, where tinderbox did not suceed without the WERROR=. i talked to dim@ on #freebsd-clang@OFTC and he was against enclosing WERROR= in a architecture or compiler condition. his statement was: " dim : I'm not going to riddle all those makefiles with endless comments, people have to learn to look in the commit history for a file. " however the problem is that most of the time the commit message by the person who added WERROR= to a Makefile doesn't state *why* the person did it and most importantly it doesn't state in *which* case tinderbox failed without the WERROR= (see *). so my suggestion would be that instead of setting WERROR= unconditionally in Makefiles, it should be enclosed in a condition to only set it in a particular case, where tinderbox *will* fail. an example is r228861. here dim@ set WERROR= in sys/modules/nve/Makefile unconditionally. however tinderbox will only fail when A) clang is used as compiler B) when building for i386 these facts aren't clear from the commit message. so i'd suggest (please see the attached patch) to conditionalise such settings. another point i'd like to make is that WERROR= might have been set in a Makefile in the past to unbreak tinderbox for a certain arch that is no longer supported. since most of the time the commit logs simply state something like: *) "...remove -Werror for now to unbreak tinderbox..." nobody will notice that after support for a certain arch was removed, the WERROR= line can be removed from that particular Makefile. that's why i'd like to propose the following patch. i ran a full tinderbox run against r228878 and it suceeded. cheers. alex --W/nzBZO5zC0uMSeA Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="remove-werror.diff" Index: sys/modules/xfs/Makefile =================================================================== --- sys/modules/xfs/Makefile (revision 228878) +++ sys/modules/xfs/Makefile (working copy) @@ -6,8 +6,6 @@ KMOD= xfs -WERROR= - SRCS = vnode_if.h \ xfs_alloc.c \ xfs_alloc_btree.c \ Index: sys/modules/sound/driver/maestro/Makefile =================================================================== --- sys/modules/sound/driver/maestro/Makefile (revision 228878) +++ sys/modules/sound/driver/maestro/Makefile (working copy) @@ -5,6 +5,5 @@ KMOD= snd_maestro SRCS= device_if.h bus_if.h pci_if.h SRCS+= maestro.c -WERROR= .include Index: sys/modules/aic7xxx/ahd/Makefile =================================================================== --- sys/modules/aic7xxx/ahd/Makefile (revision 228878) +++ sys/modules/aic7xxx/ahd/Makefile (working copy) @@ -4,7 +4,6 @@ .PATH: ${.CURDIR}/../../../dev/aic7xxx KMOD= ahd -WERROR= GENSRCS= aic79xx_seq.h aic79xx_reg.h REG_PRINT_OPT= AHD_REG_PRETTY_PRINT=1 Index: sys/modules/ie/Makefile =================================================================== --- sys/modules/ie/Makefile (revision 228878) +++ sys/modules/ie/Makefile (working copy) @@ -6,6 +6,8 @@ KMOD= if_ie SRCS= if_ie.c if_ie_isa.c \ isa_if.h bus_if.h device_if.h +.if ${MACHINE_CPUARCH} == "i386" WERROR= +.endif .include Index: sys/modules/agp/Makefile =================================================================== --- sys/modules/agp/Makefile (revision 228878) +++ sys/modules/agp/Makefile (working copy) @@ -20,7 +20,6 @@ SRCS+= device_if.h bus_if.h agp_if.h pci_if.h SRCS+= opt_agp.h opt_bus.h MFILES= kern/device_if.m kern/bus_if.m dev/agp/agp_if.m dev/pci/pci_if.m -WERROR= EXPORT_SYMS= agp_find_device \ agp_state \ Index: sys/modules/bios/smapi/Makefile =================================================================== --- sys/modules/bios/smapi/Makefile (revision 228878) +++ sys/modules/bios/smapi/Makefile (working copy) @@ -6,7 +6,6 @@ KMOD= smapi SRCS= smapi.c smapi_bios.S \ bus_if.h device_if.h -WERROR= .if ${CC:T:Mclang} == "clang" # XXX: clang integrated-as doesn't grok 16-bit assembly yet CFLAGS+= ${.IMPSRC:T:Msmapi_bios.S:C/^.+$/-no-integrated-as/} Index: sys/modules/nve/Makefile =================================================================== --- sys/modules/nve/Makefile (revision 228878) +++ sys/modules/nve/Makefile (working copy) @@ -7,7 +7,9 @@ device_if.h bus_if.h pci_if.h miibus_if.h \ os+%DIKED-nve.h OBJS+= nvenetlib.o +.if ${MACHINE_CPUARCH} == "i386" && ${CC:T:Mclang} == "clang" WERROR= +.endif CLEANFILES+= nvenetlib.o os+%DIKED-nve.h nvenetlib.o: ${.CURDIR}/../../contrib/dev/nve/${MACHINE}/${.TARGET}.bz2.uu --W/nzBZO5zC0uMSeA-- From owner-freebsd-toolchain@FreeBSD.ORG Fri Dec 30 13:06:50 2011 Return-Path: Delivered-To: freebsd-toolchain@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4D70E1065672; Fri, 30 Dec 2011 13:06:50 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from tensor.andric.com (cl-327.ede-01.nl.sixxs.net [IPv6:2001:7b8:2ff:146::2]) by mx1.freebsd.org (Postfix) with ESMTP id 092498FC13; Fri, 30 Dec 2011 13:06:50 +0000 (UTC) Received: from [IPv6:2001:7b8:3a7:0:408b:3bdd:8466:f24f] (unknown [IPv6:2001:7b8:3a7:0:408b:3bdd:8466:f24f]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by tensor.andric.com (Postfix) with ESMTPSA id 48BF75C37; Fri, 30 Dec 2011 14:06:49 +0100 (CET) Message-ID: <4EFDB76C.1030901@FreeBSD.org> Date: Fri, 30 Dec 2011 14:06:52 +0100 From: Dimitry Andric Organization: The FreeBSD Project User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20111222 Thunderbird/9.0.1 MIME-Version: 1.0 To: Alexander Best References: <20111226101040.GA6361@freebsd.org> In-Reply-To: <20111226101040.GA6361@freebsd.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-toolchain@freebsd.org, current@freebsd.org, freebsd-arch@freebsd.org Subject: Re: [rfc] removing/conditionalising WERROR= in Makefiles X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Dec 2011 13:06:50 -0000 On 2011-12-26 11:10, Alexander Best wrote: > that's why i'd like to propose the following patch. i ran a full tinderbox run > against r228878 and it suceeded. Did you also try this with clang? For example the xfs module alone gets a whole slew of warnings, which would be fatal if WERROR= was removed: sys/gnu/fs/xfs/xfs_dir2_block.c:1149:17: warning: array index of '1' indexes past the end of an array (that contains 1 element) [-Warray-bounds] dep->name[0] = dep->name[1] = '.'; ^ ~ sys/gnu/fs/xfs/xfs_dir2_data.h:90:2: note: array 'name' declared here __uint8_t name[1]; /* name bytes, no null */ ^ 1 warning generated. sys/gnu/fs/xfs/xfs_dir2_sf.c:117:27: warning: array index of '1' indexes past the end of an array (that contains 1 element) [-Warray-bounds] dep->name[0] == '.' && dep->name[1] == '.'; ^ ~ sys/gnu/fs/xfs/xfs_dir2_data.h:90:2: note: array 'name' declared here __uint8_t name[1]; /* name bytes, no null */ ^ sys/gnu/fs/xfs/xfs_dir2_sf.c:237:28: warning: array index of '1' indexes past the end of an array (that contains 1 element) [-Warray-bounds] dep->name[0] == '.' && dep->name[1] == '.') ^ ~ sys/gnu/fs/xfs/xfs_dir2_data.h:90:2: note: array 'name' declared here __uint8_t name[1]; /* name bytes, no null */ ^ 2 warnings generated. sys/gnu/fs/xfs/xfs_vfsops.c:1958:19: warning: format string is not a string literal (potentially insecure) [-Wformat-security] sbuf_printf(m, xfs_infop->str); ^~~~~~~~~~~~~~ 1 warning generated. sys/gnu/fs/xfs/FreeBSD/xfs_super.c:257:9: warning: format string is not a string literal (potentially insecure) [-Wformat-security] printf(message); ^~~~~~~ 1 warning generated. sys/gnu/fs/xfs/FreeBSD/xfs_stats.c:68:32: warning: format string is not a string literal (potentially insecure) [-Wformat-security] len += sprintf(buffer + len, xstats[i].desc); ^~~~~~~~~~~~~~ sys/gnu/fs/xfs/FreeBSD/xfs_stats.c:99:33: warning: if statement has empty body [-Wempty-body] if (&xfs_read_xfsstats != NULL); ^ 2 warnings generated. sys/gnu/fs/xfs/FreeBSD/xfs_ioctl.c:1252:10: warning: explicitly assigning a variable of type 'int' to itself [-Wself-assign] error = error; ~~~~~ ^ ~~~~~ sys/gnu/fs/xfs/FreeBSD/xfs_ioctl.c:1290:9: warning: explicitly assigning a variable of type 'int' to itself [-Wself-assign] error = error; ~~~~~ ^ ~~~~~ sys/gnu/fs/xfs/FreeBSD/xfs_ioctl.c:1299:10: warning: explicitly assigning a variable of type 'int' to itself [-Wself-assign] error = error; ~~~~~ ^ ~~~~~ sys/gnu/fs/xfs/FreeBSD/xfs_ioctl.c:1350:9: warning: explicitly assigning a variable of type 'int' to itself [-Wself-assign] error = error; ~~~~~ ^ ~~~~~ 4 warnings generated. Most of the warnings should probably be fixed, but as this is mainly contrib code, and GPL to boot, it makes not much sense to do so. From owner-freebsd-toolchain@FreeBSD.ORG Fri Dec 30 14:08:35 2011 Return-Path: Delivered-To: freebsd-toolchain@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E5270106564A; Fri, 30 Dec 2011 14:08:35 +0000 (UTC) (envelope-from theraven@theravensnest.org) Received: from theravensnest.org (theravensnest.org [109.169.23.128]) by mx1.freebsd.org (Postfix) with ESMTP id 7C5F08FC0C; Fri, 30 Dec 2011 14:08:35 +0000 (UTC) Received: from [192.168.0.2] (cpc2-cwma5-0-0-cust875.7-3.cable.virginmedia.com [86.11.39.108]) (authenticated bits=0) by theravensnest.org (8.14.4/8.14.4) with ESMTP id pBUDSpdF032333 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES128-SHA bits=128 verify=NO); Fri, 30 Dec 2011 13:28:51 GMT (envelope-from theraven@theravensnest.org) Mime-Version: 1.0 (Apple Message framework v1251.1) Content-Type: text/plain; charset=iso-8859-1 From: David Chisnall In-Reply-To: <4EFDB76C.1030901@FreeBSD.org> Date: Fri, 30 Dec 2011 13:28:51 +0000 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20111226101040.GA6361@freebsd.org> <4EFDB76C.1030901@FreeBSD.org> To: Dimitry Andric X-Mailer: Apple Mail (2.1251.1) Cc: Alexander Best , freebsd-toolchain@freebsd.org, current@freebsd.org, freebsd-arch@freebsd.org Subject: Re: [rfc] removing/conditionalising WERROR= in Makefiles X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Dec 2011 14:08:36 -0000 On 30 Dec 2011, at 13:06, Dimitry Andric wrote: > sys/gnu/fs/xfs/xfs_dir2_block.c:1149:17: warning: array index of '1' = indexes past the end of an array (that contains 1 element) = [-Warray-bounds] I recall some discussion of this warning on the clang list a few months = ago, and I believe that it should now only appear if you are compiling = in a C99 or C11 dialect mode (the rationale is that any variable-length = structures in C99 should be using a zero-sized array as the final = element, while C89 lacked any ability to do this). =20 I suspect a lot of similar warnings are caused by the difference in = default dialects between clang and gcc. Adding an explicit -std=3Dc89 = to the cflags any modules that are using an archaic dialect of C may = silence a lot of these... David= From owner-freebsd-toolchain@FreeBSD.ORG Fri Dec 30 17:04:58 2011 Return-Path: Delivered-To: freebsd-toolchain@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6D0E51065670; Fri, 30 Dec 2011 17:04:58 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from tensor.andric.com (cl-327.ede-01.nl.sixxs.net [IPv6:2001:7b8:2ff:146::2]) by mx1.freebsd.org (Postfix) with ESMTP id 283728FC19; Fri, 30 Dec 2011 17:04:58 +0000 (UTC) Received: from [IPv6:2001:7b8:3a7:0:408b:3bdd:8466:f24f] (unknown [IPv6:2001:7b8:3a7:0:408b:3bdd:8466:f24f]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by tensor.andric.com (Postfix) with ESMTPSA id F0C3C5C37; Fri, 30 Dec 2011 18:04:56 +0100 (CET) Message-ID: <4EFDEF3B.4090200@FreeBSD.org> Date: Fri, 30 Dec 2011 18:04:59 +0100 From: Dimitry Andric Organization: The FreeBSD Project User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20111222 Thunderbird/9.0.1 MIME-Version: 1.0 To: David Chisnall References: <20111226101040.GA6361@freebsd.org> <4EFDB76C.1030901@FreeBSD.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Best , freebsd-toolchain@freebsd.org, current@freebsd.org, freebsd-arch@freebsd.org Subject: Re: [rfc] removing/conditionalising WERROR= in Makefiles X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Dec 2011 17:04:58 -0000 On 2011-12-30 14:28, David Chisnall wrote: > On 30 Dec 2011, at 13:06, Dimitry Andric wrote: > >> sys/gnu/fs/xfs/xfs_dir2_block.c:1149:17: warning: array index of '1' indexes past the end of an array (that contains 1 element) [-Warray-bounds] > > I recall some discussion of this warning on the clang list a few months ago, and I believe that it should now only appear if you are compiling in a C99 or C11 dialect mode (the rationale is that any variable-length structures in C99 should be using a zero-sized array as the final element, while C89 lacked any ability to do this). Yes, that is perfectly fine, but the xfs code defines the struct in question as follows: /* * Active entry in a data block. Aligned to 8 bytes. * Tag appears as the last 2 bytes. */ typedef struct xfs_dir2_data_entry { xfs_ino_t inumber; /* inode number */ __uint8_t namelen; /* name length */ __uint8_t name[1]; /* name bytes, no null */ /* variable offset */ xfs_dir2_data_off_t tag; /* starting offset of us */ } xfs_dir2_data_entry_t; E.g there *is* an overrun, but maybe it was really supposed to be like that. Meanwhile, upstream has apparently caught on: http://oss.sgi.com/archives/xfs/2011-07/msg00024.html From owner-freebsd-toolchain@FreeBSD.ORG Fri Dec 30 21:19:51 2011 Return-Path: Delivered-To: toolchain@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9FEBE106566B for ; Fri, 30 Dec 2011 21:19:51 +0000 (UTC) (envelope-from tijl@coosemans.org) Received: from mailrelay011.isp.belgacom.be (mailrelay011.isp.belgacom.be [195.238.6.178]) by mx1.freebsd.org (Postfix) with ESMTP id 1FEE58FC16 for ; Fri, 30 Dec 2011 21:19:50 +0000 (UTC) X-Belgacom-Dynamic: yes X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEAOUh/k5bsQ3b/2dsb2JhbABDrFKBBoJPgT2IM5YcnzmDfYR4gxoEjiaZEQ Received: from 219.13-177-91.adsl-dyn.isp.belgacom.be (HELO kalimero.tijl.coosemans.org) ([91.177.13.219]) by relay.skynet.be with ESMTP; 30 Dec 2011 21:50:16 +0100 Received: from kalimero.tijl.coosemans.org (kalimero.tijl.coosemans.org [127.0.0.1]) by kalimero.tijl.coosemans.org (8.14.5/8.14.5) with ESMTP id pBUKoFiM077057 for ; Fri, 30 Dec 2011 21:50:16 +0100 (CET) (envelope-from tijl@coosemans.org) From: Tijl Coosemans To: toolchain@freebsd.org Date: Fri, 30 Dec 2011 21:50:07 +0100 User-Agent: KMail/1.13.7 (FreeBSD/10.0-CURRENT; KDE/4.7.3; i386; ; ) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2065800.D6lVRokuGV"; protocol="application/pgp-signature"; micalg=pgp-sha256 Content-Transfer-Encoding: 7bit Message-Id: <201112302150.13636.tijl@coosemans.org> Cc: Subject: system vs ports gcc -fstack-protector X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Dec 2011 21:19:51 -0000 --nextPart2065800.D6lVRokuGV Content-Type: multipart/mixed; boundary="Boundary-01=_/Pi/OPxxySsA5rU" Content-Transfer-Encoding: 7bit --Boundary-01=_/Pi/OPxxySsA5rU Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline I ran into a difference between the system and ports gcc. When using -fstack-protector the system gcc passes -lssp_nonshared to the linker and ports gcc doesn't, which causes the undefined reference below: % cat test.c int main( void ) { return( 0 ); } % gcc46 -o test test.c -fstack-protector-all -fPIE /var/tmp//ccjYQxKu.o: In function `main': test.c:(.text+0x37): undefined reference to `__stack_chk_fail_local' /usr/local/bin/ld: test: hidden symbol `__stack_chk_fail_local' isn't defined /usr/local/bin/ld: final link failed: Bad value collect2: ld returned 1 exit status The ports gcc configure script detects __stack_chk_fail in our libc and assumes libc provides everything, so I wonder if that's a wrong assumption or if something's wrong in our ssp implementation. The attached patch is a quick fix for lang/gcc46. --Boundary-01=_/Pi/OPxxySsA5rU Content-Type: text/plain; charset="us-ascii"; name="patch-ssp" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="patch-ssp" =2D-- gcc/gcc.c.orig +++ gcc/gcc.c @@ -601,7 +601,7 @@ =20 #ifndef LINK_SSP_SPEC #ifdef TARGET_LIBC_PROVIDES_SSP =2D#define LINK_SSP_SPEC "%{fstack-protector:}" +#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonsh= ared}" #else #define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonsh= ared -lssp}" #endif --Boundary-01=_/Pi/OPxxySsA5rU-- --nextPart2065800.D6lVRokuGV Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iF4EABEIAAYFAk7+JAUACgkQfoCS2CCgtiu+BwD/S3jmMTyKkTX6RB0YpoUmMvX1 iPMR2bboSjuhLnoUPCwA/RnCdboRONLV+YOr5LB43Or/sCIPyuHfSA84sJb3lFnv =pYch -----END PGP SIGNATURE----- --nextPart2065800.D6lVRokuGV--