Skip to content

Commit d5b1120

Browse files
Rollup merge of rust-lang#155521 - Urgau:runtime-symbols, r=davidtwco
Add lint againts invalid runtime symbol definitions This PR adds a deny-by-default lint againts invalid runtime symbol definitions, those runtime symbols are assumed and used by `core`[^1] and `rustc` with a specific definition. We have had multiple reports of users tripping over `std` symbols (addressed in a future PR): - [Why does `#[no_mangle] fn open() {}` make `cargo t` hang?](https://users.rust-lang.org/t/why-does-no-mangle-fn-open-make-cargo-t-hang/103423) - [Pointer becomes misaligned in test with `no_mangle`](https://users.rust-lang.org/t/pointer-becomes-misaligned-in-test-with-no-mangle/126580) This PR is a second attempt after rust-lang#146505, where T-lang had [some reservations](rust-lang#146505 (comment)) about a blanket lint that does not check the signature, which is now done with this PR, and about linting of `std` runtime symbols when std is not linked, which this PR omits by not including any std runtime symbols (for now). ## `invalid_runtime_symbol_definitions` *(deny-by-default)* The `invalid_runtime_symbol_definitions` lint checks the signature of items whose symbol name is a runtime symbols expected by `core`. ### Example ```rust,compile_fail #[unsafe(no_mangle)] pub fn memcmp() {} // invalid definition of the `memcmp` runtime symbol ``` ```text error: invalid definition of the runtime `memcmp` symbol used by the standard library --> a.rs:2:1 | 4 | fn memcmp() {} | ^^^^^^^^^^^ | = note: expected `unsafe extern "C" fn(*const c_void, *const c_void, usize) -> i32` found `fn()` = help: either fix the signature or remove any attributes like `#[unsafe(no_mangle)]`, `#[unsafe(export_name = "memcmp")]`, or `#[link_name = "memcmp"]` = note: `#[deny(invalid_runtime_symbol_definitions)]` on by default ``` ### Explanation Up-most care is required when defining runtime symbols assumed and used by the standard library. They must follow the C specification, not use any standard-library facility or undefined behavior may occur. The symbols currently checked are `memcpy`, `memmove`, `memset`, `memcmp`, `bcmp` and `strlen`. [^1]: https://doc.rust-lang.org/core/index.html#how-to-use-the-core-library
2 parents 8008927 + 87d0090 commit d5b1120

20 files changed

Lines changed: 504 additions & 38 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4267,6 +4267,7 @@ dependencies = [
42674267
"rustc_parse_format",
42684268
"rustc_session",
42694269
"rustc_span",
4270+
"rustc_symbol_mangling",
42704271
"rustc_target",
42714272
"rustc_trait_selection",
42724273
"smallvec",

compiler/rustc_error_codes/src/error_codes/E0755.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ side effects or infinite loops:
2020
2121
extern "C" {
2222
#[unsafe(ffi_pure)] // ok!
23-
pub fn strlen(s: *const i8) -> isize;
23+
pub fn strlen(s: *const std::ffi::c_char) -> usize;
2424
}
2525
# fn main() {}
2626
```

compiler/rustc_error_codes/src/error_codes/E0756.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ which have no side effects except for their return value:
2121
2222
extern "C" {
2323
#[unsafe(ffi_const)] // ok!
24-
pub fn strlen(s: *const i8) -> i32;
24+
pub fn strlen(s: *const std::ffi::c_char) -> usize;
2525
}
2626
# fn main() {}
2727
```

compiler/rustc_hir/src/lang_items.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,14 @@ language_item_table! {
448448

449449
// Used to fallback `{float}` to `f32` when `f32: From<{float}>`
450450
From, sym::From, from_trait, Target::Trait, GenericRequirement::Exact(1);
451+
452+
// Runtime symbols
453+
MemCpy, sym::memcpy_fn, memcpy_fn, Target::Fn, GenericRequirement::None;
454+
MemMove, sym::memmove_fn, memmove_fn, Target::Fn, GenericRequirement::None;
455+
MemSet, sym::memset_fn, memset_fn, Target::Fn, GenericRequirement::None;
456+
MemCmp, sym::memcmp_fn, memcmp_fn, Target::Fn, GenericRequirement::None;
457+
Bcmp, sym::bcmp_fn, bcmp_fn, Target::Fn, GenericRequirement::None;
458+
StrLen, sym::strlen_fn, strlen_fn, Target::Fn, GenericRequirement::None;
451459
}
452460

453461
/// The requirement imposed on the generics of a lang item

compiler/rustc_hir/src/weak_lang_items.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,29 @@ macro_rules! weak_lang_items {
2323
}
2424
}
2525

26+
macro_rules! weak_only_lang_items {
27+
($($item:ident,)*) => {
28+
pub static WEAK_ONLY_LANG_ITEMS: &[LangItem] = &[$(LangItem::$item,)*];
29+
30+
impl LangItem {
31+
pub fn is_weak_only(self) -> bool {
32+
matches!(self, $(LangItem::$item)|*)
33+
}
34+
}
35+
}
36+
}
37+
2638
weak_lang_items! {
2739
PanicImpl, rust_begin_unwind;
2840
EhPersonality, rust_eh_personality;
2941
EhCatchTypeinfo, rust_eh_catch_typeinfo;
3042
}
43+
44+
weak_only_lang_items! {
45+
MemCpy,
46+
MemMove,
47+
MemSet,
48+
MemCmp,
49+
Bcmp,
50+
StrLen,
51+
}

compiler/rustc_lint/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ rustc_middle = { path = "../rustc_middle" }
2222
rustc_parse_format = { path = "../rustc_parse_format" }
2323
rustc_session = { path = "../rustc_session" }
2424
rustc_span = { path = "../rustc_span" }
25+
rustc_symbol_mangling = { path = "../rustc_symbol_mangling" }
2526
rustc_target = { path = "../rustc_target" }
2627
rustc_trait_selection = { path = "../rustc_trait_selection" }
2728
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }

compiler/rustc_lint/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ mod precedence;
7070
mod ptr_nulls;
7171
mod redundant_semicolon;
7272
mod reference_casting;
73+
mod runtime_symbols;
7374
mod shadowed_into_iter;
7475
mod static_mut_refs;
7576
mod traits;
@@ -113,6 +114,7 @@ use precedence::*;
113114
use ptr_nulls::*;
114115
use redundant_semicolon::*;
115116
use reference_casting::*;
117+
use runtime_symbols::*;
116118
use rustc_hir::def_id::LocalModDefId;
117119
use rustc_middle::query::Providers;
118120
use rustc_middle::ty::TyCtxt;
@@ -242,6 +244,7 @@ late_lint_methods!(
242244
AsyncFnInTrait: AsyncFnInTrait,
243245
NonLocalDefinitions: NonLocalDefinitions::default(),
244246
InteriorMutableConsts: InteriorMutableConsts,
247+
RuntimeSymbols: RuntimeSymbols,
245248
ImplTraitOvercaptures: ImplTraitOvercaptures,
246249
IfLetRescope: IfLetRescope::default(),
247250
StaticMutRefs: StaticMutRefs,

compiler/rustc_lint/src/lints.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,33 @@ pub(crate) enum UseLetUnderscoreIgnoreSuggestion {
848848
},
849849
}
850850

851+
// runtime_symbols.rs
852+
#[derive(Diagnostic)]
853+
pub(crate) enum RedefiningRuntimeSymbolsDiag<'tcx> {
854+
#[diag(
855+
"invalid definition of the runtime `{$symbol_name}` symbol used by the standard library"
856+
)]
857+
#[note(
858+
"expected `{$expected_fn_sig}`
859+
found `{$found_fn_sig}`"
860+
)]
861+
#[help(
862+
"either fix the signature or remove any attributes like `#[unsafe(no_mangle)]`, `#[unsafe(export_name = \"{$symbol_name}\")]`, or `#[link_name = \"{$symbol_name}\"]`"
863+
)]
864+
FnDef { symbol_name: String, expected_fn_sig: Ty<'tcx>, found_fn_sig: Ty<'tcx> },
865+
#[diag(
866+
"invalid definition of the runtime `{$symbol_name}` symbol used by the standard library"
867+
)]
868+
#[note(
869+
"expected `{$expected_fn_sig}`
870+
found `static {$symbol_name}: {$static_ty}`"
871+
)]
872+
#[help(
873+
"either fix the signature or remove any attributes `#[unsafe(no_mangle)]` or `#[unsafe(export_name = \"{$symbol_name}\")]`"
874+
)]
875+
Static { symbol_name: String, static_ty: Ty<'tcx>, expected_fn_sig: Ty<'tcx> },
876+
}
877+
851878
// drop_forget_useless.rs
852879
#[derive(Diagnostic)]
853880
#[diag("calls to `std::mem::drop` with a reference instead of an owned value does nothing")]
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
use rustc_hir::def_id::{DefId, LocalDefId};
2+
use rustc_hir::{self as hir, FnSig, ForeignItemKind, LanguageItems};
3+
use rustc_infer::infer::DefineOpaqueTypes;
4+
use rustc_middle::ty::{self, Instance, Ty};
5+
use rustc_session::{declare_lint, declare_lint_pass};
6+
use rustc_span::Span;
7+
use rustc_trait_selection::infer::TyCtxtInferExt;
8+
9+
use crate::lints::RedefiningRuntimeSymbolsDiag;
10+
use crate::{LateContext, LateLintPass, LintContext};
11+
12+
declare_lint! {
13+
/// The `invalid_runtime_symbol_definitions` lint checks the signature of items whose
14+
/// symbol name is a runtime symbols expected by `core`.
15+
///
16+
/// ### Example
17+
///
18+
/// ```rust,compile_fail
19+
/// #[unsafe(no_mangle)]
20+
/// pub fn strlen() {} // invalid definition of the `strlen` function
21+
/// ```
22+
///
23+
/// {{produces}}
24+
///
25+
/// ### Explanation
26+
///
27+
/// Up-most care is required when defining runtime symbols assumed and
28+
/// used by the standard library. They must follow the C specification, not use any
29+
/// standard-library facility or undefined behavior may occur.
30+
///
31+
/// The symbols currently checked are `memcpy`, `memmove`, `memset`, `memcmp`,
32+
/// `bcmp` and `strlen`.
33+
///
34+
/// [^1]: https://doc.rust-lang.org/core/index.html#how-to-use-the-core-library
35+
pub INVALID_RUNTIME_SYMBOL_DEFINITIONS,
36+
Deny,
37+
"invalid definition of a symbol used by the standard library"
38+
}
39+
40+
declare_lint_pass!(RuntimeSymbols => [INVALID_RUNTIME_SYMBOL_DEFINITIONS]);
41+
42+
static EXPECTED_SYMBOLS: &[ExpectedSymbol] = &[
43+
ExpectedSymbol { symbol: "memcpy", lang: LanguageItems::memcpy_fn },
44+
ExpectedSymbol { symbol: "memmove", lang: LanguageItems::memmove_fn },
45+
ExpectedSymbol { symbol: "memset", lang: LanguageItems::memset_fn },
46+
ExpectedSymbol { symbol: "memcmp", lang: LanguageItems::memcmp_fn },
47+
ExpectedSymbol { symbol: "bcmp", lang: LanguageItems::bcmp_fn },
48+
ExpectedSymbol { symbol: "strlen", lang: LanguageItems::strlen_fn },
49+
];
50+
51+
#[derive(Copy, Clone, Debug)]
52+
struct ExpectedSymbol {
53+
symbol: &'static str,
54+
lang: fn(&LanguageItems) -> Option<DefId>,
55+
}
56+
57+
impl<'tcx> LateLintPass<'tcx> for RuntimeSymbols {
58+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
59+
// Bail-out if the item is not a function/method or static.
60+
match item.kind {
61+
hir::ItemKind::Fn { sig, ident: _, generics, body: _, has_body: _ } => {
62+
// Generic functions cannot have the same runtime symbol as we do not allow
63+
// any symbol attributes.
64+
if !generics.params.is_empty() {
65+
return;
66+
}
67+
68+
// Try to get the overridden symbol name of this function (our mangling
69+
// cannot ever conflict with runtime symbols, so no need to check for those).
70+
let Some(symbol_name) = rustc_symbol_mangling::symbol_name_from_attrs(
71+
cx.tcx,
72+
rustc_middle::ty::InstanceKind::Item(item.owner_id.to_def_id()),
73+
) else {
74+
return;
75+
};
76+
77+
check_fn(cx, &symbol_name, sig, item.owner_id.def_id);
78+
}
79+
hir::ItemKind::Static(..) => {
80+
// Compute the symbol name of this static (without mangling, as our mangling
81+
// cannot ever conflict with runtime symbols).
82+
let Some(symbol_name) = rustc_symbol_mangling::symbol_name_from_attrs(
83+
cx.tcx,
84+
rustc_middle::ty::InstanceKind::Item(item.owner_id.to_def_id()),
85+
) else {
86+
return;
87+
};
88+
89+
let def_id = item.owner_id.def_id;
90+
91+
check_static(cx, &symbol_name, def_id, item.span);
92+
}
93+
hir::ItemKind::ForeignMod { abi: _, items } => {
94+
for item in items {
95+
let item = cx.tcx.hir_foreign_item(*item);
96+
97+
let did = item.owner_id.def_id;
98+
let instance = Instance::new_raw(
99+
did.to_def_id(),
100+
ty::List::identity_for_item(cx.tcx, did),
101+
);
102+
let symbol_name = cx.tcx.symbol_name(instance);
103+
104+
match item.kind {
105+
ForeignItemKind::Fn(fn_sig, _idents, _generics) => {
106+
check_fn(cx, &symbol_name.name, fn_sig, did);
107+
}
108+
ForeignItemKind::Static(..) => {
109+
check_static(cx, &symbol_name.name, did, item.span);
110+
}
111+
ForeignItemKind::Type => return,
112+
}
113+
}
114+
}
115+
_ => return,
116+
}
117+
}
118+
}
119+
120+
fn check_fn(cx: &LateContext<'_>, symbol_name: &str, sig: FnSig<'_>, did: LocalDefId) {
121+
let Some(expected_symbol) = EXPECTED_SYMBOLS.iter().find(|es| es.symbol == symbol_name) else {
122+
// The symbol name does not correspond to a runtime symbols, bail out
123+
return;
124+
};
125+
126+
let Some(expected_def_id) = (expected_symbol.lang)(&cx.tcx.lang_items()) else {
127+
// Can't find the corresponding language item, bail out
128+
return;
129+
};
130+
131+
// Get the two function signatures
132+
let lang_sig = cx.tcx.normalize_erasing_regions(
133+
cx.typing_env(),
134+
cx.tcx.fn_sig(expected_def_id).instantiate_identity(),
135+
);
136+
let user_sig = cx
137+
.tcx
138+
.normalize_erasing_regions(cx.typing_env(), cx.tcx.fn_sig(did).instantiate_identity());
139+
140+
// Compare the two signatures with an inference context
141+
let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode());
142+
let cause = rustc_middle::traits::ObligationCause::misc(sig.span, did);
143+
let result = infcx.at(&cause, cx.param_env).eq(DefineOpaqueTypes::No, lang_sig, user_sig);
144+
145+
// If they don't match, emit our own mismatch signatures
146+
if let Err(_terr) = result {
147+
// Create fn pointers for diagnostics purpose
148+
let expected = Ty::new_fn_ptr(cx.tcx, lang_sig);
149+
let actual = Ty::new_fn_ptr(cx.tcx, user_sig);
150+
151+
cx.emit_span_lint(
152+
INVALID_RUNTIME_SYMBOL_DEFINITIONS,
153+
sig.span,
154+
RedefiningRuntimeSymbolsDiag::FnDef {
155+
symbol_name: symbol_name.to_string(),
156+
found_fn_sig: actual,
157+
expected_fn_sig: expected,
158+
},
159+
);
160+
}
161+
}
162+
163+
fn check_static<'tcx>(cx: &LateContext<'tcx>, symbol_name: &str, did: LocalDefId, sp: Span) {
164+
let Some(expected_symbol) = EXPECTED_SYMBOLS.iter().find(|es| es.symbol == symbol_name) else {
165+
// The symbol name does not correspond to a runtime symbols, bail out
166+
return;
167+
};
168+
169+
let Some(expected_def_id) = (expected_symbol.lang)(&cx.tcx.lang_items()) else {
170+
// Can't find the corresponding language item, bail out
171+
return;
172+
};
173+
174+
// Get the static type
175+
let static_ty = cx.tcx.type_of(did).instantiate_identity().skip_norm_wip();
176+
177+
// Peel Option<...> and get the inner type (see std weak! macro with #[linkage = "extern_weak"])
178+
let inner_static_ty: Ty<'_> = match static_ty.kind() {
179+
ty::Adt(def, args) if Some(def.did()) == cx.tcx.lang_items().option_type() => {
180+
args.type_at(0)
181+
}
182+
_ => static_ty,
183+
};
184+
185+
// Get the expected symbol function signature
186+
let lang_sig = cx.tcx.normalize_erasing_regions(
187+
cx.typing_env(),
188+
cx.tcx.fn_sig(expected_def_id).instantiate_identity(),
189+
);
190+
191+
let expected = Ty::new_fn_ptr(cx.tcx, lang_sig);
192+
193+
// Compare the expected function signature with the static type, report an error if they don't match
194+
if expected != inner_static_ty {
195+
cx.emit_span_lint(
196+
INVALID_RUNTIME_SYMBOL_DEFINITIONS,
197+
sp,
198+
RedefiningRuntimeSymbolsDiag::Static {
199+
static_ty,
200+
symbol_name: symbol_name.to_string(),
201+
expected_fn_sig: expected,
202+
},
203+
);
204+
}
205+
}

0 commit comments

Comments
 (0)