| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJ-ks9=t2rrwgXDwVtqhZz+K02E-2jNprfu-Zy5bb14b0bNhSw@mail.gmail.com>
Date: Sun, 4 Jan 2026 17:53:04 -0500
From: Tamir Duberstein <tamird@...il.com>
To: Gary Guo <gary@...yguo.net>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Fiona Behrens <me@...enk.dev>, Patrick Miller <paddymills@...ton.me>,
José Expósito <jose.exposito89@...il.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Guilherme Giacomo Simoes <trintaeoitogc@...il.com>, rust-for-linux@...r.kernel.org,
Igor Korotin <igor.korotin.linux@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/11] rust: macros: use `syn` to parse `module!` macro
On Thu, Dec 11, 2025 at 2:29 PM Gary Guo <gary@...nel.org> wrote:
>
> From: Gary Guo <gary@...yguo.net>
>
> With `syn` being available in the kernel, use it to parse the complex
> custom `module!` macro to replace existing helpers. Only parsing is
> changed in this commit, the code generation is untouched.
>
> This has the benefit of better error message when the macro is used
> incorrectly, as it can point to a concrete span on what's going wrong.
>
> For example, if a field is specified twice, previously it reads:
>
> error: proc macro panicked
> --> samples/rust/rust_minimal.rs:7:1
> |
> 7 | / module! {
> 8 | | type: RustMinimal,
> 9 | | name: "rust_minimal",
> 10 | | author: "Rust for Linux Contributors",
> 11 | | description: "Rust minimal sample",
> 12 | | license: "GPL",
> 13 | | license: "GPL",
> 14 | | }
> | |_^
> |
> = help: message: Duplicated key "license". Keys can only be specified once.
>
> now it reads:
>
> error: duplicated key "license". Keys can only be specified once.
> --> samples/rust/rust_minimal.rs:13:5
> |
> 13 | license: "GPL",
> | ^^^^^^^
>
> Co-developed-by: Benno Lossin <lossin@...nel.org>
> Signed-off-by: Benno Lossin <lossin@...nel.org>
> Signed-off-by: Gary Guo <gary@...yguo.net>
> ---
> rust/macros/helpers.rs | 84 ++++++----------
> rust/macros/lib.rs | 9 +-
> rust/macros/module.rs | 220 ++++++++++++++++++++++++++++++-----------
> 3 files changed, 202 insertions(+), 111 deletions(-)
>
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index 853527b5d9567..f6bbdb02d9d7d 100644
> --- a/rust/macros/helpers.rs
> +++ b/rust/macros/helpers.rs
> @@ -1,42 +1,21 @@
> // SPDX-License-Identifier: GPL-2.0
>
> -use proc_macro2::{token_stream, Group, Ident, TokenStream, TokenTree};
> -
> -pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
> - if let Some(TokenTree::Ident(ident)) = it.next() {
> - Some(ident.to_string())
> - } else {
> - None
> - }
> -}
> -
> -pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
> - if let Some(TokenTree::Literal(literal)) = it.next() {
> - Some(literal.to_string())
> - } else {
> - None
> - }
> -}
> -
> -pub(crate) fn try_string(it: &mut token_stream::IntoIter) -> Option<String> {
> - try_literal(it).and_then(|string| {
> - if string.starts_with('\"') && string.ends_with('\"') {
> - let content = &string[1..string.len() - 1];
> - if content.contains('\\') {
> - panic!("Escape sequences in string literals not yet handled");
> - }
> - Some(content.to_string())
> - } else if string.starts_with("r\"") {
> - panic!("Raw string literals are not yet handled");
> - } else {
> - None
> - }
> - })
> -}
> -
> -pub(crate) fn expect_ident(it: &mut token_stream::IntoIter) -> String {
> - try_ident(it).expect("Expected Ident")
> -}
> +use proc_macro2::{
> + token_stream,
> + Ident,
> + TokenStream,
> + TokenTree, //
> +};
> +use quote::ToTokens;
> +use syn::{
> + parse::{
> + Parse,
> + ParseStream, //
> + },
> + Error,
> + LitStr,
> + Result, //
> +};
>
> pub(crate) fn expect_punct(it: &mut token_stream::IntoIter) -> char {
> if let TokenTree::Punct(punct) = it.next().expect("Reached end of token stream for Punct") {
> @@ -46,27 +25,28 @@ pub(crate) fn expect_punct(it: &mut token_stream::IntoIter) -> char {
> }
> }
>
> -pub(crate) fn expect_string(it: &mut token_stream::IntoIter) -> String {
> - try_string(it).expect("Expected string")
> -}
> +/// A string literal that is required to have ASCII value only.
> +pub(crate) struct AsciiLitStr(LitStr);
>
> -pub(crate) fn expect_string_ascii(it: &mut token_stream::IntoIter) -> String {
> - let string = try_string(it).expect("Expected string");
> - assert!(string.is_ascii(), "Expected ASCII string");
> - string
> +impl Parse for AsciiLitStr {
> + fn parse(input: ParseStream<'_>) -> Result<Self> {
> + let s: LitStr = input.parse()?;
> + if !s.value().is_ascii() {
> + return Err(Error::new_spanned(s, "expected ASCII-only string literal"));
> + }
> + Ok(Self(s))
> + }
> }
>
> -pub(crate) fn expect_group(it: &mut token_stream::IntoIter) -> Group {
> - if let TokenTree::Group(group) = it.next().expect("Reached end of token stream for Group") {
> - group
> - } else {
> - panic!("Expected Group");
> +impl ToTokens for AsciiLitStr {
> + fn to_tokens(&self, ts: &mut TokenStream) {
> + self.0.to_tokens(ts);
> }
> }
>
> -pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
> - if it.next().is_some() {
> - panic!("Expected end");
> +impl AsciiLitStr {
> + pub(crate) fn value(&self) -> String {
> + self.0.value()
> }
> }
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 33935b38d11c0..4e440deaed853 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -10,6 +10,9 @@
> // which was added in Rust 1.88.0. This is why `cfg_attr` is used here, i.e.
> // to avoid depending on the full `proc_macro_span` on Rust >= 1.88.0.
> #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
> +//
> +// Stable since Rust 1.81.0.
> +#![feature(lint_reasons)]
>
> mod concat_idents;
> mod export;
> @@ -100,8 +103,10 @@
> /// - `firmware`: array of ASCII string literals of the firmware files of
> /// the kernel module.
> #[proc_macro]
> -pub fn module(ts: TokenStream) -> TokenStream {
> - module::module(ts.into()).into()
> +pub fn module(input: TokenStream) -> TokenStream {
> + module::module(parse_macro_input!(input))
> + .unwrap_or_else(|e| e.into_compile_error())
> + .into()
> }
>
> /// Declares or implements a vtable trait.
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index 6974fb04f58fa..e4d51878a5360 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -2,28 +2,27 @@
>
> use std::fmt::Write;
>
> -use proc_macro2::{token_stream, Delimiter, Literal, TokenStream, TokenTree};
> +use proc_macro2::{
> + Literal,
> + TokenStream, //
> +};
> +use syn::{
> + bracketed,
> + parse::{
> + Parse,
> + ParseStream, //
> + },
> + punctuated::Punctuated,
> + token::Bracket,
> + Error,
> + Ident,
> + LitStr,
> + Result,
> + Token, //
> +};
>
> use crate::helpers::*;
>
> -fn expect_string_array(it: &mut token_stream::IntoIter) -> Vec<String> {
> - let group = expect_group(it);
> - assert_eq!(group.delimiter(), Delimiter::Bracket);
> - let mut values = Vec::new();
> - let mut it = group.stream().into_iter();
> -
> - while let Some(val) = try_string(&mut it) {
> - assert!(val.is_ascii(), "Expected ASCII string");
> - values.push(val);
> - match it.next() {
> - Some(TokenTree::Punct(punct)) => assert_eq!(punct.as_char(), ','),
> - None => break,
> - _ => panic!("Expected ',' or end of array"),
> - }
> - }
> - values
> -}
> -
> struct ModInfoBuilder<'a> {
> module: &'a str,
> counter: usize,
> @@ -91,8 +90,107 @@ fn emit(&mut self, field: &str, content: &str) {
> }
> }
>
> +mod kw {
> + syn::custom_keyword!(name);
> + syn::custom_keyword!(authors);
> + syn::custom_keyword!(description);
> + syn::custom_keyword!(license);
> + syn::custom_keyword!(alias);
> + syn::custom_keyword!(firmware);
> +}
> +
> +#[allow(dead_code, reason = "some fields are only parsed into")]
Use #[expect(...)] so that the compiler can catch if this is no longer
dead code.
> +enum ModInfoField {
> + Type(Token![type], Token![:], Ident),
> + Name(kw::name, Token![:], AsciiLitStr),
> + Authors(
> + kw::authors,
> + Token![:],
> + Bracket,
> + Punctuated<LitStr, Token![,]>,
> + ),
Per the documentation on macros::module, authors must be ASCII.
> + Description(kw::description, Token![:], LitStr),
> + License(kw::license, Token![:], AsciiLitStr),
> + Alias(
> + kw::authors,
This should be kw::alias
> + Token![:],
> + Bracket,
> + Punctuated<LitStr, Token![,]>,
> + ),
Per the documentation on macros::module, alias must be ASCII.
> + Firmware(
> + kw::firmware,
> + Token![:],
> + Bracket,
> + Punctuated<LitStr, Token![,]>,
> + ),
Must be ASCII.
> +}
> +
> +impl ModInfoField {
> + /// Obtain the key identifying the field.
> + fn key(&self) -> Ident {
> + match self {
> + ModInfoField::Type(key, ..) => Ident::new("type", key.span),
> + ModInfoField::Name(key, ..) => Ident::new("name", key.span),
> + ModInfoField::Authors(key, ..) => Ident::new("authors", key.span),
> + ModInfoField::Description(key, ..) => Ident::new("description", key.span),
> + ModInfoField::License(key, ..) => Ident::new("license", key.span),
> + ModInfoField::Alias(key, ..) => Ident::new("alias", key.span),
> + ModInfoField::Firmware(key, ..) => Ident::new("firmware", key.span),
> + }
> + }
> +}
> +
> +impl Parse for ModInfoField {
> + fn parse(input: ParseStream<'_>) -> Result<Self> {
> + let key = input.lookahead1();
> + if key.peek(Token![type]) {
> + Ok(Self::Type(input.parse()?, input.parse()?, input.parse()?))
> + } else if key.peek(kw::name) {
> + Ok(Self::Name(input.parse()?, input.parse()?, input.parse()?))
> + } else if key.peek(kw::authors) {
> + let list;
> + Ok(Self::Authors(
> + input.parse()?,
> + input.parse()?,
> + bracketed!(list in input),
> + Punctuated::parse_terminated(&list)?,
> + ))
> + } else if key.peek(kw::description) {
> + Ok(Self::Description(
> + input.parse()?,
> + input.parse()?,
> + input.parse()?,
> + ))
> + } else if key.peek(kw::license) {
> + Ok(Self::License(
> + input.parse()?,
> + input.parse()?,
> + input.parse()?,
> + ))
> + } else if key.peek(kw::alias) {
> + let list;
> + Ok(Self::Alias(
> + input.parse()?,
> + input.parse()?,
> + bracketed!(list in input),
> + Punctuated::parse_terminated(&list)?,
> + ))
> + } else if key.peek(kw::firmware) {
> + let list;
> + Ok(Self::Firmware(
> + input.parse()?,
> + input.parse()?,
> + bracketed!(list in input),
> + Punctuated::parse_terminated(&list)?,
> + ))
> + } else {
> + Err(key.error())
> + }
> + }
> +}
> +
> #[derive(Debug, Default)]
> -struct ModuleInfo {
> +pub(crate) struct ModuleInfo {
> type_: String,
> license: String,
> name: String,
> @@ -102,9 +200,13 @@ struct ModuleInfo {
> firmware: Option<Vec<String>>,
> }
>
> -impl ModuleInfo {
> - fn parse(it: &mut token_stream::IntoIter) -> Self {
> - let mut info = ModuleInfo::default();
> +impl Parse for ModuleInfo {
> + fn parse(input: ParseStream<'_>) -> Result<Self> {
> + let mut info = Self::default();
> +
> + let span = input.span();
> + let fields = Punctuated::<ModInfoField, Token![,]>::parse_terminated(input)?;
> + let mut errors = Vec::new();
>
> const EXPECTED_KEYS: &[&str] = &[
> "type",
> @@ -118,40 +220,38 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
> const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
> let mut seen_keys = Vec::new();
>
> - loop {
> - let key = match it.next() {
> - Some(TokenTree::Ident(ident)) => ident.to_string(),
> - Some(_) => panic!("Expected Ident or end"),
> - None => break,
> - };
> + for field in fields {
> + let key = field.key();
>
> if seen_keys.contains(&key) {
> - panic!("Duplicated key \"{key}\". Keys can only be specified once.");
> + errors.push(Error::new_spanned(
> + &key,
> + format!(r#"duplicated key "{key}". Keys can only be specified once."#),
> + ));
> + continue;
> }
> + seen_keys.push(key);
>
> - assert_eq!(expect_punct(it), ':');
> -
> - match key.as_str() {
> - "type" => info.type_ = expect_ident(it),
> - "name" => info.name = expect_string_ascii(it),
> - "authors" => info.authors = Some(expect_string_array(it)),
> - "description" => info.description = Some(expect_string(it)),
> - "license" => info.license = expect_string_ascii(it),
> - "alias" => info.alias = Some(expect_string_array(it)),
> - "firmware" => info.firmware = Some(expect_string_array(it)),
> - _ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
> + match field {
> + ModInfoField::Type(_, _, ty) => info.type_ = ty.to_string(),
> + ModInfoField::Name(_, _, name) => info.name = name.value(),
> + ModInfoField::Authors(_, _, _, list) => {
> + info.authors = Some(list.into_iter().map(|x| x.value()).collect())
> + }
> + ModInfoField::Description(_, _, desc) => info.description = Some(desc.value()),
> + ModInfoField::License(_, _, license) => info.license = license.value(),
> + ModInfoField::Alias(_, _, _, list) => {
> + info.alias = Some(list.into_iter().map(|x| x.value()).collect())
> + }
> + ModInfoField::Firmware(_, _, _, list) => {
> + info.firmware = Some(list.into_iter().map(|x| x.value()).collect())
> + }
> }
> -
> - assert_eq!(expect_punct(it), ',');
> -
> - seen_keys.push(key);
> }
>
> - expect_end(it);
> -
> for key in REQUIRED_KEYS {
> if !seen_keys.iter().any(|e| e == key) {
> - panic!("Missing required key \"{key}\".");
> + errors.push(Error::new(span, format!(r#"missing required key "{key}""#)));
> }
> }
>
> @@ -163,18 +263,24 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
> }
>
> if seen_keys != ordered_keys {
> - panic!("Keys are not ordered as expected. Order them like: {ordered_keys:?}.");
> + errors.push(Error::new(
> + span,
> + format!(r#"keys are not ordered as expected. Order them like: {ordered_keys:?}."#),
Unnecessary raw string here.
> + ));
> + }
> +
> + if let Some(err) = errors.into_iter().reduce(|mut e1, e2| {
> + e1.combine(e2);
> + e1
> + }) {
> + return Err(err);
> }
>
> - info
> + Ok(info)
> }
> }
>
> -pub(crate) fn module(ts: TokenStream) -> TokenStream {
> - let mut it = ts.into_iter();
> -
> - let info = ModuleInfo::parse(&mut it);
> -
> +pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
> // Rust does not allow hyphens in identifiers, use underscore instead.
> let ident = info.name.replace('-', "_");
> let mut modinfo = ModInfoBuilder::new(ident.as_ref());
> @@ -203,7 +309,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
> std::env::var("RUST_MODFILE").expect("Unable to fetch RUST_MODFILE environmental variable");
> modinfo.emit_only_builtin("file", &file);
>
> - format!(
> + Ok(format!(
> "
> /// The module name.
> ///
> @@ -377,5 +483,5 @@ unsafe fn __exit() {{
> initcall_section = ".initcall6.init"
> )
> .parse()
> - .expect("Error parsing formatted string into token stream.")
> + .expect("Error parsing formatted string into token stream."))
> }
> --
> 2.51.2
>
Powered by blists - more mailing lists