Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let builders provide customized CFLAGS for use by the runtime build #35727

Closed
omajid opened this issue May 1, 2020 · 2 comments · Fixed by #39191
Closed

Let builders provide customized CFLAGS for use by the runtime build #35727

omajid opened this issue May 1, 2020 · 2 comments · Fixed by #39191

Comments

@omajid
Copy link
Member

omajid commented May 1, 2020

TLDR

Currently, anyone building runtime can't easily and correctly add to the C compiler flags (CFLAGS) used by runtime. This is required by Linux distributions. A naive workaround results in various side-effects and can produce broken .NET Core builds.

The Full Version

We are trying to work on getting .NET Core integrated into various Linux distributions. Some of them have a policy about using "standard" compiler flags (often via the CFLAGS environment variable). That, along with how runtime builds, makes it difficult to produce known-to-work builds of .NET Core.

Linux Distribution policies

Many Linux distributions have a packaging policy/suggestion/requirement to produce "secure" builds by using the distributions' standard CFLAGS/CXXFLAGS.

Debian has a Release goal for supporting standard (security) flags :

This goal is to update as many packages as possible to use security hardening build flags via dpkg-buildflags. These flags enable various protections against security issues such as stack smashing, predictable locations of values in memory, etc.

Even more information here: https://wiki.debian.org/Hardening. This also applies to Ubuntu, since it follows Debian's packaging rules. In fact, Ubuntu recommends you get your package into Debian first.

Fedora says:

Compilers used to build packages must honor the applicable compiler flags set in the system rpm configuration. Honoring means that the contents of that variable is used as the basis of the flags actually used by the compiler during the package build.

The Fedora compiler flags include -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong but also -Wall.

Gentoo says:

In Gentoo-based systems, set the CFLAGS and CXXFLAGS variables in /etc/portage/make.conf. Variables set in this file will be exported to the environment of programs invoked by portage such that all packages will be compiled using these options as a base.

Mixing this with .NET Core

Over in Fedora Linux, we have been building/packaging .NET Core, including coreclr and corefx (now libraries) from source. Fedora, as mentioned above, requires that we build .NET Core with distribution-standard compiler options.

The normal way to do that is by exporting environment variables like CFLAGS and CXXFLAGS. Fedora provides a bunch of values that we can use . But this doesn't play very nice with the liberal use of -Werror in corefx. Worse, the flags leak into various configure tests . For example HAVE_IN_PKTINFO would fail due to an unrelated warning. I use this to work around this sort of test: https://gist.github.com/omajid/2e31ae5262256c275716d9c374dabe20.

What makes this even worse is Unfortunately, the configure tests are quite spread out. So it's possible for me to miss saving/setting/restoring CFLAGS in a custom patch. I have had configure tests that conclude that the system is different than what it really is just because of a CFLAGS confusion :

check_c_source_compiles(
"
#include <string.h>
int main(void)
{
char buffer[1];
char c = *strerror_r(0, buffer, 0);
return 0;
}
"
HAVE_GNU_STRERROR_R)

Fedora would set CFLAGS to a value that included -Wall and this configure check would fail to compile code (because of unused variable, if I recall correctly) and decide HAVE_GNU_STRERROR_R is false.

See https://pagure.io/dotnet-sig/dotnet-2-1/c/29673bc6dc37f205890fa1090e58d757bd262afb?branch=master for a hacky workaround.

Some common/possible solutions

  1. Generally, this problem is avoided entirely by most applications/libraries using autotools and cmake because the Linux distributions let us split out the configure/cmake call (the "configuration phase") from the final make call (the actual "build phase"). It's common to not touch CFLAGS and friends for the configuration phase and only set it to the distribution's required values during the build phase. Unfortunately this doesn't work for runtime. Both cmake and make are called indirectly by a single build script ./build.sh. This makes it difficult for builders to override CFLAGS just for the make call.

  2. Another common solution is to provide some way way for users to provide CFLAGS, maybe through a flag like --with-cflags, and this value is used to initialize the environment variable CFLAGS only for invoking make (not for cmake).

  3. We could accept some special environment variables, and use that to append to CFLAGS, but only for invoking make (not for cmake).

This was originally opened as dotnet/source-build#745, but I think this is mostly a problem in the runtime code.

@omajid
Copy link
Member Author

omajid commented May 1, 2020

cc @tmds @RheaAyase

@omajid
Copy link
Member Author

omajid commented Jul 10, 2020

It seems like option 3, setting CFLAGS/CXXFLAGS/LDFLAGS just before calling cmake --build (or equivalent) is sufficient: https://gist.github.com/omajid/3033f0d77c4e6d58fd2fffb0da685c12#file-cflags-patch

@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Jul 17, 2020
@jashook jashook removed the untriaged New issue has not been triaged by the area owner label Jul 17, 2020
@jashook jashook modified the milestones: 6.0.0, 5.0.0 Jul 17, 2020
omajid added a commit to omajid/dotnet-runtime that referenced this issue Aug 4, 2020
Many Linux distributions like to use an extra set of compiler flags (via
`CFLAGS`, `CXXFLAGS` and `LDFLAGS`) to produce builds that are hardened
against vulnerabilities and exploits. The flags sometimes also enable
extra warnings to inform packagers about potential memory issues.

This pach adds support for that to dotnet/runtime.

The obvious method to make this work is to just export the `CFLAGS`,
`CXXFLAGS`, and `LDFLAGS` directly. This, however, enables those flags
during configure-time (aka `cmake` without `--build` too). That means
several cmake configure tests get executed with unexpected compiler
flags. These configure tests can then report incorrect results.

For example, https://bugzilla.redhat.com/show_bug.cgi?id=1712158
demonstrates an issue where the check for `strerror_r` in the runtime
comes to the wrong conclusion because `-Wall` is enabled and a variable
is unused.

A slightly longer fix is to support another set of environment
variables, and use them to set `CFLAGS`, `CXXFLAGS`, `LDFLAGS`, but only
for the invocation of `cmake --build`/`make`.

See dotnet#35727 for the complete details.

Fixes dotnet#35727
janvorli pushed a commit that referenced this issue Aug 10, 2020
Many Linux distributions like to use an extra set of compiler flags (via
`CFLAGS`, `CXXFLAGS` and `LDFLAGS`) to produce builds that are hardened
against vulnerabilities and exploits. The flags sometimes also enable
extra warnings to inform packagers about potential memory issues.

This pach adds support for that to dotnet/runtime.

The obvious method to make this work is to just export the `CFLAGS`,
`CXXFLAGS`, and `LDFLAGS` directly. This, however, enables those flags
during configure-time (aka `cmake` without `--build` too). That means
several cmake configure tests get executed with unexpected compiler
flags. These configure tests can then report incorrect results.

For example, https://bugzilla.redhat.com/show_bug.cgi?id=1712158
demonstrates an issue where the check for `strerror_r` in the runtime
comes to the wrong conclusion because `-Wall` is enabled and a variable
is unused.

A slightly longer fix is to support another set of environment
variables, and use them to set `CFLAGS`, `CXXFLAGS`, `LDFLAGS`, but only
for the invocation of `cmake --build`/`make`.

See #35727 for the complete details.

Fixes #35727
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants