* [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 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
* 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
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