aya: add symbol lookup in associated debug files

[swananan]

This change enhances the logic for symbol lookup in uprobe or uretprobe. If the symbol is not found in the original binary, the search continues in the debug file associated through the debuglink section. Before searching the symbol table, it compares the build IDs of the two files. The symbol lookup will only be terminated if both build IDs exist and do not match. This modification does not affect the existing symbol lookup logic.

Refs: #936

[netlify]

❌ Deploy Preview for aya-rs-docs failed.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	e6e1bfe
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/663b5067d474480007525aed

[alessandrod]

Thanks! This looks great. Could you add a test for the debuglink resolution?

[alessandrod]

Thanks! This looks great. Could you add a test for the debuglink resolution?

Actually thinking of it, given our current integration-test setup this is probably harder than it should be. Please take a look but if it's too cumbersome I'm fine just smoke testing this locally for now.

[swananan]

Thanks! This looks great. Could you add a test for the debuglink resolution?

Actually thinking of it, given our current integration-test setup this is probably harder than it should be. Please take a look but if it's too cumbersome I'm fine just smoke testing this locally for now.

I was initially planning to add UT, haha. However, I realized I needed to hook the std::fs::read function. If I use the object crate to prepare the testing file in memory (since preparing a real file might be too burdensome), and I'm unsure whether introducing a file mocking crate like fake-fs would be a good idea, especially since I am a new beginner to Rust and Aya.

I took a look at the integration test, but for this particular change, I think UT might be a more suitable choice because it doesn't affect the ebpf code part. If I am mistaken, please correct me.

If it's okay, I will add some UTs using the object and fake-fs crates as soon as possible.

[alessandrod]

I was initially planning to add UT, haha. However, I realized I needed to hook the std::fs::read function

unit test works! But why do you need to hook std::fs::read?

[swananan]

I was initially planning to add UT, haha. However, I realized I needed to hook the std::fs::read function

unit test works! But why do you need to hook std::fs::read?

Since std::fs::read is called by resolve_symbol, I think I need to mock this function. This is because I don't intend to provide a real file path for resolve_symbol to process.

[alessandrod]

Since std::fs::read is called by resolve_symbol, I think I need to mock this function. This is because I don't intend to provide a real file path for resolve_symbol to process.

You could introduce a new resolve_symbol_data(...) and call it from resolve_symbol. resolve_symbol would fs::read then call resolve_symbol_data with the content. Then you could unit test resolve_symbol_data and not need fs access?

[addisoncrump]

Super cool. I work a lot with stripped binaries for which I have separate debug symbols, so this will be a real help.

[swananan]

@alessandrod could you review this pr? thanks

[alessandrod]

@alessandrod could you review this pr? thanks

Yep sorry been travelling this week, will take a look soon

[alessandrod]

quality PR, thanks so much! Just a nit and please fix the lint issue

[alessandrod]

nit: why not unwrap here?

[swananan]

oh, I totally agree.

[swananan]

Hi @alessandrod this PR failed the public api check phase, but I am not sure if it's the root cause, could you help take a look? thanks!

[addisoncrump]

I think you can do cargo xtask public-api --bless or something similar to fix this.

[mergify]

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

[alessandrod]

Hi @alessandrod this PR failed the public api check phase, but I am not sure if it's the root cause, could you help take a look? thanks!

I think adding the write feature to the object crate pulls in a new transitive dependency - the equivalent crate. That crate has a blanket impl (smh), which impacts some of our types that we use as keys in maps. I just pushed a commit which hopefully makes CI green.

[alessandrod]

Can you take a look at the test failure under miri?

[swananan]

Can you take a look at the test failure under miri?

I looked into this failure and found an exception, Invalid ELF header size or alignment, originating from the object crate. I checked the main_bytes generated by create_elf_with_build_id, and the ELF header size is 64, which seems correct.

However, further investigation into the object crate showed that it has a strict address alignment requirement, leading to a failure in the object::File::parse API (see this link). My understanding is that in Rust, a Vec's memory comes from the heap and should be aligned to 8 bytes on x64 architectures. However, since I'm new to Rust, I'm not entirely certain about this. Obviously, this alignment is not guaranteed with the miri interpreter (this is my first time hearing about miri, haha).

Given the strict limitation in the object crate, I decided to adjust the Vec to ensure 8-byte alignment in the tests as a workaround for miri (I am not sure if it's a good idea = =||). I've added a new commit with these changes. Could you please review it and let me know your thoughts?

[alessandrod]

Yep ELF files must be aligned. Thank you again!