Recommendation around panic-handler and use of default-members

[thomaseizinger]

Instead of using default-members, I opted for the following in my eBPF code:

#[cfg(target_arch = "bpf")]
extern crate panic_halt; // Defines a no-op panic handler (we always compile with `immediate_abort` anyway).

This allows me to just run cargo check in my workspace without the compiler barking at me that I am defining two panic handlers (as the std one is part of the dependency tree via the shared crate and its optional std dependency).

This also means rust-analyzer runs cargo check for this crate as well and provides its lint output.

[tamird]

We intentionally don't want a no-op panic handler because that would potentially cause panicking programs to verify.

But I think you can just update the existing panic handlers we have in the examples to be conditional on target_arch and remove default-members.

Would you like to send a PR?

[thomaseizinger]

But I think you can just update the existing panic handlers we have in the examples to be conditional on target_arch and remove default-members.

Would you like to send a PR?

Yep can do! Was there any other reason why you use default-members?

We intentionally don't want a no-op panic handler because that would potentially cause panicking programs to verify.

The panic-halt crate is the same code though as used in the hello-xdp example: loop {}

https://github.com/korken89/panic-halt/blob/master/src/lib.rs

[tamird]

But I think you can just update the existing panic handlers we have in the examples to be conditional on target_arch and remove default-members.
Would you like to send a PR?

Yep can do! Was there any other reason why you use default-members?

I honestly don't remember. CI will let us know.

We intentionally don't want a no-op panic handler because that would potentially cause panicking programs to verify.

The panic-halt crate is the same code though as used in the hello-xdp example: loop {}

https://github.com/korken89/panic-halt/blob/master/src/lib.rs

Ah, I hadn't looked. I just went by your description.

[thomaseizinger]

I called it "no-op" because that is essentially what it is compared to crates like panic-abort which call into intrinsics.

In my experience with eBPF, code that actually wants to panic doesn't compile in the first place. LLVM complains that abort is an undefined symbol or something. So really, this is just a place-holder.

To be fair, it is probably not needed to use a crate. I just used it because I couldn't figure out how to configure rust-analyzer to type-check my code without errors. Correctly feature-gateing (which I discovered now) works so we can go back to the inline panic-handler :)

[thomaseizinger]

So I sent a bunch of PRs. Weirdly enough, I couldn't replicate in your repo what I have working in mine in regards to removing default_members. I don't need it in our workspace and cargo test / cargo build across the workspace still works fine.

[thomaseizinger]

Aha!

What we can do is the following:

diff --git a/examples/aya-tool/myapp-ebpf/src/main.rs b/examples/aya-tool/myapp-ebpf/src/main.rs
index 1892359..31b4ace 100644
--- a/examples/aya-tool/myapp-ebpf/src/main.rs
+++ b/examples/aya-tool/myapp-ebpf/src/main.rs
@@ -1,5 +1,5 @@
-#![no_std]
-#![no_main]
+#![cfg_attr(target_arch = "bpf", no_std)]
+#![cfg_attr(target_arch = "bpf", no_main)]
 
 #[allow(clippy::all)]
 #[allow(dead_code)]
@@ -51,3 +51,8 @@ unsafe fn try_task_alloc(ctx: LsmContext) -> Result<i32, i32> {
 fn panic(_info: &core::panic::PanicInfo) -> ! {
     loop {}
 }
+
+#[cfg(not(target_arch = "bpf"))]
+fn main() {
+    panic!("This program is meant to be compiled as an eBPF program.");
+}

We can conditionally opt-in to no-std and no-main only if we compile for the eBPF architecture. It means we need to add an fn main() stub though. With this however, cargo build, cargo clippy etc run all just fine on the host system, even for the eBPF crate.

Thoughts?

[tamird]

How does this build for you, without this stub?

[thomaseizinger]

What do you mean how does it build for me?

Without the main stub, it doesn't build unless you build with --target bpfel-unknown-none. That is the issue I'd like to solve (and is what necessitates default-members as far as I understand).

The idea was, if we feature-gate no_std and no_main on the correct architecture, then you can actually build (but not run) the eBPF kernel and thus tools like rust-analyzer and cargo check in the workspace just work.

[tamird]

I meant in your repo, with which your experience motivated this line of PRs.

[tamird]

Clarifying further: you said at the top of this issue that you opted for that panic_halt crate and removed default-members. What I'm asking is: how does that work, given it doesn't work here in the aya repo?