[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Brendan Heading]

Fixes:
http://autobuild.buildroot.net/results/123/123a5b3f72ba8c1a4aa1cea5b7b846a04fd4e923/
http://autobuild.buildroot.net/results/38c/38cfa4e7249a8770b06dbd392acba79303d3f9bc/

.. and others.

GCC's configure stage assumes that if the glibc version, as denoted by
__GLIBC__ and __GLIBC_MINOR__, is greater or equal to 2.4 then stack
protection must be available in the C library. This results in the compiler
not attempting to link SSP helpers during the final link.

The problem is seen with uclibc-ng 1.0.6 (and likely greater) where they
updated the __GLIBC_MINOR__ value to 10. It will be seen in any libc
that permits stack protection to be disabled while exporting a glibc
version >= 2.4.

This patch overrides GCC to expect/not expect SSP support in libc based on
the toolchain's capability as understood by buildroot.

Signed-off-by: Brendan Heading <brendanheading@gmail.com>
---
Patch V1 :
This fix definitely solves the problem, however I doubt it's acceptable to
export environment variables in this way.

I had initially tried adding it to HOST_GCC_COMMON_CONF_ENV, and I
confirmed that this is passed into GCC's top level configure, however it
does not seem to propagate to the gcc/configure script. Another way would
be to patch GCC, however this would involve maintaining patches for all
the supported GCC versions.

Improvement suggestions welcome!
---
 package/gcc/gcc.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk
index 501fcea..43835af 100644
--- a/package/gcc/gcc.mk
+++ b/package/gcc/gcc.mk
@@ -123,6 +123,14 @@ endif
 HOST_GCC_COMMON_CONF_ENV += CFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_CFLAGS)"
 HOST_GCC_COMMON_CONF_ENV += CXXFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_CXXFLAGS)"
 
+# Work around issue with detecting SSP support in the C library
+
+ifeq ($(BR2_TOOLCHAIN_HAS_SSP),y)
+export gcc_cv_libc_provides_ssp=yes
+else
+export gcc_cv_libc_provides_ssp=no
+endif
+
 # libitm needs sparc V9+
 ifeq ($(BR2_sparc_v8)$(BR2_sparc_leon3),y)
 HOST_GCC_COMMON_CONF_OPTS += --disable-libitm
-- 
2.4.3

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Thomas Petazzoni]

Brendan,

On Thu, 17 Sep 2015 15:37:58 +0100, Brendan Heading wrote:

> GCC's configure stage assumes that if the glibc version, as denoted by
> __GLIBC__ and __GLIBC_MINOR__, is greater or equal to 2.4 then stack
> protection must be available in the C library. This results in the compiler
> not attempting to link SSP helpers during the final link.
> 
> The problem is seen with uclibc-ng 1.0.6 (and likely greater) where they
> updated the __GLIBC_MINOR__ value to 10. It will be seen in any libc
> that permits stack protection to be disabled while exporting a glibc
> version >= 2.4.

Cc'ing Waldemar here. It's an interesting consequence of bumping the
GLIBC_MINOR version exposed by uClibc, which happened recently to solve
an eventfd_read() problem in Boost on ARC, investigated by Alexey (also
in Cc).

> 
> This patch overrides GCC to expect/not expect SSP support in libc based on
> the toolchain's capability as understood by buildroot.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>
> ---
> Patch V1 :
> This fix definitely solves the problem, however I doubt it's acceptable to
> export environment variables in this way.
> 
> I had initially tried adding it to HOST_GCC_COMMON_CONF_ENV, and I
> confirmed that this is passed into GCC's top level configure, however it
> does not seem to propagate to the gcc/configure script. Another way would
> be to patch GCC, however this would involve maintaining patches for all
> the supported GCC versions.
> 
> Improvement suggestions welcome!
> ---
>  package/gcc/gcc.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk
> index 501fcea..43835af 100644
> --- a/package/gcc/gcc.mk
> +++ b/package/gcc/gcc.mk
> @@ -123,6 +123,14 @@ endif
>  HOST_GCC_COMMON_CONF_ENV += CFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_CFLAGS)"
>  HOST_GCC_COMMON_CONF_ENV += CXXFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_CXXFLAGS)"
>  
> +# Work around issue with detecting SSP support in the C library
> +
> +ifeq ($(BR2_TOOLCHAIN_HAS_SSP),y)
> +export gcc_cv_libc_provides_ssp=yes
> +else
> +export gcc_cv_libc_provides_ssp=no
> +endif

Exporting that globally is really horrible. I think the appropriate fix
is probably to teach gcc about uClibc, like we're doing for musl already
(http://git.buildroot.net/buildroot/tree/package/gcc/4.9.3/900-musl-support.patch#n452).
So we could do a check here like:

 *uclibc*)
	test if __UCLIBC_HAS_SSP__ is defined or not. If yes -> we have
	SSP, if not -> we don't have SSP.

And contribute that to upstream gcc.

What do you think?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Brendan Heading]

>> The problem is seen with uclibc-ng 1.0.6 (and likely greater) where they
>> updated the __GLIBC_MINOR__ value to 10. It will be seen in any libc
>> that permits stack protection to be disabled while exporting a glibc
>> version >= 2.4.
>
> Cc'ing Waldemar here. It's an interesting consequence of bumping the
> GLIBC_MINOR version exposed by uClibc, which happened recently to solve
> an eventfd_read() problem in Boost on ARC, investigated by Alexey (also
> in Cc).

Yup, that's exactly what has exposed the problem. And the "problem" is
really a deficiency in the way GCC detects SSP support in the libc.

> Exporting that globally is really horrible.

Yeah I knew that exporting it globally was not going to fly - I just
submitted it as a starting point for discussion how to solve this.

> I think the appropriate fix
> is probably to teach gcc about uClibc, like we're doing for musl already
> (http://git.buildroot.net/buildroot/tree/package/gcc/4.9.3/900-musl-support.patch#n452).
> So we could do a check here like:
>
>  *uclibc*)
>         test if __UCLIBC_HAS_SSP__ is defined or not. If yes -> we have
>         SSP, if not -> we don't have SSP.
>
> And contribute that to upstream gcc.
>
> What do you think?

It is up to you. My only quibble would be that layering another patch
in this area (where there are already patches adding stuff as you
noted) might get a bit fiddly. Are we okay with patches being
order-dependent ? This does mean that they're not so easy to submit
upstream ..

Aside from that .. GCC actually already has a block which checks
UCLIBC_HAS_SSP. The problem is that it reaches it only if the glibc
version check returns that the version is 2.3 or lower. The fix might
simply be to reorder the check.

I'm curious why GCC have done this rather than doing a check for the
__stack_chk symbols. Would be interested in hearing from Alexey and
Waldemar.

Brendan

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Waldemar Brodkorb]

Hi,
Brendan Heading wrote,

> Aside from that .. GCC actually already has a block which checks
> UCLIBC_HAS_SSP. The problem is that it reaches it only if the glibc
> version check returns that the version is 2.3 or lower. The fix might
> simply be to reorder the check.
> 
> I'm curious why GCC have done this rather than doing a check for the
> __stack_chk symbols. Would be interested in hearing from Alexey and
> Waldemar.

I vote for the GCC patch, if there is already a __UCLIBC_HAS_SSP__
check.

best regards
 Waldemar

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Thomas Petazzoni]

Brendan,

On Thu, 17 Sep 2015 18:29:12 +0100, Brendan Heading wrote:

> > Cc'ing Waldemar here. It's an interesting consequence of bumping the
> > GLIBC_MINOR version exposed by uClibc, which happened recently to solve
> > an eventfd_read() problem in Boost on ARC, investigated by Alexey (also
> > in Cc).
> 
> Yup, that's exactly what has exposed the problem. And the "problem" is
> really a deficiency in the way GCC detects SSP support in the libc.

Correct.

> > Exporting that globally is really horrible.
> 
> Yeah I knew that exporting it globally was not going to fly - I just
> submitted it as a starting point for discussion how to solve this.

Sure, no problem with that. It's already much appreciated that you did
all this investigation.

> > I think the appropriate fix
> > is probably to teach gcc about uClibc, like we're doing for musl already
> > (http://git.buildroot.net/buildroot/tree/package/gcc/4.9.3/900-musl-support.patch#n452).
> > So we could do a check here like:
> >
> >  *uclibc*)
> >         test if __UCLIBC_HAS_SSP__ is defined or not. If yes -> we have
> >         SSP, if not -> we don't have SSP.
> >
> > And contribute that to upstream gcc.
> >
> > What do you think?
> 
> It is up to you. My only quibble would be that layering another patch
> in this area (where there are already patches adding stuff as you
> noted) might get a bit fiddly. Are we okay with patches being
> order-dependent ? This does mean that they're not so easy to submit
> upstream ..

We are fine with patches being order-dependent. That's fine we have a
sequence number for all patches in the first place.

So far, the effort to push upstream our gcc patches has been very
limited. It would be good to push some of them upstream, but in the
mean time, our stack of gcc patches is not that big, and is not causing
too many problems when bumping gcc.

> Aside from that .. GCC actually already has a block which checks
> UCLIBC_HAS_SSP. The problem is that it reaches it only if the glibc
> version check returns that the version is 2.3 or lower. The fix might
> simply be to reorder the check.

If it's that simple, then it should be done :) In any case, version
based tests are often not a good idea, so when uClibc is used, gcc
should really rely on UCLIBC_HAS_SSP.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Brendan Heading]

>> Yeah I knew that exporting it globally was not going to fly - I just
>> submitted it as a starting point for discussion how to solve this.
>
> Sure, no problem with that. It's already much appreciated that you did
> all this investigation.

Thank you for that!

> We are fine with patches being order-dependent. That's fine we have a
> sequence number for all patches in the first place.
>
> So far, the effort to push upstream our gcc patches has been very
> limited. It would be good to push some of them upstream, but in the
> mean time, our stack of gcc patches is not that big, and is not causing
> too many problems when bumping gcc.

Yeah I also get the sense that getting patches upstream in GCC might
be difficult, and when I was googling this I saw other examples of
people running into problems similar to this. I would guess it's
especially unlikely we would get patches into the versions that are in
maintenance mode. We might have more luck with next (ie GCC 6.0).

>> Aside from that .. GCC actually already has a block which checks
>> UCLIBC_HAS_SSP. The problem is that it reaches it only if the glibc
>> version check returns that the version is 2.3 or lower. The fix might
>> simply be to reorder the check.
>
> If it's that simple, then it should be done :) In any case, version
> based tests are often not a good idea, so when uClibc is used, gcc
> should really rely on UCLIBC_HAS_SSP.

Okay, I am now looking at putting together an interim patch set. I
don't think there is any point in trying to submit them into
maintenance releases of the existing versions but it's worth taking a
shot at getting them into -next.

regards

Brendan

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Thomas Petazzoni]

Brendan,

On Thu, 17 Sep 2015 22:22:52 +0100, Brendan Heading wrote:

> > We are fine with patches being order-dependent. That's fine we have a
> > sequence number for all patches in the first place.
> >
> > So far, the effort to push upstream our gcc patches has been very
> > limited. It would be good to push some of them upstream, but in the
> > mean time, our stack of gcc patches is not that big, and is not causing
> > too many problems when bumping gcc.
> 
> Yeah I also get the sense that getting patches upstream in GCC might
> be difficult, and when I was googling this I saw other examples of
> people running into problems similar to this. I would guess it's
> especially unlikely we would get patches into the versions that are in
> maintenance mode. We might have more luck with next (ie GCC 6.0).

It is indeed not necessarily easy to get patches upstream, but when it
is the proper fix, it is nonetheless what we should do.

> >> Aside from that .. GCC actually already has a block which checks
> >> UCLIBC_HAS_SSP. The problem is that it reaches it only if the glibc
> >> version check returns that the version is 2.3 or lower. The fix might
> >> simply be to reorder the check.
> >
> > If it's that simple, then it should be done :) In any case, version
> > based tests are often not a good idea, so when uClibc is used, gcc
> > should really rely on UCLIBC_HAS_SSP.
> 
> Okay, I am now looking at putting together an interim patch set. I
> don't think there is any point in trying to submit them into
> maintenance releases of the existing versions but it's worth taking a
> shot at getting them into -next.

Great, thanks!

In fact, maybe uClibc-ng should have some sort of side project to
collect the gcc patches needed to build a uClibc based toolchain, a bit
like musl is doing at
https://bitbucket.org/GregorR/musl-gcc-patches/src.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH RFC v1 1/1] gcc: fix problem with detecting SSP under uclibc-ng

[Alexey Brodkin]

Hi Brendan,

On Thu, 2015-09-17 at 18:29 +0100, Brendan Heading wrote:
The problem is seen with uclibc-ng 1.0.6 (and likely greater) where they
> > > updated the __GLIBC_MINOR__ value to 10. It will be seen in any libc
> > > that permits stack protection to be disabled while exporting a glibc
> > > version >= 2.4.
> > 
> > Cc'ing Waldemar here. It's an interesting consequence of bumping the
> > GLIBC_MINOR version exposed by uClibc, which happened recently to solve
> > an eventfd_read() problem in Boost on ARC, investigated by Alexey (also
> > in Cc).
> 
> Yup, that's exactly what has exposed the problem. And the "problem" is
> really a deficiency in the way GCC detects SSP support in the libc.

Well I expected new issues to show up after that change but I'm surprised
it's the first one reported - so IMHO we're dealing quite well.

As for the nature of that new problem I'd say it's also expected.
Because I see people use glibc version to determine which feature
or set of features could be used. And IMHO that's not very correct
especially compared to checking each particular feature via compilation.

And frankly I was not very happy with that blind bump of GLIBC_MINOR
from one magical version (I was not able to find any justification why
it was the value it was) to another not justified also... just in attempt
to pretend we're now covering more things in uClibc. But it was low
hanging fruit and so we decided to go that way instead of fixing
affected projects... which in its turn could be not that easy as
patch submission in either uClibc or Buildroot :)

> __stack_chk symbols. Would be interested in hearing from Alexey and
> Waldemar

I think I'll vote for GCC patch as well. So there will be a check for
__UCLIBC_HAS_SSP__ regardless that GLIBC_MINOR value.

-Alexey