mathieu/os-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Move linkage type check to HIR analysis and fix semantics issues.

Browse source 
This ensures that the error is printed even for unused variables,
as well as unifying the handling between the LLVM and GCC backends.

This also fixes unusual behavior around exported Rust-defined variables
with linkage attributes. With the previous behavior, it appears to be
impossible to define such a variable such that it can actually be imported
and used by another crate. This is because on the importing side, the
variable is required to be a pointer, but on the exporting side, the
type checker rejects static variables of pointer type because they do
not implement `Sync`. Even if it were possible to import such a type, it
appears that code generation on the importing side would add an unexpected
additional level of pointer indirection, which would break type safety.

This highlighted that the semantics of linkage on Rust-defined variables
is different to linkage on foreign items. As such, we now model the
difference with two different codegen attributes: linkage for Rust-defined
variables, and import_linkage for foreign items.

This change gives semantics to the test
src/test/ui/linkage-attr/auxiliary/def_illtyped_external.rs which was
previously expected to fail to compile. Therefore, convert it into a
test that is expected to successfully compile.

The update to the GCC backend is speculative and untested.
This commit is contained in:
Peter Collingbourne 2022-11-23 18:13:30 -08:00
parent e1d819583f
commit 5873ebeef3
19 changed files with 56 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
compiler/rustc_codegen_gcc/src/consts.rs
View file

@ -8,13 +8,11 @@ use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::mir::interpret::{self, ConstAllocation, ErrorHandled, Scalar as InterpScalar, read_target_uint};
use rustc_span::Span;
use rustc_span::def_id::DefId;
use rustc_target::abi::{self, Align, HasDataLayout, Primitive, Size, WrappingRange};
use crate::base;
use crate::context::CodegenCx;
use crate::errors::LinkageConstOrMutType;
use crate::type_of::LayoutGccExt;
impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
@ -239,12 +237,12 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
}
Node::ForeignItem(&hir::ForeignItem {
span,
span: _,
kind: hir::ForeignItemKind::Static(..),
..
}) => {
let fn_attrs = self.tcx.codegen_fn_attrs(def_id);
check_and_apply_linkage(&self, &fn_attrs, ty, sym, span)
check_and_apply_linkage(&self, &fn_attrs, ty, sym)
}
item => bug!("get_static: expected static, found {:?}", item),
@ -257,8 +255,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
//debug!("get_static: sym={} item_attr={:?}", sym, self.tcx.item_attrs(def_id));
let attrs = self.tcx.codegen_fn_attrs(def_id);
let span = self.tcx.def_span(def_id);
let global = check_and_apply_linkage(&self, &attrs, ty, sym, span);
let global = check_and_apply_linkage(&self, &attrs, ty, sym);
let needs_dll_storage_attr = false; // TODO(antoyo)
@ -355,24 +352,12 @@ pub fn codegen_static_initializer<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, def_id
Ok((const_alloc_to_gcc(cx, alloc), alloc))
}
fn check_and_apply_linkage<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, attrs: &CodegenFnAttrs, ty: Ty<'tcx>, sym: &str, span: Span) -> LValue<'gcc> {
fn check_and_apply_linkage<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, attrs: &CodegenFnAttrs, ty: Ty<'tcx>, sym: &str) -> LValue<'gcc> {
let is_tls = attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL);
let llty = cx.layout_of(ty).gcc_type(cx, true);
if let Some(linkage) = attrs.linkage {
// If this is a static with a linkage specified, then we need to handle
// it a little specially. The typesystem prevents things like &T and
// extern "C" fn() from being non-null, so we can't just declare a
// static and call it a day. Some linkages (like weak) will make it such
// that the static actually has a null value.
let llty2 =
if let ty::RawPtr(ref mt) = ty.kind() {
cx.layout_of(mt.ty).gcc_type(cx, true)
}
else {
cx.sess().emit_fatal(LinkageConstOrMutType { span: span })
};
if let Some(linkage) = attrs.import_linkage {
// Declare a symbol `foo` with the desired linkage.
let global1 = cx.declare_global_with_linkage(&sym, llty2, base::global_linkage_to_gcc(linkage));
let global1 = cx.declare_global_with_linkage(&sym, cx.type_i8(), base::global_linkage_to_gcc(linkage));
// Declare an internal global `extern_with_linkage_foo` which
// is initialized with the address of `foo`. If `foo` is

7
compiler/rustc_codegen_gcc/src/errors.rs
View file

@ -211,13 +211,6 @@ pub(crate) struct InvalidMonomorphizationUnsupportedOperation<'a> {
pub in_elem: Ty<'a>,
}
#[derive(Diagnostic)]
#[diag(codegen_gcc_linkage_const_or_mut_type)]
pub(crate) struct LinkageConstOrMutType {
#[primary_span]
pub span: Span,
}
#[derive(Diagnostic)]
#[diag(codegen_gcc_lto_not_supported)]
pub(crate) struct LTONotSupported;

18
compiler/rustc_codegen_llvm/src/consts.rs
View file

@ -1,7 +1,7 @@
use crate::base;
use crate::common::{self, CodegenCx};
use crate::debuginfo;
use crate::errors::{InvalidMinimumAlignment, LinkageConstOrMutType, SymbolAlreadyDefined};
use crate::errors::{InvalidMinimumAlignment, SymbolAlreadyDefined};
use crate::llvm::{self, True};
use crate::llvm_util;
use crate::type_::Type;
@ -162,22 +162,12 @@ fn check_and_apply_linkage<'ll, 'tcx>(
def_id: DefId,
) -> &'ll Value {
let llty = cx.layout_of(ty).llvm_type(cx);
if let Some(linkage) = attrs.linkage {
if let Some(linkage) = attrs.import_linkage {
debug!("get_static: sym={} linkage={:?}", sym, linkage);
// If this is a static with a linkage specified, then we need to handle
// it a little specially. The typesystem prevents things like &T and
// extern "C" fn() from being non-null, so we can't just declare a
// static and call it a day. Some linkages (like weak) will make it such
// that the static actually has a null value.
let llty2 = if let ty::RawPtr(ref mt) = ty.kind() {
cx.layout_of(mt.ty).llvm_type(cx)
} else {
cx.sess().emit_fatal(LinkageConstOrMutType { span: cx.tcx.def_span(def_id) })
};
unsafe {
// Declare a symbol `foo` with the desired linkage.
let g1 = cx.declare_global(sym, llty2);
let g1 = cx.declare_global(sym, cx.type_i8());
llvm::LLVMRustSetLinkage(g1, base::linkage_to_llvm(linkage));
// Declare an internal global `extern_with_linkage_foo` which
@ -195,7 +185,7 @@ fn check_and_apply_linkage<'ll, 'tcx>(
})
});
llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage);
llvm::LLVMSetInitializer(g2, g1);
llvm::LLVMSetInitializer(g2, cx.const_ptrcast(g1, llty));
g2
}
} else if cx.tcx.sess.target.arch == "x86" &&

7
compiler/rustc_codegen_llvm/src/errors.rs
View file

@ -61,13 +61,6 @@ pub(crate) struct InvalidMinimumAlignment {
pub err: String,
}
#[derive(Diagnostic)]
#[diag(codegen_llvm_linkage_const_or_mut_type)]
pub(crate) struct LinkageConstOrMutType {
#[primary_span]
pub span: Span,
}
#[derive(Diagnostic)]
#[diag(codegen_llvm_sanitizer_memtag_requires_mte)]
pub(crate) struct SanitizerMemtagRequiresMte;

3
compiler/rustc_error_messages/locales/en-US/codegen_gcc.ftl
View file

@ -1,6 +1,3 @@
codegen_gcc_linkage_const_or_mut_type =
must have type `*const T` or `*mut T` due to `#[linkage]` attribute
codegen_gcc_unwinding_inline_asm =
GCC backend does not support unwinding from inline asm

3
compiler/rustc_error_messages/locales/en-US/codegen_llvm.ftl
View file

@ -23,9 +23,6 @@ codegen_llvm_branch_protection_requires_aarch64 =
codegen_llvm_invalid_minimum_alignment =
invalid minimum global alignment: {$err}
codegen_llvm_linkage_const_or_mut_type =
must have type `*const T` or `*mut T` due to `#[linkage]` attribute
codegen_llvm_sanitizer_memtag_requires_mte =
`-Zsanitizer=memtag` requires `-Ctarget-feature=+mte`

3
compiler/rustc_error_messages/locales/en-US/hir_analysis.ftl
View file

@ -113,3 +113,6 @@ hir_analysis_const_bound_for_non_const_trait =
hir_analysis_self_in_impl_self =
`Self` is not valid in the self type of an impl block
.note = replace `Self` with a different type
hir_analysis_linkage_type =
must have type `*const T` or `*mut T` due to `#[linkage]` attribute

14
compiler/rustc_hir_analysis/src/check/check.rs
View file

@ -1,4 +1,5 @@
use crate::check::intrinsicck::InlineAsmCtxt;
use crate::errors::LinkageType;
use super::compare_method::check_type_bounds;
use super::compare_method::{compare_impl_method, compare_ty_impl};
@ -478,6 +479,17 @@ fn check_opaque_meets_bounds<'tcx>(
let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
}
fn check_static_linkage<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) {
if tcx.codegen_fn_attrs(def_id).import_linkage.is_some() {
if match tcx.type_of(def_id).kind() {
ty::RawPtr(_) => false,
_ => true,
} {
tcx.sess.emit_err(LinkageType { span: tcx.def_span(def_id) });
}
}
}
fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) {
debug!(
"check_item_type(it.def_id={:?}, it.name={})",
@ -490,6 +502,7 @@ fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) {
tcx.ensure().typeck(id.owner_id.def_id);
maybe_check_static_with_link_section(tcx, id.owner_id.def_id);
check_static_inhabited(tcx, id.owner_id.def_id);
check_static_linkage(tcx, id.owner_id.def_id);
}
DefKind::Const => {
tcx.ensure().typeck(id.owner_id.def_id);
@ -627,6 +640,7 @@ fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) {
}
hir::ForeignItemKind::Static(..) => {
check_static_inhabited(tcx, def_id);
check_static_linkage(tcx, def_id);
}
_ => {}
}

7
compiler/rustc_hir_analysis/src/collect.rs
View file

@ -1814,7 +1814,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs {
);
} else if attr.has_name(sym::linkage) {
if let Some(val) = attr.value_str() {
codegen_fn_attrs.linkage = Some(linkage_by_name(tcx, did, val.as_str()));
let linkage = Some(linkage_by_name(tcx, did, val.as_str()));
if tcx.is_foreign_item(did) {
codegen_fn_attrs.import_linkage = linkage;
} else {
codegen_fn_attrs.linkage = linkage;
}
}
} else if attr.has_name(sym::link_section) {
if let Some(val) = attr.value_str() {

7
compiler/rustc_hir_analysis/src/errors.rs
View file

@ -285,3 +285,10 @@ pub struct SelfInImplSelf {
#[note]
pub note: (),
}
#[derive(Diagnostic)]
#[diag(hir_analysis_linkage_type)]
pub(crate) struct LinkageType {
#[primary_span]
pub span: Span,
}

5
compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
View file

@ -26,8 +26,10 @@ pub struct CodegenFnAttrs {
/// The `#[target_feature(enable = "...")]` attribute and the enabled
/// features (only enabled features are supported right now).
pub target_features: Vec<Symbol>,
/// The `#[linkage = "..."]` attribute and the value we found.
/// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
pub linkage: Option<Linkage>,
/// The `#[linkage = "..."]` attribute on foreign items and the value we found.
pub import_linkage: Option<Linkage>,
/// The `#[link_section = "..."]` attribute, or what executable section this
/// should be placed in.
pub link_section: Option<Symbol>,
@ -113,6 +115,7 @@ impl CodegenFnAttrs {
link_ordinal: None,
target_features: vec![],
linkage: None,
import_linkage: None,
link_section: None,
no_sanitize: SanitizerSet::empty(),
instruction_set: None,

2
src/test/ui/feature-gates/feature-gate-linkage.rs
View file

@ -1,5 +1,5 @@
extern "C" {
#[linkage = "extern_weak"] static foo: isize;
#[linkage = "extern_weak"] static foo: *mut isize;
//~^ ERROR: the `linkage` attribute is experimental and not portable
}

2
src/test/ui/feature-gates/feature-gate-linkage.stderr
View file

@ -1,7 +1,7 @@
error[E0658]: the `linkage` attribute is experimental and not portable across platforms
--> $DIR/feature-gate-linkage.rs:2:5
|
LL | #[linkage = "extern_weak"] static foo: isize;
LL | #[linkage = "extern_weak"] static foo: *mut isize;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #29603 <https://github.com/rust-lang/rust/issues/29603> for more information

0
src/test/ui/linkage-attr/auxiliary/def_illtyped_external.rs → src/test/ui/linkage-attr/auxiliary/def_external.rs
View file

8
src/test/ui/linkage-attr/linkage-import.rs Normal file
View file

@ -0,0 +1,8 @@
// build-pass
// aux-build:def_external.rs
extern crate def_external as dep;
fn main() {
println!("{:p}", &dep::EXTERN);
}

11
src/test/ui/linkage-attr/linkage-requires-raw-ptr.rs
View file

@ -1,11 +0,0 @@
// rust-lang/rust#59548: We used to ICE when trying to use a static
// with a type that violated its own `#[linkage]`.
// build-fail
// aux-build:def_illtyped_external.rs
extern crate def_illtyped_external as dep;
fn main() {
println!("{:p}", &dep::EXTERN);
}

8
src/test/ui/linkage-attr/linkage-requires-raw-ptr.stderr
View file

@ -1,8 +0,0 @@
error: must have type `*const T` or `*mut T` due to `#[linkage]` attribute
--> $DIR/auxiliary/def_illtyped_external.rs:5:1
|
LL | pub static EXTERN: u32 = 0;
| ^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error

7
src/test/ui/linkage-attr/linkage2.rs
View file

@ -1,9 +1,4 @@
// FIXME https://github.com/rust-lang/rust/issues/59774
// build-fail
// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> ""
// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> ""
// ignore-sgx no weak linkages permitted
// check-fail
#![feature(linkage)]

2
src/test/ui/linkage-attr/linkage2.stderr
View file

@ -1,5 +1,5 @@
error: must have type `*const T` or `*mut T` due to `#[linkage]` attribute
--> $DIR/linkage2.rs:12:5
--> $DIR/linkage2.rs:7:5
|
LL | static foo: i32;
| ^^^^^^^^^^^^^^^