Re: [PATCH] rust: print: Fix issue with rust_build_error

[Gary Guo <gary@garyguo.net>]

On Sun, 21 Sep 2025 09:12:45 +0200
"Benno Lossin" <lossin@kernel.org> wrote:

> On Sun Sep 21, 2025 at 2:45 AM CEST, Joel Fernandes wrote:
> > On Sat, Sep 20, 2025 at 10:09:30PM +0200, Benno Lossin wrote:  
> >> On Sat Sep 20, 2025 at 6:19 PM CEST, Joel Fernandes wrote:  
> >> > When printing just before calling io.write32(), modpost fails due to
> >> > build_assert's missing rust_build_error symbol. The issue is that, the
> >> > printk arguments are passed as reference in bindings code, thus Rust
> >> > cannot trust its value and fails to optimize away the build_assert.
> >> >
> >> > The issue can be reproduced with the following simple snippet:
> >> >   let offset = 0;
> >> >   pr_err!("{}", offset);
> >> >   io.write32(base, offset);  
> >> 
> >> I took a long time to understand this and I think I got it now, but
> >> maybe I'm still wrong: passing `offset` to `printk` via a reference
> >> results in the compiler thinking that the value of `offset` might be
> >> changed (even though its a shared reference I assume). For this reason
> >> the `build_assert!` used by `io.write32` cannot be optimized away.  
> >
> > Yes, that's correct and that's my understanding. I in fact also dumped the IR
> > and see that the compiler is trying to reload the pointer to the offset. I
> > feel this is a compiler bug, and for immutable variables, the compiler should
> > not be reloading them even if they are passed to external code? Because if
> > external code is modifying immutable variables, that is buggy anyway.  
> 
> It would be great if you could add all of the extra info that you looked
> at into the commit message here. So all of the code to reproduce the
> issue, the IR you looked at...
> 
> >> > Fix it by just using a closure to call printk. Rust captures the
> >> > arguments into the closure's arguments thus breaking the dependency.
> >> > This can be fixed by simply creating a variable alias for each variable
> >> > however the closure is a simple and concise fix.  
> >> 
> >> I don't think this is the fix we want to have.  
> >
> > Yeah its ugly, though a small workaround. IMO ideally the fix should be in
> > the compiler. I want to send a proposal for this in fact. I looked at the MIR
> > and I believe with my limited understanding, that the MIR should be issuing a
> > copy before making a pointer of the immutable variable if pointers to
> > immutable variables are being passed to external functions.  
> 
> ... and definitely the MIR.
> 
> > If I were to send a proposal to the Rust language to start a discussion
> > leading to a potential RFC stage, would you mind guiding me on doing so?  
> 
> I agree that this should be fixed in the compiler if we aren't missing
> some attribute on one of our functions.
> 
> Note that this probably doesn't require an RFC, since it's not a big
> feature and I imagine that there isn't much controversy about it. We can
> mention this in our sync meeting on Wednesday with the Rust folks & see
> what they have to say about it. After that we probably want to open an
> issue about it, I'll let you know.
> 
> >> In fact it already
> >> doesn't compile all of the existing code:  
> >
> > Oh gosh, I should have run doctests. I just did a normal build. Let me see
> > if I can fix this closure issue.  
> 
> We might just want to live with this until we get the compiler fixed. Or
> is there a different workaround like this:
> 
>     let offset = 0;
>     let offset_pr = offset;
>     pr_err!("{}", offset_pr);
>     io.write32(base, offset);

I suggested to Joel during Kangrejos that just

	pr_err!("{}", {offset});

should work, as it would create a new copy.

Best,
Gary