dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #130301 - RalfJung:clashing_extern_declarations, r=compiler-errors 

some fixes for clashing_extern_declarations lint

There were two issues with the clashing_extern_declarations lint:
- It would accept non-`repr(C)` structs as compatible with each other by comparing their fields in declaration order, but the fields could have different memory order (and with `-Zrandomize-layout`, this can really happen).
- It would accept two types as compatible if `compare_layouts` returns `true`, but that function actually just compared the *ABI*, not the fully layout -- and all sized structs with more than 2 fields have the same ABI (`Abi::Aggregate`), so this missed a *lot* of cases.

We don't currently have a clear spec for what we *want* to consider "clashing" and what is fine, so I otherwise kept the original logic. I hope to have a t-lang discussion about this at some point. But meanwhile, these changes seem like clear bugfixes.
This commit is contained in:
Matthias Krüger 2024-09-13 18:25:46 +02:00 committed by GitHub
parent b0e323549a f362a59c3e
commit 7d36bfa6c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 172 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
compiler/rustc_lint/src/foreign_modules.rs
View File

@ -3,8 +3,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_middle::query::Providers;
use rustc_middle::ty::layout::LayoutError;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_middle::ty::{self, AdtDef, Instance, Ty, TyCtxt};
use rustc_session::declare_lint;
use rustc_span::{sym, Span, Symbol};
use rustc_target::abi::FIRST_VARIANT;
@ -212,7 +211,17 @@ fn structurally_same_type<'tcx>(
ckind: types::CItemKind,
) -> bool {
let mut seen_types = UnordSet::default();
structurally_same_type_impl(&mut seen_types, tcx, param_env, a, b, ckind)
let result = structurally_same_type_impl(&mut seen_types, tcx, param_env, a, b, ckind);
if cfg!(debug_assertions) && result {
// Sanity-check: must have same ABI, size and alignment.
// `extern` blocks cannot be generic, so we'll always get a layout here.
let a_layout = tcx.layout_of(param_env.and(a)).unwrap();
let b_layout = tcx.layout_of(param_env.and(b)).unwrap();
assert_eq!(a_layout.abi, b_layout.abi);
assert_eq!(a_layout.size, b_layout.size);
assert_eq!(a_layout.align, b_layout.align);
}
result
}
fn structurally_same_type_impl<'tcx>(
@ -266,30 +275,21 @@ fn structurally_same_type_impl<'tcx>(
// Do a full, depth-first comparison between the two.
use rustc_type_ir::TyKind::*;
let compare_layouts = |a, b| -> Result<bool, &'tcx LayoutError<'tcx>> {
debug!("compare_layouts({:?}, {:?})", a, b);
let a_layout = &tcx.layout_of(param_env.and(a))?.layout.abi();
let b_layout = &tcx.layout_of(param_env.and(b))?.layout.abi();
debug!(
"comparing layouts: {:?} == {:?} = {}",
a_layout,
b_layout,
a_layout == b_layout
);
Ok(a_layout == b_layout)
};
let is_primitive_or_pointer =
|ty: Ty<'tcx>| ty.is_primitive() || matches!(ty.kind(), RawPtr(..) | Ref(..));
ensure_sufficient_stack(|| {
match (a.kind(), b.kind()) {
(Adt(a_def, _), Adt(b_def, _)) => {
// We can immediately rule out these types as structurally same if
// their layouts differ.
match compare_layouts(a, b) {
Ok(false) => return false,
_ => (), // otherwise, continue onto the full, fields comparison
(&Adt(a_def, _), &Adt(b_def, _)) => {
// Only `repr(C)` types can be compared structurally.
if !(a_def.repr().c() && b_def.repr().c()) {
return false;
}
// If the types differ in their packed-ness, align, or simd-ness they conflict.
let repr_characteristica =
|def: AdtDef<'tcx>| (def.repr().pack, def.repr().align, def.repr().simd());
if repr_characteristica(a_def) != repr_characteristica(b_def) {
return false;
}
// Grab a flattened representation of all fields.
@ -311,9 +311,9 @@ fn structurally_same_type_impl<'tcx>(
},
)
}
(Array(a_ty, a_const), Array(b_ty, b_const)) => {
// For arrays, we also check the constness of the type.
a_const.kind() == b_const.kind()
(Array(a_ty, a_len), Array(b_ty, b_len)) => {
// For arrays, we also check the length.
a_len == b_len
&& structurally_same_type_impl(
seen_types, tcx, param_env, *a_ty, *b_ty, ckind,
)
@ -357,10 +357,9 @@ fn structurally_same_type_impl<'tcx>(
ckind,
)
}
(Tuple(a_args), Tuple(b_args)) => {
a_args.iter().eq_by(b_args.iter(), |a_ty, b_ty| {
structurally_same_type_impl(seen_types, tcx, param_env, a_ty, b_ty, ckind)
})
(Tuple(..), Tuple(..)) => {
// Tuples are not `repr(C)` so these cannot be compared structurally.
false
}
// For these, it's not quite as easy to define structural-sameness quite so easily.
// For the purposes of this lint, take the conservative approach and mark them as
@ -380,24 +379,21 @@ fn structurally_same_type_impl<'tcx>(
// An Adt and a primitive or pointer type. This can be FFI-safe if non-null
// enum layout optimisation is being applied.
(Adt(..), _) if is_primitive_or_pointer(b) => {
if let Some(ty) = types::repr_nullable_ptr(tcx, param_env, a, ckind) {
ty == b
if let Some(a_inner) = types::repr_nullable_ptr(tcx, param_env, a, ckind) {
a_inner == b
} else {
compare_layouts(a, b).unwrap_or(false)
false
}
}
(_, Adt(..)) if is_primitive_or_pointer(a) => {
if let Some(ty) = types::repr_nullable_ptr(tcx, param_env, b, ckind) {
ty == a
if let Some(b_inner) = types::repr_nullable_ptr(tcx, param_env, b, ckind) {
b_inner == a
} else {
compare_layouts(a, b).unwrap_or(false)
false
}
}
// Otherwise, just compare the layouts. This may fail to lint for some
// incompatible types, but at the very least, will stop reads into
// uninitialised memory.
_ => compare_layouts(a, b).unwrap_or(false),
_ => false,
}
})
}

2
tests/ui/lint/clashing-extern-fn-recursion.rs
View File

@ -92,6 +92,7 @@ mod ref_recursion_once_removed {
reffy: &'a Reffy2<'a>,
}
#[repr(C)]
struct Reffy2<'a> {
reffy: &'a Reffy1<'a>,
}
@ -107,6 +108,7 @@ mod ref_recursion_once_removed {
reffy: &'a Reffy2<'a>,
}
#[repr(C)]
struct Reffy2<'a> {
reffy: &'a Reffy1<'a>,
}

99
tests/ui/lint/clashing-extern-fn.rs
View File

@ -1,7 +1,6 @@
//@ check-pass
//@ aux-build:external_extern_fn.rs
#![crate_type = "lib"]
#![warn(clashing_extern_declarations)]
mod redeclared_different_signature {
mod a {
@ -132,7 +131,7 @@ mod banana {
mod three {
// This _should_ trigger the lint, because repr(packed) should generate a struct that has a
// different layout.
#[repr(packed)]
#[repr(C, packed)]
struct Banana {
weight: u32,
length: u16,
@ -143,6 +142,79 @@ mod banana {
//~^ WARN `weigh_banana` redeclared with a different signature
}
}
mod four {
// This _should_ trigger the lint, because the type is not repr(C).
struct Banana {
weight: u32,
length: u16,
}
#[allow(improper_ctypes)]
extern "C" {
fn weigh_banana(count: *const Banana) -> u64;
//~^ WARN `weigh_banana` redeclared with a different signature
}
}
}
// 3-field structs can't be distinguished by ScalarPair, side-stepping some shortucts
// the logic used to (incorrectly) take.
mod banana3 {
mod one {
#[repr(C)]
struct Banana {
weight: u32,
length: u16,
color: u8,
}
extern "C" {
fn weigh_banana3(count: *const Banana) -> u64;
}
}
mod two {
#[repr(C)]
struct Banana {
weight: u32,
length: u16,
color: u8,
} // note: distinct type
// This should not trigger the lint because two::Banana is structurally equivalent to
// one::Banana.
extern "C" {
fn weigh_banana3(count: *const Banana) -> u64;
}
}
mod three {
// This _should_ trigger the lint, because repr(packed) should generate a struct that has a
// different layout.
#[repr(C, packed)]
struct Banana {
weight: u32,
length: u16,
color: u8,
}
#[allow(improper_ctypes)]
extern "C" {
fn weigh_banana3(count: *const Banana) -> u64;
//~^ WARN `weigh_banana3` redeclared with a different signature
}
}
mod four {
// This _should_ trigger the lint, because the type is not repr(C).
struct Banana {
weight: u32,
length: u16,
color: u8,
}
#[allow(improper_ctypes)]
extern "C" {
fn weigh_banana3(count: *const Banana) -> u64;
//~^ WARN `weigh_banana3` redeclared with a different signature
}
}
}
mod sameish_members {
@ -223,27 +295,6 @@ mod transparent {
}
}
#[allow(improper_ctypes)]
mod zst {
mod transparent {
#[repr(transparent)]
struct TransparentZst(());
extern "C" {
fn zst() -> ();
fn transparent_zst() -> TransparentZst;
}
}
mod not_transparent {
struct NotTransparentZst(());
extern "C" {
// These shouldn't warn since all return types are zero sized
fn zst() -> NotTransparentZst;
fn transparent_zst() -> NotTransparentZst;
}
}
}
mod missing_return_type {
mod a {
extern "C" {
@ -384,6 +435,7 @@ mod unknown_layout {
extern "C" {
pub fn generic(l: Link<u32>);
}
#[repr(C)]
pub struct Link<T> {
pub item: T,
pub next: *const Link<T>,
@ -394,6 +446,7 @@ mod unknown_layout {
extern "C" {
pub fn generic(l: Link<u32>);
}
#[repr(C)]
pub struct Link<T> {
pub item: T,
pub next: *const Link<T>,

86
tests/ui/lint/clashing-extern-fn.stderr
View File

@ -1,5 +1,5 @@
warning: `extern` block uses type `Option<TransparentNoNiche>`, which is not FFI-safe
--> $DIR/clashing-extern-fn.rs:429:55
--> $DIR/clashing-extern-fn.rs:482:55
|
LL | fn hidden_niche_transparent_no_niche() -> Option<TransparentNoNiche>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
@ -9,7 +9,7 @@ LL | fn hidden_niche_transparent_no_niche() -> Option<TransparentNoN
= note: `#[warn(improper_ctypes)]` on by default
warning: `extern` block uses type `Option<UnsafeCell<NonZero<usize>>>`, which is not FFI-safe
--> $DIR/clashing-extern-fn.rs:433:46
--> $DIR/clashing-extern-fn.rs:486:46
|
LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usize>>>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
@ -18,7 +18,7 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
= note: enum has no representation hint
warning: `clash` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:14:13
--> $DIR/clashing-extern-fn.rs:13:13
|
LL | fn clash(x: u8);
| ---------------- `clash` previously declared here
@ -28,14 +28,10 @@ LL | fn clash(x: u64);
|
= note: expected `unsafe extern "C" fn(u8)`
found `unsafe extern "C" fn(u64)`
note: the lint level is defined here
--> $DIR/clashing-extern-fn.rs:4:9
|
LL | #![warn(clashing_extern_declarations)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `#[warn(clashing_extern_declarations)]` on by default
warning: `extern_link_name` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:52:9
--> $DIR/clashing-extern-fn.rs:51:9
|
LL | #[link_name = "extern_link_name"]
| --------------------------------- `extern_link_name` previously declared here
@ -47,7 +43,7 @@ LL | fn extern_link_name(x: u32);
found `unsafe extern "C" fn(u32)`
warning: `some_other_extern_link_name` redeclares `some_other_new_name` with a different signature
--> $DIR/clashing-extern-fn.rs:55:9
--> $DIR/clashing-extern-fn.rs:54:9
|
LL | fn some_other_new_name(x: i16);
| ------------------------------- `some_other_new_name` previously declared here
@ -59,7 +55,7 @@ LL | #[link_name = "some_other_new_name"]
found `unsafe extern "C" fn(u32)`
warning: `other_both_names_different` redeclares `link_name_same` with a different signature
--> $DIR/clashing-extern-fn.rs:59:9
--> $DIR/clashing-extern-fn.rs:58:9
|
LL | #[link_name = "link_name_same"]
| ------------------------------- `link_name_same` previously declared here
@ -71,7 +67,7 @@ LL | #[link_name = "link_name_same"]
found `unsafe extern "C" fn(u32)`
warning: `different_mod` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:72:9
--> $DIR/clashing-extern-fn.rs:71:9
|
LL | fn different_mod(x: u8);
| ------------------------ `different_mod` previously declared here
@ -83,7 +79,7 @@ LL | fn different_mod(x: u64);
found `unsafe extern "C" fn(u64)`
warning: `variadic_decl` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:82:9
--> $DIR/clashing-extern-fn.rs:81:9
|
LL | fn variadic_decl(x: u8, ...);
| ----------------------------- `variadic_decl` previously declared here
@ -95,7 +91,7 @@ LL | fn variadic_decl(x: u8);
found `unsafe extern "C" fn(u8)`
warning: `weigh_banana` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:142:13
--> $DIR/clashing-extern-fn.rs:141:13
|
LL | fn weigh_banana(count: *const Banana) -> u64;
| --------------------------------------------- `weigh_banana` previously declared here
@ -103,11 +99,47 @@ LL | fn weigh_banana(count: *const Banana) -> u64;
LL | fn weigh_banana(count: *const Banana) -> u64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn(*const one::Banana) -> u64`
found `unsafe extern "C" fn(*const three::Banana) -> u64`
= note: expected `unsafe extern "C" fn(*const banana::one::Banana) -> u64`
found `unsafe extern "C" fn(*const banana::three::Banana) -> u64`
warning: `weigh_banana` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:154:13
|
LL | fn weigh_banana(count: *const Banana) -> u64;
| --------------------------------------------- `weigh_banana` previously declared here
...
LL | fn weigh_banana(count: *const Banana) -> u64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn(*const banana::one::Banana) -> u64`
found `unsafe extern "C" fn(*const banana::four::Banana) -> u64`
warning: `weigh_banana3` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:200:13
|
LL | fn weigh_banana3(count: *const Banana) -> u64;
| ---------------------------------------------- `weigh_banana3` previously declared here
...
LL | fn weigh_banana3(count: *const Banana) -> u64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn(*const banana3::one::Banana) -> u64`
found `unsafe extern "C" fn(*const banana3::three::Banana) -> u64`
warning: `weigh_banana3` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:214:13
|
LL | fn weigh_banana3(count: *const Banana) -> u64;
| ---------------------------------------------- `weigh_banana3` previously declared here
...
LL | fn weigh_banana3(count: *const Banana) -> u64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn(*const banana3::one::Banana) -> u64`
found `unsafe extern "C" fn(*const banana3::four::Banana) -> u64`
warning: `draw_point` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:171:13
--> $DIR/clashing-extern-fn.rs:243:13
|
LL | fn draw_point(p: Point);
| ------------------------ `draw_point` previously declared here
@ -119,7 +151,7 @@ LL | fn draw_point(p: Point);
found `unsafe extern "C" fn(sameish_members::b::Point)`
warning: `origin` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:197:13
--> $DIR/clashing-extern-fn.rs:269:13
|
LL | fn origin() -> Point3;
| ---------------------- `origin` previously declared here
@ -131,7 +163,7 @@ LL | fn origin() -> Point3;
found `unsafe extern "C" fn() -> same_sized_members_clash::b::Point3`
warning: `transparent_incorrect` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:220:13
--> $DIR/clashing-extern-fn.rs:292:13
|
LL | fn transparent_incorrect() -> T;
| -------------------------------- `transparent_incorrect` previously declared here
@ -143,7 +175,7 @@ LL | fn transparent_incorrect() -> isize;
found `unsafe extern "C" fn() -> isize`
warning: `missing_return_type` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:259:13
--> $DIR/clashing-extern-fn.rs:310:13
|
LL | fn missing_return_type() -> usize;
| ---------------------------------- `missing_return_type` previously declared here
@ -155,7 +187,7 @@ LL | fn missing_return_type();
found `unsafe extern "C" fn()`
warning: `non_zero_usize` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:277:13
--> $DIR/clashing-extern-fn.rs:328:13
|
LL | fn non_zero_usize() -> core::num::NonZero<usize>;
| ------------------------------------------------- `non_zero_usize` previously declared here
@ -167,7 +199,7 @@ LL | fn non_zero_usize() -> usize;
found `unsafe extern "C" fn() -> usize`
warning: `non_null_ptr` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:279:13
--> $DIR/clashing-extern-fn.rs:330:13
|
LL | fn non_null_ptr() -> core::ptr::NonNull<usize>;
| ----------------------------------------------- `non_null_ptr` previously declared here
@ -179,7 +211,7 @@ LL | fn non_null_ptr() -> *const usize;
found `unsafe extern "C" fn() -> *const usize`
warning: `option_non_zero_usize_incorrect` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:373:13
--> $DIR/clashing-extern-fn.rs:424:13
|
LL | fn option_non_zero_usize_incorrect() -> usize;
| ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here
@ -191,7 +223,7 @@ LL | fn option_non_zero_usize_incorrect() -> isize;
found `unsafe extern "C" fn() -> isize`
warning: `option_non_null_ptr_incorrect` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:375:13
--> $DIR/clashing-extern-fn.rs:426:13
|
LL | fn option_non_null_ptr_incorrect() -> *const usize;
| --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here
@ -203,7 +235,7 @@ LL | fn option_non_null_ptr_incorrect() -> *const isize;
found `unsafe extern "C" fn() -> *const isize`
warning: `hidden_niche_transparent_no_niche` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:429:13
--> $DIR/clashing-extern-fn.rs:482:13
|
LL | fn hidden_niche_transparent_no_niche() -> usize;
| ------------------------------------------------ `hidden_niche_transparent_no_niche` previously declared here
@ -215,7 +247,7 @@ LL | fn hidden_niche_transparent_no_niche() -> Option<TransparentNoN
found `unsafe extern "C" fn() -> Option<TransparentNoNiche>`
warning: `hidden_niche_unsafe_cell` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:433:13
--> $DIR/clashing-extern-fn.rs:486:13
|
LL | fn hidden_niche_unsafe_cell() -> usize;
| --------------------------------------- `hidden_niche_unsafe_cell` previously declared here
@ -226,5 +258,5 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
= note: expected `unsafe extern "C" fn() -> usize`
found `unsafe extern "C" fn() -> Option<UnsafeCell<NonZero<usize>>>`
warning: 19 warnings emitted
warning: 22 warnings emitted