Re: PIE static library builds

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Mar 15, 2023 at 04:45:26PM +0100, Mattias Rönnblom wrote:
> On 2023-03-15 15:18, Bruce Richardson wrote:
> > On Tue, Mar 14, 2023 at 06:31:41PM +0000, Bruce Richardson wrote:
> > > On Tue, Mar 14, 2023 at 07:04:19PM +0100, Mattias Rönnblom wrote:
> > > > On 2023-03-14 17:29, Bruce Richardson wrote:
> > > > > On Tue, Mar 14, 2023 at 05:22:02PM +0100, Mattias Rönnblom wrote:
> > > > > > Hi.
> > > > > > 
> > > > > > Is the "b_staticpic" meson build option supposed to work with DPDK?
> > > > > > 
> > > > > > Setting it to "false" (default is "true") causes link failures on
> > > > > > Ubuntu 22.04, with GCC 9 and 11, on v23.03rc1 and v22.11:
> > > > > > 
> > > > > > /usr/bin/ld: lib/librte_eal.a.p/eal_common_eal_common_errno.c.o:
> > > > > > relocation R_X86_64_TPOFF32 against `per_lcore_retval.1' can not be
> > > > > > used when making a shared object; recompile with -fPIC /usr/bin/ld:
> > > > > > failed to set dynamic section sizes: bad value collect2: error: ld
> > > > > > returned 1 exit status
> > > > > > 
> > > > > > Does something per-lcore/TLS-related require PIC builds, even for
> > > > > > static libraries?
> > > > > > 
> > > > > I don't think that is the issue. The "issue" is that DPDK always does
> > > > > both static and shared builds from the same object files, so without
> > > > > -fPIC the shared library parts of the build fails. To support not
> > > > > using staticpic, we'd have to disable building the .so's in those
> > > > > cases, or each C file built twice.
> > > > > 
> > > > 
> > > > With "default_library" set to "static", shouldn't the shared objects be
> > > > skipped? I can see now, they are not.
> > > > 
> > > 
> > > Yep, they aren't skipped. The reasons for this are partially historical,
> > > and partially due to meson limitations around linking (which may now also
> > > be historical).
> > > 
> > > When we originally switched over to meson, IIRC there was no
> > > "both_libraries" option, but we still had a situation where: * we wanted
> > > to use and link staticly by default * we had *lots* of issues with
> > > patches breaking builds as submitters had forgotten about shared libs
> > > e.g. updating the version map Therefore, from the earliest versions of
> > > the meson builds we had DPDK always build both libraries - using our own
> > > logic. [This did have the desired effect of mostly eliminating version
> > > map issues once everyone whiched over, which was nice!]
> > > 
> 
> Symbols missing in version.map will be caught by the build bots, correct?
> Provided they build shared object builds, as well as the default. This
> feedback is received only after the patches have been submitted, but usually
> there are still several revisions of a patch set anyways.
> 

Yes, they are caught by buildbots, but it's much better that they are
caught by the submitter before they even get to pushing out a patch. While
it may be a fairly minor issue, we used to see a *lot* of patches submitted
previously which did not pass a shared-library build (and not everyone
checked the build bot results for their patches).

> > > As things moved on, meson did add support for "both_libraries", and I did
> > > investigate using it in DPDK to have proper static-only, shared-only and
> > > both-library builds. Unfortunately, the assumption in meson was that if
> > > both libraries were built, the apps would link against the shared
> > > versions.  Therefore, any change to use "both_library" support in DPDK
> > > would unfortunately lead to a change in default behaviour as our builds
> > > would all be shared, rather than static. [I have not checked recently to
> > > see if this can be overcome.]
> > > 
> > > This is why things as where they are right now. :-)
> > > 
> > For the sake of completeness: one other complication I forgot to mention -
> > using function versioning.  When we have a library containing versioned
> > functions the build needs to be performed slightly differently depending on
> > whether we are building it as a static or a shared library. This is because
> > the verisoning macro need to expand slightly differently depending on the
> > build type. This prevents us from using "both_libraries" in these cases.
> > [And why, right now, we need to explicitly tag any libs with versioned
> > functions, so we can compile all the source files twice, with different
> > flags].
> > 
> 
> I'm not sure I follow here. Are separate object files built for static and
> shared libraries, or not? Here it sounds like they have to be built with
> different flags, but earlier I thought you said static and dynamic libraries
> were assembled from the same object files.

Normally, they are only built once. However, if a library is using function
versioning then the object files are built twice.

> 
> If you set "b_staticpic=false" the build is still partially successful, and
> you can build separate applications (e.g., dpdk-test). I ran some
> performance tests, and it seems like there may be some performance to gain
> from building with -fPIE.

Great. There is probably a way we can make the DPDK build work to enable
proper support for just static, just shared, or both libraries being built.
It's probably not entirely straight-forward though. The biggest sticking
point is like the function versioning in the "both_library" case, but we
may be able to make that work with some overloading of the "pic" flag -
since setting that to false will force double-compilation of the files in
the "both" case. I think we'll also need a DPDK-specific build option too
for specifying the link-preference of static/shared for the "both" case
too, so as to keep static linking as default.

/Bruce