Page MenuHomeSolus

Inappropriate gcc default optimization level
Closed, WontfixPublic

Description

Currently gcc is intentionally patched to alter the default optimization level from -O0 to -O1. This is done in package.yml at the line '%patch -p1 < $pkgfiles/optimize-at-least-some.patch'. A comment in this file indicates the purpose as 'without at least -O1 optimizations, many security checks just do not work so default to at least O1'. Without knowing the security checks that require this setting, it certainly seems a worthy goal. However, this does now mean that gcc behaves differently to what is indicated in the man page supplied with the distribution as well as the typical behavior in most other distributions and the on-line gcc documentation (including no doubt countless programming tutorial pages).

This means that unless users explicitly set -O0 for debug builds, certain optimizations that make debugging difficult (such as elimination of variables) will occur. Depending on the build system in use, and the users expectations, there is the potential for this to be very difficult to diagnose and correct. Bearing in mind that when working on large open source projects it is normal to just follow the build instructions provided and not attempt to understand the intricacies of the build system in use.

For example, if a user is working on a project using a build system based on cmake, then the expected command to generate a debug build would include the option '-DCMAKE_BUILD_TYPE=Debug'. Unless the CMakeLists.txt file includes custom debug options, this will result in only the gcc option '-g' being emitted. With the default optimization changed to -O1, unnecessary variables will be optimized out, leading to a sub-optimal debug experience. To find out the reason for this undesirable behavior the user will probably inspect the compile commands output by cmake and will check with the gcc man page or on-line documentation to see what optimization should result - they will be misled. No matter how deeply they inspect the complicated network of makefiles generated by cmake they will not find the cause, which will only be found by explicitly setting -O0.

The solution is to remove the patch and restore the expected behavior of gcc. If, for the purpose of security checks, other optimization levels are required for package code, then this should be set explicitly in the build system for the respective packages not in gcc itself where it will affect all subsequent software build within Solus.

twelsby created this task.May 27 2018, 8:22 AM

So, this isn't a security issue in the slightest tbh. And many features like FORTIFY only work with an optimisation level set, regardless of what the manpage leads you to believe. The patch is there due to inconsistent build systems that remove or ignore flags entirely, for this very reason, to ensure builds are consistent and secure. Likewise, -g2 is set as the default for proper debug symbols and we build separate dbginfo packages, which have never presented an issue before.

Note that DCE is post applied and will not degrade debugging experience at all. Sticking to default -g vs -g2 will lead to a degraded debugging experience, so I recommend altering the CFLAGS/CXXFLAGS that you pass.
DCE doesn't affect the -g2 source lines and gdb can still step through correctly.

Typically for normal builds you actually want to try and match the distribution defaults of -O2 -g2 which leads to working debug information and working security features (like FORTIFY SOURCE for glibc based builds)

I see no further action required for this bug but if you disagree please feel free to say why.

No this is not a security issue in my opinion (although, as stated the reason given for the patching gcc is apparently security related). I apologise if I have inadvertently characterised this as a security issue. I should also point out that I consider this a low priority issue as there are simple workarounds, once the user is aware of the true nature of the problem.

My argument is not that software or projects that use tools like FORTIFY should not be compiled with optimization, and in any event I lack the necessary knowledge to meaningfully contribute to any such discussion. My argument is that this outcome should not be achieved by modifying the default behaviour of gcc, because any time you modify the default behaviour of a tool (especially a foundation tool such as gcc) you run the risk of breaking a whole heap of dependant software and causing all sorts of headaches for your users.

I feel fairly confident in stating that very little software is written under Solus, except for that which is part of the Solus project itself, and most of that is likely derived from upstream software that was not developed under Solus. So the build systems have been set up expecting the normal behaviour of gcc and probably most of the developers working on a particular project are also using some other distribution or OS. A new user of Solus working on such a project would expect their development experience will be the same as someone using another distribution (and indeed their own experience under another distribution). They will not expect to need to modify the build system to deal with undocumented changes to the build tools on their distribution. Then there is the issue that if they do make changes to the build system then they have to somehow prevent those changes from being included in any commits that they make. Your suggestion of altering CFLAGS/CXXFLAGS would be such a change, but -g2 is already the default I believe.

The new defaults certainly do affect the debugging experience. The particular case that drew this to my attention a small project I created to independently test an algorithm, for which I was using a minimal cmake configuration. The algorithm included a recursive function, the main path of which, among other things, incremented an iterator argument. The debug build, using cmake defaults, optimized out the iterator argument and altered the order of execution of statements. As I desired to step through this function and inspect the iterator, this was certainly inconvenient. In my case my test project was not part of a body of code that I needed to share with others, so once I worked out the problem (several hours work as I was operating under the assumption that the man pages are always accurate) I could simply change the cmake file to include the appropriate flags.

Now, as I said, I accept your reasoning regarding the value of using -02 when using other features that depend on this optimisation level, however these settings should be applied to the particular projects that require them. If it is felt that this should become the new default for gcc, then make that argument upstream so that everyone, whether on Solus or some other distribution, is on the same page. If there is a problem with the build system for a particular project then fix it in the project, either in Solus or upstream. Changing gcc to fix bugs in projects built with it is like using a nuclear bomb to dig a hole - sure you will get a hole, but there's going to be some undesirable side effects.

Ultimately this is your decision to make but I urge you to give very careful consideration to the negative consequences of altering the default behaviour of foundation tools. At the very least, if this change is to persist then the man page for gcc and --help needs to be updated, as well as any other documentation. This needs to be very clearly documented.

For a minimal example that illustrates the problem consider the following:

#include <iostream>
#include <string>

int main()
{
    int i = 0;

    int j = i + 2;

    std::cout << j << std::endl;

    int k = std::stoi("10");

    std::cout << k << std::endl;

    return j;
}

When compiled under Solus with g++ -g main.cpp the variables i, j and k are all optimised out, however when compiled with g++ -g main.cpp -O0 they are not. Under Ubuntu 18.04 with the same version of gcc/g++ (7.3.0) then the command g++ -g main.cpp does not result in the variables being optimised out. Not being able to easily inspect these variables certainly does affect the debugging experience.

I feel fairly confident in stating that very little software is written under Solus, except for that which is part of the Solus project itself, and most of that is likely derived from upstream software that was not developed under Solus

We're more than capable of writing our own software without requiring upstreams. You should look at our GitHub. Do note the primary purpose of the toolchain provided in Solus is to build Solus itself.

Your argument regarding upstream integration of different defaults is moot in the face of an opinionated project that doesn't always agree with upstream defaults and those having firm, unshifting stances. Also note that upstreams
in these areas are incredibly slow to respond to real issues (for an example check out the glibc patch we have to carry for rtld-audit performance)

Personally I don't really care too much about the patch as it originally comes from Clear Linux (which I used to work on), and we have gradually removed a great number of Clear Linux patches from Solus over the last year because they've
taken the approach of shooting the wings off a fly with a bazooka. So, I'll happily nuke the patch with one exception.

We'll need to put together a very trivial build log scanner to identify common issues from packages ignoring/overriding our CFLAGS + CXXFLAGS + LDFLAGS, so we can automatically fail any build that isn't correctly propagating the
defaults, and this also ensures we have FORTIFY_SOURCE working. Failing that we could just directly scan the resulting binaries and ensure they're RELRO, fPIC, and for glibc builds contain _chk symbols to validate we have FORTIFY_SOURCE working.

To remove the patch without doing that presents a large surface area for regressions.

BTW your example is pretty poor, there is no valid reason to need to debug i and j as they're never referenced, and the main() will always return 2 with even the most basic optimisation. The argument to debug something that is never used makes incredibly little sense, and you should consider forcing the hand of the compiler with static volatile int. Also note you can force local optimisation levels with a #pragma. Poorly written code shouldn't really be a defence.

JoshStrobl closed this task as Wontfix.Jul 20 2018, 10:51 AM
JoshStrobl claimed this task.