[Bug 5887] New: Add support for -Bsymbolic-functions to LD checks

http://bugzilla.xfce.org/show_bug.cgi?id=5887

           Summary: Add support for -Bsymbolic-functions to LD checks
    Classification: Xfce
           Product: Xfce4-dev-tools
           Version: Unspecified
          Platform: Other
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: Medium
         Component: General
        AssignedTo: xfce-bugs@...
        ReportedBy: nick@...


Created an attachment (id=2618)
 --> (http://bugzilla.xfce.org/attachment.cgi?id=2618)
Add -Bsymbolic-functions to XDT_FEATURE_LINKER_OPTS.

Although removing the visibility checks from several modules makes the whole
thing easier to understand, it does increase the PLT overhead a bit.
Fortunately recent versions of ld support -Bsymbolic-functions which will
decrease PLT overhead of shared libraries.

It works fine here and I think it safe to use (also used by OOo). -Bsymbolic
however could result in problems, so better not touch that.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #1 from Brian J. Tarricone <bjt23@...> 2009-10-19 09:23:08 PDT ---
Hmm... it does have one potentially-unwanted side-effect: you can't override
symbols in the library later using LD_PRELOAD or similar.  We might not care
for some libraries, but maybe we would for others.  (Though one could make the
argument that this is bad practice and should be discouraged, it can be useful
for debugging in some cases.)

Another option might be to have some opt 'levels' that the macro can take,
similar to how XDT_FEATURE_DEBUG works... but that feels like overdoing it a
bit.

Regardless of this, maybe we should disable linker opts when the debug level is
'full', maybe even 'yes' also.

Need to think about this a bit.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

Nick Schermer <nick@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2618|0                           |1
        is obsolete|                            |

--- Comment #2 from Nick Schermer <nick@...> 2009-10-20 10:37:49 CEST ---
Created an attachment (id=2619)
 --> (http://bugzilla.xfce.org/attachment.cgi?id=2619)
Add -Bsymbolic-functions to XDT_FEATURE_LINKER_OPTS, v2

Disabling the opts when debugging is active sounds like a good idea. I've
disabled it when debugging is yes or full, and it throws an error when
$enable_debug is not defined (self check if we call XDT_FEATURE_LINKER_OPTS
before XDT_FEATURE_DEBUG).

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

Nick Schermer <nick@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2619|0                           |1
        is obsolete|                            |

--- Comment #3 from Nick Schermer <nick@...> 2009-10-20 10:39:56 CEST ---
Created an attachment (id=2620)
 --> (http://bugzilla.xfce.org/attachment.cgi?id=2620)
Add -Bsymbolic-functions to XDT_FEATURE_LINKER_OPTS, v2.1

Whoops, forgot the save before creating the patch...

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

Brian J. Tarricone <bjt23@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #4 from Brian J. Tarricone <bjt23@...> 2009-10-20 02:42:53 PDT ---
I'll look at this tomorrow, but just a quick thought to toss out before I try
to go back to bed:

Would it make sense to AC_REQUIRE() XDT_FEATURE_DEBUG at the top of
XDT_FEATURE_LINKER_OPTS?  That way you don't even need to specify the former if
you do the latter.  Though I guess that could make the definition get emitted
twice if they're both included.  Is there a way to tell m4/autoconf "only
expand this macro if it hasn't been expanded before, otherwise expand to
nothing"?

The problem with your idea of checking $enable_debug is that it doesn't allow
using XDT_FEATURE_LINKER_OPTS without _DEBUG.  While personally I think not
using _DEBUG is silly, maybe someone might want the linker opts and not have a
debug flag.

At any rate, since _LINKER_OPTS has been around for such a short time, I think
it's totally fine to require _DEBUG before _LINKER_OPTS in the next released
version.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #5 from Brian J. Tarricone <bjt23@...> 2009-10-20 02:58:28 PDT ---
Another issue I just thought of...  you probably only want this flag when
building shared libraries, not when building applications.  See:

https://bugs.launchpad.net/ubuntu/+source/samba/+bug/234901

That just presents one example using gethostbyname(), but I imagine there are
other cases.

Of course XDT_FEATURE_LINKER_OPTS is useful for both apps and shared libs:
--as-needed is useful for both, and -O1 fine to use for both (it currently only
has an effect on shared libs, but may in the future do something nice for apps
too).  -Bsymbolic and -Bsymbolic-functions, however, shouldn't be used with
apps.

Ignoring that bit, packages like xfconf that have both a library and an
application also hit this problem.

Any thoughts on how to fix this?  I guess we could only add safe flags to
LDFLAGS, and do something like XDT_LIBRARY_LDFLAGS for library-only flags like
this, but that strikes me as ugly and requires that the developer add that
variable to their Makefile.am for the library target only, which is probably
easy to forget (of course, forgetting doesn't break anything, so maybe it's not
a big deal).

And also I found this (see 2009-03-20):

http://wiki.services.openoffice.org/wiki/Performance/Meetings/2009_03

Unless something changed since March, they actually decided for OOo *not* to
use -Bsymbolic-functions.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #6 from Nick Schermer <nick@...> 2009-10-20 12:54:06 CEST ---
I wouldn't have any problems with library specific LDFLAGS, since I can't think
of another way to handle this in a package like xfconf/exo too.

Configure in ooo looks for bsymbolic, so it looks like it is included in the
source: http://svn.services.openoffice.org/ooo/trunk/configure.in (see also
set_soenv.in)

Anyway, with the library specific ldflags and disable all the ld flags when
debugging; I think we should just give it a try. We can easily disable this at
any time and while we're still early in the development cycle we can give it
some good testing.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #7 from Brian J. Tarricone <bjt23@...> 2009-10-20 10:07:48 PDT ---
Hmm, I dunno... I still dislike this.  Why are we doing this again?  What do
you mean about *removing* the visibility stuff?

Given the relatively small number of relocatable symbols in our libraries, esp
with gnuc visibility enabled, this seems like a risk with very little payoff.
Any cold-cache benchmarks of load time with and without -Bsymbolic-functions?
I think this is a case of needing to convince me that it's useful and doesn't
break things up-front, not "it might be a good idea, so let's try it."

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #8 from Nick Schermer <nick@...> 2009-10-21 11:12:29 CEST ---
The previous alias stuff we used works in 2 ways: it handles public visibility
of functions and reduces the PLT overhead for internal calls. However this is a
somewhat complicated and a lot of times people forgot the update the symbols
file.

So, right now we handle the first part by using -export-symbols-regex and mark
all private functions with _ or optionally use a .ver file. This technique is
recommended by people like Drepper and the binutils folks.

The other part is the internal calls. The alias code creates files to make
aliases for the public functions starting with IA__ so all function calls
inside the library are not going over the PLT, this is what
-Bsymbolic-functions could do for us.

I admit I did not know about the internal PLT stuff when removing this from
some of our libraries and if -Bsymbolic-functions will be rejected I'll put the
alias code back in the libraries. I only prefer -Bsymbolic-functions since it's
less complicated.

No benchmarks done on this, but you can see the result of -Bsymbolic-functions
when using size on the library:

   text       data        bss        dec        hex    filename
 263976       7280        560     271816      425c8    libexo-1.so.normal
 259416       6368        560     266344      41068  
libexo-1.so.with.Bsymbolic.functions

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #9 from Nick Schermer <nick@...> 2009-10-21 11:33:48 CEST ---
For reference, compiled with visibility and the aliasdef technique, gives this:

   text       data        bss        dec        hex    filename
 261372       6320        560     268252      417dc  
/opt/xfce/lib/libexo-1.so.with.alias

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

Christian Dywan <christian@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |christian@...

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

Jannis Pohlmann <jannis@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jannis@...

--- Comment #10 from Jannis Pohlmann <jannis@...> 2009-10-21 13:46:16 UTC ---
After talking to Nick about this on IRC, I understand the situation as:

We had alias code in exo before to create IA_ aliases for exo functions so that
when an exo function called another exo function it wouldn't have to go through
the PLT. -Bsymbolic-functions provides the same thing and with it we wouldn't
have to manage aliases manually anymore. -Bsymbolic-functions however can break
applications if passed to them when linking to e.g. exo.

So, like the alias code we had for exo which was only applied to the library,
not to any applications, we want to pass -Bsymbolic-functions only to
libraries, right?

I guess I'd be ok with an additional variable for library makefiles. The alias
code is a real pain so I'd be happy to be able to get rid of it without losing
functionality. A new makefile variable sounds like a fair trade-off.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

Brian J. Tarricone <bjt23@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |WONTFIX

--- Comment #11 from Brian J. Tarricone <bjt23@...> 2009-10-21 11:02:59 PDT ---
Honestly, why bother?  We're talking about a tiny tiny tiny reduction in binary
size and a probably negligible performance increase, and the risk is breaking
things in unknown ways by using a linker flag that clearly we don't completely
understand.

The visibility and alias stuff is a little annoying, but it's a known quantity
and is something well-used by other projects... and we already have (had?) it
set up.

If you really want to find ways to increase performance, profile your app and
fix bottlenecks.  Unless I see real-world benchmarks showing that
-Bsymbolic-functions actually has a material effect on performance, I don't see
a reason to use it.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #12 from Jannis Pohlmann <jannis@...> 2009-10-22 10:28:34 UTC ---
(In reply to comment #11)
> Honestly, why bother?  We're talking about a tiny tiny tiny reduction in binary
> size and a probably negligible performance increase, and the risk is breaking
> things in unknown ways by using a linker flag that clearly we don't completely
> understand.
>
> The visibility and alias stuff is a little annoying, but it's a known quantity
> and is something well-used by other projects... and we already have (had?) it
> set up.

AFAIK, we've always had it for thunar-vfs, exo and thunarx. We recently removed
it from thunarx and from exo. Nick just added it back to exo.

The aliasdef stuff is kinda ugly but since we're planning to keep the .symbols
file around anyway, I guess it'd be ok to add it back to thunarx as well and
continue to use it.

> If you really want to find ways to increase performance, profile your app and
> fix bottlenecks.  Unless I see real-world benchmarks showing that
> -Bsymbolic-functions actually has a material effect on performance, I don't see
> a reason to use it.

I'm wondering about this, too. The GLib guys told me that PLT lookups and
relocations are a real concern to them which is why they are stuffing
everything into GIO nowadays instead of separate GLib modules. But they
couldn't back this argument up with numbers either.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #13 from Nick Schermer <nick@...> 2009-10-22 16:00:54 CEST ---
My final goal was using a .ver file (which is more or less like the symbols
file in our case) and pass that to -Wl,--version-script (instead of the
-export-symbols-regex, which creates such a file on the fly). Then we have a
'symbols' file to maintain/check the api, which is directly passed to the
linker and can also be used for the abicheck.sh test, -Bsymbolic-functions
would've solved the internal PLT stuff. That is the simplest and also the most
effective setup from a performance (whether that is measurable or not) and
maintenance point of view.

Anyway, already restored (and fixed) most of the alias stuff, but I do get a
feeling out of all this that some people don't care because it works for them
and so you don't have to look forward and try something. The performance thing
is an easy one to toss because we all know it is hard to measure, but for that
same bit of extra performance we also use the alias code.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs

http://bugzilla.xfce.org/show_bug.cgi?id=5887

--- Comment #14 from Brian J. Tarricone <bjt23@...> 2009-10-22 14:50:36 PDT ---
Is it actually hard to measure?

It's true that I don't really want to change it because what we have seems to
work fine, and there's some risk in changing it.  But it's a question of the
benefit: if you're going to propose a change, then the burden is on your
shoulders to prove that there's actually a benefit.

Do you know how to benchmark this?  If not, then you're just making a baseless
claim about some unknown performance case.

The only evidence of improvement you've provided is a 6% decrease (less than
2kB) in binary size for a single (relatively small 260kB) library.  Not only is
it an unreliable sample, I just don't think saving 6% (assuming it's 6% for
every single library, which I doubt) of our total shared library footprint is
all that impressive.

If you prefer to use --version-script, why not just generate the .ver file from
the .symbols file (or vice versa)?

Also note that -Bsymbolic isn't about reducing PLT entries.  It does do that as
a side-effect, but its purpose is to bind symbol definitions so they can't be
overridden.

The modern/correct way of defining the public ABI is through visibility
attributes.  The version script method is also correct, though maybe "the old
way" (not that there's anything wrong with that).  -Bsymbolic has nothing to do
with ABI.  The safest way to reduce PLT entries is with the aliasing trick.

Performance wise, I can't really say too much, but I'd guess there's less of a
win here than you'd think.  To call shared lib functions, the application
*still* has to call into the GOT.  Just with -Bsymbolic, instead of resolving
the function address with GOT+PLT, it should get resolved with GOT+a static
offset.  This offset still needs to be determined, so you're not speeding up
initial application load time (the relocation still has to happen anyway).

As for actual calls to the function, I'm not sure how the runtime linker
handles this.  Either it will more or less generate a PLT on the fly (which is
why the .so size is smaller), or it'll patch references in the binary at
callsites with GOT+offset either at load time or on first call.  If it does the
former, I don't think there's really any benefit.  If the latter, successive
calls to the function should be faster, but I'm not sure how much.

Anyway, that's all hand-wavy from memory and intuition (haven't looked at this
stuff in years), so I might be off on how that works.  But still, that doesn't
really change the fact that the benefit seems negligible (pending proof either
way), and what we have works well enough already.

Now, if you're saying the alias stuff is a pain to set up, I'd certainly agree.
 But that doesn't justify throwing away the stuff that's already set up.  I
believe the script that generates the alias .c/.h files is in perl... if that
could be rewritten in portable sh, we could stuff that in x-d-t as well, and
then just require that the maintainer:

1.  Put the macro in configure.ac
2.  Maintain a file with a list of exported symbols
3.  Put a rule substitution in the Makefile.am (similar to
@INTLTOOL_DESKTOP_RULE@) that generates the alias .c/.h files
4.  Include the files in the appropriate .c files.

That would maintain the status quo with regard to functionality, but make
setting up new libraries easier.  The only annoying bit is #4, but maybe we can
come up with a workaround for that, too.

--
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
Xfce-bugs mailing list
Xfce-bugs@...
http://foo-projects.org/mailman/listinfo/xfce-bugs