CFLAGS overridden by distribution build system

CFLAGS overridden by distribution build system

[Laurent Bigonville]

Hello,

The current build system of the userspace is setting a lot of CFLAGS, 
but most of these are overridden by the distributions when building.

Today I received a bug report[0] from Christian Göttsche asking me to 
set -fno-semantic-interposition again in libsepol. I see also the same 
flag and also a lot of others set in libselinux and libsemanage build 
system.

For what I understand some of these are just needed for code quality 
(-W) and could be controlled by distributions but others might actually 
need to be always set (-f?).

Shouldn't the flags that always need to be set (which ones?) be moved to 
a "override CFLAGS" directive to avoid these to be unset by distributions?

Kind regards,

Laurent Bigonville

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=961329

Re: CFLAGS overridden by distribution build system

[William Roberts]

On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
>
> Hello,
>
> The current build system of the userspace is setting a lot of CFLAGS,
> but most of these are overridden by the distributions when building.
>
> Today I received a bug report[0] from Christian Göttsche asking me to
> set -fno-semantic-interposition again in libsepol. I see also the same
> flag and also a lot of others set in libselinux and libsemanage build
> system.

Why would it be again? The old DSO design made that option impotent.
Clang does it by default.

>
> For what I understand some of these are just needed for code quality
> (-W) and could be controlled by distributions but others might actually
> need to be always set (-f?).

If you look at the Makefiles overall in all the projects, you'll see hardening
flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
they all get overridden by build time CFLAGS if you want.

>
> Shouldn't the flags that always need to be set (which ones?) be moved to
> a "override CFLAGS" directive to avoid these to be unset by distributions?

If you we're cleaver with CFLAGS before, you could acually circumvent
the buildtime
DSO stuff. While i'm not opposed to adding it to immutable flag, if
you're setting
CFLAGS, you're on your own. At least with the current design.

I have no issues with adding it to override, but we would have to
overhaul a bunch
of them and make them consistent.

>
> Kind regards,
>
> Laurent Bigonville
>
> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=961329
>

Re: CFLAGS overridden by distribution build system

[Stephen Smalley]

On Sat, May 23, 2020 at 11:59 AM William Roberts
<bill.c.roberts@gmail.com> wrote:
>
> On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> >
> > Hello,
> >
> > The current build system of the userspace is setting a lot of CFLAGS,
> > but most of these are overridden by the distributions when building.
> >
> > Today I received a bug report[0] from Christian Göttsche asking me to
> > set -fno-semantic-interposition again in libsepol. I see also the same
> > flag and also a lot of others set in libselinux and libsemanage build
> > system.
>
> Why would it be again? The old DSO design made that option impotent.
> Clang does it by default.
>
> >
> > For what I understand some of these are just needed for code quality
> > (-W) and could be controlled by distributions but others might actually
> > need to be always set (-f?).
>
> If you look at the Makefiles overall in all the projects, you'll see hardening
> flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> they all get overridden by build time CFLAGS if you want.
>
> >
> > Shouldn't the flags that always need to be set (which ones?) be moved to
> > a "override CFLAGS" directive to avoid these to be unset by distributions?
>
> If you we're cleaver with CFLAGS before, you could acually circumvent
> the buildtime
> DSO stuff. While i'm not opposed to adding it to immutable flag, if
> you're setting
> CFLAGS, you're on your own. At least with the current design.
>
> I have no issues with adding it to override, but we would have to
> overhaul a bunch
> of them and make them consistent.

I'm inclined to agree with Laurent here - we should always set this
flag in order to preserve the same behavior prior to the patches that
removed hidden_def/hidden_proto and switched to relying on this
instead.

Re: CFLAGS overridden by distribution build system

[William Roberts]

On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Sat, May 23, 2020 at 11:59 AM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> >
> > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> > >
> > > Hello,
> > >
> > > The current build system of the userspace is setting a lot of CFLAGS,
> > > but most of these are overridden by the distributions when building.
> > >
> > > Today I received a bug report[0] from Christian Göttsche asking me to
> > > set -fno-semantic-interposition again in libsepol. I see also the same
> > > flag and also a lot of others set in libselinux and libsemanage build
> > > system.
> >
> > Why would it be again? The old DSO design made that option impotent.
> > Clang does it by default.
> >
> > >
> > > For what I understand some of these are just needed for code quality
> > > (-W) and could be controlled by distributions but others might actually
> > > need to be always set (-f?).
> >
> > If you look at the Makefiles overall in all the projects, you'll see hardening
> > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> > they all get overridden by build time CFLAGS if you want.
> >
> > >
> > > Shouldn't the flags that always need to be set (which ones?) be moved to
> > > a "override CFLAGS" directive to avoid these to be unset by distributions?
> >
> > If you we're cleaver with CFLAGS before, you could acually circumvent
> > the buildtime
> > DSO stuff. While i'm not opposed to adding it to immutable flag, if
> > you're setting
> > CFLAGS, you're on your own. At least with the current design.
> >
> > I have no issues with adding it to override, but we would have to
> > overhaul a bunch
> > of them and make them consistent.
>
> I'm inclined to agree with Laurent here - we should always set this
> flag in order to preserve the same behavior prior to the patches that
> removed hidden_def/hidden_proto and switched to relying on this
> instead.

Theirs a lot of features that rely on particular cflags, consider hardening
options. A lot of the warnings/error stuff is not just a code hygiene, but also
spotting potential issues that could arise as security issues. If one starts
tinkering with cflags, you can really change the code quite a bit. This is why
some places only approve sanctioned builds of crypto libraries.

But one of the things to consider is that for clang builds, clang
doesn't support
this option, so we would need to move the compiler checking code into each
projects root makefile and ensure any consuming subordinate leafs add a
default, or move it to the global makefile and make sure each leaf that needs it
has a default.

Re: CFLAGS overridden by distribution build system

[Stephen Smalley]

On Fri, May 29, 2020 at 12:05 PM William Roberts
<bill.c.roberts@gmail.com> wrote:
>
> On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Sat, May 23, 2020 at 11:59 AM William Roberts
> > <bill.c.roberts@gmail.com> wrote:
> > >
> > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> > > >
> > > > Hello,
> > > >
> > > > The current build system of the userspace is setting a lot of CFLAGS,
> > > > but most of these are overridden by the distributions when building.
> > > >
> > > > Today I received a bug report[0] from Christian Göttsche asking me to
> > > > set -fno-semantic-interposition again in libsepol. I see also the same
> > > > flag and also a lot of others set in libselinux and libsemanage build
> > > > system.
> > >
> > > Why would it be again? The old DSO design made that option impotent.
> > > Clang does it by default.
> > >
> > > >
> > > > For what I understand some of these are just needed for code quality
> > > > (-W) and could be controlled by distributions but others might actually
> > > > need to be always set (-f?).
> > >
> > > If you look at the Makefiles overall in all the projects, you'll see hardening
> > > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> > > they all get overridden by build time CFLAGS if you want.
> > >
> > > >
> > > > Shouldn't the flags that always need to be set (which ones?) be moved to
> > > > a "override CFLAGS" directive to avoid these to be unset by distributions?
> > >
> > > If you we're cleaver with CFLAGS before, you could acually circumvent
> > > the buildtime
> > > DSO stuff. While i'm not opposed to adding it to immutable flag, if
> > > you're setting
> > > CFLAGS, you're on your own. At least with the current design.
> > >
> > > I have no issues with adding it to override, but we would have to
> > > overhaul a bunch
> > > of them and make them consistent.
> >
> > I'm inclined to agree with Laurent here - we should always set this
> > flag in order to preserve the same behavior prior to the patches that
> > removed hidden_def/hidden_proto and switched to relying on this
> > instead.
>
> Theirs a lot of features that rely on particular cflags, consider hardening
> options. A lot of the warnings/error stuff is not just a code hygiene, but also
> spotting potential issues that could arise as security issues. If one starts
> tinkering with cflags, you can really change the code quite a bit. This is why
> some places only approve sanctioned builds of crypto libraries.

I think the difference here is that we made a change in the source
code (hidden_def/hidden_proto removal) that requires use of
-fno-semantic-interposition to preserve existing behavior.

> But one of the things to consider is that for clang builds, clang
> doesn't support
> this option, so we would need to move the compiler checking code into each
> projects root makefile and ensure any consuming subordinate leafs add a
> default, or move it to the global makefile and make sure each leaf that needs it
> has a default.

I think clang does support it now? https://reviews.llvm.org/D72724

Re: CFLAGS overridden by distribution build system

[William Roberts]

On Fri, May 29, 2020 at 11:40 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, May 29, 2020 at 12:05 PM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> >
> > On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > On Sat, May 23, 2020 at 11:59 AM William Roberts
> > > <bill.c.roberts@gmail.com> wrote:
> > > >
> > > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > The current build system of the userspace is setting a lot of CFLAGS,
> > > > > but most of these are overridden by the distributions when building.
> > > > >
> > > > > Today I received a bug report[0] from Christian Göttsche asking me to
> > > > > set -fno-semantic-interposition again in libsepol. I see also the same
> > > > > flag and also a lot of others set in libselinux and libsemanage build
> > > > > system.
> > > >
> > > > Why would it be again? The old DSO design made that option impotent.
> > > > Clang does it by default.
> > > >
> > > > >
> > > > > For what I understand some of these are just needed for code quality
> > > > > (-W) and could be controlled by distributions but others might actually
> > > > > need to be always set (-f?).
> > > >
> > > > If you look at the Makefiles overall in all the projects, you'll see hardening
> > > > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> > > > they all get overridden by build time CFLAGS if you want.
> > > >
> > > > >
> > > > > Shouldn't the flags that always need to be set (which ones?) be moved to
> > > > > a "override CFLAGS" directive to avoid these to be unset by distributions?
> > > >
> > > > If you we're cleaver with CFLAGS before, you could acually circumvent
> > > > the buildtime
> > > > DSO stuff. While i'm not opposed to adding it to immutable flag, if
> > > > you're setting
> > > > CFLAGS, you're on your own. At least with the current design.
> > > >
> > > > I have no issues with adding it to override, but we would have to
> > > > overhaul a bunch
> > > > of them and make them consistent.
> > >
> > > I'm inclined to agree with Laurent here - we should always set this
> > > flag in order to preserve the same behavior prior to the patches that
> > > removed hidden_def/hidden_proto and switched to relying on this
> > > instead.
> >
> > Theirs a lot of features that rely on particular cflags, consider hardening
> > options. A lot of the warnings/error stuff is not just a code hygiene, but also
> > spotting potential issues that could arise as security issues. If one starts
> > tinkering with cflags, you can really change the code quite a bit. This is why
> > some places only approve sanctioned builds of crypto libraries.
>
> I think the difference here is that we made a change in the source
> code (hidden_def/hidden_proto removal) that requires use of
> -fno-semantic-interposition to preserve existing behavior.
>
> > But one of the things to consider is that for clang builds, clang
> > doesn't support
> > this option, so we would need to move the compiler checking code into each
> > projects root makefile and ensure any consuming subordinate leafs add a
> > default, or move it to the global makefile and make sure each leaf that needs it
> > has a default.
>
> I think clang does support it now? https://reviews.llvm.org/D72724

Yeah but that bug is all 2020 stuff. It is in the clang-10 release. I
verified that
with a local build from here:
  - https://apt.llvm.org/
So anything sub clang-10 won't support it, do you want to tie us to that
new of a clang?

Re: CFLAGS overridden by distribution build system

[Stephen Smalley]

On Fri, May 29, 2020 at 1:15 PM William Roberts
<bill.c.roberts@gmail.com> wrote:
>
> On Fri, May 29, 2020 at 11:40 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, May 29, 2020 at 12:05 PM William Roberts
> > <bill.c.roberts@gmail.com> wrote:
> > >
> > > On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > >
> > > > On Sat, May 23, 2020 at 11:59 AM William Roberts
> > > > <bill.c.roberts@gmail.com> wrote:
> > > > >
> > > > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > The current build system of the userspace is setting a lot of CFLAGS,
> > > > > > but most of these are overridden by the distributions when building.
> > > > > >
> > > > > > Today I received a bug report[0] from Christian Göttsche asking me to
> > > > > > set -fno-semantic-interposition again in libsepol. I see also the same
> > > > > > flag and also a lot of others set in libselinux and libsemanage build
> > > > > > system.
> > > > >
> > > > > Why would it be again? The old DSO design made that option impotent.
> > > > > Clang does it by default.
> > > > >
> > > > > >
> > > > > > For what I understand some of these are just needed for code quality
> > > > > > (-W) and could be controlled by distributions but others might actually
> > > > > > need to be always set (-f?).
> > > > >
> > > > > If you look at the Makefiles overall in all the projects, you'll see hardening
> > > > > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> > > > > they all get overridden by build time CFLAGS if you want.
> > > > >
> > > > > >
> > > > > > Shouldn't the flags that always need to be set (which ones?) be moved to
> > > > > > a "override CFLAGS" directive to avoid these to be unset by distributions?
> > > > >
> > > > > If you we're cleaver with CFLAGS before, you could acually circumvent
> > > > > the buildtime
> > > > > DSO stuff. While i'm not opposed to adding it to immutable flag, if
> > > > > you're setting
> > > > > CFLAGS, you're on your own. At least with the current design.
> > > > >
> > > > > I have no issues with adding it to override, but we would have to
> > > > > overhaul a bunch
> > > > > of them and make them consistent.
> > > >
> > > > I'm inclined to agree with Laurent here - we should always set this
> > > > flag in order to preserve the same behavior prior to the patches that
> > > > removed hidden_def/hidden_proto and switched to relying on this
> > > > instead.
> > >
> > > Theirs a lot of features that rely on particular cflags, consider hardening
> > > options. A lot of the warnings/error stuff is not just a code hygiene, but also
> > > spotting potential issues that could arise as security issues. If one starts
> > > tinkering with cflags, you can really change the code quite a bit. This is why
> > > some places only approve sanctioned builds of crypto libraries.
> >
> > I think the difference here is that we made a change in the source
> > code (hidden_def/hidden_proto removal) that requires use of
> > -fno-semantic-interposition to preserve existing behavior.
> >
> > > But one of the things to consider is that for clang builds, clang
> > > doesn't support
> > > this option, so we would need to move the compiler checking code into each
> > > projects root makefile and ensure any consuming subordinate leafs add a
> > > default, or move it to the global makefile and make sure each leaf that needs it
> > > has a default.
> >
> > I think clang does support it now? https://reviews.llvm.org/D72724
>
> Yeah but that bug is all 2020 stuff. It is in the clang-10 release. I
> verified that
> with a local build from here:
>   - https://apt.llvm.org/
> So anything sub clang-10 won't support it, do you want to tie us to that
> new of a clang?

No, I guess not.  So I guess our options are to leave it alone and
just make sure it is well documented or complicate the Makefiles.

Re: CFLAGS overridden by distribution build system

[William Roberts]

On Fri, May 29, 2020 at 1:18 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, May 29, 2020 at 1:15 PM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> >
> > On Fri, May 29, 2020 at 11:40 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > On Fri, May 29, 2020 at 12:05 PM William Roberts
> > > <bill.c.roberts@gmail.com> wrote:
> > > >
> > > > On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
> > > > <stephen.smalley.work@gmail.com> wrote:
> > > > >
> > > > > On Sat, May 23, 2020 at 11:59 AM William Roberts
> > > > > <bill.c.roberts@gmail.com> wrote:
> > > > > >
> > > > > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> > > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > The current build system of the userspace is setting a lot of CFLAGS,
> > > > > > > but most of these are overridden by the distributions when building.
> > > > > > >
> > > > > > > Today I received a bug report[0] from Christian Göttsche asking me to
> > > > > > > set -fno-semantic-interposition again in libsepol. I see also the same
> > > > > > > flag and also a lot of others set in libselinux and libsemanage build
> > > > > > > system.
> > > > > >
> > > > > > Why would it be again? The old DSO design made that option impotent.
> > > > > > Clang does it by default.
> > > > > >
> > > > > > >
> > > > > > > For what I understand some of these are just needed for code quality
> > > > > > > (-W) and could be controlled by distributions but others might actually
> > > > > > > need to be always set (-f?).
> > > > > >
> > > > > > If you look at the Makefiles overall in all the projects, you'll see hardening
> > > > > > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> > > > > > they all get overridden by build time CFLAGS if you want.
> > > > > >
> > > > > > >
> > > > > > > Shouldn't the flags that always need to be set (which ones?) be moved to
> > > > > > > a "override CFLAGS" directive to avoid these to be unset by distributions?
> > > > > >
> > > > > > If you we're cleaver with CFLAGS before, you could acually circumvent
> > > > > > the buildtime
> > > > > > DSO stuff. While i'm not opposed to adding it to immutable flag, if
> > > > > > you're setting
> > > > > > CFLAGS, you're on your own. At least with the current design.
> > > > > >
> > > > > > I have no issues with adding it to override, but we would have to
> > > > > > overhaul a bunch
> > > > > > of them and make them consistent.
> > > > >
> > > > > I'm inclined to agree with Laurent here - we should always set this
> > > > > flag in order to preserve the same behavior prior to the patches that
> > > > > removed hidden_def/hidden_proto and switched to relying on this
> > > > > instead.
> > > >
> > > > Theirs a lot of features that rely on particular cflags, consider hardening
> > > > options. A lot of the warnings/error stuff is not just a code hygiene, but also
> > > > spotting potential issues that could arise as security issues. If one starts
> > > > tinkering with cflags, you can really change the code quite a bit. This is why
> > > > some places only approve sanctioned builds of crypto libraries.
> > >
> > > I think the difference here is that we made a change in the source
> > > code (hidden_def/hidden_proto removal) that requires use of
> > > -fno-semantic-interposition to preserve existing behavior.
> > >
> > > > But one of the things to consider is that for clang builds, clang
> > > > doesn't support
> > > > this option, so we would need to move the compiler checking code into each
> > > > projects root makefile and ensure any consuming subordinate leafs add a
> > > > default, or move it to the global makefile and make sure each leaf that needs it
> > > > has a default.
> > >
> > > I think clang does support it now? https://reviews.llvm.org/D72724
> >
> > Yeah but that bug is all 2020 stuff. It is in the clang-10 release. I
> > verified that
> > with a local build from here:
> >   - https://apt.llvm.org/
> > So anything sub clang-10 won't support it, do you want to tie us to that
> > new of a clang?
>
> No, I guess not.  So I guess our options are to leave it alone and
> just make sure it is well documented or complicate the Makefiles.

Pretty much, which is why I go back to, "If your setting CFLAGS, you better
make sure you get em right". There are so many options and so many things
that can affect the build output, you really need to get em right.
This is the one area autoconf/automake is nice (I have a hate/hate
relationship with it),
but it does give one easy-peasy feature test macros, so this problem
is simple to fix.

The problem with documentation is no one reads it :-p. We could do something
like issue a warning, compiler agnostic, from make if we don't detect the
-fsemantic-interposition option (easy). That would be simple, or I can
work up the
gcc/clang split patches (slightly harder).

Or we can just say, if your messing with CFLAGS do it right and you should
probably add -fsemantic-interposition if your using gcc documentation (easiest).

If they didn't change the CFLAGS for their build, they wouldn't have this issue.
Which brings up the question, "can they add the flags they need to the
Makefile?"
or "What do they need added/removed/changed in CFLAGS overall?"

I'd like to start with why they need to change CFLAGS, and can we just
make that the default?

Re: CFLAGS overridden by distribution build system

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 9141 bytes --]

On Fri, May 29, 2020 at 03:59:13PM -0500, William Roberts wrote:
> On Fri, May 29, 2020 at 1:18 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, May 29, 2020 at 1:15 PM William Roberts
> > <bill.c.roberts@gmail.com> wrote:
> > >
> > > On Fri, May 29, 2020 at 11:40 AM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > >
> > > > On Fri, May 29, 2020 at 12:05 PM William Roberts
> > > > <bill.c.roberts@gmail.com> wrote:
> > > > >
> > > > > On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
> > > > > <stephen.smalley.work@gmail.com> wrote:
> > > > > >
> > > > > > On Sat, May 23, 2020 at 11:59 AM William Roberts
> > > > > > <bill.c.roberts@gmail.com> wrote:
> > > > > > >
> > > > > > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> wrote:
> > > > > > > >
> > > > > > > > Hello,
> > > > > > > >
> > > > > > > > The current build system of the userspace is setting a lot of CFLAGS,
> > > > > > > > but most of these are overridden by the distributions when building.
> > > > > > > >
> > > > > > > > Today I received a bug report[0] from Christian Göttsche asking me to
> > > > > > > > set -fno-semantic-interposition again in libsepol. I see also the same
> > > > > > > > flag and also a lot of others set in libselinux and libsemanage build
> > > > > > > > system.
> > > > > > >
> > > > > > > Why would it be again? The old DSO design made that option impotent.
> > > > > > > Clang does it by default.
> > > > > > >
> > > > > > > >
> > > > > > > > For what I understand some of these are just needed for code quality
> > > > > > > > (-W) and could be controlled by distributions but others might actually
> > > > > > > > need to be always set (-f?).
> > > > > > >
> > > > > > > If you look at the Makefiles overall in all the projects, you'll see hardening
> > > > > > > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but
> > > > > > > they all get overridden by build time CFLAGS if you want.
> > > > > > >
> > > > > > > >
> > > > > > > > Shouldn't the flags that always need to be set (which ones?) be moved to
> > > > > > > > a "override CFLAGS" directive to avoid these to be unset by distributions?
> > > > > > >
> > > > > > > If you we're cleaver with CFLAGS before, you could acually circumvent
> > > > > > > the buildtime
> > > > > > > DSO stuff. While i'm not opposed to adding it to immutable flag, if
> > > > > > > you're setting
> > > > > > > CFLAGS, you're on your own. At least with the current design.
> > > > > > >
> > > > > > > I have no issues with adding it to override, but we would have to
> > > > > > > overhaul a bunch
> > > > > > > of them and make them consistent.
> > > > > >
> > > > > > I'm inclined to agree with Laurent here - we should always set this
> > > > > > flag in order to preserve the same behavior prior to the patches that
> > > > > > removed hidden_def/hidden_proto and switched to relying on this
> > > > > > instead.
> > > > >
> > > > > Theirs a lot of features that rely on particular cflags, consider hardening
> > > > > options. A lot of the warnings/error stuff is not just a code hygiene, but also
> > > > > spotting potential issues that could arise as security issues. If one starts
> > > > > tinkering with cflags, you can really change the code quite a bit. This is why
> > > > > some places only approve sanctioned builds of crypto libraries.
> > > >
> > > > I think the difference here is that we made a change in the source
> > > > code (hidden_def/hidden_proto removal) that requires use of
> > > > -fno-semantic-interposition to preserve existing behavior.
> > > >
> > > > > But one of the things to consider is that for clang builds, clang
> > > > > doesn't support
> > > > > this option, so we would need to move the compiler checking code into each
> > > > > projects root makefile and ensure any consuming subordinate leafs add a
> > > > > default, or move it to the global makefile and make sure each leaf that needs it
> > > > > has a default.
> > > >
> > > > I think clang does support it now? https://reviews.llvm.org/D72724
> > >
> > > Yeah but that bug is all 2020 stuff. It is in the clang-10 release. I
> > > verified that
> > > with a local build from here:
> > >   - https://apt.llvm.org/
> > > So anything sub clang-10 won't support it, do you want to tie us to that
> > > new of a clang?
> >
> > No, I guess not.  So I guess our options are to leave it alone and
> > just make sure it is well documented or complicate the Makefiles.
> 
> Pretty much, which is why I go back to, "If your setting CFLAGS, you better
> make sure you get em right". There are so many options and so many things
> that can affect the build output, you really need to get em right.
> This is the one area autoconf/automake is nice (I have a hate/hate
> relationship with it),
> but it does give one easy-peasy feature test macros, so this problem
> is simple to fix.
> 
> The problem with documentation is no one reads it :-p. We could do something
> like issue a warning, compiler agnostic, from make if we don't detect the
> -fsemantic-interposition option (easy). That would be simple, or I can
> work up the
> gcc/clang split patches (slightly harder).
> 
> Or we can just say, if your messing with CFLAGS do it right and you should
> probably add -fsemantic-interposition if your using gcc documentation (easiest).
> 
> If they didn't change the CFLAGS for their build, they wouldn't have this issue.
> Which brings up the question, "can they add the flags they need to the
> Makefile?"
> or "What do they need added/removed/changed in CFLAGS overall?"
> 
> I'd like to start with why they need to change CFLAGS, and can we just
> make that the default?
> 

Fedora packager point of view:

From time to time, Fedora project decides to use new/different flags for
different reasons. In order to make it easier for packagers there are Fedora
distribution macros available which can be used in .spec files and which set all
needed flags, e.g:

$ grep CFLAGS libsepol.spec  
make %{?_smp_mflags} CFLAGS="%{optflags}" LDFLAGS="%{?__global_ldflags}"

$ rpm --eval 'make %{?_smp_mflags} CFLAGS="%{optflags}" LDFLAGS="%{?__global_ldflags}"'           
make -j4 CFLAGS="-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" LDFLAGS="-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld"	


There's a simpler way using %set_build_flags in .spec file:

$ rpm --eval '%set_build_flags'                                                                       
  CFLAGS="${CFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -I/usr/lib64/gfortran/modules}" ; export FFLAGS ; 
  FCFLAGS="${FCFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -I/usr/lib64/gfortran/modules}" ; export FCFLAGS ; 
  LDFLAGS="${LDFLAGS:--Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld}" ; export LDFLAGS ; 
  LT_SYS_LIBRARY_PATH="${LT_SYS_LIBRARY_PATH:-/usr/lib64:}" ; export LT_SYS_LIBRARY_PATH


From this POV it would be great to have -fno-semantic-interposition in CFLAGS
override in libsepol Makefile.

But given the compatibility problem described in this thread, I, as Fedora
packager, would consider as sufficient to have this requirement in the build instructions in
documentation, and also as part of new release notes as it's changed in the latest release.


Petr


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] README: start a section for documenting CFLAGS

[bill.c.roberts]

From: William Roberts <william.c.roberts@intel.com>

Start a section in the README for documenting that custom CFLAGS yeilds
CUSTOM results and that your mileage may vary. The first CFLAG to
document that you likely want to include is -fsemantic-interposition.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
 README.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/README.md b/README.md
index 9d64f0b5cf90..f37a9f91f51e 100644
--- a/README.md
+++ b/README.md
@@ -120,6 +120,17 @@ lacks library functions or other dependencies relied upon by your
 distribution.  If it breaks, you get to keep both pieces.
 
 
+## Setting CFLAGS
+
+Setting CFLAGS during the make process will cause the omission of many default CFLAGS. While the project strives
+to provide a sane set of default CFLAGS, custom CFLAGS could break the build, or have other undesired changes
+on the build output. Thus, be very careful when setting CFLAGS. CFLAGS that we encourage to be set when
+overriding CFLAGS are:
+
+- -fsemantic-interposition for gcc or compilers that do not do this. clang does this by default. clang-10 and up
+   will support passing this flag, but ignore it. Previous clang versions fail.
+
+
 macOS
 -----
 
-- 
2.17.1

Re: [PATCH] README: start a section for documenting CFLAGS

[Stephen Smalley]

On Tue, Jun 2, 2020 at 2:55 PM <bill.c.roberts@gmail.com> wrote:
>
> From: William Roberts <william.c.roberts@intel.com>
>
> Start a section in the README for documenting that custom CFLAGS yeilds
> CUSTOM results and that your mileage may vary. The first CFLAG to
> document that you likely want to include is -fsemantic-interposition.

Spelling error (yields), capitalization (CUSTOM), and it should be
-fno-semantic-interposition.

>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
>  README.md | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/README.md b/README.md
> index 9d64f0b5cf90..f37a9f91f51e 100644
> --- a/README.md
> +++ b/README.md
> @@ -120,6 +120,17 @@ lacks library functions or other dependencies relied upon by your
>  distribution.  If it breaks, you get to keep both pieces.
>
>
> +## Setting CFLAGS
> +
> +Setting CFLAGS during the make process will cause the omission of many default CFLAGS. While the project strives
> +to provide a sane set of default CFLAGS, custom CFLAGS could break the build, or have other undesired changes
> +on the build output. Thus, be very careful when setting CFLAGS. CFLAGS that we encourage to be set when
> +overriding CFLAGS are:
> +
> +- -fsemantic-interposition for gcc or compilers that do not do this. clang does this by default. clang-10 and up
> +   will support passing this flag, but ignore it. Previous clang versions fail.
> +
> +

-fno-semantic-interposition.  Also overly repetitive use of CFLAGS above.

[PATCH v2] README: start a section for documenting CFLAGS

[bill.c.roberts]

From: William Roberts <william.c.roberts@intel.com>

Start a section in the README for documenting that custom CFLAGS yields
CUSTOM results and that your mileage may vary. The first CFLAG to
document that you likely want to include is -fsemantic-interposition.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
v2:
  - Fixed commit message spelling of yields.
  - Reduced usages of CFLAGS in documentation.

 README.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/README.md b/README.md
index 9d64f0b5cf90..eb8e170ea1f7 100644
--- a/README.md
+++ b/README.md
@@ -120,6 +120,17 @@ lacks library functions or other dependencies relied upon by your
 distribution.  If it breaks, you get to keep both pieces.
 
 
+## Setting CFLAGS
+
+Setting CFLAGS during the make process will cause the omission of many defaults. While the project strives
+to provide a reasonable set of default flags, custom CFLAGS could break the build, or have other undesired
+changes on the build output. Thus, be very careful when setting CFLAGS. CFLAGS that are encouraged to be
+set when overriding are:
+
+- -fsemantic-interposition for gcc or compilers that do not do this. clang does this by default. clang-10 and up
+   will support passing this flag, but ignore it. Previous clang versions fail.
+
+
 macOS
 -----
 
-- 
2.17.1

Re: [PATCH v2] README: start a section for documenting CFLAGS

[Stephen Smalley]

On Mon, Jun 8, 2020 at 11:37 AM <bill.c.roberts@gmail.com> wrote:
>
> From: William Roberts <william.c.roberts@intel.com>
>
> Start a section in the README for documenting that custom CFLAGS yields
> CUSTOM results and that your mileage may vary. The first CFLAG to
> document that you likely want to include is -fsemantic-interposition.

CUSTOM is all-caps for some reason, and it should be
-fno-semantic-interposition.

> diff --git a/README.md b/README.md
> index 9d64f0b5cf90..eb8e170ea1f7 100644
> --- a/README.md
> +++ b/README.md
> +
> +- -fsemantic-interposition for gcc or compilers that do not do this. clang does this by default. clang-10 and up
> +   will support passing this flag, but ignore it. Previous clang versions fail.

-fno-semantic-interposition

Re: [PATCH v2] README: start a section for documenting CFLAGS

[William Roberts]

On Mon, Jun 8, 2020 at 11:21 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jun 8, 2020 at 11:37 AM <bill.c.roberts@gmail.com> wrote:
> >
> > From: William Roberts <william.c.roberts@intel.com>
> >
> > Start a section in the README for documenting that custom CFLAGS yields
> > CUSTOM results and that your mileage may vary. The first CFLAG to
> > document that you likely want to include is -fsemantic-interposition.
>
> CUSTOM is all-caps for some reason, and it should be

Meant to emphasize CUSTOM RESULTS, but it only really emphasizes
it correctly if RESULTS is also capitalized (which it wasn't).

> -fno-semantic-interposition.

Argh, I forgot to change that, sorry.

>
> > diff --git a/README.md b/README.md
> > index 9d64f0b5cf90..eb8e170ea1f7 100644
> > --- a/README.md
> > +++ b/README.md
> > +
> > +- -fsemantic-interposition for gcc or compilers that do not do this. clang does this by default. clang-10 and up
> > +   will support passing this flag, but ignore it. Previous clang versions fail.
>
> -fno-semantic-interposition

[PATCH v3] README: start a section for documenting CFLAGS

[bill.c.roberts]

From: William Roberts <william.c.roberts@intel.com>

Start a section in the README for documenting that custom CFLAGS yields
custom results and that your mileage may vary. The first CFLAG to
document that you likely want to include is -fno-semantic-interposition.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
v3:
  - lowercased CUSTOM in the commit message.
  - Updated -fsemantic-interposition to -fno-semantic-interposition in
      the commit message and README file.
v2:
  - Fixed commit message spelling of yields.
  - Reduced usages of CFLAGS in documentation.
 README.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/README.md b/README.md
index 9d64f0b5cf90..4d33686da1f8 100644
--- a/README.md
+++ b/README.md
@@ -120,6 +120,17 @@ lacks library functions or other dependencies relied upon by your
 distribution.  If it breaks, you get to keep both pieces.
 
 
+## Setting CFLAGS
+
+Setting CFLAGS during the make process will cause the omission of many defaults. While the project strives
+to provide a reasonable set of default flags, custom CFLAGS could break the build, or have other undesired
+changes on the build output. Thus, be very careful when setting CFLAGS. CFLAGS that are encouraged to be
+set when overriding are:
+
+- -fno-semantic-interposition for gcc or compilers that do not do this. clang does this by default. clang-10 and up
+   will support passing this flag, but ignore it. Previous clang versions fail.
+
+
 macOS
 -----
 
-- 
2.17.1

Re: [PATCH v3] README: start a section for documenting CFLAGS

[Stephen Smalley]

On Mon, Jun 8, 2020 at 6:38 PM <bill.c.roberts@gmail.com> wrote:
>
> From: William Roberts <william.c.roberts@intel.com>
>
> Start a section in the README for documenting that custom CFLAGS yields
> custom results and that your mileage may vary. The first CFLAG to
> document that you likely want to include is -fno-semantic-interposition.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>