Skip to content

Support Option and similar enums as type of static variable with linkage attribute #104799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 6 additions & 21 deletions compiler/rustc_codegen_gcc/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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),
Expand All @@ -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)

Expand Down Expand Up @@ -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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might need to do something else with GCC, but I don't have a setup for testing the GCC backend.

I asked around on Zulip and I was told that developers do not need to keep the GCC backend working; their changes just needs to pass CI.


// Declare an internal global `extern_with_linkage_foo` which
// is initialized with the address of `foo`. If `foo` is
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_codegen_gcc/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 4 additions & 14 deletions compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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" &&
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_codegen_llvm/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_error_codes/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ E0786: include_str!("./error_codes/E0786.md"),
E0787: include_str!("./error_codes/E0787.md"),
E0788: include_str!("./error_codes/E0788.md"),
E0790: include_str!("./error_codes/E0790.md"),
E0791: include_str!("./error_codes/E0791.md"),
;
// E0006, // merged with E0005
// E0008, // cannot bind by-move into a pattern guard
Expand Down
41 changes: 41 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0791.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
Static variables with the `#[linkage]` attribute within external blocks
must have one of the following types, which are equivalent to a nullable
pointer in C:

* `*mut T` or `*const T`, where `T` may be any type.

* An enumerator type with no `#[repr]` attribute and with two variants, where
one of the variants has no fields, and the other has a single field of one of
the following non-nullable types:
* Reference type
* Function pointer type

The variants can appear in either order.

For example, the following declaration is invalid:

```compile_fail,E0791
#![feature(linkage)]

extern "C" {
#[linkage = "extern_weak"]
static foo: i8;
}
```

The following declarations are valid:

```
#![feature(linkage)]

extern "C" {
#[linkage = "extern_weak"]
static foo: Option<unsafe extern "C" fn()>;

#[linkage = "extern_weak"]
static bar: Option<&'static i8>;

#[linkage = "extern_weak"]
static baz: *mut i8;
}
```
3 changes: 0 additions & 3 deletions compiler/rustc_error_messages/locales/en-US/codegen_gcc.ftl
Original file line number Diff line number Diff line change
@@ -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

Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_error_messages/locales/en-US/codegen_llvm.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/hir_analysis.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
invalid type for variable with `#[linkage]` attribute
35 changes: 34 additions & 1 deletion compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -20,7 +21,7 @@ use rustc_middle::middle::stability::EvalResult;
use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES};
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable};
use rustc_middle::ty::{self, AdtDef, ParamEnv, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable};
use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS};
use rustc_span::symbol::sym;
use rustc_span::{self, Span};
Expand Down Expand Up @@ -478,6 +479,36 @@ fn check_opaque_meets_bounds<'tcx>(
let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
}

fn is_enum_of_nonnullable_ptr<'tcx>(
tcx: TyCtxt<'tcx>,
adt_def: AdtDef<'tcx>,
substs: SubstsRef<'tcx>,
) -> bool {
if adt_def.repr().inhibit_enum_layout_opt() {
return false;
}

let [var_one, var_two] = &adt_def.variants().raw[..] else {
return false;
};
let (([], [field]) | ([field], [])) = (&var_one.fields[..], &var_two.fields[..]) else {
return false;
};
matches!(field.ty(tcx, substs).kind(), ty::FnPtr(..) | ty::Ref(..))
}

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,
ty::Adt(adt_def, substs) => !is_enum_of_nonnullable_ptr(tcx, *adt_def, *substs),
_ => 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={})",
Expand All @@ -490,6 +521,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);
Expand Down Expand Up @@ -627,6 +659,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);
}
_ => {}
}
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,10 @@ pub struct SelfInImplSelf {
#[note]
pub note: (),
}

#[derive(Diagnostic)]
#[diag(hir_analysis_linkage_type, code = "E0791")]
pub(crate) struct LinkageType {
#[primary_span]
pub span: Span,
}
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 36 additions & 1 deletion library/std/src/sys/unix/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,21 @@ use crate::ptr;
use crate::sync::atomic::{self, AtomicPtr, Ordering};

// We can use true weak linkage on ELF targets.
#[cfg(not(any(target_os = "macos", target_os = "ios")))]
#[cfg(all(not(any(target_os = "macos", target_os = "ios")), not(bootstrap)))]
pub(crate) macro weak {
(fn $name:ident($($t:ty),*) -> $ret:ty) => (
let ref $name: ExternWeak<unsafe extern "C" fn($($t),*) -> $ret> = {
extern "C" {
#[linkage = "extern_weak"]
static $name: Option<unsafe extern "C" fn($($t),*) -> $ret>;
}
#[allow(unused_unsafe)]
ExternWeak::new(unsafe { $name })
};
)
}

#[cfg(all(not(any(target_os = "macos", target_os = "ios")), bootstrap))]
pub(crate) macro weak {
(fn $name:ident($($t:ty),*) -> $ret:ty) => (
let ref $name: ExternWeak<unsafe extern "C" fn($($t),*) -> $ret> = {
Expand All @@ -47,18 +61,39 @@ pub(crate) macro weak {
#[cfg(any(target_os = "macos", target_os = "ios"))]
pub(crate) use self::dlsym as weak;

#[cfg(not(bootstrap))]
pub(crate) struct ExternWeak<F: Copy> {
weak_ptr: Option<F>,
}

#[cfg(not(bootstrap))]
impl<F: Copy> ExternWeak<F> {
#[inline]
pub(crate) fn new(weak_ptr: Option<F>) -> Self {
ExternWeak { weak_ptr }
}

#[inline]
pub(crate) fn get(&self) -> Option<F> {
self.weak_ptr
}
}

#[cfg(bootstrap)]
pub(crate) struct ExternWeak<F> {
weak_ptr: *const libc::c_void,
_marker: PhantomData<F>,
}

#[cfg(bootstrap)]
impl<F> ExternWeak<F> {
#[inline]
pub(crate) fn new(weak_ptr: *const libc::c_void) -> Self {
ExternWeak { weak_ptr, _marker: PhantomData }
}
}

#[cfg(bootstrap)]
impl<F> ExternWeak<F> {
#[inline]
pub(crate) fn get(&self) -> Option<F> {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/feature-gates/feature-gate-linkage.rs
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down
Loading