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
>
> 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 >
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"