Skip to content

Commit 9669223

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 9669223

6 files changed

Lines changed: 207 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: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
use rustc_hir::{Expr, ExprKind, TyKind};
2+
use rustc_lint::{LateContext, LateLintPass};
3+
use rustc_middle::ty::Ty;
4+
use rustc_session::declare_lint_pass;
5+
6+
use clippy_utils::diagnostics::span_lint_and_help;
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+
let ExprKind::Call(callee, args) = expr.kind else {
46+
return;
47+
};
48+
// where the signature of the callee shows it is
49+
// extern "C"
50+
if is_extern_c_fn(cx, callee) {
51+
// and takes a raw pointer as an argument
52+
let arg_idxs = raw_pointer_arg_idxs(cx, callee);
53+
if arg_idxs.is_empty() {
54+
return;
55+
}
56+
let relevant_args = arg_idxs
57+
.into_iter()
58+
.filter_map(|idx| args.get(idx))
59+
.map(Clone::clone)
60+
.collect::<Vec<_>>();
61+
let problem_args = args_str_ptr_cast(cx, &relevant_args);
62+
if problem_args.is_empty() {
63+
return;
64+
}
65+
let span: rustc_errors::MultiSpan = problem_args
66+
.into_iter()
67+
.map(|arg| arg.span)
68+
.collect::<Vec<rustc_span::Span>>()
69+
.into();
70+
span_lint_and_help(
71+
cx,
72+
STR_PTR_IN_C_ABI,
73+
span,
74+
"giving a Rust str pointer to an `extern \"C\" fn` can cause undefined behavior",
75+
/* help_span */ None,
76+
"first convert the str to a `std::ffi::CString` and then get a pointer from there",
77+
);
78+
}
79+
}
80+
}
81+
82+
/// Does the expression represent an `extern "C" fn` of some type?
83+
fn is_extern_c_fn<'tcx>(cx: &LateContext<'tcx>, callee: &'tcx Expr<'tcx>) -> bool {
84+
let callee_ty = cx.typeck_results().expr_ty(callee);
85+
if !(callee_ty.is_fn() || callee_ty.is_fn_ptr()) {
86+
return false;
87+
}
88+
matches!(callee_ty.fn_sig(cx.tcx).abi(), rustc_abi::ExternAbi::C { .. })
89+
}
90+
91+
/// A list of indices into callee's arg list where the type of the value should be `*mut _` or
92+
/// `*const _`.
93+
///
94+
/// If `callee` is not the head of a `Call` expression, this will return `None`.
95+
fn raw_pointer_arg_idxs<'tcx>(cx: &LateContext<'tcx>, callee: &'tcx Expr<'tcx>) -> Vec<usize> {
96+
let callee_ty = cx.typeck_results().expr_ty(callee);
97+
if !(callee_ty.is_fn() || callee_ty.is_fn_ptr()) {
98+
return Vec::new();
99+
}
100+
callee_ty
101+
.fn_sig(cx.tcx)
102+
.inputs()
103+
.iter()
104+
.enumerate()
105+
.filter(|(_i, ty)| ty.skip_binder().is_raw_ptr())
106+
.map(|(i, _)| i)
107+
.collect()
108+
}
109+
110+
/// Returns elements from `args` which have a Rust `str` as a raw pointer cast to a raw pointer.
111+
fn args_str_ptr_cast<'tcx>(cx: &LateContext<'tcx>, args: &[Expr<'tcx>]) -> Vec<Expr<'tcx>> {
112+
args.iter()
113+
.filter(|arg| {
114+
// just a simple check for
115+
// `let s: &str; libc::strlen(s.as_ptr() as *const _);`
116+
// or similarly with `as_mut_ptr`/`*mut`
117+
if let ExprKind::Cast(expr, ty) = arg.kind
118+
&& matches!(ty.kind, TyKind::Ptr(_))
119+
&& let ExprKind::MethodCall(method, this, _args, _span) = expr.kind
120+
&& ["as_ptr", "as_mut_ptr"].contains(&method.ident.name.as_str())
121+
&& is_ref_str(cx.typeck_results().expr_ty(this))
122+
{
123+
true
124+
} else {
125+
false
126+
}
127+
})
128+
.map(Clone::clone)
129+
.collect()
130+
}
131+
132+
/// is `ty` either `&str` or `&mut str`?
133+
fn is_ref_str(ty: Ty<'_>) -> bool {
134+
if let rustc_middle::ty::Ref(_, reffed_ty, _) = ty.kind() {
135+
return reffed_ty.is_str();
136+
}
137+
false
138+
}

tests/ui/str_ptr_in_c_abi.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
28+
#[allow(unused)]
29+
unsafe extern "C" fn strcpy(dst: *mut i8, src: *const i8) -> *mut i8 {
30+
unimplemented!()
31+
}
32+
33+
/// The same signature as in the libc crate, but without variadics
34+
#[allow(unused)]
35+
unsafe extern "C" fn printf(format: *const i8) -> i32 {
36+
unimplemented!()
37+
}

tests/ui/str_ptr_in_c_abi.stderr

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error: giving a Rust str pointer 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 Rust str pointer 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 Rust str pointer 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: aborting due to 3 previous errors
28+

0 commit comments

Comments
 (0)