Skip to content

Commit e2d13b9

Browse files
committed
Lint str-ptr-in-c-abi discourage str pointers in C ABI fns
This discourages using a pointer to a `str` instead of to a CString for `extern "C"` functions.
1 parent 6412d74 commit e2d13b9

7 files changed

Lines changed: 195 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7455,6 +7455,7 @@ Released 2018-09-13
74557455
[`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
74567456
[`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc
74577457
[`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core
7458+
[`str_ptr_in_c_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_ptr_in_c_abi
74587459
[`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline
74597460
[`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
74607461
[`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
714714
crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
715715
crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
716716
crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,
717+
crate::str_ptr_in_c_abi::STR_PTR_IN_C_ABI_INFO,
717718
crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO,
718719
crate::string_patterns::SINGLE_CHAR_PATTERN_INFO,
719720
crate::strings::STR_TO_STRING_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ mod size_of_in_element_count;
348348
mod size_of_ref;
349349
mod slow_vector_initialization;
350350
mod std_instead_of_core;
351+
mod str_ptr_in_c_abi;
351352
mod string_patterns;
352353
mod strings;
353354
mod strlen_on_c_strings;
@@ -866,6 +867,7 @@ rustc_lint::late_lint_methods!(
866867
RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
867868
RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
868869
RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
870+
StrPtrInCAbi: str_ptr_in_c_abi::StrPtrInCAbi = str_ptr_in_c_abi::StrPtrInCAbi,
869871
// add late passes here, used by `cargo dev new_lint`
870872
]]
871873
);
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use rustc_hir::{Expr, ExprKind, TyKind};
2+
use rustc_lint::{LateContext, LateLintPass};
3+
use rustc_session::declare_lint_pass;
4+
5+
use clippy_utils::diagnostics::span_lint_and_help;
6+
use clippy_utils::sym;
7+
8+
declare_clippy_lint! {
9+
/// ### What it does
10+
///
11+
/// This lint triggers if a pointer to a Rust `str` is passed into an `extern "C"` interface
12+
/// where you should instead be providing a pointer to a `CString`.
13+
///
14+
/// ### Why is this bad?
15+
///
16+
/// Foreign functions under the C ABI expect that a string ends with a null byte (`'\0'`).
17+
/// Rust's `str` doesn't provide a null byte. Instead it contains a length for the string.
18+
/// This leads to two problems.
19+
///
20+
/// 1. The length parameter of the Rust string will be misinterpreted as a character, which is logically invalid.
21+
/// 2. Without a null byte, foreign functions will read beyond the memory allocated to the string searching for the null terminator, causing undefined behavior (UB).
22+
///
23+
/// ### Example
24+
/// ```no_run
25+
/// # unsafe extern "C" fn strlen(s: *const i8) -> usize { unimplemented!() }
26+
/// unsafe { strlen("Hello".as_ptr() as *const _) };
27+
/// ```
28+
/// Use instead:
29+
/// ```no_run
30+
/// # unsafe extern "C" fn strlen(s: *const i8) -> usize { unimplemented!() }
31+
/// let cstring = std::ffi::CString::new("Hello".as_bytes()).unwrap();
32+
/// unsafe { strlen(cstring.as_ptr()) };
33+
/// ```
34+
#[clippy::version = "1.99.0"]
35+
pub STR_PTR_IN_C_ABI,
36+
nursery,
37+
"discourage str pointers in C ABI fns"
38+
}
39+
40+
declare_lint_pass!(StrPtrInCAbi => [STR_PTR_IN_C_ABI]);
41+
42+
impl<'tcx> LateLintPass<'tcx> for StrPtrInCAbi {
43+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
44+
// find call expressions
45+
if let ExprKind::Call(callee, args) = expr.kind
46+
// where the signature of the callee shows it is
47+
// extern "C" fn
48+
&& is_extern_c_fn(cx, callee)
49+
// and takes a raw pointer as an argument
50+
// and to that argument gives a pointer to a Rust `str`
51+
&& let span = args
52+
.iter()
53+
.filter(|arg| is_cast_str_ptr_to_raw_ptr(cx, arg))
54+
.map(|arg| arg.span)
55+
.collect::<Vec<_>>()
56+
&& !span.is_empty()
57+
{
58+
span_lint_and_help(
59+
cx,
60+
STR_PTR_IN_C_ABI,
61+
span,
62+
"giving a pointer to a Rust `str` to an `extern \"C\" fn` can cause undefined behavior",
63+
/* help_span */ None,
64+
"first convert the `str` to a `std::ffi::CString` and then get a pointer from there",
65+
);
66+
}
67+
}
68+
}
69+
70+
/// Does the expression represent an `extern "C" fn` of some type?
71+
fn is_extern_c_fn<'tcx>(cx: &LateContext<'tcx>, callee: &'tcx Expr<'tcx>) -> bool {
72+
let callee_ty = cx.typeck_results().expr_ty(callee);
73+
if !(callee_ty.is_fn() || callee_ty.is_fn_ptr()) {
74+
return false;
75+
}
76+
matches!(callee_ty.fn_sig(cx.tcx).abi(), rustc_abi::ExternAbi::C { .. })
77+
}
78+
79+
/// If `arg` is `derefs_to_str.as(_mut)_ptr() as *const(mut) _`, then true.
80+
fn is_cast_str_ptr_to_raw_ptr<'tcx>(cx: &LateContext<'tcx>, arg: &Expr<'tcx>) -> bool {
81+
let typeck = cx.typeck_results();
82+
if let ExprKind::Cast(expr, ty) = arg.kind
83+
&& matches!(ty.kind, TyKind::Ptr(_))
84+
&& let ExprKind::MethodCall(method, this, _args, _span) = expr.kind
85+
&& matches!(method.ident.name, sym::as_ptr | sym::as_mut_ptr)
86+
&& typeck.expr_ty_adjusted(this).peel_refs().is_str()
87+
{
88+
return true;
89+
}
90+
false
91+
}

clippy_utils/src/sym.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ generate! {
138138
as_deref,
139139
as_deref_mut,
140140
as_mut,
141+
as_mut_ptr,
141142
as_path,
142143
as_ptr,
143144
as_str,

tests/ui/str_ptr_in_c_abi.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#![warn(clippy::str_ptr_in_c_abi)]
2+
3+
use std::ffi::CString;
4+
5+
fn main() {
6+
// This should use a pointer to a CString
7+
unsafe { printf("Hello".as_ptr() as *const _) };
8+
//~^ str_ptr_in_c_abi
9+
10+
// Like this
11+
let cstring = CString::new("Hello".as_bytes()).unwrap();
12+
unsafe { printf(cstring.as_ptr()) };
13+
14+
// One should also use mut pointers to CStrings
15+
let mut buffer = String::new();
16+
let mut buffer = buffer.as_mut_str(); // this lint can only detect `str`s for now
17+
unsafe { strcpy(buffer.as_mut_ptr() as *mut _, cstring.as_ptr()) };
18+
//~^ str_ptr_in_c_abi
19+
20+
let mut cstring_mut = CString::new([]).unwrap();
21+
unsafe { strcpy(cstring_mut.into_raw(), cstring.as_ptr()) };
22+
23+
// Two rust strings at once!
24+
unsafe { strcpy(buffer.as_mut_ptr() as *mut _, "Hello".as_ptr() as *const _) };
25+
//~^ str_ptr_in_c_abi
26+
27+
// It can detect smart pointers to str
28+
let hello_string: String = "Hello".into();
29+
let hello_box: Box<str> = "Hello".into();
30+
let hello_rc: std::rc::Rc<str> = "Hello".into();
31+
unsafe { printf(hello_string.as_ptr() as *const _) };
32+
//~^ str_ptr_in_c_abi
33+
unsafe { printf(hello_box.as_ptr() as *const _) };
34+
//~^ str_ptr_in_c_abi
35+
unsafe { printf(hello_rc.as_ptr() as *const _) };
36+
//~^ str_ptr_in_c_abi
37+
38+
// It detects str to ptr casts in variadics
39+
let fmt = CString::new("%s\n".as_bytes()).unwrap();
40+
unsafe { printf(fmt.as_ptr(), "I'm (incorrectly) printf-ing a str!".as_ptr() as *const _) };
41+
//~^ str_ptr_in_c_abi
42+
}
43+
44+
unsafe extern "C" {
45+
fn strcpy(dst: *mut i8, src: *const i8) -> *mut i8;
46+
fn printf(format: *const i8, ...) -> i32;
47+
}

tests/ui/str_ptr_in_c_abi.stderr

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
2+
--> tests/ui/str_ptr_in_c_abi.rs:7:21
3+
|
4+
LL | unsafe { printf("Hello".as_ptr() as *const _) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
8+
= note: `-D clippy::str-ptr-in-c-abi` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::str_ptr_in_c_abi)]`
10+
11+
error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
12+
--> tests/ui/str_ptr_in_c_abi.rs:17:21
13+
|
14+
LL | unsafe { strcpy(buffer.as_mut_ptr() as *mut _, cstring.as_ptr()) };
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
= help: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
18+
19+
error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
20+
--> tests/ui/str_ptr_in_c_abi.rs:24:21
21+
|
22+
LL | unsafe { strcpy(buffer.as_mut_ptr() as *mut _, "Hello".as_ptr() as *const _) };
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
24+
|
25+
= help: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
26+
27+
error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
28+
--> tests/ui/str_ptr_in_c_abi.rs:31:21
29+
|
30+
LL | unsafe { printf(hello_string.as_ptr() as *const _) };
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
|
33+
= help: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
34+
35+
error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
36+
--> tests/ui/str_ptr_in_c_abi.rs:33:21
37+
|
38+
LL | unsafe { printf(hello_box.as_ptr() as *const _) };
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40+
|
41+
= help: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
42+
43+
error: giving a pointer to a Rust `str` to an `extern "C" fn` can cause undefined behavior
44+
--> tests/ui/str_ptr_in_c_abi.rs:35:21
45+
|
46+
LL | unsafe { printf(hello_rc.as_ptr() as *const _) };
47+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
48+
|
49+
= help: first convert the `str` to a `std::ffi::CString` and then get a pointer from there
50+
51+
error: aborting due to 6 previous errors
52+

0 commit comments

Comments
 (0)