Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit 05445c7

Browse files
Aaron1011mvines
authored andcommitted
Fix hygiene issues in declare_program! and declare_loader!
The `declare_program!` and `declare_loader!` macros both expand to new macro definitions (based on the `$name` argument). These 'inner' macros make use of the special `$crate` metavariable to access items in the crate where the 'inner' macros is defined. However, this only works due to a bug in rustc. When a macro is expanded, all `$crate` tokens in its output are 'marked' as being resolved in the defining crate of that macro. An inner macro (including the body of its arms) is 'just' another set of tokens that appears in the body of the outer macro, so any `$crate` identifiers used there are resolved relative to the 'outer' macro. For example, consider the following code: ```rust macro_rules! outer { () => { macro_rules! inner { () => { $crate::Foo } } } } ``` The path `$crate::Foo` will be resolved relative to the crate that defines `outer`, **not** the crate which defines `inner`. However, rustc currently loses this extra resolution information (referred to as 'hygiene' information) when a crate is serialized. In the above example, this means that the macro `inner` (which gets defined in whatever crate invokes `outer!`) will behave differently depending on which crate it is invoked from: When `inner` is invoked from the same crate in which it is defined, the hygiene information will still be available, which will cause `$crate::Foo` to be resolved in the crate which defines 'outer'. When `inner` is invoked from a different crate, it will be loaded from the metadata of the crate which defines 'inner'. Since the hygiene information is currently lost, rust will 'forget' that `$crate::Foo` is supposed to be resolved in the context of 'outer'. Instead, it will be resolved relative to the crate which defines 'inner', which can cause incorrect code to compile. This bug will soon be fixed in rust (see rust-lang/rust#72121), which will break `declare_program!` and `declare_loader!`. Fortunately, it's possible to obtain the desired behavior (`$crate` resolving in the context of the 'inner' macro) by use of a procedural macro. This commit adds a `respan!` proc-macro to the `sdk/macro` crate. Using the newly-stabilized (on Nightly) `Span::resolved_at` method, the `$crate` identifier can be made to be resolved in the context of the proper crate. Since `Span::resolved_at` is only stable on the latest nightly, referencing it on an earlier version of Rust will cause a compilation error. This requires the `rustversion` crate to be used, which allows conditionally compiling code epending on the Rust compiler version in use. Since this method is already stabilized in the latest nightly, there will never be a situation where the hygiene bug is fixed (e.g. rust-lang/rust#72121) is merged but we are unable to call `Span::resolved_at`.
1 parent 0b919f0 commit 05445c7

File tree

8 files changed

+155
-15
lines changed

8 files changed

+155
-15
lines changed

Cargo.lock

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

programs/bpf_loader/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ thiserror = "1.0"
2121

2222
[dev-dependencies]
2323
rand = "0.7.3"
24+
rustversion = "1.0.3"
2425

2526
[lib]
2627
crate-type = ["lib", "cdylib"]

programs/bpf_loader/src/lib.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,14 @@ mod tests {
340340
}
341341
}
342342

343+
#[rustversion::since(1.46.0)]
344+
#[test]
345+
fn test_bpf_loader_same_crate() {
346+
// Ensure that we can invoke this macro from the same crate
347+
// where it is defined.
348+
solana_bpf_loader_program!();
349+
}
350+
343351
#[test]
344352
#[should_panic(expected = "ExceededMaxInstructions(10)")]
345353
fn test_bpf_loader_non_terminating_program() {

sdk/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ solana-crate-features = { path = "../crate-features", version = "1.3.0", optiona
5555
solana-logger = { path = "../logger", version = "1.3.0", optional = true }
5656
solana-sdk-macro = { path = "macro", version = "1.3.0" }
5757
solana-sdk-macro-frozen-abi = { path = "macro-frozen-abi", version = "1.3.0" }
58+
rustversion = "1.0.3"
5859

5960
[dev-dependencies]
6061
tiny-bip39 = "0.7.0"

sdk/macro/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ bs58 = "0.3.0"
1616
proc-macro2 = "1.0"
1717
quote = "1.0"
1818
syn = { version = "1.0", features = ["full", "extra-traits"] }
19+
rustversion = "1.0.3"
1920

2021
[package.metadata.docs.rs]
2122
targets = ["x86_64-unknown-linux-gnu"]

sdk/macro/src/lib.rs

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
extern crate proc_macro;
66

77
use proc_macro::TokenStream;
8-
use proc_macro2::Span;
8+
use proc_macro2::{Delimiter, Span, TokenTree};
99
use quote::{quote, ToTokens};
1010
use std::convert::TryFrom;
1111
use syn::{
@@ -14,7 +14,7 @@ use syn::{
1414
parse_macro_input,
1515
punctuated::Punctuated,
1616
token::Bracket,
17-
Expr, Ident, LitByte, LitStr, Token,
17+
Expr, Ident, LitByte, LitStr, Path, Token,
1818
};
1919

2020
struct Id(proc_macro2::TokenStream);
@@ -63,6 +63,75 @@ impl ToTokens for Id {
6363
}
6464
}
6565

66+
#[allow(dead_code)] // `respan` may be compiled out
67+
struct RespanInput {
68+
to_respan: Path,
69+
respan_using: Span,
70+
}
71+
72+
impl Parse for RespanInput {
73+
fn parse(input: ParseStream) -> Result<Self> {
74+
let to_respan: Path = input.parse()?;
75+
let _comma: Token![,] = input.parse()?;
76+
let respan_tree: TokenTree = input.parse()?;
77+
match respan_tree {
78+
TokenTree::Group(g) if g.delimiter() == Delimiter::None => {
79+
let ident: Ident = syn::parse2(g.stream())?;
80+
Ok(RespanInput {
81+
to_respan,
82+
respan_using: ident.span(),
83+
})
84+
}
85+
val @ _ => Err(syn::Error::new_spanned(
86+
val,
87+
"expected None-delimited group",
88+
)),
89+
}
90+
}
91+
}
92+
93+
/// A proc-macro which respans the tokens in its first argument (a `Path`)
94+
/// to be resolved at the tokens of its second argument.
95+
/// For internal use only.
96+
///
97+
/// There must be exactly one comma in the input,
98+
/// which is used to separate the two arguments.
99+
/// The second argument should be exactly one token.
100+
///
101+
/// For example, `respan!($crate::foo, with_span)`
102+
/// produces the tokens `$crate::foo`, but resolved
103+
/// at the span of `with_span`.
104+
///
105+
/// The input to this function should be very short -
106+
/// its only purpose is to override the span of a token
107+
/// sequence containing `$crate`. For all other purposes,
108+
/// a more general proc-macro should be used.
109+
#[rustversion::since(1.46.0)] // `Span::resolved_at` is stable in 1.46.0 and above
110+
#[proc_macro]
111+
pub fn respan(input: TokenStream) -> TokenStream {
112+
// Obtain the `Path` we are going to respan, and the ident
113+
// whose span we will be using.
114+
let RespanInput {
115+
to_respan,
116+
respan_using,
117+
} = parse_macro_input!(input as RespanInput);
118+
// Respan all of the tokens in the `Path`
119+
let to_respan: proc_macro2::TokenStream = to_respan
120+
.into_token_stream()
121+
.into_iter()
122+
.map(|mut t| {
123+
// Combine the location of the token with the resolution behavior of `respan_using`
124+
// Note: `proc_macro2::Span::resolved_at` is currently gated with cfg(procmacro2_semver_exempt)
125+
// Once this gate is removed, we will no longer need to use 'unwrap()' to call
126+
// the underling `proc_macro::Span::resolved_at` method.
127+
let new_span: Span = t.span().unwrap().resolved_at(respan_using.unwrap()).into();
128+
t.set_span(new_span);
129+
t
130+
})
131+
.collect();
132+
return TokenStream::from(to_respan);
133+
}
134+
66135
#[proc_macro]
67136
pub fn declare_id(input: TokenStream) -> TokenStream {
68137
let id = parse_macro_input!(input as Id);

sdk/src/entrypoint_native.rs

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,60 @@ pub type LoaderEntrypoint = unsafe extern "C" fn(
3030
invoke_context: &dyn InvokeContext,
3131
) -> Result<(), InstructionError>;
3232

33+
#[rustversion::since(1.46.0)]
34+
#[macro_export]
35+
macro_rules! declare_name {
36+
($name:ident) => {
37+
#[macro_export]
38+
macro_rules! $name {
39+
() => {
40+
// Subtle:
41+
// The outer `declare_name!` macro may be expanded in another
42+
// crate, causing the macro `$name!` to be defined in that
43+
// crate. We want to emit a call to `$crate::id()`, and have
44+
// `$crate` be resolved in the crate where `$name!` gets defined,
45+
// *not* in this crate (where `declare_name! is defined).
46+
//
47+
// When a macro_rules! macro gets expanded, any $crate tokens
48+
// in its output will be 'marked' with the crate they were expanded
49+
// from. This includes nested macros like our macro `$name` - even
50+
// though it looks like a separate macro, Rust considers it to be
51+
// just another part of the output of `declare_program!`.
52+
//
53+
// We pass `$name` as the second argument to tell `respan!` to
54+
// apply use the `Span` of `$name` when resolving `$crate::id`.
55+
// This causes `$crate` to behave as though it was written
56+
// at the same location as the `$name` value passed
57+
// to `declare_name!` (e.g. the 'foo' in
58+
// `declare_name(foo)`
59+
//
60+
// See the `respan!` macro for more details.
61+
// FIXME: Use `crate::respan!` once https://github.com/rust-lang/rust/pull/72121
62+
// is merged.
63+
// `respan!` respans the path `$crate::id`, which we then call (hence the extra
64+
// parens)
65+
(
66+
stringify!($name).to_string(),
67+
::solana_sdk::respan!($crate::id, $name)(),
68+
)
69+
};
70+
}
71+
};
72+
}
73+
74+
#[rustversion::not(since(1.46.0))]
75+
#[macro_export]
76+
macro_rules! declare_name {
77+
($name:ident) => {
78+
#[macro_export]
79+
macro_rules! $name {
80+
() => {
81+
(stringify!($name).to_string(), $crate::id())
82+
};
83+
}
84+
};
85+
}
86+
3387
/// Convenience macro to declare a native program
3488
///
3589
/// bs58_string: bs58 string representation the program's id
@@ -102,13 +156,7 @@ pub type LoaderEntrypoint = unsafe extern "C" fn(
102156
macro_rules! declare_program(
103157
($bs58_string:expr, $name:ident, $entrypoint:expr) => (
104158
$crate::declare_id!($bs58_string);
105-
106-
#[macro_export]
107-
macro_rules! $name {
108-
() => {
109-
(stringify!($name).to_string(), $crate::id())
110-
};
111-
}
159+
$crate::declare_name!($name);
112160

113161
#[no_mangle]
114162
pub extern "C" fn $name(
@@ -126,13 +174,9 @@ macro_rules! declare_program(
126174
macro_rules! declare_loader(
127175
($bs58_string:expr, $name:ident, $entrypoint:expr) => (
128176
$crate::declare_id!($bs58_string);
177+
$crate::declare_name!($name);
178+
129179

130-
#[macro_export]
131-
macro_rules! $name {
132-
() => {
133-
(stringify!($name).to_string(), $crate::id())
134-
};
135-
}
136180

137181
#[no_mangle]
138182
pub extern "C" fn $name(

sdk/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ pub mod timing;
6767
/// ```
6868
pub use solana_sdk_macro::declare_id;
6969
pub use solana_sdk_macro::pubkeys;
70+
#[rustversion::since(1.46.0)]
71+
pub use solana_sdk_macro::respan;
7072

7173
// On-chain program specific modules
7274
pub mod account_info;

0 commit comments

Comments
 (0)