Re: [PATCH v2 08/13] rust: cleanup module_init!, use it from #[derive(Object)]

[Kevin Wolf <kwolf@redhat.com>]

Am 22.10.2024 um 08:00 hat Junjie Mao geschrieben:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > On Tue, Oct 22, 2024 at 4:12 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
> >> > +    ($type:ident => $body:block) => {
> >> > +        const _: () = {
> >> > +            #[used]
> >> > +            #[cfg_attr(
> >> > +                not(any(target_vendor = "apple", target_os = "windows")),
> >> > +                link_section = ".init_array"
> >> > +            )]
> >> > +            #[cfg_attr(target_vendor = "apple", link_section = "__DATA,__mod_init_func")]
> >> > +            #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
> >> > +            pub static LOAD_MODULE: extern "C" fn() = {
> >> > +                extern "C" fn init_fn() {
> >>
> >> init_fn() should be unsafe fn according to the signature of
> >> register_module_init.
> >
> > I think it *can* be unsafe (which bindgen does by default). It's
> > always okay to pass a non-unsafe function as unsafe function pointer:
> >
> > fn f() {
> >     println!("abc");
> > }
> >
> > fn g(pf: unsafe fn()) {
> >     unsafe {
> >         pf();
> >     }
> > }
> >
> > fn main() {
> >     g(f);
> > }
> >
> >> Being unsafe fn also makes sense here because it
> >> is the caller, not init_fn() itself, that is responsible for the safety of
> >> the unsafe code in the body.
> >
> > Isn't it the opposite? Since the caller of module_init! is responsible
> > for the safety, init_fn() itself can be safe.
> 
> My understanding is that:
> 
>   1. init_fn() is called by QEMU QOM subsystem with certain timing
>      (e.g., called after main()) and concurrency (e.g., all init_fn()
>      are called sequentially) preconditions.

Though these conditions are not a matter of safety, but of correctness.

>   2. The caller of module_init! is responsible for justifying the safety
>      of their $body under the preconditions established in #1.
> 
> "unsafe fn" in Rust is designed to mark functions with safety
> preconditions so that the compiler can raise an error if the caller
> forgets that it has those preconditions to uphold [1].

I don't think we expect to have preconditions here that are required for
safety (in the sense with which the term is used in Rust).

But safe code is not automatically correct.

If you added "unsafe" for every function that requires some
preconditions to be met so that it functions correctly (instead of only
so that it doesn't have undefined behaviour on the language level), then
you would annotate almost every function as "unsafe".

I think the rule of thumb for marking a function unsafe is when you have
unsafe code inside of it whose safety (not correctness!) depends on a
condition that the caller must reason about because you can't guarantee
it in the function itself.

This macro should probably only be used with code that can guarantee the
safety of its unsafe blocks in itself. The C side of constructors can't
make many guarantees anyway, and there is nobody who would reason about
them.

Kevin