[rfc] a few kern.mk and bsd.sys.mk related changes

[Alexander Best]

On Fri May 27 11, Warner Losh wrote:
> 
> On May 27, 2011, at 12:14 PM, Alexander Best wrote:
> 
> > On Fri May 27 11, Warner Losh wrote:
> >> These look generally good.  Just one thing I had a question on:
> >> 
> >> #
> >> +# Enable FreeBSD kernel-specific printf format specifiers. Also instruct gcc to
> >> +# enable some diagnostics, which make it easier to pinpoint tinderbox failures.
> >> +CFLAGS+=	-fformat-extensions -fdiagnostics-show-option
> >> +
> >> 
> >> Does this put sand in the gears of clang support?
> > 
> > i don't think so. the code that features -fformat-extensions support is freebsd
> > specific and gets plugged into the vendor code during every new import. dim@
> > and rdivacky@ have been discussing the possibility of getting this committed
> > upstream as something like -ffreebsd-extensions, but they haven't decided, yet.
> 
> OK.  I have a bunch of uncommitted changes to support external toolchains, and this sort of thing might bite me when them...  But I'll fight with that when I get there.

well the main problem imho is -fformat-extensions. the code that supports this
flag has not been pushed upstream neither to clang, nor to gcc. so compiling
the freebsd kernel with anything external, like clang tot or gcc 4.6 will fail
to recognise the special printf formatting.

personally i think getting this committed upsteam to clang as
-ffreebsd-extensions would be a nice thing to have. however it's not my
decision to make. ;)

the -fdiagnostics-show-option flag is a standard clang and gcc flag and won't
cause any problems. the only difference between gcc and clang is: gcc has it
disabled by default; clang has it enabled by default.

i've reworked my patch yet again. it now features these changes:

1) -W is now called -Wextra
2) move -fformat-extensions and -fdiagnostics-show-option out of CWARNFLAGS,
   since they aren't really warnings. this will now let anybody use custom
   CWARNFLAGS without having to remember to add -fformat-extensions in order
   not to break buildworld.
3) rearrange "\" characters, as to avoid double spaces
4) when building kernel modules, -fstack-protector was added to CFLAGS twice.
   this should now be solved.
5) only enable -frename-registers, when gcc is used, since clang doesn't
   understand this flag
6) when debugging is enabled, also use the -O flag for amd64 and not -O2. when
   people enable debugging they don't really want optimisations such as -O2
   turned on.

the patch is available here:

http://people.freebsd.org/~arundel/patches/kern.mk-bsd.sys.mk.patch

NB:
regarding point 2), i'd like to encurrage people to play around with CWARNFLAGS
a bit. maybe we find some nice -Wwarning options which are reasonable to have
and - most importantly - don't break tinderbox. for far i've played around with
the following flags and can verify that they break tinderbox:

-Wshadow, -Wwrite-strings, -Wextra, -Wbad-function-cast and
-Wmissing-declarations

-Wmissing-declarations almost works, but there are ~ 20 undeclared functions in
the dtrace code. if those could be fixed it would be possible to add that flag
to CWARNFLAGS.

cheers.
alex

> 
> > regarding -fdiagnostics-show-option: this is intended purely for gcc, since it
> > is disabled per default. under clang the flag exists, too, but it is enabled by
> > default. so specifying it under clang is pretty much a nop.
> 
> Cool.  This is less of a concern, since it will be present in most external toolchains I care about.
> 
> Warner
> 
> > cheers.
> > alex
> > 
> >> 
> >> Warner
> >> 
> >> 
> >> On May 27, 2011, at 5:51 AM, Alexander Best wrote:
> >> 
> >>> hi there,
> >>> 
> >>> i would like to hear people's opinion regarding the following patch. it
> >>> features the following semantic changes:
> >>> 
> >>> 1) -W is now called -Wextra
> >>> 2) move -fformat-extensions and -fdiagnostics-show-option out of CWARNFLAGS,
> >>>  since they aren't really warnings. this will now let anybody use custom
> >>>  CWARNFLAGS without having to remember to add -fformat-extensions in order
> >>>  not to break buildworld.
> >>> 3) rearrange "\" characters, as to avoid double spaces
> >>> 4) when building kernel modules, -fstack-protector was added to CFLAGS twice.
> >>>  this should now be solved.
> >>> 
> >>> cheers.
> >>> alex
> >>> 
> >>> -- 
> >>> a13x
> >>> <kern.mk-bsy.sys.mk.patch>_______________________________________________
> >>> freebsd-toolchain at freebsd.org mailing list
> >>> http://lists.freebsd.org/mailman/listinfo/freebsd-toolchain
> >>> To unsubscribe, send any mail to "freebsd-toolchain-unsubscribe at freebsd.org"
> > 
> > -- 
> > a13x
> > 
> > 

-- 
a13x