-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Fix stack overflow when macro_rules! macros shadow builtin attributes or types #142946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Compiler also crashes with SIGBUS when shadowing type with RUST_MIN_STACK=1048576: #[macro_export]
macro_rules! String {
() => {
fn get_string() -> String {
"hello".to_owned()
}
};
}
#[cfg(test)]
mod tests {
use super::*;
String!();
#[test]
fn test_string_shadow() {
assert_eq!(get_string(), "hello");
}
} Name collision between proc macros and macro rules (SIGBUS): // Trigger name resolution bug before
// compiler validates crate structure
extern crate proc_macro;
use proc_macro::TokenStream;
#[proc_macro_attribute]
pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream {
}
#[test]
fn my_test_function() {
assert_eq!(2 + 2, 4);
}
// Never gets here
fn main() {
println!("Hi!");
} More cases (SIGBUS): [package]
name = "test_proc_macro"
version = "0.1.0"
edition = "2024"
[lib]
proc-macro = true
[dependencies]
extern crate proc_macro;
use proc_macro::TokenStream;
/// A procedural attribute macro whose only purpose is to create a name collision.
#[proc_macro_attribute]
pub fn ambiguous_macro(_attr: TokenStream, item: TokenStream) -> TokenStream {
item
}
[package]
name = "my_app"
version = "0.1.0"
edition = "2024"
[dependencies]
test_proc_macro = { path = "../test_proc_macro" }
use test_proc_macro::ambiguous_macro;
#[macro_export]
macro_rules! ambiguous_macro {
() => {
fn created_by_macro_rules() {}
};
}
fn main() {
ambiguous_macro!();
created_by_macro_rules();
} RUST_MIN_STACK=1048576 cargo build --manifest-path my_app/Cargo.toml Just shadowing keywords in general (SIGBUS after syntax errors): #[macro_export]
macro_rules! fn {
() => {
// This macro's name shadows the `fn` keyword.
// The goal is to see if this confuses the parser, which operates
// before the name resolution stage where previous bugs were found.
println!("The `fn` macro was called.");
};
}
// Now, we attempt to define a regular function using the `fn` keyword.
// If the parser incorrectly identifies `fn` as the macro, it could lead
// to a crash or a deeply confusing error message, as the fundamental
// structure of the language is being challenged.
fn my_test_function() {
println!("This is a standard function.");
}
fn main() {
// We can invoke the macro to make sure it's active.
fn!();
// And call the regular function.
my_test_function();
} |
This seems misguided.
There are numerous way to trigger this recursion limit, and most of them do not involve any shadowing and are not affected by changes to ident.rs made in this PR. (The change doesn't affect the reproducer with Also, this is not the only recursion limit error in rustc, there are similar errors in type checking, in const evaluation, probably somewhere else. |
Thanks for the quick reply. Here's the output of the original issue, encountered while testing a gpui project:
|
There are a few dozens of targeted uses of |
Compiler crashes with SIGSEGV when
macro_rules!
shadows a builtin attribute like#[test]
and uses that attribute within its expansion.Repro
RUST_MIN_STACK=1048576 rustc --test crash_test.rs # Segmentation fault (core dumped)
Can crash in regular situations without limiting stack as well, like gpui tests in Zed.
Fix adds a check in
early_resolve_ident_in_lexical_scope
to prevent infinite recursion by always preferring the builtin attribute when there's an ambiguity between amacro_rules!
macro and a builtin attribute. This only applies tomacro_rules!
macros to preserve existing proc-macro ambiguity detection.Questions
Should there be a warning when a
macro_rules!
macro shadows a builtin attribute? What's the precedence order?With a very small stack, a crash can still happen before execution gets here. Move fix?
r? @WaffleLapkin bug originally found in gpui 😸