Skip to content

Commit e7cf3cd

Browse files
RalfJungRalfJung
committed
declare imported functions with a nondescript signature
This means when the function is imported with multiple different signatures, they don't clash
1 parent 26b2b8d commit e7cf3cd

23 files changed

+332
-231
lines changed

‎compiler/rustc_ast_passes/src/feature_gate.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
324324
&self,
325325
link_llvm_intrinsics,
326326
i.span,
327-
"linking to LLVM intrinsics is experimental"
327+
"linking to LLVM intrinsics is an internal feature reserved for the standard library"
328328
);
329329
}
330330
}

‎compiler/rustc_codegen_llvm/src/abi.rs

+22-3
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,12 @@ pub(crate) trait FnAbiLlvmExt<'ll, 'tcx> {
321321
);
322322

323323
/// Apply attributes to a function call.
324-
fn apply_attrs_callsite(&self, bx: &mut Builder<'_, 'll, 'tcx>, callsite: &'ll Value);
324+
fn apply_attrs_callsite(
325+
&self,
326+
bx: &mut Builder<'_, 'll, 'tcx>,
327+
callsite: &'ll Value,
328+
instance: Option<ty::Instance<'tcx>>,
329+
);
325330
}
326331

327332
impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
@@ -527,11 +532,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
527532

528533
// If the declaration has an associated instance, compute extra attributes based on that.
529534
if let Some(instance) = instance {
530-
llfn_attrs_from_instance(cx, llfn, instance);
535+
llfn_attrs_from_instance(cx, instance, Some(llfn), |place, attrs| {
536+
attributes::apply_to_llfn(llfn, place, attrs)
537+
});
531538
}
532539
}
533540

534-
fn apply_attrs_callsite(&self, bx: &mut Builder<'_, 'll, 'tcx>, callsite: &'ll Value) {
541+
fn apply_attrs_callsite(
542+
&self,
543+
bx: &mut Builder<'_, 'll, 'tcx>,
544+
callsite: &'ll Value,
545+
instance: Option<ty::Instance<'tcx>>,
546+
) {
535547
let mut func_attrs = SmallVec::<[_; 2]>::new();
536548
if self.ret.layout.abi.is_uninhabited() {
537549
func_attrs.push(llvm::AttributeKind::NoReturn.create_attr(bx.cx.llcx));
@@ -649,6 +661,13 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
649661
&[element_type_attr],
650662
);
651663
}
664+
665+
// If the call site has an associated instance, compute extra attributes based on that.
666+
if let Some(instance) = instance {
667+
llfn_attrs_from_instance(bx.cx, instance, None, |place, attrs| {
668+
attributes::apply_to_callsite(callsite, place, attrs)
669+
});
670+
}
652671
}
653672
}
654673

‎compiler/rustc_codegen_llvm/src/attributes.rs

+39-29
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ use crate::context::CodegenCx;
1414
use crate::errors::{MissingFeatures, SanitizerMemtagRequiresMte, TargetFeatureDisableOrEnable};
1515
use crate::llvm::AttributePlace::Function;
1616
use crate::llvm::{self, AllocKindFlags, Attribute, AttributeKind, AttributePlace, MemoryEffects};
17+
use crate::llvm_util;
1718
use crate::value::Value;
18-
use crate::{attributes, llvm_util};
1919

2020
pub(crate) fn apply_to_llfn(llfn: &Value, idx: AttributePlace, attrs: &[&Attribute]) {
2121
if !attrs.is_empty() {
@@ -324,13 +324,18 @@ fn create_alloc_family_attr(llcx: &llvm::Context) -> &llvm::Attribute {
324324
llvm::CreateAttrStringValue(llcx, "alloc-family", "__rust_alloc")
325325
}
326326

327-
/// Helper for `FnAbi::apply_attrs_llfn`:
327+
/// Helper for `FnAbi::apply_attrs_*`:
328328
/// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
329329
/// attributes.
330+
///
331+
/// `apply_attrs` is called to apply the attributes, so this can be used both for declarations and
332+
/// calls. However, some things are not represented as attributes and can only be set on
333+
/// declarations, so `declare_llfn` should be `Some` if this is a declaration.
330334
pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
331335
cx: &CodegenCx<'ll, 'tcx>,
332-
llfn: &'ll Value,
333336
instance: ty::Instance<'tcx>,
337+
declare_llfn: Option<&'ll Value>,
338+
apply_attrs: impl Fn(AttributePlace, &[&Attribute]),
334339
) {
335340
let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(instance.def_id());
336341

@@ -445,7 +450,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
445450
to_add.push(create_alloc_family_attr(cx.llcx));
446451
// apply to argument place instead of function
447452
let alloc_align = AttributeKind::AllocAlign.create_attr(cx.llcx);
448-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(1), &[alloc_align]);
453+
apply_attrs(AttributePlace::Argument(1), &[alloc_align]);
449454
to_add.push(llvm::CreateAllocSizeAttr(cx.llcx, 0));
450455
let mut flags = AllocKindFlags::Alloc | AllocKindFlags::Aligned;
451456
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) {
@@ -456,7 +461,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
456461
to_add.push(llvm::CreateAllocKindAttr(cx.llcx, flags));
457462
// apply to return place instead of function (unlike all other attributes applied in this function)
458463
let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
459-
attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
464+
apply_attrs(AttributePlace::ReturnValue, &[no_alias]);
460465
}
461466
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::REALLOCATOR) {
462467
to_add.push(create_alloc_family_attr(cx.llcx));
@@ -466,26 +471,28 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
466471
));
467472
// applies to argument place instead of function place
468473
let allocated_pointer = AttributeKind::AllocatedPointer.create_attr(cx.llcx);
469-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(0), &[allocated_pointer]);
474+
apply_attrs(AttributePlace::Argument(0), &[allocated_pointer]);
470475
// apply to argument place instead of function
471476
let alloc_align = AttributeKind::AllocAlign.create_attr(cx.llcx);
472-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(2), &[alloc_align]);
477+
apply_attrs(AttributePlace::Argument(2), &[alloc_align]);
473478
to_add.push(llvm::CreateAllocSizeAttr(cx.llcx, 3));
474479
let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
475-
attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
480+
apply_attrs(AttributePlace::ReturnValue, &[no_alias]);
476481
}
477482
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::DEALLOCATOR) {
478483
to_add.push(create_alloc_family_attr(cx.llcx));
479484
to_add.push(llvm::CreateAllocKindAttr(cx.llcx, AllocKindFlags::Free));
480485
// applies to argument place instead of function place
481486
let allocated_pointer = AttributeKind::AllocatedPointer.create_attr(cx.llcx);
482-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(0), &[allocated_pointer]);
487+
apply_attrs(AttributePlace::Argument(0), &[allocated_pointer]);
483488
}
484489
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::CMSE_NONSECURE_ENTRY) {
485490
to_add.push(llvm::CreateAttrString(cx.llcx, "cmse_nonsecure_entry"));
486491
}
487492
if let Some(align) = codegen_fn_attrs.alignment {
488-
llvm::set_alignment(llfn, align);
493+
if let Some(llfn) = declare_llfn {
494+
llvm::set_alignment(llfn, align);
495+
}
489496
}
490497
if let Some(backchain) = backchain_attr(cx) {
491498
to_add.push(backchain);
@@ -503,24 +510,27 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
503510
let function_features =
504511
codegen_fn_attrs.target_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();
505512

506-
if let Some(f) = llvm_util::check_tied_features(
507-
cx.tcx.sess,
508-
&function_features.iter().map(|f| (*f, true)).collect(),
509-
) {
510-
let span = cx
511-
.tcx
512-
.get_attrs(instance.def_id(), sym::target_feature)
513-
.next()
514-
.map_or_else(|| cx.tcx.def_span(instance.def_id()), |a| a.span);
515-
cx.tcx
516-
.dcx()
517-
.create_err(TargetFeatureDisableOrEnable {
518-
features: f,
519-
span: Some(span),
520-
missing_features: Some(MissingFeatures),
521-
})
522-
.emit();
523-
return;
513+
// HACK: Avoid emitting the lint twice.
514+
if declare_llfn.is_some() {
515+
if let Some(f) = llvm_util::check_tied_features(
516+
cx.tcx.sess,
517+
&function_features.iter().map(|f| (*f, true)).collect(),
518+
) {
519+
let span = cx
520+
.tcx
521+
.get_attrs(instance.def_id(), sym::target_feature)
522+
.next()
523+
.map_or_else(|| cx.tcx.def_span(instance.def_id()), |a| a.span);
524+
cx.tcx
525+
.dcx()
526+
.create_err(TargetFeatureDisableOrEnable {
527+
features: f,
528+
span: Some(span),
529+
missing_features: Some(MissingFeatures),
530+
})
531+
.emit();
532+
return;
533+
}
524534
}
525535

526536
let function_features = function_features
@@ -562,7 +572,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
562572
to_add.push(llvm::CreateAttrStringValue(cx.llcx, "target-features", &target_features));
563573
}
564574

565-
attributes::apply_to_llfn(llfn, Function, &to_add);
575+
apply_attrs(Function, &to_add);
566576
}
567577

568578
fn wasm_import_module(tcx: TyCtxt<'_>, id: DefId) -> Option<&String> {

‎compiler/rustc_codegen_llvm/src/builder.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
262262
)
263263
};
264264
if let Some(fn_abi) = fn_abi {
265-
fn_abi.apply_attrs_callsite(self, invoke);
265+
fn_abi.apply_attrs_callsite(self, invoke, instance);
266266
}
267267
invoke
268268
}
@@ -1307,7 +1307,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
13071307
)
13081308
};
13091309
if let Some(fn_abi) = fn_abi {
1310-
fn_abi.apply_attrs_callsite(self, call);
1310+
fn_abi.apply_attrs_callsite(self, call, instance);
13111311
}
13121312
call
13131313
}
@@ -1657,7 +1657,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
16571657
)
16581658
};
16591659
if let Some(fn_abi) = fn_abi {
1660-
fn_abi.apply_attrs_callsite(self, callbr);
1660+
fn_abi.apply_attrs_callsite(self, callbr, instance);
16611661
}
16621662
callbr
16631663
}

‎compiler/rustc_codegen_llvm/src/declare.rs

+38-1
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
//! * When in doubt, define.
1313
1414
use itertools::Itertools;
15-
use rustc_codegen_ssa::traits::TypeMembershipMethods;
15+
use rustc_codegen_ssa::traits::{BaseTypeMethods, TypeMembershipMethods};
1616
use rustc_data_structures::fx::FxIndexSet;
17+
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
1718
use rustc_middle::ty::{Instance, Ty};
1819
use rustc_sanitizers::{cfi, kcfi};
1920
use smallvec::SmallVec;
@@ -127,6 +128,42 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
127128
) -> &'ll Value {
128129
debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
129130

131+
// FFI imports need to be treated specially: in Rust one can import the same function
132+
// with different signatures, but LLVM can only import each function once. Within a
133+
// codegen unit, whatever declaration we set up last will "win"; across codegen units,
134+
// LLVM is doing some sort of unspecified merging as part of LTO.
135+
// This is partially unsound due to LLVM bugs (https://github.com/llvm/llvm-project/issues/58976),
136+
// but on the Rust side we try to ensure soundness by making the least amount of promises
137+
// for these imports: no matter the signatures, we declare all functions as `fn()`.
138+
// If the same symbol is declared both as a function and a static, we just hope that
139+
// doesn't lead to unsoundness (or LLVM crashes)...
140+
let is_foreign_item = instance.is_some_and(|i| self.tcx.is_foreign_item(i.def_id()));
141+
// If this is a Rust native allocator function, we want its attributes, so we do not
142+
// treat it like the other FFI imports. If a user declares `__rust_alloc` we're going to have
143+
// one import with the attributes and one with the default signature; that should hopefully be fine
144+
// even if LLVM only sees the default one.
145+
let is_rust_alloc_fn = instance.is_some_and(|i| {
146+
let codegen_fn_flags = self.tcx.codegen_fn_attrs(i.def_id()).flags;
147+
codegen_fn_flags.contains(CodegenFnAttrFlags::ALLOCATOR)
148+
|| codegen_fn_flags.contains(CodegenFnAttrFlags::ALLOCATOR_ZEROED)
149+
|| codegen_fn_flags.contains(CodegenFnAttrFlags::REALLOCATOR)
150+
|| codegen_fn_flags.contains(CodegenFnAttrFlags::DEALLOCATOR)
151+
});
152+
// If this is calling an LLVM intrinsic, we must set the right signature or LLVM complains.
153+
// These are also unstable to call so nobody but us can screw up their signature.
154+
let is_llvm_builtin = name.starts_with("llvm.");
155+
if is_foreign_item && !is_rust_alloc_fn && !is_llvm_builtin {
156+
return declare_raw_fn(
157+
self,
158+
name,
159+
llvm::CallConv::CCallConv,
160+
llvm::UnnamedAddr::Global,
161+
llvm::Visibility::Default,
162+
// The function type for `fn()`.
163+
self.type_func(&[], self.type_void()),
164+
);
165+
}
166+
130167
// Function addresses in Rust are never significant, allowing functions to
131168
// be merged.
132169
let llfn = declare_raw_fn(

‎tests/codegen/aarch64-struct-align-128.rs

+36-16
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,17 @@ pub struct Wrapped8 {
3939
}
4040

4141
extern "C" {
42-
// linux: declare void @test_8([2 x i64], [2 x i64], [2 x i64])
43-
// darwin: declare void @test_8([2 x i64], [2 x i64], [2 x i64])
44-
// win: declare void @test_8([2 x i64], [2 x i64], [2 x i64])
4542
fn test_8(a: Align8, b: Transparent8, c: Wrapped8);
4643
}
4744

45+
#[no_mangle]
46+
fn call_test_8(a: Align8, b: Transparent8, c: Wrapped8) {
47+
// linux: call void @test_8([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, [2 x i64] {{%.*}})
48+
// darwin: call void @test_8([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, [2 x i64] {{%.*}})
49+
// win: call void @test_8([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, [2 x i64] {{%.*}})
50+
unsafe { test_8(a, b, c) }
51+
}
52+
4853
// Passed as `i128`, since it's an aggregate with size <= 128 bits, align = 128 bits.
4954
// EXCEPT on Linux, where there's a special case to use its unadjusted alignment,
5055
// making it the same as `Align8`, so it's be passed as `[i64 x 2]`.
@@ -69,12 +74,17 @@ pub struct Wrapped16 {
6974
}
7075

7176
extern "C" {
72-
// linux: declare void @test_16([2 x i64], [2 x i64], i128)
73-
// darwin: declare void @test_16(i128, i128, i128)
74-
// win: declare void @test_16(i128, i128, i128)
7577
fn test_16(a: Align16, b: Transparent16, c: Wrapped16);
7678
}
7779

80+
#[no_mangle]
81+
fn call_test_16(a: Align16, b: Transparent16, c: Wrapped16) {
82+
// linux: call void @test_16([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, i128 {{%.*}})
83+
// darwin: call void @test_16(i128 {{%.*}}, i128 {{%.*}}, i128 {{%.*}})
84+
// win: call void @test_16(i128 {{%.*}}, i128 {{%.*}}, i128 {{%.*}})
85+
unsafe { test_16(a, b, c) }
86+
}
87+
7888
// Passed as `i128`, since it's an aggregate with size <= 128 bits, align = 128 bits.
7989
#[repr(C)]
8090
pub struct I128 {
@@ -94,12 +104,17 @@ pub struct WrappedI128 {
94104
}
95105

96106
extern "C" {
97-
// linux: declare void @test_i128(i128, i128, i128)
98-
// darwin: declare void @test_i128(i128, i128, i128)
99-
// win: declare void @test_i128(i128, i128, i128)
100107
fn test_i128(a: I128, b: TransparentI128, c: WrappedI128);
101108
}
102109

110+
#[no_mangle]
111+
fn call_test_i128(a: I128, b: TransparentI128, c: WrappedI128) {
112+
// linux: call void @test_i128(i128 {{%.*}}, i128 {{%.*}}, i128 {{%.*}})
113+
// darwin: call void @test_i128(i128 {{%.*}}, i128 {{%.*}}, i128 {{%.*}})
114+
// win: call void @test_i128(i128 {{%.*}}, i128 {{%.*}}, i128 {{%.*}})
115+
unsafe { test_i128(a, b, c) }
116+
}
117+
103118
// Passed as `[2 x i64]`, since it's an aggregate with size <= 128 bits, align < 128 bits.
104119
// Note that the Linux special case does not apply, because packing is not considered "adjustment".
105120
#[repr(C)]
@@ -121,12 +136,17 @@ pub struct WrappedPacked {
121136
}
122137

123138
extern "C" {
124-
// linux: declare void @test_packed([2 x i64], [2 x i64], [2 x i64])
125-
// darwin: declare void @test_packed([2 x i64], [2 x i64], [2 x i64])
126-
// win: declare void @test_packed([2 x i64], [2 x i64], [2 x i64])
127139
fn test_packed(a: Packed, b: TransparentPacked, c: WrappedPacked);
128140
}
129141

142+
#[no_mangle]
143+
fn call_test_packed(a: Packed, b: TransparentPacked, c: WrappedPacked) {
144+
// linux: call void @test_packed([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, [2 x i64] {{%.*}})
145+
// darwin: call void @test_packed([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, [2 x i64] {{%.*}})
146+
// win: call void @test_packed([2 x i64] {{%.*}}, [2 x i64] {{%.*}}, [2 x i64] {{%.*}})
147+
unsafe { test_packed(a, b, c) }
148+
}
149+
130150
pub unsafe fn main(
131151
a1: Align8,
132152
a2: Transparent8,
@@ -141,8 +161,8 @@ pub unsafe fn main(
141161
d2: TransparentPacked,
142162
d3: WrappedPacked,
143163
) {
144-
test_8(a1, a2, a3);
145-
test_16(b1, b2, b3);
146-
test_i128(c1, c2, c3);
147-
test_packed(d1, d2, d3);
164+
call_test_8(a1, a2, a3);
165+
call_test_16(b1, b2, b3);
166+
call_test_i128(c1, c2, c3);
167+
call_test_packed(d1, d2, d3);
148168
}

‎tests/codegen/align-byval-vector.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,18 @@ pub struct DoubleFoo {
3737
}
3838

3939
extern "C" {
40-
// x86-linux: declare void @f({{.*}}byval([32 x i8]) align 4{{.*}})
41-
// x86-darwin: declare void @f({{.*}}byval([32 x i8]) align 16{{.*}})
4240
fn f(foo: Foo);
4341

44-
// x86-linux: declare void @g({{.*}}byval([64 x i8]) align 4{{.*}})
45-
// x86-darwin: declare void @g({{.*}}byval([64 x i8]) align 16{{.*}})
4642
fn g(foo: DoubleFoo);
4743
}
4844

4945
pub fn main() {
46+
// x86-linux: call void @f({{.*}}byval([32 x i8]) align 4 {{.*}})
47+
// x86-darwin: call void @f({{.*}}byval([32 x i8]) align 16 {{.*}})
5048
unsafe { f(Foo { a: i32x4(1, 2, 3, 4), b: 0 }) }
5149

50+
// x86-linux: call void @g({{.*}}byval([64 x i8]) align 4 {{.*}})
51+
// x86-darwin: call void @g({{.*}}byval([64 x i8]) align 16 {{.*}})
5252
unsafe {
5353
g(DoubleFoo {
5454
one: Foo { a: i32x4(1, 2, 3, 4), b: 0 },

0 commit comments

Comments
 (0)