* [PATCH] rust: fmt: reject {:p} format specifier for raw pointers
@ 2025-12-23 3:30 Ke Sun
2025-12-23 6:41 ` Miguel Ojeda
0 siblings, 1 reply; 6+ messages in thread
From: Ke Sun @ 2025-12-23 3:30 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Tamir Duberstein, rust-for-linux, Ke Sun
Add compile-time check in fmt! macro to detect {:p} format specifier
and emit a compile error when it's used.
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
Changes:
- According to Alice's suggestion, attempting to print raw pointers with
{:p} format specifier should not compile at all.
- Link: https://lore.kernel.org/all/aUO1nRdaAISMjMTG@google.com/
---
rust/macros/fmt.rs | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/rust/macros/fmt.rs b/rust/macros/fmt.rs
index 2f4b9f6e22110..76dd3d1ee56ad 100644
--- a/rust/macros/fmt.rs
+++ b/rust/macros/fmt.rs
@@ -36,6 +36,23 @@ pub(crate) fn fmt(input: TokenStream) -> TokenStream {
}
if let Some((name, rest)) = first_str.split_once('}') {
first_str = rest;
+ // Check for {:p} format specifier and emit compile error
+ if name.contains(':') {
+ let (_, format_spec) = name.split_once(':').unwrap_or(("", name));
+ if format_spec.trim() == "p" {
+ let error_msg = "{:p} formatting for raw pointers is not supported!";
+ // Generate compile_error! that returns a format_args! placeholder
+ // This ensures the macro expansion produces a valid expression
+ // Use raw string literal for format! to avoid manual escaping
+ // error_msg already contains escaped quotes, so regular string literal is sufficient
+ return format!(
+ r##"{{ ::core::compile_error!("{}"); ::core::format_args!("") }}"##,
+ error_msg
+ )
+ .parse::<TokenStream>()
+ .unwrap();
+ }
+ }
let name = name.split_once(':').map_or(name, |(name, _)| name);
if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
names.insert(name);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] rust: fmt: reject {:p} format specifier for raw pointers 2025-12-23 3:30 [PATCH] rust: fmt: reject {:p} format specifier for raw pointers Ke Sun @ 2025-12-23 6:41 ` Miguel Ojeda 2025-12-23 8:03 ` Benno Lossin 2025-12-23 11:27 ` Danilo Krummrich 0 siblings, 2 replies; 6+ messages in thread From: Miguel Ojeda @ 2025-12-23 6:41 UTC (permalink / raw) To: Ke Sun, Gary Guo, Benno Lossin, Trevor Gross Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Danilo Krummrich, Tamir Duberstein, rust-for-linux On Tue, Dec 23, 2025 at 4:30 AM Ke Sun <sunke@kylinos.cn> wrote: > > Add compile-time check in fmt! macro to detect {:p} format specifier > and emit a compile error when it's used. > > Signed-off-by: Ke Sun <sunke@kylinos.cn> Please use the `git format-patch -vN` flag so that you can set a version for the patch (it is true that it is fairly different than the other one, so perhaps you did not use v2 for that reason). Regarding the patch itself, ideally what we probably want is to do something similar to the C side, i.e. by default hash the pointers like C does, and then have an explicit way to request the real address when really needed. For the real address case, we could require wrapping the pointer-like argument into another type, so that it is very explicit and opt-in (since having our own custom formatting is harder, though perhaps we should finally look into that -- Cc'ing Gary/Benno may have some ideas in this domain). Otherwise, if we land something like this, how does one print a pointer when needed without having to manually replicate the formatting? `:?` perhaps? But that is almost as easy to make a mistake with as `:p`, so that should probably be included. It is actually what was done in https://github.com/rust-lang/rust-clippy/pull/14792, i.e. both are caught by the lint. Cc'ing Trevor who was involved in reviewing that one. Speaking about the lint: since it got implemented, assuming it works well, then we could just use the lint, which allows developers to at least opt-out when needed, e.g. debugging, until we get the hashing etc. Finally, when the macro shouldn't work, normally we just panic the macro, is the `compiler_error` meant for diagnostics? (By the way, eventually (i.e. not for this patch), it would be nice to point to the user to the docs on pointers are printed, i.e. we should probably expand on pointers on the `fmt!` docs linking to `prink-formats.rst` to explain the hashing when we have that etc.). Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: fmt: reject {:p} format specifier for raw pointers 2025-12-23 6:41 ` Miguel Ojeda @ 2025-12-23 8:03 ` Benno Lossin 2025-12-23 9:12 ` Ke Sun 2025-12-23 15:48 ` Timur Tabi 2025-12-23 11:27 ` Danilo Krummrich 1 sibling, 2 replies; 6+ messages in thread From: Benno Lossin @ 2025-12-23 8:03 UTC (permalink / raw) To: Miguel Ojeda, Ke Sun, Gary Guo, Trevor Gross Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Danilo Krummrich, Tamir Duberstein, rust-for-linux On Tue Dec 23, 2025 at 7:41 AM CET, Miguel Ojeda wrote: > Regarding the patch itself, ideally what we probably want is to do > something similar to the C side, i.e. by default hash the pointers > like C does, and then have an explicit way to request the real address > when really needed. Could that just be some kconfig option or do people want to do that without recompiling? But yeah, as always with these things, we should use the principle of "add those capabilities, which already are available on the C side to Rust". > For the real address case, we could require wrapping the pointer-like > argument into another type, so that it is very explicit and opt-in > (since having our own custom formatting is harder, though perhaps we > should finally look into that -- Cc'ing Gary/Benno may have some ideas > in this domain). Well, we could have something like `PrintRealPointerAddress`. But if someone would really want to print the address of a pointer without using it, they could just do: let ptr: *mut T = ...; pr_info!("pointer value: {:x}", ptr.addr()); So unless we also have a lint on printing `<*mut T>::addr`, then I'd say it's only useful for people that know of it and reviewers still need to be on the lookout for people trying to print pointers. (but I guess this is similar to the C side, where you can just cast to an size_t or whatever is the correct thing, right? (or are there lints for that?)) > Otherwise, if we land something like this, how does one print a > pointer when needed without having to manually replicate the > formatting? `:?` perhaps? But that is almost as easy to make a mistake > with as `:p`, so that should probably be included. It is actually what > was done in https://github.com/rust-lang/rust-clippy/pull/14792, i.e. > both are caught by the lint. Cc'ing Trevor who was involved in > reviewing that one. > > Speaking about the lint: since it got implemented, assuming it works > well, then we could just use the lint, which allows developers to at > least opt-out when needed, e.g. debugging, until we get the hashing > etc. We'd still need something for the older compiler versions, or is it enough to have the lint in the latest/only in some stable releases? > Finally, when the macro shouldn't work, normally we just panic the > macro, is the `compiler_error` meant for diagnostics? Emitting a `compiler_error` normally is more ergonomic wrt. diagnostics, since the erroneous code still gets emitted, which prevents other errors from occurring. Here it probably isn't necessary, but an example where it is important is `#[pin_data]`, if it panics, it also doesn't emit the struct definitions, leading to undefined type errors everywhere the type is used. Cheers, Benno ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: fmt: reject {:p} format specifier for raw pointers 2025-12-23 8:03 ` Benno Lossin @ 2025-12-23 9:12 ` Ke Sun 2025-12-23 15:48 ` Timur Tabi 1 sibling, 0 replies; 6+ messages in thread From: Ke Sun @ 2025-12-23 9:12 UTC (permalink / raw) To: Benno Lossin, Miguel Ojeda, Ke Sun, Gary Guo, Trevor Gross Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Danilo Krummrich, Tamir Duberstein, rust-for-linux On 12/23/25 16:03, Benno Lossin wrote: > On Tue Dec 23, 2025 at 7:41 AM CET, Miguel Ojeda wrote: >> Regarding the patch itself, ideally what we probably want is to do >> something similar to the C side, i.e. by default hash the pointers >> like C does, and then have an explicit way to request the real address >> when really needed. > Could that just be some kconfig option or do people want to do that > without recompiling? But yeah, as always with these things, we should > use the principle of "add those capabilities, which already are > available on the C side to Rust". > >> For the real address case, we could require wrapping the pointer-like >> argument into another type, so that it is very explicit and opt-in >> (since having our own custom formatting is harder, though perhaps we >> should finally look into that -- Cc'ing Gary/Benno may have some ideas >> in this domain). > Well, we could have something like `PrintRealPointerAddress`. But if > someone would really want to print the address of a pointer without > using it, they could just do: > > let ptr: *mut T = ...; > pr_info!("pointer value: {:x}", ptr.addr()); > > So unless we also have a lint on printing `<*mut T>::addr`, then I'd say > it's only useful for people that know of it and reviewers still need to > be on the lookout for people trying to print pointers. (but I guess this > is similar to the C side, where you can just cast to an size_t or > whatever is the correct thing, right? (or are there lints for that?)) Add wrapper types matching C printk's pointer format specifiers: HashedPtr → %p (hashed, default) RestrictedPtr → %pK (restricted, respects kptr_restrict) RawPtr → %px (raw address, debug only) Require using these wrapper types when formatting pointers? > >> Otherwise, if we land something like this, how does one print a >> pointer when needed without having to manually replicate the >> formatting? `:?` perhaps? But that is almost as easy to make a mistake >> with as `:p`, so that should probably be included. It is actually what >> was done in https://github.com/rust-lang/rust-clippy/pull/14792, i.e. >> both are caught by the lint. Cc'ing Trevor who was involved in >> reviewing that one. >> >> Speaking about the lint: since it got implemented, assuming it works >> well, then we could just use the lint, which allows developers to at >> least opt-out when needed, e.g. debugging, until we get the hashing >> etc. > We'd still need something for the older compiler versions, or is it > enough to have the lint in the latest/only in some stable releases? > >> Finally, when the macro shouldn't work, normally we just panic the >> macro, is the `compiler_error` meant for diagnostics? > Emitting a `compiler_error` normally is more ergonomic wrt. diagnostics, > since the erroneous code still gets emitted, which prevents other errors > from occurring. Here it probably isn't necessary, but an example where > it is important is `#[pin_data]`, if it panics, it also doesn't emit the > struct definitions, leading to undefined type errors everywhere the type > is used. > > Cheers, > Benno > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: fmt: reject {:p} format specifier for raw pointers 2025-12-23 8:03 ` Benno Lossin 2025-12-23 9:12 ` Ke Sun @ 2025-12-23 15:48 ` Timur Tabi 1 sibling, 0 replies; 6+ messages in thread From: Timur Tabi @ 2025-12-23 15:48 UTC (permalink / raw) To: gary@garyguo.net, tmgross@umich.edu, lossin@kernel.org, miguel.ojeda.sandonis@gmail.com, sunke@kylinos.cn Cc: a.hindborg@kernel.org, rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com, boqun.feng@gmail.com, dakr@kernel.org, tamird@gmail.com, ojeda@kernel.org, aliceryhl@google.com On Tue, 2025-12-23 at 09:03 +0100, Benno Lossin wrote: > > Regarding the patch itself, ideally what we probably want is to do > > something similar to the C side, i.e. by default hash the pointers > > like C does, and then have an explicit way to request the real address > > when really needed. > > Could that just be some kconfig option or do people want to do that > without recompiling? But yeah, as always with these things, we should > use the principle of "add those capabilities, which already are > available on the C side to Rust". There already is a command-line parameter, hash_pointers, that does this. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: fmt: reject {:p} format specifier for raw pointers 2025-12-23 6:41 ` Miguel Ojeda 2025-12-23 8:03 ` Benno Lossin @ 2025-12-23 11:27 ` Danilo Krummrich 1 sibling, 0 replies; 6+ messages in thread From: Danilo Krummrich @ 2025-12-23 11:27 UTC (permalink / raw) To: Miguel Ojeda Cc: Ke Sun, Gary Guo, Benno Lossin, Trevor Gross, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Tamir Duberstein, rust-for-linux On Tue Dec 23, 2025 at 7:41 AM CET, Miguel Ojeda wrote: > Regarding the patch itself, ideally what we probably want is to do > something similar to the C side, i.e. by default hash the pointers > like C does, and then have an explicit way to request the real address > when really needed. Indeed, but I think the latter is even more important: In C %p is commonly used for debugging when the actual address does not matter, but a unique object needs to be identified (in quite a lot of cases to hunt down memory bugs). In Rust we should have quite fewer memory bugs to begin with (there may still be ones in unsafe code, e.g. in abstractions) and we have built-in language capabilities to easily derive debug prints for objects, which are more useful then just a semi-unique identifier in a lot of cases. So, what we are (mostly) left with I think are cases where we do want to print the actual address temporarily for debug purposes, e.g. when interacting with hardware (DMA memory, shared memory, etc.). ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-12-23 15:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-23 3:30 [PATCH] rust: fmt: reject {:p} format specifier for raw pointers Ke Sun
2025-12-23 6:41 ` Miguel Ojeda
2025-12-23 8:03 ` Benno Lossin
2025-12-23 9:12 ` Ke Sun
2025-12-23 15:48 ` Timur Tabi
2025-12-23 11:27 ` Danilo Krummrich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox