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

add DI support #35

Merged
merged 13 commits into from
Aug 1, 2023
Merged

add DI support #35

merged 13 commits into from
Aug 1, 2023

Conversation

davibe
Copy link
Contributor

@davibe davibe commented Apr 14, 2023

This branch contains my efforts to enable DI support in bpf-linker and modify the LLVM IR output so that it can be loaded and potentially interoperable with C-based eBPF programs.

In short, I created an additional bpf-linker pass that visits and patches DI nodes generated by LLVM. This required two simple patches that I added to LLVM, which have been accepted and merged, but they have not been released yet. You can find the patches here:

While we wait for them to be released, I also rebased them on the rustc fork of LLVM, which you can find here:

In addition, the above requires a version of rust-llvm.sys that exposes such functions to Rust, which you can find here:

Looking forward, due to my limited time, I am unable to commit to anything specific. I welcome anyone to help out and contribute. What I have done so far works for some programs, but it’s unclear what is left to do to support the full Rust syntax. These are the next steps in my mind:

Feel free to reach out to me on Discord (dade). In the meantime, I will be slowly working on the above.

@davibe davibe force-pushed the fix-di branch 8 times, most recently from 01a9e6b to 0f66cdd Compare April 18, 2023 19:46
vadorovsky added a commit to vadorovsky/aya that referenced this pull request Apr 29, 2023
The new `btf-maps` feature enables usage of BTF-defined BPF map
definitions[0] instead of legacy ones.

This is still work in progress and it needs modification of debug info
with bpf-linker, which is being worked on in aya-rs/bpf-linker#35. This
draft PR is a reference for the bpf-linker work for now.

[0] https://lwn.net/Articles/790177/
vadorovsky added a commit to vadorovsky/aya that referenced this pull request May 9, 2023
The new `btf-maps` feature enables usage of BTF-defined BPF map
definitions[0] instead of legacy ones.

This is still work in progress and it needs modification of debug info
with bpf-linker, which is being worked on in aya-rs/bpf-linker#35. This
draft PR is a reference for the bpf-linker work for now.

[0] https://lwn.net/Articles/790177/
vadorovsky added a commit to vadorovsky/aya that referenced this pull request May 9, 2023
The new `btf-maps` feature enables usage of BTF-defined BPF map
definitions[0] instead of legacy ones.

This is still work in progress and it needs modification of debug info
with bpf-linker, which is being worked on in aya-rs/bpf-linker#35. This
draft PR is a reference for the bpf-linker work for now.

[0] https://lwn.net/Articles/790177/
vadorovsky added a commit to vadorovsky/aya that referenced this pull request May 9, 2023
The new `btf-maps` feature enables usage of BTF-defined BPF map
definitions[0] instead of legacy ones.

This is still work in progress and it needs modification of debug info
with bpf-linker, which is being worked on in aya-rs/bpf-linker#35. This
draft PR is a reference for the bpf-linker work for now.

[0] https://lwn.net/Articles/790177/
vadorovsky added a commit to vadorovsky/aya that referenced this pull request May 9, 2023
The new `btf-maps` feature enables usage of BTF-defined BPF map
definitions[0] instead of legacy ones.

This is still work in progress and it needs modification of debug info
with bpf-linker, which is being worked on in aya-rs/bpf-linker#35. This
draft PR is a reference for the bpf-linker work for now.

[0] https://lwn.net/Articles/790177/
@shaneutt
Copy link

@davibe I just wanted to take the time to let you know how much we in the community appreciate the work you're putting in here, thank you very much for your time on this you're opening a lot of doors for the rest of us! 🍻

@tamird tamird force-pushed the fix-di branch 3 times, most recently from e45d59e to 8cbce59 Compare July 26, 2023 19:43
@tamird tamird force-pushed the fix-di branch 6 times, most recently from b5e841b to e4c4eb2 Compare July 28, 2023 18:39
davibe and others added 13 commits July 28, 2023 22:42
This disables testing with rustc's LLVM until our changes have been
cherry-picked to their LLVM fork, and disables installing LLVM from apt
until those changes are included in an LLVM release.
The previous way of stripping (instead of sanitizing) generic names was
not correct. When we define `MyStruct<T>`, then each implementation like
`MyStruct<u32>` or `MyStruct<u64>` is in fact a separate type. So to
differentiate them in BTF, we need to keep all of them with distinct
names.

However, Linux kernel doesn't accept BTF type names with the following
characters: `<`, `>`, `::`, `,`, ` `. So we need to sanitize them.
First of all, change the name of `strip_generics` function to
`sanitize_type_name`. The goal of it is more general - it makes sure
that Rust type names are sanitized to be compatible with C.

Then, instead of using `replace`, iterate over all characters once and
replace characters unsupported by C with their hex representations.
The memory layout of a struct in Rust is undefined[0]; when struct
fields are reordered by the compiler, we can end up with DI which has
non-monotonic field offsets. The verifier does not like this[1].

Sort struct fields by offset in DI to please the verifier.

[0] https://github.com/rust-lang/reference/blob/master/src/types/struct.md
[1] torvalds/linux@6283fa3
Previously we were sanitizing only struct types
(`DW_tag_structure_type`). Here it's done also for functions
(`DW_tag_subprogram`).

This change also fixes the `sanitize_type_name` function to accept the
`_` character. It's a valid character in C types and converting it to
`_5F_` is not great for readability.
Requires a compiletest-rs release containing
Manishearth/compiletest-rs#278.
@tamird tamird changed the base branch from main to feature/fix-di August 1, 2023 12:52
@vadorovsky vadorovsky marked this pull request as ready for review August 1, 2023 12:57
Copy link
Member

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

Thanks for your work and getting the whole BTF effort started! 🙏

@vadorovsky vadorovsky merged commit 4403d05 into aya-rs:feature/fix-di Aug 1, 2023
7 checks passed
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.

4 participants