Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 12 Nov 2019 22:19:49 -0600
From:      Kyle Evans <kevans@freebsd.org>
To:        Pedro Giffuni <pfg@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>,  svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r354672 - head/lib/libc/secure
Message-ID:  <CACNAnaGYZwLkEdUj%2BhQHyCRx=V1AW7P5s33i8xuBVsqo=9hKAA@mail.gmail.com>
In-Reply-To: <80479651-f60d-e352-1f40-f01939aff9fd@FreeBSD.org>
References:  <201911130300.xAD30WUE099996@repo.freebsd.org> <80479651-f60d-e352-1f40-f01939aff9fd@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Nov 12, 2019, 22:04 Pedro Giffuni <pfg@freebsd.org> wrote:

>
> On 12/11/2019 22:00, Kyle Evans wrote:
>
> Author: kevans
> Date: Wed Nov 13 03:00:32 2019
> New Revision: 354672
> URL: https://svnweb.freebsd.org/changeset/base/354672
>
> Log:
>   ssp: rework the logic to use priority=200 on clang builds
>
>   The preproc logic was added at the last minute to appease GCC 4.2, and
>   kevans@ did clearly not go back and double-check that the logic worked out
>   for clang builds to use the new variant.
>
>   It turns out that clang defines __GNUC__ == 4. Flip it around and check
>   __clang__ as well, leaving a note to remove it later.
>
>
> clang reports itself as GCC 4.2, the priority argument was introduced in
> GCC 4.3.
>
>   Reported by:	cem
>
> Modified:
>   head/lib/libc/secure/stack_protector.c
>
> Modified: head/lib/libc/secure/stack_protector.c
> ==============================================================================
> --- head/lib/libc/secure/stack_protector.c	Wed Nov 13 02:22:00 2019	(r354671)
> +++ head/lib/libc/secure/stack_protector.c	Wed Nov 13 03:00:32 2019	(r354672)
> @@ -47,13 +47,15 @@ __FBSDID("$FreeBSD$");
>   * they're either not usually statically linked or they simply don't do things
>   * in constructors that would be adversely affected by their positioning with
>   * respect to this initialization.
> + *
> + * This conditional should be removed when GCC 4.2 is removed.
>   */
> -#if defined(__GNUC__) && __GNUC__ <= 4
> -#define	_GUARD_SETUP_CTOR_ATTR	\
> -    __attribute__((__constructor__, __used__));
> -#else
> +#if defined(__clang__) || (defined(__GNUC__) && __GNUC__ > 4)
>  #define	_GUARD_SETUP_CTOR_ATTR	 \
>      __attribute__((__constructor__ (200), __used__));
> +#else
> +#define	_GUARD_SETUP_CTOR_ATTR	\
> +    __attribute__((__constructor__, __used__));
>  #endif
>
>  extern int __sysctl(const int *name, u_int namelen, void *oldp,
>
> Please fix properly. Assuming clang always supported it, something like:
>
> #if __GNUC_PREREQ__(4, 3) || __has_attribute(__constructor__)
>
> should work
>
> Cheers,
>

I considered something of this sort, but searching for information on the
priority argument in the first place was annoying enough that I had too
much search-fatigue to figure out when GCC actually corrected this, thus
assuming that GCC5 (which seemed to be an all-around good release if memory
serves) and later (since I confirmed GCC6) was sufficient.

I'll fix it in the morning (~8 hours) if I receive no further objections to
address.

Thanks!

>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaGYZwLkEdUj%2BhQHyCRx=V1AW7P5s33i8xuBVsqo=9hKAA>