freebsd stable - Jul 2014 - [HEADS-UP] Problem with clang in 9-stable [was: r268244 (stable/9) seems to break "sysctl hw.ncpu"]

On 4 Jul 2014, at 19:18, David Wolfskill <david at catwhisker.org> wrote:
> clang -O2 -pipe  -std=gnu99 -Qunused-arguments  -fstack-protector
-Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-uninitialized
-Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int
-Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value
-Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion  -o sysctl
sysctl.o
This compile line is turning off a lot of warnings.  In particular,
-Wno-uninitialized and -Wno-parentheses-equality are likely to hide warnings
that refer to real errors.  It sounds like this case was one of them - if these
warnings were on then we'd have got a build failure rather than an
executable that depended on undefined behaviour.

David

On Sat, Jul 05, 2014 at 01:49:44PM +0100, David Chisnall
wrote:
> On 4 Jul 2014, at 19:18, David Wolfskill <david at catwhisker.org>
wrote:
> 
> > clang -O2 -pipe  -std=gnu99 -Qunused-arguments  -fstack-protector
-Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-uninitialized
-Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int
-Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value
-Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion  -o sysctl
sysctl.o
> 
> This compile line is turning off a lot of warnings.  In particular,
-Wno-uninitialized and -Wno-parentheses-equality are likely to hide warnings
that refer to real errors.  It sounds like this case was one of them - if these
warnings were on then we'd have got a build failure rather than an
executable that depended on undefined behaviour.
> ....
So I checked src/sbin/sysctl/Makefile first; it's fairly
"vanilla":

#       @(#)Makefile    8.1 (Berkeley) 6/6/93
# $FreeBSD: stable/9/sbin/sysctl/Makefile 203917 2010-02-15 14:08:06Z
uqs $

PROG=   sysctl
WARNS?= 3
MAN=    sysctl.8

.include <bsd.prog.mk>

And the -Wno-uninitialized (at least) comes from bsd.sys.mk:

.if ${WARNS} >= 2 && ${WARNS} <= 4
# XXX Delete -Wuninitialized by default for now -- the compiler doesn't
# XXX always get it right.
CWARNFLAGS+=    -Wno-uninitialized
.endif # WARNS >=2 && WARNS <= 4

A bit later, we see the origin of -Wno-parentheses-equality:

# Clang has more warnings enabled by default, and when using -Wall, so if WARNS
# is set to low values, these have to be disabled explicitly.
.if ${COMPILER_TYPE} == "clang" && !defined(EARLY_BUILD)
.if ${WARNS} <= 6
CWARNFLAGS+=    -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable
.endif # WARNS <= 6
.if ${WARNS} <= 3
CWARNFLAGS+=    -Wno-tautological-compare -Wno-unused-value\
                -Wno-parentheses-equality -Wno-unused-function
-Wno-enum-conversion
.endif # WARNS <= 3
.if ${WARNS} <= 2
CWARNFLAGS+=    -Wno-switch -Wno-switch-enum -Wno-knr-promoted-parameter
.endif # WARNS <= 2
.if ${WARNS} <= 1
CWARNFLAGS+=    -Wno-parentheses
.endif # WARNS <= 1
.if defined(NO_WARRAY_BOUNDS)
CWARNFLAGS+=    -Wno-array-bounds
.endif # NO_WARRAY_BOUNDS
.endif # CLANG


I don't know that there's likely to be a huge amount of interest
in addressing the issue for stable/9, but stable/10 looks similar,
and while I see some differences in head, the code in head's
bsd.sys.mk may well be functionally equivalent.

I'm happy to help test if someone wants to put together patches to
(at least) reduce the extent to which we have executables depending
on undefined behavior.

Peace,
david
-- 
David H. Wolfskill				david at catwhisker.org
Taliban: Evil cowards with guns afraid of truth from a 14-year old girl.

See http://www.catwhisker.org/~david/publickey.gpg for my public key.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 949 bytes
Desc: not available
URL:
<http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20140705/21255146/attachment.sig>

On 05 Jul 2014, at 14:49, David Chisnall <theraven at theravensnest.org>
wrote:
> On 4 Jul 2014, at 19:18, David Wolfskill <david at catwhisker.org>
wrote:
> 
>> clang -O2 -pipe  -std=gnu99 -Qunused-arguments  -fstack-protector
-Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-uninitialized
-Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int
-Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value
-Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion  -o sysctl
sysctl.o
> 
> This compile line is turning off a lot of warnings.  In particular,
-Wno-uninitialized and -Wno-parentheses-equality are likely to hide warnings
that refer to real errors.
Interestingly, -Wno-uninitialized has been in bsd.sys.mk since r76861,
and the accompanying comment ("XXX Delete -Wuninitialized by default for
now -- the compiler doesn't always get it right") has never been
changed. :-)

It is probably time to re-enable that warning after 13 years, at least.

I'm not so sure about -Wno-parentheses-equality, because that might give
quite a few false positives, especially in old, contributed code.

-Dimitry

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 203 bytes
Desc: Message signed with OpenPGP using GPGMail
URL:
<http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20140705/20931958/attachment.sig>

On 5 Jul 2014, at 14:07, Dimitry Andric <dim@FreeBSD.org> wrote:
> Interestingly, -Wno-uninitialized has been in bsd.sys.mk since r76861,
> and the accompanying comment ("XXX Delete -Wuninitialized by default
for
> now -- the compiler doesn't always get it right") has never been
> changed. :-)
> 
> It is probably time to re-enable that warning after 13 years, at least.
It probably only wants enabling for clang.  GCC (at least, GCC 4.2.1) performs
this analysis based on analyses run by the optimisers and so the warnings are
dependent on optimisation level.

David

_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to
"freebsd-stable-unsubscribe@freebsd.org"