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

Fix cross-compilation for static,vendored #95

Merged
merged 3 commits into from
Aug 19, 2024

Conversation

kxxt
Copy link
Contributor

@kxxt kxxt commented Aug 16, 2024

This PR fixes cross-compilation for static,vendored.

@kxxt kxxt mentioned this pull request Aug 16, 2024
@kxxt kxxt changed the title Only build libelf and disable demangler for elfutils Fix cross-compilation for static,vendored Aug 16, 2024
Copy link
Collaborator

@danielocfb danielocfb left a comment

Choose a reason for hiding this comment

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

W00t, this is great! Feeling a tad uneasy about the unconditional setting of --host, but I think it's the right thing to do and if we got some target wrong we can add it as supported in CI later.

I don't see a reason why we'd need the demangler, but it's possible libbpf may need/use it for uprobe attachment or similar? @anakryiko does libbpf use/rely on demangling functionality somewhere?

@kxxt
Copy link
Contributor Author

kxxt commented Aug 16, 2024

if we got some target wrong we can add it as supported in CI later.

Yes. I think that's the right way to go. For riscv64, rust uses riscv64gc but autotools uses riscv64. There might be similar problems for some other architectures but I think we can wait until someone actually using those architectures opens an issue/PR.

the demangler

For reference, here's a workflow run with elfutils 0.190 without --disable-demangler that fails because of missing libstdc++: https://github.com/kxxt/libbpf-sys-cross/actions/runs/10400687991/job/28801712892#step:8:330

@danielocfb
Copy link
Collaborator

For reference, here's a workflow run with elfutils 0.190 without --disable-demangler that fails because of missing libstdc++: https://github.com/kxxt/libbpf-sys-cross/actions/runs/10400687991/job/28801712892#step:8:330

Got it. But it was being enabled with the current snapshot in elfutils-mirror (which I guess is 0.189 or whatever), right? So we should make sure that we are not disabling functionality that would actually be used.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 19, 2024

It seems that for elfutils 0.189. the demangler support requires libstdc++ already.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 19, 2024

A quick rg __cxa_demangle shows that this function is not referenced in libelf. So I think it should be safe to disable demangler

configure.ac
497:AC_CHECK_LIB([stdc++], [__cxa_demangle], [dnl
499:AM_CONDITIONAL(DEMANGLE, test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes")
500:AS_IF([test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes"],
502:      [AC_MSG_ERROR([[__cxa_demangle not found in libstdc++, use --disable-demangler to disable demangler support.]])]),

ChangeLog
16:     __cxa_demangle cannot be found.
827:    * configure.ac: Check for __cxa_demangle in libstdc++.

src/stack.c
238:      char *dsymname = __cxa_demangle (symname, demangle_buffer,

src/nm.c
811:      char *dmsymstr = __cxa_demangle (symstr, demangle_buffer,
964:      char *dmsymstr = __cxa_demangle (symstr, demangle_buffer,
1096:     char *dmsymstr = __cxa_demangle (symstr, demangle_buffer,
1375:         char *dmsymstr = __cxa_demangle (symstr, demangle_buffer,

src/addr2line.c
293:      char *dsymname = __cxa_demangle (name, demangle_buffer,

lib/system.h
204:extern char *__cxa_demangle (const char *mangled_name, char *output_buffer,

lib/ChangeLog
318:    * system.h: Declare __cxa_demangle.

Previously, build.rs builds all elfutils including other libraries that we
actually don't need and that will require C++ toolchains for elfutils>=0.190

This commit passes --disable-demangler when configuring elfutils to remove C++
dependency and changes build.rs to only build libelf when building elfutils.
It shortens the build time significantly.
(From 22 seconds to 13 seconds for a clean build on my machine)
@anakryiko
Copy link
Member

I don't see a reason why we'd need the demangler, but it's possible libbpf may need/use it for uprobe attachment or similar? @anakryiko does libbpf use/rely on demangling functionality somewhere?

No, libbpf doesn't do any demangling.

@danielocfb danielocfb merged commit 9b7afd7 into libbpf:master Aug 19, 2024
10 checks passed
@kxxt
Copy link
Contributor Author

kxxt commented Aug 24, 2024

Hi @danielocfb, it would be nice if you could release a new version of libbpf-sys to make the fix available on crates.io.

@danielocfb
Copy link
Collaborator

Yes, we should do that. Will probably take a few days if that's alright. I want to get #98 in first.

@danielocfb
Copy link
Collaborator

danielocfb commented Aug 28, 2024

Hi @danielocfb, it would be nice if you could release a new version of libbpf-sys to make the fix available on crates.io.

We just released https://crates.io/crates/libbpf-sys/1.4.5+v1.4.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants