Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 May 2008 11:11:08 +0200
From:      Jeremie Le Hen <jeremie@le-hen.org>
To:        Marcel Moolenaar <xcllnt@mac.com>
Cc:        freebsd-arch@FreeBSD.org
Subject:   Re: Integration of ProPolice in FreeBSD
Message-ID:  <20080514091108.GD70896@obiwan.tataz.chchile.org>
In-Reply-To: <8ED24288-618C-4B55-A27E-C5FAB2E046E8@mac.com>
References:  <20080502070147.GE74500@obiwan.tataz.chchile.org> <8ED24288-618C-4B55-A27E-C5FAB2E046E8@mac.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Marcel,

Thank you for your comments.

On Fri, May 02, 2008 at 08:52:37AM -0700, Marcel Moolenaar wrote:
>  Index: share/mk/bsd.sys.mk
>  ===================================================================
>  RCS file: /mnt/octobre/space/freebsd-cvs/src/share/mk/bsd.sys.mk,v
>  retrieving revision 1.44
>  diff -u -p -r1.44 bsd.sys.mk
>  --- share/mk/bsd.sys.mk	22 Nov 2007 23:21:12 -0000	1.44
>  +++ share/mk/bsd.sys.mk	29 Mar 2008 23:13:06 -0000
>  @@ -74,5 +74,10 @@ CWARNFLAGS	+=	-Werror
>   CWARNFLAGS	+=	-Wno-unknown-pragmas
>   .endif
> 
>  +.if ${MK_SSP} != "no" && ${CC} != "icc"
>  +CFLAGS		+=	-fstack-protector
>  +# Don't use -Wstack-protector as it breaks world with -Werror.
>  +.endif
>  +
>   # Allow user-specified additional warning flags
>   CFLAGS		+=	${CWARNFLAGS}
> 
> 
>  I may be better to explicitly test for GCC. I would not assume
>  that GCC and ICC are the only options, even if they are now.
>  There's a second place as well...

I would make sense to test for GCC indeed but in the same file there is
a number of explicit tests for ICC.  Thus I prefer to keep to the
current "style" at the expense of a little more work for those who will
try to compile with another compiler.


>  Index: sys/boot/i386/Makefile.inc
>  ===================================================================
>  RCS file: /mnt/octobre/space/freebsd-cvs/src/sys/boot/i386/Makefile.inc,v
>  retrieving revision 1.12
>  diff -u -p -r1.12 Makefile.inc
>  --- sys/boot/i386/Makefile.inc	28 Sep 2006 10:02:04 -0000	1.12
>  +++ sys/boot/i386/Makefile.inc	28 Mar 2008 07:41:32 -0000
>  @@ -24,3 +24,5 @@ BTXDIR=		${.CURDIR}/../btx
>   BTXLDR=		${BTXDIR}/btxldr/btxldr
>   BTXKERN=	${BTXDIR}/btx/btx
>   BTXCRT=		${BTXDIR}/lib/crt0.o
>  +
>  +.include "../Makefile.inc"
>  Index: sys/boot/i386/loader/Makefile
>  ===================================================================
>  RCS file: /mnt/octobre/space/freebsd-cvs/src/sys/boot/i386/loader/Makefile,v
>  retrieving revision 1.85
>  diff -u -p -r1.85 Makefile
>  --- sys/boot/i386/loader/Makefile	29 May 2007 14:35:57 -0000	1.85
>  +++ sys/boot/i386/loader/Makefile	16 Apr 2008 09:14:10 -0000
>  @@ -1,5 +1,7 @@
>   # $FreeBSD: src/sys/boot/i386/loader/Makefile,v 1.85 2007/05/29 14:35:57 
>  simokawa Exp $
> 
>  +WITHOUT_SSP=
>  +
>   .include <bsd.own.mk>
> 
>   PROG=		loader.sym
> 
>  Maybe second and third level makefiles should include
>  ../../Makefile.inc and ../../../Makefile.inc resp.
>  If, for arguments sake, we want to enable SSP in boot,
>  then it's best if that only requires a single knob to
>  be changed. This may not be a strong argument for SSP,
>  but with Makefile.inc in place, I don't see a possible
>  future in which another knob is added that controls
>  overall behavior (e.g. something like the Watcom option
>  to use argument passing in registers instead of on the
>  stack for i386 -- you definitely want to have that apply
>  to all code or none).

For now, I prefer leaving it as is and let anyone else make this change.
While I agree with your argument, I'm not sure this benefit is worth the
complexity it adds for now, given that the only knob is WITHOUT_SSP and
it would require more than a simple switch to use SSP for /boot (import
SSP symbols).

>  Also, please make sure MK_SSP defaults to "no" on ia64.

Ok, done.

Best regards,
-- 
Jeremie Le Hen
< jeremie at le-hen dot org >< ttz at chchile dot org >



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080514091108.GD70896>