Skip to content
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

Add new lint no_mangle_with_rust_abi #10369

Merged
merged 1 commit into from
Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4622,6 +4622,7 @@ Released 2018-09-13
[`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect
[`no_effect_replace`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_replace
[`no_effect_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding
[`no_mangle_with_rust_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
[`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
[`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::no_effect::NO_EFFECT_INFO,
crate::no_effect::NO_EFFECT_UNDERSCORE_BINDING_INFO,
crate::no_effect::UNNECESSARY_OPERATION_INFO,
crate::no_mangle_with_rust_abi::NO_MANGLE_WITH_RUST_ABI_INFO,
crate::non_copy_const::BORROW_INTERIOR_MUTABLE_CONST_INFO,
crate::non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST_INFO,
crate::non_expressive_names::JUST_UNDERSCORES_AND_DIGITS_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ mod neg_cmp_op_on_partial_ord;
mod neg_multiply;
mod new_without_default;
mod no_effect;
mod no_mangle_with_rust_abi;
mod non_copy_const;
mod non_expressive_names;
mod non_octal_unix_permissions;
Expand Down Expand Up @@ -921,6 +922,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
avoid_breaking_exported_api,
))
});
store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
65 changes: 65 additions & 0 deletions clippy_lints/src/no_mangle_with_rust_abi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_hir::{Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_target::spec::abi::Abi;

declare_clippy_lint! {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to move this to src/attrs.rs or let it remain in it's own file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to leave in its own file.

/// ### What it does
/// Checks for Rust ABI functions with the `#[no_mangle]` attribute.
///
/// ### Why is this bad?
/// The Rust ABI is not stable, but in many simple cases matches
/// enough with the C ABI that it is possible to forget to add
/// `extern "C"` to a function called from C. Changes to the
/// Rust ABI can break this at any point.
///
/// ### Example
/// ```rust
/// #[no_mangle]
/// fn example(arg_one: u32, arg_two: usize) {}
/// ```
///
/// Use instead:
/// ```rust
/// #[no_mangle]
/// extern "C" fn example(arg_one: u32, arg_two: usize) {}
/// ```
#[clippy::version = "1.69.0"]
pub NO_MANGLE_WITH_RUST_ABI,
pedantic,
"convert Rust ABI functions to C ABI"
}
declare_lint_pass!(NoMangleWithRustAbi => [NO_MANGLE_WITH_RUST_ABI]);

impl<'tcx> LateLintPass<'tcx> for NoMangleWithRustAbi {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Fn(fn_sig, _, _) = &item.kind {
let attrs = cx.tcx.hir().attrs(item.hir_id());
let mut applicability = Applicability::MachineApplicable;
let snippet = snippet_with_applicability(cx, fn_sig.span, "..", &mut applicability);
for attr in attrs {
if let Some(ident) = attr.ident()
&& ident.name == rustc_span::sym::no_mangle
&& fn_sig.header.abi == Abi::Rust
&& !snippet.contains("extern") {

let suggestion = snippet.split_once("fn")
.map_or(String::new(), |(first, second)| format!(r#"{first}extern "C" fn{second}"#));

span_lint_and_sugg(
cx,
NO_MANGLE_WITH_RUST_ABI,
fn_sig.span,
"attribute #[no_mangle] set on a Rust ABI function",
"try",
suggestion,
applicability
);
}
}
}
}
}
2 changes: 1 addition & 1 deletion tests/ui/must_use_candidates.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub unsafe fn mutates_static() -> usize {
}

#[no_mangle]
pub fn unmangled(i: bool) -> bool {
pub extern "C" fn unmangled(i: bool) -> bool {
!i
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/must_use_candidates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub unsafe fn mutates_static() -> usize {
}

#[no_mangle]
pub fn unmangled(i: bool) -> bool {
pub extern "C" fn unmangled(i: bool) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just applying this lint. Helps us avoid adding noise to tests/ui/must_use_candidates.fixed

!i
}

Expand Down
48 changes: 48 additions & 0 deletions tests/ui/no_mangle_with_rust_abi.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// run-rustfix

#![allow(unused)]
#![warn(clippy::no_mangle_with_rust_abi)]

#[no_mangle]
extern "C" fn rust_abi_fn_one(arg_one: u32, arg_two: usize) {}

#[no_mangle]
pub extern "C" fn rust_abi_fn_two(arg_one: u32, arg_two: usize) {}

/// # Safety
/// This function shouldn't be called unless the horsemen are ready
#[no_mangle]
pub unsafe extern "C" fn rust_abi_fn_three(arg_one: u32, arg_two: usize) {}

/// # Safety
/// This function shouldn't be called unless the horsemen are ready
#[no_mangle]
unsafe extern "C" fn rust_abi_fn_four(arg_one: u32, arg_two: usize) {}

#[no_mangle]
extern "C" fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
arg_one: u32,
arg_two: usize,
) -> u32 {
0
}

// Must not run on functions that explicitly opt in to Rust ABI with `extern "Rust"`
#[no_mangle]
#[rustfmt::skip]
extern "Rust" fn rust_abi_fn_explicit_opt_in(arg_one: u32, arg_two: usize) {}

fn rust_abi_fn_again(arg_one: u32, arg_two: usize) {}

#[no_mangle]
extern "C" fn c_abi_fn(arg_one: u32, arg_two: usize) {}

extern "C" fn c_abi_fn_again(arg_one: u32, arg_two: usize) {}

extern "C" {
fn c_abi_in_block(arg_one: u32, arg_two: usize);
}

fn main() {
// test code goes here
}
48 changes: 48 additions & 0 deletions tests/ui/no_mangle_with_rust_abi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// run-rustfix

#![allow(unused)]
#![warn(clippy::no_mangle_with_rust_abi)]

#[no_mangle]
fn rust_abi_fn_one(arg_one: u32, arg_two: usize) {}

#[no_mangle]
pub fn rust_abi_fn_two(arg_one: u32, arg_two: usize) {}

/// # Safety
/// This function shouldn't be called unless the horsemen are ready
#[no_mangle]
pub unsafe fn rust_abi_fn_three(arg_one: u32, arg_two: usize) {}

/// # Safety
/// This function shouldn't be called unless the horsemen are ready
#[no_mangle]
unsafe fn rust_abi_fn_four(arg_one: u32, arg_two: usize) {}

#[no_mangle]
fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
arg_one: u32,
arg_two: usize,
) -> u32 {
0
}

// Must not run on functions that explicitly opt in to Rust ABI with `extern "Rust"`
#[no_mangle]
#[rustfmt::skip]
extern "Rust" fn rust_abi_fn_explicit_opt_in(arg_one: u32, arg_two: usize) {}

fn rust_abi_fn_again(arg_one: u32, arg_two: usize) {}

#[no_mangle]
extern "C" fn c_abi_fn(arg_one: u32, arg_two: usize) {}

extern "C" fn c_abi_fn_again(arg_one: u32, arg_two: usize) {}

extern "C" {
fn c_abi_in_block(arg_one: u32, arg_two: usize);
}

fn main() {
// test code goes here
}
45 changes: 45 additions & 0 deletions tests/ui/no_mangle_with_rust_abi.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error: attribute #[no_mangle] set on a Rust ABI function
--> $DIR/no_mangle_with_rust_abi.rs:7:1
|
LL | fn rust_abi_fn_one(arg_one: u32, arg_two: usize) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `extern "C" fn rust_abi_fn_one(arg_one: u32, arg_two: usize)`
|
= note: `-D clippy::no-mangle-with-rust-abi` implied by `-D warnings`

error: attribute #[no_mangle] set on a Rust ABI function
--> $DIR/no_mangle_with_rust_abi.rs:10:1
|
LL | pub fn rust_abi_fn_two(arg_one: u32, arg_two: usize) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `pub extern "C" fn rust_abi_fn_two(arg_one: u32, arg_two: usize)`

error: attribute #[no_mangle] set on a Rust ABI function
--> $DIR/no_mangle_with_rust_abi.rs:15:1
|
LL | pub unsafe fn rust_abi_fn_three(arg_one: u32, arg_two: usize) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `pub unsafe extern "C" fn rust_abi_fn_three(arg_one: u32, arg_two: usize)`

error: attribute #[no_mangle] set on a Rust ABI function
--> $DIR/no_mangle_with_rust_abi.rs:20:1
|
LL | unsafe fn rust_abi_fn_four(arg_one: u32, arg_two: usize) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe extern "C" fn rust_abi_fn_four(arg_one: u32, arg_two: usize)`

error: attribute #[no_mangle] set on a Rust ABI function
--> $DIR/no_mangle_with_rust_abi.rs:23:1
|
LL | / fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
LL | | arg_one: u32,
LL | | arg_two: usize,
LL | | ) -> u32 {
| |________^
|
help: try
|
LL + extern "C" fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
LL + arg_one: u32,
LL + arg_two: usize,
LL ~ ) -> u32 {
|

error: aborting due to 5 previous errors