As for KDE bug #101542, kde 3.4 has some troubles with symbol hiding provided by -fvisibility=hidden. Those problems aren't limited to amd64 and ppc archs as were the ones with inline visibilities. Those made kasteroids crashing, but still they can have other implications in KDE 3.4. KDE 3.3 is not affected, as it doesn't make use of -fvisibility=hidden. I'll attach two patches (one to kde eclass, the other one to kdelibs) which fixes the problem for me, using the workaround stated in the bur report (changed to make sure that it DOES compile, please note that you can't simply add it to the flags, as assing them drives configure crazy!). Sorry if kdelibs has also a cleanup and the fam useflag adding, but I had those changes there trying to fix the same bug, so I left it there. HTH, Diego
Created attachment 54621 [details, diff] kde-eclass.patch
Created attachment 54622 [details, diff] kdelibs-3.4.0.patch
Just to confirm that compiling kdelibs, libkdegames and kasteroids (3.4.0) with a GCC that had all visibility patches excluded was enough to fix this issue here. I am going to start looking at whether other issues with KDE may be related to this too. AFAIK this bug should affect all archs with a GCC patched with visibility support.
It seems that adding this patch messes up a bit configure checks... i'll try to fix it better.
Created attachment 55153 [details, diff] Eclass patch to *disable* visibility configuration Seems like visibility mess around with other software, too. After a bad day with my knetload because of an added call of it in his module, I thought of this solution. I'm building now kdelibs to test it but I'm pretty sure it's what we need at this point, as seems like visibility support for now is just... crap.
Ok this patch works, but breaks ABI compatibility with the apps and libraries compiled with the visibility stuff (well not everything is involved in this, but there are a few cases in which this can lead to problems, see for example kasteroids which still crashes if you don't recompile kdelibs, libkdegames and kasteroids with this patch applied. OTOH, this should re-enable binary comptibility with kde 3.3 series, which was broke mainly by visibility stuff.
Created attachment 55173 [details, diff] updated patch Forgot to say, that is needed for split ebuild where configure.in doesn't exists in first place.
Is there any news on this from the KDE herd? This is only really a problem for us because we have a GCC tool chain with backported visibility patches... It does cause segfaults in certain applications, such as kasteroids. There is an upstream bug (http://bugs.kde.org/show_bug.cgi?id=101542) on this issue. I think it is important that we fix this - and it looks like the best way is probably to disable visibility stuff on all archs until KDE/Qt 4 where visibility will be properly supported. I would welcome feedback from the KDE herd on this.
I agree, disabling visibility support for everything is a tough decision, but it seems the only possibility here. If others agree, we can patch the eclass as proposed in comment 5.
There're news about this? Is going to be a blocker to make kde 3.4 stable, and it's also a *very* big bug at the moment, as we can't see all the implications of this, and we risk to have bug opened for crashes which are related to this on 3rd party apps which uses the latest admin dir and tries to use visibility.
This is indeed a blocker. What we need is an official announcement and the patch applied a few days later. herd: I'm not the one to write an pidgin-english announcement. Please let this fix soon. */me raises an eyebrow looking especially at Caleb and Dan ;)*
What kind of announcement would you like?
Some information that those who built kde with -fvisibility=hidden already should redo and an explanation why. Or would you take the ugly patch from bugs.kde.org #101542 into account?
I'll look into it this weekend. I'm still on gcc 3.3, so I haven't been paying much attention to the issue.
is -fvisibility=hidden a flag that must be set in CFLAGS, or is it picked up automatically, or other? Should we filter-flags it, and encourage people who compiled KDE with it to recompile? Or just tell people not to use it when compiling KDE?
It's enabled by default on ./configure by kde. If the flag was only passed by users, they are on their own with that :)
> It's enabled by default on ./configure by kde. Maybe I should have had a look at the patch. Frankly, I did not notice any problem yet. :) > If the flag was only passed by users, they are on their own with that :) Nah, bug nice reports float in and I'm not fine with developers not clearly defining the api they want to expose. Caleb: cxxflag, from the man page: -fvisibility=default|internal|hidden|protected Set the default ELF image symbol visibility to the specified option - all symbols will be marked with this unless overrided within the code. Using this feature can very substantially improve linking and load times of shared object libraries, produce more optimised code, provide near-perfect API export and prevent symbol clashes. It is strongly recommended that you use this in any shared objects you distribute. Despite the nomenclature, "default" always means public ie; available to be linked against from outside the shared object. "protected" and "internal" are pretty useless in real-world usage so the only other commonly used option will be "hidden". The default if -fvisibility isn't specified is "default" ie; make every symbol public - this causes the same behaviour as previous versions of GCC. A good explanation of the benefits offered by ensuring ELF symbols have the correct visibility is given by ``How To Write Shared Libraries'' by Ulrich Drepper (which can be found at <http://people.redhat.com/~drepper/>) - however a superior solution made possible by this option to marking things hidden when the default is public is to make the default hidden and mark things public. This is the norm with DLL's on Windows and with -fvisibility=hidden and "_ _attribute_ _ ((visibility("default")))" instead of "_ _declspec(dllexport)" you get almost identical semantics with identical syntax. This is a great boon to those working with cross-platform projects. For those adding visibility support to existing code, you may find #pragma GCC visibility of use. This works by you enclosing the declarations you wish to set visibility for with (for example) #pragma GCC visibility push(hidden) and #pragma GCC visibility pop. These can be nested up to sixteen times. Bear in mind that symbol visibility should be viewed as part of the API interface contract and thus all new code should always specify visibility when it is not the default ie; declarations only for use within the local DSO should always be marked explicitly as hidden as so to avoid PLT indirection overheads - making this abundantly clear also aids readability and self-documentation of the code. Note that due to ISO C++ specification requirements, operator new and operator delete must always be of default visibility.
I agree, we're forced to disable visibility support. Recompiling kde with this patch now...
...As for binary incompatibility (comment 6): how bad is it? If I take a kde compiled with visibility=hidden and recompile some parts without visibility, what breaks that wasn't broken already (like kasteroids)? Since we're adding not removing symbols, just why would things break?
It would break wrt the version with visibility... but it will remain broken.
Finished recompiling, everything seems OK. I'm attaching the patch against kde-meta.eclass (for split ebuilds). I don't understand comment #20... If we commit these patches, and people recompile only part of their systems, what exactly would break that isn't already broken the way kasteroids is? IOW: does the announcement have to say, 'recompile _all_ of KDE 3.4 without running it while half-done'?
Created attachment 57077 [details, diff] Patch with the same effect for kde-meta.eclass
Caleb's away... Everyone else - can we commit this? I'd still like an answer about comment #20. If a mixed system (some packages with hidden visibility, some without) is not a problem, then we don't have to tell people to upgrade everything, just e.g. kdelibs and kasteroids (in that case we could even create new revisions). Otherwise, the announcement would be: Hello KDE users, If you use KDE 3.4 and GCC 3.4/4.0 (~x86, amd64 [what other archs?]), read on. Otherwise, you can ignore this announcement. A bug has been found[link here] in the way KDE 3.4 uses the new GCC 3.4/4.0 hidden visibility feature. There is no complete fix as yet, so we are forced to disable hidden visibility in KDE for now. Everyone who has KDE 3.4 installed, compiled with GCC 3.4/4.0, needs to emerge --sync and then re-emerge kdelibs and all other KDE packages installed. A system with hidden visibility enabled can cause unpredictable behavior of KDE apps, but the only actual such case found to date is a crash in kasteroids. If KDE is working, you don't have to rush to recompile it; you can wait for a comfortable night.
A mixed system isn't more messed up than a all-visibility-enabled system, but it's still probably messed up, we can't really be sure of which extension the problem has.
I think this should be committed ASAP. It is a known issue with our GCC 3.4, and 4.0 once that starts getting used. The best fix is to turn it off, and it seems to work better here. Mixing doesn't cause an issue here either.
Reviewing the comments it seems like everyone's agreed on this. I intend to commit this (patches as attached) and announce tomorrow (have to run now). If anyone else gets to it first please feel free to commit.
We should tell how to do it. emerge ~arts-3.4.0 ~kdelibs-3.4.0 revdep-rebuild -X --soname libkdecore.so.4 should catch everything, if I'm not wrong.
um, bullshit, kde3.3/4 ship the same libkdecore
You're right, we should tell people how to do this... An evil single pipeline: ls -1 /var/db/pkg/kde-base | egrep '*-3.4.0(-r.*)?' | sed -e 's:^:>=kde-base/:g' | xargs emerge -pv Or a shell construct of which I'm a bit more certain: list="" for x in `ls -1 /var/db/pkg/kde-base | egrep '*-3.4.0(-r.*)?'`; do list="$list >=kde-base/$x" done emerge $list -pv There could be a hole in this somewhere though ;-(
Had such a solution first, too, but what about non kde-base/* stuff? We'd need to unpack environment.bz2 for every application built against some kdelibs version and check if $KDEDIR==/usr/kde/3.4, when we go the /var/db/pkg way. Too bad revdep-rebuild doesn't allow to check for the full path. `revdep-rebuild -X --soname /usr/kde/3.4/lib/libkdecore.so.4` would be a lot easier.
This simple change is hardly visible - use a space instead a tab ;) Just need to get it into gentoolkit. --- /usr/bin/revdep-rebuild.orig 2005-05-05 02:25:21.000000000 +0200 +++ /usr/bin/revdep-rebuild 2005-05-05 02:26:47.000000000 +0200 @@ -128,7 +128,12 @@ OK_TEXT="Dynamic linking on your system is consistent" WORKING_TEXT=" consistency" else - SONAME_SEARCH=" $SONAME " + # first case is needed to test against /path/to/foo.so + if [ ${SONAME:0:1} == '/' ] ; then + SONAME_SEARCH=" $SONAME " + else + SONAME_SEARCH=" $SONAME " + fi LLIST=${LIST}_$(echo "$SONAME_SEARCH$SONAME" | md5sum | head -c 8) HEAD_TEXT="using given shared object name" OK_TEXT="There are no dynamic links to $SONAME"
Does the patch need to be applied to both kde-meta.eclass and kde.eclass? I have applied it to both, and in my testing kasteroids stopped segfaulting, and there were no additional issues encountered. I hope we can get this done before KDE 3.4.1, as that would be a good opportunity to coincide with the version bump and get most stuff recompiled anyway.
I applied the patch and recompiled KDE. Besides kasteroids not crashing and disabling keyboard repeat, the download manager icon (kget) has appeared in Konqueror. Everything else seems the same. Nothing new broke.
As soon as these patches are applied, I think there no other showstoppers for marking kde-3.4 stable (now that bug 40698 and bug 89870 have been addressed). Anyway, as Marcus suggested, if we wait a bit and target 3.4.1 for stable (which contains quite a lot of bugfixes anyway) we can avoid making an announcement to make everyone recompile kde. kde-3.4.1 is expected around june 1, and it could go stable about a couple of weeks later, in my opinion.
I think this is the best plan. The patches should be committed ASAP though in my opinion, as during all my testing I haven't found any problems with mixed binaries (some compiled with, and some without the patched eclasses). So the sooner we apply it, the quicker packages will be recompiled in the course of normal upgrades. Recompiling just kdelibs and kasteroids is enough to fix the most obvious bug produced. Much of x86 on 3.3.* compilers are also unaffected by this issue.
Re: comment #32: yes, we need to patch both eclasses. Re: waiting for 3.4.1: 3.4.1 will be tagged on the 23rd, so released on the 30th at the earliest. That's a lot of waiting. I vote for patching and releasing the announcement, because: - People running stable x86 have gcc 3.3.x, without visibility support and there's no point in making them wait for kde 3.4 even longer. - People with gcc 3.4.x might end up with mixed systems, but there's no evidence that's any worse than a system built entirely with visibility support, so we won't be making matters any worse. And if anyone complains we can just tell them to recompile, which they'd have to do anyway at some point, so might as well do it now. I think we should increase the revs on kdelibs and kasteroids, and nothing else will (probably) get badly broken until 3.4.1 gets here and people recompile anyway.
There's also another one of these undisclosed securty issues pending, I'd like to adress before we mark stable. I don't see the point, why we can't wait another week or two.
I really think we should commit this and make the announcement sooner rather than later. I hope that the eclasses will be patched before 3.4.1 is committed to CVS. I don't mind committing this, and making the announcement on -dev and -user (others) but don't want to do it if there are still reasons for us to hold back on this. I personally can't see any, and had hoped this would be done already. Feedback?
re #38: I agree, we should at least commit - but Carsten seemed to disagree? I'll probably commit masked 3.4.1 ebuilds tomorrow, so is it finally OK to commit these patches now? (Although we might have to change them for 3.4.1, please check that if you're going to commit them.)
The name and the place of the macro is the same in current 3.4 branch for SVN, so I think the patch is still valid.
I have applied the patches to the kde.eclass and kde-meta.eclass. I have also made an announcement to gentoo-dev, gentoo-user and gentoo-amd64. Most people should just recompile once KDE 3.4.1 is released, but recompiling kdelibs and kasteroids/kdegames is enough to fix the obvious bug. Please let me know if I have forgotten anything. Thanks to FlameEyes for the patches, I will be around on IRC most of the night if there are any issues.
Created attachment 59879 [details, diff] kde-visibility-fix.patch It seems after the patch was applied all the kde-derived ebuilds, even if not supporting visibility stuff, regenerate their configure scripts at compile-time, and this is a bit annoying. Please take a look at the attached patch, which works as expected here and should fix the issue. I can commit it later if it looks good.
Looks good to me, and seems to have the desired affect here too. I hadn't tested the older 3.3/3.2 stuff with the earlier patches, and it would certainly be annoying on slower systems if there is no need for it to be done.
committed.
*** Bug 97320 has been marked as a duplicate of this bug. ***
