Bug 2739 - [PATCH] StaticNeedsPicForShared YES is often required on x86
Summary: [PATCH] StaticNeedsPicForShared YES is often required on x86
Status: RESOLVED WONTFIX
Alias: None
Product: xorg
Classification: Unclassified
Component: Build/Monolithic (show other bugs)
Version: git
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact:
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2005-03-15 06:48 UTC by Mike A. Harris
Modified: 2006-06-03 02:52 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
xorg-x11-cvs-head-config-StaticNeedsPicForShared.patch (429 bytes, patch)
2005-03-15 06:53 UTC, Mike A. Harris
no flags Details | Splinter Review

Description Mike A. Harris 2005-03-15 06:48:52 UTC
StaticNeedsPicForShared is currently set to YES on all architectures
but x86.  When linking static libraries into shared libraries, PIC is
required on x86 or things will not link properly.  Parts of KDE and
other applications that link to static libraries will not compile
properly for example.

This is needed for all static libs to which no shared library is shipped,
in case some shared lib wants to link to that lib.
Comment 1 Mike A. Harris 2005-03-15 06:53:33 UTC
Created attachment 2121 [details] [review]
xorg-x11-cvs-head-config-StaticNeedsPicForShared.patch
Comment 2 Adam Jackson 2005-03-15 10:15:28 UTC
i'm not convinced of this.  my understanding was that x86 allowed you to build
shared libraries non-PIC, but that they would have massive text relocations as a
result.  this isn't necessarily a link-time failure on stock x86, though it can
be one for (for example) PaX systems, and it'll make prelink very unhappy.

not that i disagree with the patch, textrels are bad things, but i don't think
non-PIC static libs are necessarily fatal for x86.  could you clarify the
motivation for this patch?
Comment 3 Mike A. Harris 2005-03-17 15:57:34 UTC
Without this patch, KDE and all KDE apps would not compile on x86.  KDE
libraries link to various X libraries including static only libs.  If the
static only libs, are not PIC, then it either wont compile, or it wont
prelink or both.  I don't remember the details, but we have solved the
problem for about 2 years now by setting StaticNeedsPicForShared YES.

Rather than it continuing to be a Red Hat specific modification, I figured
it would be best to merge it upstream and drop the local change.

I can probably hunt down RHAT bugzilla bugs this fixes if that is
really necessary.  Alternatively, I can disable the local modification,
and get our KDE maintainer to rebuild KDE and file bugs here for review
in case someone has ideas of a better solution rather than setting
"StaticNeedsPicForShared YES".
Comment 4 Adam Jackson 2005-03-18 19:11:47 UTC
yeah, i got that.  my point was that historically x86's toolchain has not required static objects linked
into shared libs to have been compiled with -fPIC.  there are recent toolchain changes that do require
this though, and that these changes are becoming more widespread.

updating summary to match.  i think it's a good change, i just want to clarify the motivation.  people
have complained bitterly in the past when i've threatened to turn on -fPIC for libGL on x86, in defense
of some claimed 1% performance improvement.
Comment 5 Mike A. Harris 2005-03-19 00:30:31 UTC
That may be so, but either X.Org CVS has this turned on by default so
that software compiles correctly on x86, or else every distribution known
to man, shipping a modern KDE and other software which links shared libs
to static X libs will not work properly.  I suppose another solution is
to provide shared versions of all libraries included, so that these libs
link to the shared versions instead.  That is done for most libs now,
but there are some left that don't do it.

I'm going to disable StaticNeedsPicForShared in our spec file and wait to
see what breaks.  I'll send out a notice to our devel mailing list for
people to rebuild things that link to X libraries to see if they fail
without PIC shared libs or not, and provide feedback back here.

There might be other solutions we could consider if the problem is still
as prevalent as it was since around 4.3.0 or so.

I'm not sure what this has to do with libGL though, it is perfectly fine
as is.  Also, Jakub's original libGL patches reduced the number of
symbols needing relocations by marking symbols private that were internal.
It made it possible to make libGL PIC *without* performance problems,
because the performance critical code wasn't hit.

I never did understand why Jakub's patches were rejected outright and
reimplemented by other people.  I don't know if the alternative
implementations did all of the things that Jakub's patches did, so
I don't know if libGL would be negatively affected by building it
PIC now or not.
Comment 6 Adam Jackson 2005-03-19 16:42:40 UTC
(In reply to comment #5)
> I'm not sure what this has to do with libGL though, it is perfectly fine
> as is.

x86 performance weenies hate -fPIC, irrationally.  If we start PICifying static
libs they are likely to complain.  Just a heads-up.

> Also, Jakub's original libGL patches reduced the number of
> symbols needing relocations by marking symbols private that were internal.
> It made it possible to make libGL PIC *without* performance problems,
> because the performance critical code wasn't hit.

Agreed.
 
> I never did understand why Jakub's patches were rejected outright and
> reimplemented by other people.  I don't know if the alternative
> implementations did all of the things that Jakub's patches did, so
> I don't know if libGL would be negatively affected by building it
> PIC now or not.

Primarily because they edited generated source rather than the generating
scripts, and also because (we suspected) they subtly broke binary
compatibility between libGL and the drivers, such that you couldn't use
an old libGL with a new driver, e.g.
Comment 7 Mike A. Harris 2005-04-11 15:08:34 UTC
testing bugzilla
Comment 8 Mike A. Harris 2005-11-30 20:56:36 UTC
All of the libraries are shipped in shared form nowadays, so this bug is
not really relevant anymore I guess.  As an update to this though, I did
disable StaticNeedsPicForShared override in our host.def, and have received
bug reports for both x86, and s390 of which were not listed by default in
xorg.cf as "YES".

Since this isn't relevent for X11R7 now anymore, and it isn't clear what
the future of 6.8.x will be, I'm going to just sledgehammer the option to
YES in our host.def for all architectures to avoid the problem.

I don't need to track this anymore as we'll likely permanently use this
workaround for 6.8.x now, but I'll leave it open in case someone else
considers it important enough for 6.8.3.  Feel free to close it "WONTFIX"
or "FIXED in X11R7" or whatever seems most appropriate if desired however.

Comment 9 Erik Andren 2006-04-18 05:05:50 UTC
Adding patch keyword
Comment 10 Daniel Stone 2006-06-03 02:52:10 UTC
GLEEFUL IMAKE DEATH


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.