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

Cross-compilation is broken #94

Closed
kxxt opened this issue Aug 10, 2024 · 9 comments
Closed

Cross-compilation is broken #94

kxxt opened this issue Aug 10, 2024 · 9 comments

Comments

@kxxt
Copy link
Contributor

kxxt commented Aug 10, 2024

libbpf-sys cannot be cross-compiled.

default-features

There's no way to specify include path for cross-compilation. I opened #93 to fix it.

cargo build --target aarch64-unknown-linux-gnu 
  In file included from bpf_prog_linfo.c:9:
  libbpf_internal.h:19:10: fatal error: libelf.h: No such file or directory
     19 | #include <libelf.h>
        |          ^~~~~~~~~~
  compilation terminated.
  In file included from nlattr.c:14:
  libbpf_internal.h:19:10: fatal error: libelf.h: No such file or directory
     19 | #include <libelf.h>
        |          ^~~~~~~~~~
  compilation terminated.
  In file included from libbpf_errno.c:15:
  libbpf_internal.h:19:10: fatal error: libelf.h: No such file or directory
     19 | #include <libelf.h>
        |          ^~~~~~~~~~
  compilation terminated.
  make: *** [Makefile:134: /home/kxxt/repos/libbpf-sys/target/aarch64-unknown-linux-gnu/debug/build/libbpf-sys-099efd81f8db105f/out/obj/staticobjs/nlattr.o] Error 1
  make: *** Waiting for unfinished jobs....
  make: *** [Makefile:134: /home/kxxt/repos/libbpf-sys/target/aarch64-unknown-linux-gnu/debug/build/libbpf-sys-099efd81f8db105f/out/obj/staticobjs/bpf_prog_linfo.o] Error 1
  make: *** [Makefile:134: /home/kxxt/repos/libbpf-sys/target/aarch64-unknown-linux-gnu/debug/build/libbpf-sys-099efd81f8db105f/out/obj/staticobjs/libbpf_errno.o] Error 1
  btf.c:18:10: fatal error: gelf.h: No such file or directory
     18 | #include <gelf.h>
        |          ^~~~~~~~
  compilation terminated.

static, vendored

cargo build --target aarch64-unknown-linux-gnu -F static,vendored

  Makefile.am: installing './INSTALL'
  backends/Makefile.am: installing 'config/depcomp'
  configure.ac: installing 'config/ylwrap'
  parallel-tests: installing 'config/test-driver'
  configure: error: in '/home/kxxt/repos/libbpf-sys/elfutils':
  configure: error: cannot run C compiled programs.
  If you meant to cross compile, use '--host'.
  See 'config.log' for more details
  thread 'main' panicked at build.rs:305:5:
  make failed
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

After I modify build.rs to pass --host aarch64-linux-gnu to configure, another error:

  --- stderr
  configure: WARNING: not running biarch tests,  does not work
  /home/kxxt/repos/libbpf-sys/elfutils/libcpu/i386_parse.y:243.1-8: warning: POSIX Yacc does not support %defines [-Wyacc]
    243 | %defines
        | ^~~~~~~~
  /bin/sh: line 1: ./i386_gendis: cannot execute binary file: Exec format error
  /bin/sh: line 1: ./i386_gendis: cannot execute binary file: Exec format error
  make[1]: *** [Makefile:814: i386_dis.h] Error 126
  make[1]: *** Waiting for unfinished jobs....
  make[1]: *** [Makefile:814: x86_64_dis.h] Error 126
  make: *** [Makefile:542: install-recursive] Error 1
  thread 'main' panicked at build.rs:318:5:
  make failed
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This is an upstream bug(https://sourceware.org/bugzilla/show_bug.cgi?id=28891). Unfortunately, to fix it, we need to import upstream provided release tarballs into this repo instead of using git submodule. Is that acceptable?

@kxxt
Copy link
Contributor Author

kxxt commented Aug 10, 2024

Unfortunately, to fix it, we need to import upstream provided release tarballs into this repo instead of using git submodule. Is that acceptable?

I see that libbpf-sys is already using a mirrored repo of elfutils: https://github.com/libbpf/elfutils-mirror.git

I think the best way to solve the cross-compilation failure is

  1. Convert elfutils-mirror to a fork of elfutils
  2. Disable maintainer mode in build.rs
  3. When updating elfutils, generate the missing header files and commit them:
autoreconf --install --force
./configure --disable-debuginfod --enable-maintainer-mode --without-zstd --disable-libdebuginfod
make -C lib libeu.a
make dist
git add -f libdw/known-dwarf.h libcpu/x86_64_dis.h libcpu/i386_dis.h
git commit -m "generate headers"
git push

This way we have all the required generated headers and cross-compilation is working.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 14, 2024

I see that libbpf-sys is already using a mirrored repo of elfutils: https://github.com/libbpf/elfutils-mirror.git

I think the best way to solve the cross-compilation failure is

1. Convert elfutils-mirror to a fork of elfutils

2. Disable `maintainer mode` in `build.rs`

3. When updating elfutils, generate the missing header files and commit them:
autoreconf --install --force
./configure --disable-debuginfod --enable-maintainer-mode --without-zstd --disable-libdebuginfod
make -C lib libeu.a
make dist
git add -f libdw/known-dwarf.h libcpu/x86_64_dis.h libcpu/i386_dis.h
git commit -m "generate headers"
git push

This way we have all the required generated headers and cross-compilation is working.

I have created a cross-compilation friendly fork: https://github.com/kxxt/libbpf-sys-cross/tree/libbpf-sys-cross using the above workflow. But unfortunately I cannot really use it in my application because of rust-lang/cargo#9227

If this workflow can be accepted, please let me know and I will create a pull request.

@d-e-s-o
Copy link
Contributor

d-e-s-o commented Aug 14, 2024

Very cool! Thanks for digging into this stuff and fixing it. Not the maintainer of libbpf-sys, but my take:

  • for the adjustments to https://github.com/libbpf/elfutils-mirror.git (as per Cross-compilation is broken #94 (comment)): somewhat reluctant to make this a fork of sorts, but adding a few headers until upstream gets their act together (which admittedly may not happen at all...) is probably in the realm of possibilities. My suggestion would be to create a GitHub Action that generates the headers and commits and force-pushes them to a different branch, from which they can be consumed. In my opinion, we should strive for this happening without human input. If you agree, feel free to open a pull request against the mirror and we can work to get it in.
  • the libbpf-sys changes seem sensible to me and I don't see any conceptual roadblocks to merging, but @alexforster should comment. Why do we need libstdc++? Did you update elfutils? I recall newer versions requiring it, but that was not the case with the version we currently mirror.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 15, 2024

until upstream gets their act together (which admittedly may not happen at all...)

I don't think elfutils will fix their bug. This bug has been there for 14 years: https://lists.fedorahosted.org/archives/list/[email protected]/thread/TK47UHJMSDVHMJB5W4A3SLVKHLF7JEGA/

My suggestion would be to create a GitHub Action that generates the headers and commits and force-pushes them to a different branch, from which they can be consumed. In my opinion, we should strive for this happening without human input. If you agree, feel free to open a pull request against the mirror and we can work to get it in.

I totally agree with you. I will try to set up GitHub Actions to do that. But I probably won't be able to open a PR to against the mirror. The GitHub actions should be placed on a new orphan branch to avoid conflicts with upstream. And GitHub PR cannot do that.

the libbpf-sys changes seem sensible to me and I don't see any conceptual roadblocks to merging

Thanks!

Why do we need libstdc++? Did you update elfutils? I recall newer versions requiring it, but that was not the case with the version we currently mirror.

Yes, the security report in my CI warns me about CVE-2024-25260 found in libbpf-sys so I updated elfutils to 0.191

@kxxt
Copy link
Contributor Author

kxxt commented Aug 15, 2024

I have get the GitHub Actions solution working.

So here's how it works:

The CI is updated to test cross-compilation for x86_64, aarch64 and riscv64 gnu targets.

  • We add a workflow to libbpf-sys (not elfutils-mirror)
  • When elfutils need to be updated, trigger the workflow manually with a tag from elfutils-mirror repo(for example: elfutils-0.190) .
  • Then the workflow will generate the missing headers and commit them to elfutils-0.190-cross branch. This branch is then published to elfutils-mirror repo.
  • After that, the workflow will open a pull request to libbpf-sys repo to bump the elfutils submodule.
    • One unfortunately thing is that the CI won't run for this PR because it is opened by a bot. A workaround is manually close and reopen the PR.
    • When the CI is green(hopefully), we can merge that into libbpf-sys repo.
    • Otherwise, we can continue to fix things in that PR to make the CI green and then merge it. (example: build(deps): update elfutils to elfutils-0.190-cross kxxt/libbpf-sys-cross#3)

I will open a PR to libbpf-sys after my previous PR is done: #93

Why do we need libstdc++?

libstdc++ is actually not required if we only build libelf. The build script build all the tool binaries() which not only wastes time and also pulls in C++ dependencies. I fixed it in kxxt#3

@danielocfb
Copy link
Collaborator

danielocfb commented Aug 15, 2024

We add a workflow to libbpf-sys (not elfutils-mirror)

Why not elfutils-mirror, though? I'd rather not have cross-repo dependencies like this. Particularly given that they are not maintained by the same folks.

We can just move the code of elfutils-mirror into a different branch and land the action code on main, assuming that was the issue. Then we should be able to use a cronjob action or a branch pattern to run on all other or specifically patterned branches and create new <...>-cross branches in response.

It is possible I am missing something else, of course.

Why do we need libstdc++?

libstdc++ is actually not required if we only build libelf. The build script build all the tool binaries() which not only wastes time and also pulls in C++ dependencies. I fixed it in kxxt#3

This is great. Feel free to open a pull request.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 16, 2024

Why not elfutils-mirror, though? I'd rather not have cross-repo dependencies like this. Particularly given that they are not maintained by the same folks.

We can just move the code of elfutils-mirror into a different branch and land the action code on main, assuming that was the issue.

Yes, I wanted to do it that way in the first place as well.

Then we should be able to use a cronjob action

That might be possible. But I am not sure how you want to do it in a cronjob.

or a branch pattern to run on all other or specifically patterned branches and create new <...>-cross branches in response.

That's not how GitHub Actions works. Initially I tried to create a workflow that runs on new tags. GitHub Actions uses the workflow file from whatever branch/tag it is running on instead of using the yml file from the default branch. If the branch/tag doesn't have the workflow yml file, the workflow won't run at all.

It is possible I am missing something else, of course.

The workflow yaml file I am using now is a workflow_dispatch one that needs to be triggered manually.
So it is not impossible to place it at the elfutils-mirror repo. But I feel it is not the right place if the workflow isn't fully automatic. By placing it in libbpf-sys repo, we only manually trigger the workflow when we want to update the elfutils submodule and the workflow opens a pull request for it.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 16, 2024

This is great. Feel free to open a pull request.

@danielocfb #95 for optimizing the build time.

@kxxt
Copy link
Contributor Author

kxxt commented Aug 16, 2024

Well, now I think we no longer need to touch elfutils-mirror repo at all.

The needed generated headers are only necessary when building libcpu, which is a library we actually don't need. I am changing #95 into a PR for fixing cross-compilation with static,vendored features.

@kxxt kxxt closed this as completed Aug 21, 2024
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

No branches or pull requests

3 participants