Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slibtool build fixes #163

Closed
wants to merge 5 commits into from
Closed

Slibtool build fixes #163

wants to merge 5 commits into from

Conversation

orbea
Copy link
Contributor

@orbea orbea commented May 13, 2021

When building lttng-tools with slibtool (https://dev.midipix.org/cross/slibtool) it fails in many places.

  • Many of the tests in tests/unit/ fail with undefined references because they depend on functions compiled into programs and then are linked directly with the .o object files. Build projects that use $(LIBTOOL) should not link the object files themselves and should use .la libtool archives instead. So I created the liblttng-sessiond-comm.la convenience library that will be linked with instead.
  • GNU libtool will silently ignore arguments it does not understand in many cases while slibtool will print an error which happens with --no-as-needed which is a linker argument. To solve this it needs to be passed directly to the linker with -Wl,--no-as-needed.

NOTE:

  • Each individual commit will still work with GNU libtool.
  • This does not solve all of the slibtool issues, the next one may need resolution in slibtool itself and I am making this PR now to avoid having to rebase the work later. Additionally these commits offer general improvements which remove hacks and reduce overlinking.

For reference the next slibtool build failure I see is:

Making install in baddr-statedump
make[4]: Entering directory '/tmp/lttng-tools/tests/regression/ust/baddr-statedump'
objcopy --only-keep-debug prog prog.debug
objcopy: prog: file format not recognized
make[4]: *** [Makefile:810: prog.debug] Error 1
make[4]: Leaving directory '/tmp/lttng-tools/tests/regression/ust/baddr-statedump'
make[3]: *** [Makefile:569: install-recursive] Error 1
make[3]: Leaving directory '/tmp/lttng-tools/tests/regression/ust'
make[2]: *** [Makefile:831: install-recursive] Error 1
make[2]: Leaving directory '/tmp/lttng-tools/tests/regression'
make[1]: *** [Makefile:557: install-recursive] Error 1
make[1]: Leaving directory '/tmp/lttng-tools/tests'
make: *** [Makefile:656: install-recursive] Error 1

This happens because slibtool places compiled binaries in the .libs directory while prog is a shell wrapper script. Ideally some method of --mode=execute could be used to solve this, but may require further work in slibtool itself to work. I will follow up on this PR in the future.

@orbea
Copy link
Contributor Author

orbea commented May 13, 2021

Also see this downstream issue:

https://bugs.gentoo.org/789636

And also reported independently on the slibtool tracker:

https://dev.midipix.org/cross/slibtool/issue/40

@PSRCode
Copy link
Contributor

PSRCode commented May 27, 2021

Hi,

Overall looking good.

Do you have any reference for this:

Build projects that use $(LIBTOOL) should not link the object files themselves and should use .la libtool archives instead.

Cheers

@jgalar jgalar requested a review from mjeanson May 27, 2021 22:05
Copy link
Member

@mjeanson mjeanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general idea looks good to me, I just have questions on the use of LDFLAGS.

src/bin/lttng-sessiond/Makefile.am Outdated Show resolved Hide resolved
src/bin/lttng-sessiond/Makefile.am Show resolved Hide resolved
@orbea
Copy link
Contributor Author

orbea commented May 28, 2021

Do you have any reference for this:

Build projects that use $(LIBTOOL) should not link the object files themselves and should use .la libtool archives instead.

Now that I looked again it appears the automake documentation does allow .$(OBJEXT), .a and .la as valid files for LDADD when linking programs.

maude_LDADD

    Extra objects (*.$(OBJEXT)) and libraries (*.a, *.la) can be added to a program by listing them
    in the _LDADD variable. For instance, this should be used for objects determined by configure
    (see Linking).

    _LDADD and _LIBADD are inappropriate for passing program-specific linker flags (except for
    -l, -L, -dlopen and -dlpreopen). Use the _LDFLAGS variable for this purpose.

    For instance, if your configure.ac uses AC_PATH_XTRA, you could link your program against
    the X libraries like so:

        maude_LDADD = $(X_PRE_LIBS) $(X_LIBS) $(X_EXTRA_LIBS)

    We recommend that you use -l and -L only when referring to third-party libraries, and give the
    explicit file names of any library built by your package. Doing so will ensure that
    maude_DEPENDENCIES (see below) is correctly defined by default.
    Since these dependencies are associated with the link rule used to create the programs they
    should normally list files used by the link command. That is *.$(OBJEXT), *.a, or *.la files for
    programs; *.lo and *.la files for Libtool libraries; and *.$(OBJEXT) files for static libraries. 

https://www.gnu.org/software/automake/manual/html_node/Program-and-Library-Variables.html

However as demonstrated by lttng-tools that linking with .$(OBJEXT) is significantly more complicated and fragile than using a libtool convenience library. With slibtool this results in many undefined references, maybe because of differences in linking order between GNU libtool and slibtool?

@PSRCode
Copy link
Contributor

PSRCode commented May 28, 2021

Thank you linking that info.

However as demonstrated by lttng-tools that linking with .$(OBJEXT) is significantly more complicated and fragile than using a libtool convenience library.

From a quick look, I do agree with you here.

With slibtool this results in many undefined references, maybe because of differences in linking order between GNU libtool and slibtool?

There is a lot more chance that you have a better hunch regarding this than me.

Cheers

@jgalar
Copy link
Member

jgalar commented May 28, 2021

To prevent any confusion, can you rename liblttng-sessiond-comm.la to liblttng-sessiond-common.la?
sessiond-comm already refers to the session daemon's communication facilities internally (e.g. sessiond-comm.h)

Thanks!

@orbea
Copy link
Contributor Author

orbea commented May 29, 2021

To prevent any confusion, can you rename liblttng-sessiond-comm.la to liblttng-sessiond-common.la?

Done.

I added one more commit to fix a new problem I encountered when rebasing the PR.

liblttng-ust-ctl.so: undefined reference to `lttng_ust_sigbus_state'

The fix is described in lttng-ust.

https://github.com/lttng/lttng-ust/blob/21d04cc41c966d687aab6e0ba60b2cb1466356ca/include/lttng/ust-sigbus.h#L36

@jgalar
Copy link
Member

jgalar commented Jun 4, 2021

@orbea do you mind if I add your Signed-off-by line?

orbea added 5 commits June 4, 2021 13:52
This allows correctly linking test_kernel_data with slibtool.

Signed-off-by: orbea <orbea@riseup.net>
This allows correctly linking test_session and test_ust_data
with slibtool.

Signed-off-by: orbea <orbea@riseup.net>
This allow correctly linking live_test with slibtool.

Signed-off-by: orbea <orbea@riseup.net>
This is not a libtool argument, but rather a linker argument.
GNU libtool will silently ignore arguments it doesn't understand
in many cases while slibtool does not.

Signed-off-by: orbea <orbea@riseup.net>
Fixes:

  liblttng-ust-ctl.so: undefined reference to `lttng_ust_sigbus_state'

Signed-off-by: orbea <orbea@riseup.net>
@orbea
Copy link
Contributor Author

orbea commented Jun 4, 2021

@jgalar I don't mind, but I updated the commits to include it. :)

@jgalar
Copy link
Member

jgalar commented Jun 7, 2021

Merged manually, thanks!

@jgalar jgalar closed this Jun 7, 2021
@orbea orbea deleted the tests branch May 15, 2022 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants