-
Notifications
You must be signed in to change notification settings - Fork 96
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
Get rid on strip-flags/replace-flags for some packages #57
Comments
Ok, I found an variable in if [[ -z ${_FLAG_O_MATIC_ECLASS} ]]; then
_FLAG_O_MATIC_ECLASS=1
... So something like Trick with exporting variable in package.cflags also works: |
I share your sentiments about these ebuild functions. I was actually just experimenting with GCC and Firefox, and I managed to build GCC with -O3, LTO, and PGO without issue. No codegen problems discovered so far, and I rebuilt Qt + KDE. Firefox needs more testing, as I am getting crashes--however these could be nailed down to a particular problematic set of flags (I recall I think it would be super nice to perhaps extend package.cflags with a I like the idea you propose with the
If you wanted to be daring and have it apply to everything, I think this would work:
However this would give us the power to vet individual ebuilds. I'll still try to get the changes upstreamed, of course, but at least no one would have to wait to receive the benefits (provided they use GentooLTO). I'll leave this issue open for more discussion -- if there are better ways of accomplishing the above I'm all ears. |
Add an experimental USE flag to override strip-flags and replace-flags in flag-o-matic. Address #57 Signed-off-by: Shane Peelar <[email protected]>
I've put in the basics for this as a |
I spoke too soon. I ended up adding overrides for Next up will be a controllable mechanism for this. Ideally, something that could be added in |
Add a control mechanism to re-enable flag-o-matic on a per-package basis. This may be made more granular if needed. For now, it simply enables or disables the functions that modify `*FLAGS`. Add firefox as an example that needs this workaround. Address #57 Signed-off-by: Shane Peelar <[email protected]>
For now, I've added a simple flag, |
I tested out a bunch of packages and found no problems with most, and added the exceptions as usual. I now have |
Looking into this I saw that python ebuilds had the following line: This is perplexing to me and I'm strugging to find any context that lead for the line to be included in the first place. It dates back several years, back from when the portage tree was still versioned under CVS. Do we know if there's a reason not to apply |
My bad. I looked things up and apparently managed to miss the obvious comment in the ebuild itself. Sorry for the noise. Context is here: https://bugs.gentoo.org/50309 TL;DR: 2004-2005, stack smashing protection didn't play well with |
Nice catch! As far as I know, the only reason -O3 hasn't been used on a hardened system is because it exposes undefined behaviour in C and C++ programs more than -O2 does, which could lead to a security problem. But the fault there isn't really with -O3, it's with the code that is compiled using it. |
Did some more looking at |
I'll try to remember to kick off a world rebuild before bed.
…On Mon, Oct 29, 2018, 20:23 Shane Peelar ***@***.*** wrote:
Did some more looking at flag-o-matic.eclass. So it looks like these four
functions are the only ones we should override, as append-c[xx]flags
appear to do meaningful things, as well as append-ldflags. Now it's just
a matter of testing this out and seeing if this is something we can enable
by default after we get a few confirmed world rebuilds working properly.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#57 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB74jyPgKZ_oCjNsoEBuk7Qda2fxnPtks5up8YUgaJpZM4QFVdj>
.
|
world rebuild succeeded, with the exception of inkscape, which I feel like has a race condition generating this header file? I saw this failure before at some point: ege-color-prof-tracker.cpp:54:10: fatal error: helper/sp-marshal.h: No such file or directory |
Strange! I have inkscape installed without issue. Could you post your |
I just removed all my custom flag stripping and kicked off another world rebuild. I'll open a new issue for inkscape if it fails again, but it looks like the failure got cleaned up by portage already. |
It seems if glibc is built with "custom-cflags", then machine_name during the gcc fixincludes fails resulting in all "fixes" being applied. I'm not sure how or why yet, or whether it's specific to flags I have set. |
Could you post a |
I'll try to reproduce it. |
Glibc 2.27-r6 builds with current ltoize default CFLAGS and |
@InBetweenNames Yeah, append-flags() should be allowed more leniency rather than straight up override. Often it's used to add things like -fno-strict-aliasing -pthread -fopenmp -fPIC -DSOMETHING and other things like that (I've run a grep on *.ebuild to get the idea). But then it sometimes add -O2 which supersede -O3, which is what we want stopped (overriding -O3 is truly rarely needed anymore, these issues did exist in the past but not so much anymore) I'm no bash expert so it could be written better I'm sure, but (for example) I've been using this in my bashrc (I don't use bashrc-mv): if declare -F append-flags >/dev/null; then
if ! declare -F "orig_append-flags" >/dev/null; then
eval "orig_$(declare -f append-flags)"
fi
append-flags() {
local -a argv
local -i argc=0
while [ $# -gt 0 ]; do
case "$1" in
-O?) ewarn "append-flags(): prevented addition of $1";;
*) argv[argc++]="$1";;
esac
shift
done
if [ $argc != 0 ]; then
ewarn "append-flags(): allowing ${argv[@]}"
orig_append-flags "${argv[@]}"
fi
}
fi Which just calls the original function without arguments we don't want (done this way it also won't mess up spaces in single arguments), more could be added. It's not needed here but I do a similar thing with other functions like The declare -F checks are needed in case flag-o-matic isn't loaded or if bashrc is sourced multiple times (it happens), in which case it does nothing and avoid errors. Unlike other functions I wrap, technically append-flags() is simple and doesn't need its original copied, but I also wrap more complex functions and generally will prevent having to keep up with changes in those functions on portage's side. Edit: oh yeah, should replace orig_ by something like LTOcopy_ to protect the namespace (the only thing that is set non-locally. Portage doesn't use orig_... but who knows. |
@jelinekto I have a belief that -fipa-pta is bad on glibc and the one causing issues. It builds fine but I had a few unexpected behavior (while building gcc in my case) that go away when I remove -fipa-pta on glibc. This was on glibc 2.28 though. Edit: Oh right, in your case, it could also be related to append-flags() preventing addition of -fno-strict-aliasing (in my case I'm already allowing this to be added, see above post) |
If it can help, thought I'd share my painless experience with overriding flag-o-matic system-wide... I've built a system almost without issues all the way up to firefox/wine-staging and everything else I personally use with flag-o-matic basic overrides (replace/filter/strip) on everything. append-flags() is setup the way mentioned in above posts. Nothing seems wrong ""yet"" (this is just a test system built in QEMU). I do need to check for other flag-o-matic functions though, I hadn't took care of those yet and used the overrides here as reference. That aside there's some differences with my setup:
This is my own thing but gold only needed a workaround for berkeley db (only because I don't let the ebuild override gold, which then proceed to use a bfd-supported-only option), even glibc which used to be notorious for failing with it is fine (the ebuild attempts to override it, but I blocked that) In case of firefox, did remove the normally-stripped Similar deal with ffmpeg, should remove I've also let firefox build with Kernel was also built with the same CFLAGS+gold minus I use xorg+xfce4 and no issues there, don't ask me about wayland/gnome/kde/qt. On a side-note, looking at current workarounds in the overlay that I built the package for:
For the rest I've had similar problems and had to do the same workarounds, not that I've build every packages mentioned in the ltoworkarounds.conf file. Now I just need to mess with the system and see if anything strange, so far so good. |
@ionenwks Interesting, I just tried building both 2.27 and 2.28 with -fno-strict-aliasing enabled, -fipa-pta disabled and I keep getting the same symbol lookup error. Might be something wrong specifically with my setup. |
@jelinekto Hrmm, I thought it may be a locale difference (since I don't use ISO8859-1 and only have UTF-8 locales generated), so I made one, set it and started bash but no problems still. Afraid I don't know what's causing it. |
Wow, phenomenal work and writeup! @ionenwks I agree -- we should be smarter about how we override append-flags. For now, we can enable exclusions on a per-package basis with @jelinekto Even with |
Seems to be the same for me, no |
I just checked on my Debian system and there's no gconv_end there either, and Debian should be as normal as it gets. So I'm not sure why it's looking for it. |
Hah, it actually just happened to me! It turns out it's |
@InBetweenNames Thanks, the workaround solves the problem. |
@InBetweenNames It may not be that surprising, it mostly affect shared library code (libc sure is one) and it's supposed to allow to break some standards with ELF semantics. Personally was already not using that one (as mentioned above), so guess not surprising it works for me. |
What I find interesting is that clang actually uses |
Yeah at the low level I wouldn't trust anything that breaks standards. I do want to throw -ffast-math on everything anyway though. |
I'm wondering if we should perhaps have a couple of GentooLTO defaults. A "plain" default that is just Perhaps something like:
|
^ I do think that would be good, especially if the number of flags to pick from keep increasing. It'd be less confusing to have presets. When flags (and stripping prevention of) are known to work for most people along with needed workarounds, they could be moved to a default preset from experimental. Something like "strict" should probably stay more limited though, or at least in the don't-break-standards sense. On a related note, it would be handy to have a way to disable workarounds on-the-fly without editing files. These need to be re-tested regularly to know if still needed or it'll just become a bunch of outdated stripping flags like those that remain in ebuilds. There's also the issue of which version of packages people are using (mostly just stable vs ~testing). Stable packages will typically need more workarounds (when it comes to optimizations that is) and may need a separate file, unless supporting only one of the two (given glibc 2.28 currently gets recommend in the Edit: Speaking of To go back on this:
Guess it's still no good, don't do it. It works on the surface but the console is outputting additional errors with I do wonder which optimization it dislikes from Edit: on a new side-note, don't use -fno-plt on xorg-server, its reliance on lazy bindings seems to interfere and breaks modules |
I'm confused about the whole overriding flagomatic ordeal, what and where should I put to actually allow firefox to build firefox with lto [with gcc],? |
There's no need to override anything for firefox, flagomatic should be left to work as intended and strip For gcc+LTO firefox, you only need to set the |
@ionenwks thanks. Compiled it with pgo, O3, lto and it works [65.0.2], previously it would segfault, not sure if PGO is actually working, as I'm not seeing it running for profiling, possibly instantly closes, seen it before. Plus it's 15-20% slower from averaged test than clang 8 /lto/pgo/O3 build. Even more so in graphical benchmarks. Guess I'm saying gbye to building browsers with gcc. |
@barolo I'd advise against That aside, you may not have used And yeah, firefox is putting all optimizations efforts geared toward clang (clang+LTO+PGO is the default build for releases) and barely support/test gcc at all anymore and only an old version. But did you do those benchmarks yourself? clang+LTO+PGO builds tend to get compared to a non-lto non-pgo gcc build in benchmarks given that was broken for a while, not that I'd be surprised if it's faster anyway given the lack of effort with gcc. And (if you meant seeing it visually) you wouldn't see it run for profiling, not only does firefox have a headless mode now, but gentoo even forces you to install xvfb (it used to but not sure if xvfb still get used for profiling, I think it uses the new headless mode). Not that I'm 100% sure it's working as intended myself, especially with gcc, given that doesn't really get tested upstream. |
@ionenwks about profiling, what I meant is that I was watching console's output in the background , you can see when it runs tests in headless mode, it doesn't seem to happen in case of gcc. |
@barolo I see, maybe I'll switch back to clang as well then. I used it for a while but when PGO started to not segfault with gcc again I went back to it. I do have my doubts as to whether profiling is being used correctly anyway, especially when gcc sometimes continue silently without optimizing if there's a problem with the profile. Shame browsers are such a pain to experiment with, both huge code base with convoluted build systems and sensitive to optimizations, makes me not do the tests I should just be doing. |
@ionenwks My conclusion is that if you're building ffox with GCC for a speed gain you're wasting your time, since it's no faster than upstream binary builds which are lto/pgoed , at least in the current state of things. It's even worse with chromium [ which is even more fine-tuned for clang ] I've been reading Honza Hubicka's blog and apparently there are fixes/discussions about bringing GCC builds up to par, we'll see where it ends. He also outlines where are the issues |
There is a lot of packages w/ strip-flags/replace-flags/filter-flags/... which was added years ago and probably can be dropped in now days.
For example you can get a list to analyse via
$ grep -P -A1 -B1 "(replace-flags|strip-flags|filter-flags)" /var/db/pkg/*/*/*
command. many of them useful butreplace-flags -O3 -O2
andstrip-flags
can be retested.So i wonder can we somehow override them. Maybe via package.cflags somehow or custom bashrc.d script or maybe custom eclass. Maybe there is a way to add overlay's internal use-flag like custom-cflags and let users to chose which packages this internal stuff will affect.
Other variants is to modify ebuilds itself or ask upstream to add custom-cflags or remove
*-flags
.So far I tested
x11-libs/gtk+-2.24.31-r1
,net-im/pidgin-2.12.0
,media-libs/libsdl2-2.0.7
,app-editors/vim-8.0.1188
andapp-editors/vim-core-8.0.1188
with removed flag modifiers. And everything work w/o any issues for me. Moreover despite all this scary thing was written in 2005 ( https://bugs.gentoo.org/76331 ) vim works noticeable faster, I have more than dozen plugins installed and startup was slow for editor but now startup and navigation as fast as mcedit/mcview for example.Looks like vim is one of the packages where -O3/graphite stuff really helps.
The text was updated successfully, but these errors were encountered: