Skip to content

Commit ca3dc99

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 d6c8169 commit ca3dc99

23 files changed

+332
-231
lines changed

Diff for: ‎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
}

Diff for: ‎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

Diff for: ‎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

@@ -440,7 +445,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
440445
to_add.push(create_alloc_family_attr(cx.llcx));
441446
// apply to argument place instead of function
442447
let alloc_align = AttributeKind::AllocAlign.create_attr(cx.llcx);
443-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(1), &[alloc_align]);
448+
apply_attrs(AttributePlace::Argument(1), &[alloc_align]);
444449
to_add.push(llvm::CreateAllocSizeAttr(cx.llcx, 0));
445450
let mut flags = AllocKindFlags::Alloc | AllocKindFlags::Aligned;
446451
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) {
@@ -451,7 +456,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
451456
to_add.push(llvm::CreateAllocKindAttr(cx.llcx, flags));
452457
// apply to return place instead of function (unlike all other attributes applied in this function)
453458
let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
454-
attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
459+
apply_attrs(AttributePlace::ReturnValue, &[no_alias]);
455460
}
456461
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::REALLOCATOR) {
457462
to_add.push(create_alloc_family_attr(cx.llcx));
@@ -461,26 +466,28 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
461466
));
462467
// applies to argument place instead of function place
463468
let allocated_pointer = AttributeKind::AllocatedPointer.create_attr(cx.llcx);
464-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(0), &[allocated_pointer]);
469+
apply_attrs(AttributePlace::Argument(0), &[allocated_pointer]);
465470
// apply to argument place instead of function
466471
let alloc_align = AttributeKind::AllocAlign.create_attr(cx.llcx);
467-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(2), &[alloc_align]);
472+
apply_attrs(AttributePlace::Argument(2), &[alloc_align]);
468473
to_add.push(llvm::CreateAllocSizeAttr(cx.llcx, 3));
469474
let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
470-
attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
475+
apply_attrs(AttributePlace::ReturnValue, &[no_alias]);
471476
}
472477
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::DEALLOCATOR) {
473478
to_add.push(create_alloc_family_attr(cx.llcx));
474479
to_add.push(llvm::CreateAllocKindAttr(cx.llcx, AllocKindFlags::Free));
475480
// applies to argument place instead of function place
476481
let allocated_pointer = AttributeKind::AllocatedPointer.create_attr(cx.llcx);
477-
attributes::apply_to_llfn(llfn, AttributePlace::Argument(0), &[allocated_pointer]);
482+
apply_attrs(AttributePlace::Argument(0), &[allocated_pointer]);
478483
}
479484
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::CMSE_NONSECURE_ENTRY) {
480485
to_add.push(llvm::CreateAttrString(cx.llcx, "cmse_nonsecure_entry"));
481486
}
482487
if let Some(align) = codegen_fn_attrs.alignment {
483-
llvm::set_alignment(llfn, align);
488+
if let Some(llfn) = declare_llfn {
489+
llvm::set_alignment(llfn, align);
490+
}
484491
}
485492
if let Some(backchain) = backchain_attr(cx) {
486493
to_add.push(backchain);
@@ -499,24 +506,27 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
499506
let function_features =
500507
codegen_fn_attrs.target_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();
501508

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

522532
let function_features = function_features
@@ -558,7 +568,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
558568
to_add.push(llvm::CreateAttrStringValue(cx.llcx, "target-features", &target_features));
559569
}
560570

561-
attributes::apply_to_llfn(llfn, Function, &to_add);
571+
apply_attrs(Function, &to_add);
562572
}
563573

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

Diff for: ‎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
}

Diff for: ‎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(

Diff for: ‎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
}

Diff for: ‎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)