mbox series

[0/2] remove use of weak functions from libraries

Message ID 20190410134517.63896-1-bruce.richardson@intel.com (mailing list archive)
Headers
Series remove use of weak functions from libraries |

Message

Bruce Richardson April 10, 2019, 1:45 p.m. UTC
  Weak functions don't work well with static library builds as the linker
always picks the first version of a function irrespective of whether it is
weak or not. The solution to this is to use the "whole-archive" flag when
linking, but this has the nasty side-effect that it causes the resulting
binary to be larger than it needs to be.


A further problem with this approach of using "whole-archive" is that one
either needs to link all libraries with this flag or track what libraries
need it or not - the latter being especially a problem for apps not using
the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
For meson builds this information needs to make its way all the way through
to the pkgconfig file generated - not a trivial undertaking.

Thankfully, though, the use of weak functions is limited to use for
multiple functions within a single library, meaning that when the lib is
being built, the build system itself can know whether the "weak" function
is needed or not. This allows us to change the weak function to a
conditionally compiled regular function used in fallback case. This makes
the need for "whole-archive" flag, and any special linking measures for the
library, go away.

[This set does not touch the drivers, only the libraries, since there are
other special linker flags needed for drivers in general, making the
problem less severe for driver .a files.]

Bruce Richardson (2):
  acl: remove use of weak functions
  bpf: remove use of weak functions

 lib/librte_acl/meson.build |  7 ++++++-
 lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
 lib/librte_bpf/bpf_load.c  |  4 +++-
 lib/librte_bpf/meson.build |  1 +
 mk/rte.app.mk              |  3 ---
 5 files changed, 24 insertions(+), 9 deletions(-)
  

Comments

David Marchand May 27, 2019, 2:13 p.m. UTC | #1
Hello,

On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> Weak functions don't work well with static library builds as the linker
> always picks the first version of a function irrespective of whether it is
> weak or not. The solution to this is to use the "whole-archive" flag when
> linking, but this has the nasty side-effect that it causes the resulting
> binary to be larger than it needs to be.
>
>
> A further problem with this approach of using "whole-archive" is that one
> either needs to link all libraries with this flag or track what libraries
> need it or not - the latter being especially a problem for apps not using
> the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
> For meson builds this information needs to make its way all the way through
> to the pkgconfig file generated - not a trivial undertaking.
>
> Thankfully, though, the use of weak functions is limited to use for
> multiple functions within a single library, meaning that when the lib is
> being built, the build system itself can know whether the "weak" function
> is needed or not. This allows us to change the weak function to a
> conditionally compiled regular function used in fallback case. This makes
> the need for "whole-archive" flag, and any special linking measures for the
> library, go away.
>
> [This set does not touch the drivers, only the libraries, since there are
> other special linker flags needed for drivers in general, making the
> problem less severe for driver .a files.]
>
>
Was there something preventing this patchset from getting merged ?
  
Bruce Richardson May 27, 2019, 3:41 p.m. UTC | #2
On Mon, May 27, 2019 at 04:13:53PM +0200, David Marchand wrote:
>    Hello,
>    On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      Weak functions don't work well with static library builds as the
>      linker
>      always picks the first version of a function irrespective of whether
>      it is
>      weak or not. The solution to this is to use the "whole-archive" flag
>      when
>      linking, but this has the nasty side-effect that it causes the
>      resulting
>      binary to be larger than it needs to be.
>      A further problem with this approach of using "whole-archive" is
>      that one
>      either needs to link all libraries with this flag or track what
>      libraries
>      need it or not - the latter being especially a problem for apps not
>      using
>      the DPDK build system itself (i.e. most apps not shipped with DPDK
>      itself).
>      For meson builds this information needs to make its way all the way
>      through
>      to the pkgconfig file generated - not a trivial undertaking.
>      Thankfully, though, the use of weak functions is limited to use for
>      multiple functions within a single library, meaning that when the
>      lib is
>      being built, the build system itself can know whether the "weak"
>      function
>      is needed or not. This allows us to change the weak function to a
>      conditionally compiled regular function used in fallback case. This
>      makes
>      the need for "whole-archive" flag, and any special linking measures
>      for the
>      library, go away.
>      [This set does not touch the drivers, only the libraries, since
>      there are
>      other special linker flags needed for drivers in general, making the
>      problem less severe for driver .a files.]
> 
>    Was there something preventing this patchset from getting merged ?
>    --
>    David Marchand
> 
I believe Aaron Conole had some changes in the same area and was looking to
roll these changes into a bigger patchset of his. Aaron, can you please
confirm?

/Bruce
  
Aaron Conole May 27, 2019, 8:57 p.m. UTC | #3
Bruce Richardson <bruce.richardson@intel.com> writes:

> On Mon, May 27, 2019 at 04:13:53PM +0200, David Marchand wrote:
>>    Hello,
>>    On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson
>>    <[1]bruce.richardson@intel.com> wrote:
>> 
>>      Weak functions don't work well with static library builds as the
>>      linker
>>      always picks the first version of a function irrespective of whether
>>      it is
>>      weak or not. The solution to this is to use the "whole-archive" flag
>>      when
>>      linking, but this has the nasty side-effect that it causes the
>>      resulting
>>      binary to be larger than it needs to be.
>>      A further problem with this approach of using "whole-archive" is
>>      that one
>>      either needs to link all libraries with this flag or track what
>>      libraries
>>      need it or not - the latter being especially a problem for apps not
>>      using
>>      the DPDK build system itself (i.e. most apps not shipped with DPDK
>>      itself).
>>      For meson builds this information needs to make its way all the way
>>      through
>>      to the pkgconfig file generated - not a trivial undertaking.
>>      Thankfully, though, the use of weak functions is limited to use for
>>      multiple functions within a single library, meaning that when the
>>      lib is
>>      being built, the build system itself can know whether the "weak"
>>      function
>>      is needed or not. This allows us to change the weak function to a
>>      conditionally compiled regular function used in fallback case. This
>>      makes
>>      the need for "whole-archive" flag, and any special linking measures
>>      for the
>>      library, go away.
>>      [This set does not touch the drivers, only the libraries, since
>>      there are
>>      other special linker flags needed for drivers in general, making the
>>      problem less severe for driver .a files.]
>> 
>>    Was there something preventing this patchset from getting merged ?
>>    --
>>    David Marchand
>> 
> I believe Aaron Conole had some changes in the same area and was looking to
> roll these changes into a bigger patchset of his. Aaron, can you please
> confirm?

Yes - Sorry the patches are in my queue.  Maybe it just makes sense to
merge these though?

> /Bruce
  
Bruce Richardson May 28, 2019, 8:06 a.m. UTC | #4
On Mon, May 27, 2019 at 04:57:15PM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > On Mon, May 27, 2019 at 04:13:53PM +0200, David Marchand wrote:
> >>    Hello,
> >>    On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson
> >>    <[1]bruce.richardson@intel.com> wrote:
> >> 
> >>      Weak functions don't work well with static library builds as the
> >>      linker
> >>      always picks the first version of a function irrespective of whether
> >>      it is
> >>      weak or not. The solution to this is to use the "whole-archive" flag
> >>      when
> >>      linking, but this has the nasty side-effect that it causes the
> >>      resulting
> >>      binary to be larger than it needs to be.
> >>      A further problem with this approach of using "whole-archive" is
> >>      that one
> >>      either needs to link all libraries with this flag or track what
> >>      libraries
> >>      need it or not - the latter being especially a problem for apps not
> >>      using
> >>      the DPDK build system itself (i.e. most apps not shipped with DPDK
> >>      itself).
> >>      For meson builds this information needs to make its way all the way
> >>      through
> >>      to the pkgconfig file generated - not a trivial undertaking.
> >>      Thankfully, though, the use of weak functions is limited to use for
> >>      multiple functions within a single library, meaning that when the
> >>      lib is
> >>      being built, the build system itself can know whether the "weak"
> >>      function
> >>      is needed or not. This allows us to change the weak function to a
> >>      conditionally compiled regular function used in fallback case. This
> >>      makes
> >>      the need for "whole-archive" flag, and any special linking measures
> >>      for the
> >>      library, go away.
> >>      [This set does not touch the drivers, only the libraries, since
> >>      there are
> >>      other special linker flags needed for drivers in general, making the
> >>      problem less severe for driver .a files.]
> >> 
> >>    Was there something preventing this patchset from getting merged ?
> >>    --
> >>    David Marchand
> >> 
> > I believe Aaron Conole had some changes in the same area and was looking to
> > roll these changes into a bigger patchset of his. Aaron, can you please
> > confirm?
> 
> Yes - Sorry the patches are in my queue.  Maybe it just makes sense to
> merge these though?
>
Funnily enough, I've no objections to that :-)
  
Thomas Monjalon June 5, 2019, 2:41 p.m. UTC | #5
10/04/2019 15:45, Bruce Richardson:
> Weak functions don't work well with static library builds as the linker
> always picks the first version of a function irrespective of whether it is
> weak or not. The solution to this is to use the "whole-archive" flag when
> linking, but this has the nasty side-effect that it causes the resulting
> binary to be larger than it needs to be.
> 
> 
> A further problem with this approach of using "whole-archive" is that one
> either needs to link all libraries with this flag or track what libraries
> need it or not - the latter being especially a problem for apps not using
> the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
> For meson builds this information needs to make its way all the way through
> to the pkgconfig file generated - not a trivial undertaking.
> 
> Thankfully, though, the use of weak functions is limited to use for
> multiple functions within a single library, meaning that when the lib is
> being built, the build system itself can know whether the "weak" function
> is needed or not. This allows us to change the weak function to a
> conditionally compiled regular function used in fallback case. This makes
> the need for "whole-archive" flag, and any special linking measures for the
> library, go away.
> 
> [This set does not touch the drivers, only the libraries, since there are
> other special linker flags needed for drivers in general, making the
> problem less severe for driver .a files.]
> 
> Bruce Richardson (2):
>   acl: remove use of weak functions
>   bpf: remove use of weak functions

Applied, thanks