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

Support more LTOed packages #48

Merged
merged 1 commit into from
Oct 20, 2017
Merged

Support more LTOed packages #48

merged 1 commit into from
Oct 20, 2017

Conversation

pchome
Copy link
Contributor

@pchome pchome commented Oct 19, 2017

I have all packages built w/ this for now and no noticeable problems so far. The only -flto overrides left is (as have some build errors):

media-libs/alsa-lib
media-libs/flac

and this two (unstable w/ LTO):

net-misc/openssh
media-sound/jack2

Some other locally patched to be built w/ LTO or have additional flags.
So for now I have ~1K packages installed and still no any -ffat-lto-objects flags set.

I have all packages built w/ this for now and no noticeable problems so far. The only `-flto` overrides left is (as have some build errors):
```sh
media-libs/alsa-lib
media-libs/flac
```

and this two (unstable w/ LTO):
```sh
net-misc/openssh
media-sound/jack2
```
Some other locally patched to be built w/ LTO or have additional flags.
So for now I have ~1K packages installed and still no any `-ffat-lto-objects` flags set.
@InBetweenNames
Copy link
Owner

Oh wow, glad to see some upstream discussion about this -- thanks for linking the thread! I understand now why strip causes the problem it sometimes does.

It looks like perhaps a Portage patch is in order to fix this. According to the linked thread, gcc-ranlib needs to be run in order to fix the static libraries. I'm going to check into that to see if I can attach a patch to the bug report. It would be nice to have it handled properly.

@InBetweenNames
Copy link
Owner

InBetweenNames commented Oct 19, 2017

Okay, I was able to patch Portage to do it, but of course the patch will need work.

File /usr/lib64/portage/python3.6/ebuild-helpers/prepstrip (will vary from system to system) line 347:

if ! ${FEATURES_splitdebug} || ${RESTRICT_splitdebug} ; then
    ${STRIP} -g "${x}"
    ${CHOST}-ranlib "${x}" #add this here
fi

This did the trick for me. Obviously there are a few things that need to be addressed:

  • STRIP seems to be defined but RANLIB is not. Not sure where STRIP gets defined by default, but it seems RANLIB should be too, or a way of getting it at least.
  • Different Python versions and architectures
  • Breaks checksums

While that's being worked on, instead of having a blanket nostrip, what if we applied that only to the individual packages that are affected?

@InBetweenNames
Copy link
Owner

Ahh yes, I found where STRIP is defined:

# look up the tools we might be using
for t in STRIP:strip OBJCOPY:objcopy READELF:readelf ; do
        v=${t%:*} # STRIP
        t=${t#*:} # strip
        eval ${v}=\"${!v:-${CHOST}-${t}}\"
        type -P -- ${!v} >/dev/null || eval ${v}=${t}
done

So RANLIB seems to be defined correctly according to that definition.

Of course, this still has the drawback of breaking checksums. Hmm.

@pchome
Copy link
Contributor Author

pchome commented Oct 19, 2017

STRIP_MASK is internal portage env_var AFAIK
and "*.a" mask applied only for static libs (preinstall strip don't applied to this files in other words)

Not-stripped *.a won't hurt your system match cause there is a few packages provided them and I believe final size almost equal to fat objects . So why not to do things more complex and just use this STRIP_MASK variable until objdump (strip is alias(?)) will support plugins?

edit:
/usr/lib64/portage/python3.6/ebuild-helpers/prepstrip:308

        if ! ${SKIP_STRIP} ; then
                # The noglob funk is to support STRIP_MASK="/*/booga" and to keep
                #  the for loop from expanding the globs.
                # The eval echo is to support STRIP_MASK="/*/{booga,bar}" sex.
                set -o noglob
                strip_this=true
                for m in $(eval echo ${STRIP_MASK}) ; do
                        [[ /${x#${ED}} == ${m} ]] && strip_this=false && break
                done
                set +o noglob
        else
                strip_this=false
        fi

@pchome
Copy link
Contributor Author

pchome commented Oct 19, 2017

what if we applied that only to the individual packages that are affected?

Sure you can use pkg/name STRIP_MASK="*.a" or so for individual packages in your packages.cflags but you'll face build problems caused by packages not in the list and other packages will be false-positive reported as -fno-lto ones. I know what I'm talking about cause I've done this before: #32 (comment) 😉 .

@pchome
Copy link
Contributor Author

pchome commented Oct 19, 2017

One more example to be clear:
$ equery f binutils-libs | grep "\.a$"

/usr/lib32/libiberty.a
/usr/lib64/libiberty.a

but you have binutils-libs marked as knownworking so every package using -liberty flag will fail to build (sys-devel/prelink in my case) and probably will be reported as -fno-lto.

But it builds fine for me w/ LTOed and nostripped libiberty.a and it takes time to understand why.
So I rejected idea to add something like nostrip into my config and decided to set STRIP_MASK="*.a" globally to avoid similar problems in future.

@InBetweenNames
Copy link
Owner

Wow, thanks for the in-depth information @pchome ! I'm sufficiently convinced now that we should go ahead with your approach on this. It really does seem like this workaround is required until strip has LTO support, just as you said. I will merge this and update the README.md to reflect this information shortly.

@InBetweenNames InBetweenNames merged commit fd287f0 into InBetweenNames:master Oct 20, 2017
@pchome pchome deleted the patch-3 branch October 20, 2017 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants