Skip to content

Don't use llvm_type as the type of statics. #165371

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.

Sign up for GitHub

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
rust-bors merged 1 commit into from
Nov 3, 2026
+50 −44
Merged

Don't use llvm_type as the type of statics. #165371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 27 additions & 27 deletions compiler/rustc_codegen_llvm/src/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use tracing::{debug, instrument, trace};
use crate::common::CodegenCx;
use crate::diagnostics::SymbolAlreadyDefined;
use crate::llvm::{self, Type, Value, const_ptr_auth};
use crate::type_of::LayoutLlvmExt;
use crate::{base, debuginfo};

/// Indicates whether a value originates from a `static`.
Expand Down Expand Up @@ -200,7 +199,7 @@ fn check_and_apply_linkage<'ll, 'tcx>(
let mut should_sign = false;
// Declare a symbol `foo`. If `foo` is an extern_weak symbol, we declare
// an extern_weak function, otherwise a global with the desired linkage.
let g1 = if matches!(attrs.import_linkage, Some(Linkage::ExternalWeak)) {
let actual_global = if matches!(attrs.import_linkage, Some(Linkage::ExternalWeak)) {
// An `extern_weak` function is represented as an `Option<unsafe extern ...>`,
// we extract the function signature and declare it as an extern_weak function
// instead of an extern_weak i8.
Expand All @@ -224,28 +223,11 @@ fn check_and_apply_linkage<'ll, 'tcx>(
} else {
cx.declare_global(sym, cx.type_i8())
};
llvm::set_linkage(g1, base::linkage_to_llvm(linkage));
llvm::set_linkage(actual_global, base::linkage_to_llvm(linkage));

// Normally this is done in `get_static_inner`, but when as we generate an internal global,
// it will apply the dso_local to the internal global instead, so do it here, too.
cx.assume_dso_local(g1, true);

// Declare an internal global `extern_with_linkage_foo` which
// is initialized with the address of `foo`. If `foo` is
// discarded during linking (for example, if `foo` has weak
// linkage and there are no definitions), then
// `extern_with_linkage_foo` will instead be initialized to
// zero.
let real_name =
format!("_rust_extern_with_linkage_{:016x}_{sym}", cx.tcx.stable_crate_id(LOCAL_CRATE));
let g2 = cx.define_global(&real_name, llty).unwrap_or_else(|| {
cx.sess().dcx().emit_fatal(SymbolAlreadyDefined {
span: cx.tcx.def_span(def_id),
symbol_name: sym,
})
});
llvm::set_linkage(g2, llvm::Linkage::InternalLinkage);
llvm::set_unnamed_address(g2, llvm::UnnamedAddr::Global);
cx.assume_dso_local(actual_global, true);

// Sign the function pointer that is used to initialize the global
let initializer = if should_sign {
Expand All @@ -256,18 +238,36 @@ fn check_and_apply_linkage<'ll, 'tcx>(
.expect("pointer authentication functions must be available when signing");

const_ptr_auth(
cx.const_bitcast(g1, llty),
actual_global,
schema.key as u32,
schema.constant_discriminator as u64,
None, /* address_diversity */
)
} else {
g1
actual_global
};

llvm::set_initializer(g2, initializer);

g2
// Declare an internal global `extern_with_linkage_foo` which
// is initialized with the address of `foo`. If `foo` is
// discarded during linking (for example, if `foo` has weak
// linkage and there are no definitions), then
// `extern_with_linkage_foo` will instead be initialized to
// zero.
let real_name =
format!("_rust_extern_with_linkage_{:016x}_{sym}", cx.tcx.stable_crate_id(LOCAL_CRATE));
let pointer_global =
cx.define_global(&real_name, cx.val_ty(initializer)).unwrap_or_else(|| {
cx.sess().dcx().emit_fatal(SymbolAlreadyDefined {
span: cx.tcx.def_span(def_id),
symbol_name: sym,
})
});
llvm::set_linkage(pointer_global, llvm::Linkage::InternalLinkage);
llvm::set_unnamed_address(pointer_global, llvm::UnnamedAddr::Global);

llvm::set_initializer(pointer_global, initializer);

pointer_global
} else if cx.tcx.sess.target.arch == Arch::X86
&& common::is_using_dlltool(&cx.tcx.sess.target)
&& let Some(dllimport) = crate::common::get_dllimport(cx.tcx, def_id, sym)
Expand Down Expand Up @@ -358,7 +358,7 @@ impl<'ll> CodegenCx<'ll, '_> {
} else {
let ty = instance.ty(self.tcx, self.typing_env());
trace!(?ty);
self.layout_of(ty).llvm_type(self)
self.type_array(self.type_i8(), self.layout_of(ty).size.bytes())
};
self.get_static_inner(def_id, llty)
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ pub(crate) fn add_global<'ll>(llmod: Module<'ll>, ty: &'ll Type, name_cstr: &CSt
}

pub(crate) fn set_initializer(llglobal: &Value, constant_val: &Value) {
unsafe {
assert_eq!(
crate::llvm::LLVMGlobalGetValueType(llglobal),
crate::llvm::LLVMTypeOf(constant_val)
);
}
unsafe {
LLVMSetInitializer(llglobal, constant_val);
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_llvm/src/mono_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use crate::abi::FnAbiLlvmExt;
use crate::builder::Builder;
use crate::context::CodegenCx;
use crate::diagnostics::SymbolAlreadyDefined;
use crate::type_of::LayoutLlvmExt;
use crate::{base, llvm};

impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
Expand All @@ -37,7 +36,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
// out the llvm type from the actual evaluated initializer.
let ty =
if nested { self.tcx.types.unit } else { instance.ty(self.tcx, self.typing_env()) };
let llty = self.layout_of(ty).llvm_type(self);
let llty = self.type_array(self.type_i8(), self.layout_of(ty).size.bytes());

let g = self.define_global(symbol_name, llty).unwrap_or_else(|| {
self.sess()
Expand Down
2 changes: 1 addition & 1 deletion tests/assembly-llvm/cstring-merging.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::ffi::CStr;
#[unsafe(no_mangle)]
static CSTR: &[u8; 4] = b"foo\0";

// CHECK-NOT: .section
// CHECK: .section .rodata.str1.{{[12]}},"aMS"
// CHECK: {{(\.L|\$)}}anon.{{.+}}:
// CHECK-NEXT: .{{asciz|string}} "bar"
#[unsafe(no_mangle)]
Expand Down
3 changes: 2 additions & 1 deletion tests/assembly-llvm/static-relocation-model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ pub fn peach() -> u8 {
// x64: movq EXOCHORDA{{(\(%[a-z0-9]+\))?}}, %[[REG:[a-z0-9]+]]
// x64-NEXT: {{movb|movzbl}} (%[[REG]]), %{{[a-z0-9]+}}
// A64: adrp [[REG2:[a-z0-9]+]], EXOCHORDA
// A64-NEXT: ldr {{[a-z0-9]+}}, {{\[}}[[REG2]], :lo12:EXOCHORDA]
// A64-NEXT: add [[REG2]], [[REG2]], :lo12:EXOCHORDA
// A64-NEXT: ldr {{[a-z0-9]+}}, {{\[}}[[REG2]]{{\]}}
#[no_mangle]
pub fn mango() -> u8 {
unsafe { *EXOCHORDA }

@folkertdev folkertdev Nov 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

curiously, mangoes are not in exochorda.

Anyway, are we worried about this additional instruction? The docs note that the address given to ldr

Is the optional positive immediate byte offset, a multiple of 8 in the range 0 to 32760, defaulting to 0.

So I guess the address of EXOCHORDA is not longer appropriately aligned? Which, I suppose it was by luck before given that u8 has an alignment of 1 anyway?

@workingjubilee workingjubilee Nov 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

an exochorda shrub, demonstrating its white flowers

Huh, at least it's pretty.

@beetrees beetrees Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The ldr is the load of the *mut u8 itself (which is the type of the static), the u8 is loaded by a ldrb afterwards. I'm guessing LLVM is presuming the static has an alignment based on the default alignment of the type of the static - this seems incorrect as BackendRepr::Scalar doesn't provide any alignment guarantees. This PR incidentally fixes this by always using a type with an alignment of 1 - I think not fixing this would mean that miscompilations would occur after #159033 is merged.

@beetrees beetrees Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The LangRef says

An explicit alignment may be specified for a global, which must be a power of 2. If not present, or if the alignment is set to zero, the alignment of the global is set by the target to whatever it feels convenient.

which isn't especially reassuring. This PR is an improvement upon the status quo, but is there any reason we don't set an explicit alignment on global declarations?

@workingjubilee workingjubilee Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is no reason as far as I am aware of, and I know most all of the other curses in this area. Globals should receive explicitly-specified alignments for the same reason everything else gets explicitly-specified alignments.

Expand Down
12 changes: 6 additions & 6 deletions tests/codegen-llvm/debuginfo-generic-closure-env-names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
//@ edition: 2021

// non_generic_closure()
// NONMSVC: !DICompositeType(tag: DW_TAG_structure_type, name: "{closure_env#0}", scope: ![[non_generic_closure_NAMESPACE:[0-9]+]],
// MSVC: !DICompositeType(tag: DW_TAG_structure_type, name: "closure_env$0", scope: ![[non_generic_closure_NAMESPACE:[0-9]+]],
// CHECK: ![[non_generic_closure_NAMESPACE]] = !DINamespace(name: "non_generic_closure"
// NONMSVC-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "{closure_env#0}", scope: ![[non_generic_closure_NAMESPACE:[0-9]+]],
// MSVC-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "closure_env$0", scope: ![[non_generic_closure_NAMESPACE:[0-9]+]],
// CHECK-DAG: ![[non_generic_closure_NAMESPACE]] = !DINamespace(name: "non_generic_closure"

// CHECK: ![[function_containing_closure_NAMESPACE:[0-9]+]] = !DINamespace(name: "function_containing_closure"
// CHECK: ![[generic_async_function_NAMESPACE:[0-9]+]] = !DINamespace(name: "generic_async_function"
// CHECK: ![[generic_async_block_NAMESPACE:[0-9]+]] = !DINamespace(name: "generic_async_block"
// CHECK-DAG: ![[function_containing_closure_NAMESPACE:[0-9]+]] = !DINamespace(name: "function_containing_closure"
// CHECK-DAG: ![[generic_async_function_NAMESPACE:[0-9]+]] = !DINamespace(name: "generic_async_function"
// CHECK-DAG: ![[generic_async_block_NAMESPACE:[0-9]+]] = !DINamespace(name: "generic_async_block"

// function_containing_closure<u32>()
// NONMSVC-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "{closure_env#0}<u32>", scope: ![[function_containing_closure_NAMESPACE]]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-llvm/direct-access-external-data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ unsafe extern "C" {
// PIE-NOT: dso_local
// DIRECT-SAME: dso_local
// INDIRECT-NOT: dso_local
// CHECK-SAME: global i32
// CHECK-SAME: global [4 x i8]
safe static VAR: i32;

// When "linkage" is used, we generate an indirection global.
Expand Down
8 changes: 4 additions & 4 deletions tests/codegen-llvm/dllimports/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ extern crate wrapper;
// Check that external symbols coming from foreign dylibs are adorned with 'dllimport',
// whereas symbols coming from foreign staticlibs are not. (RFC-1717)

// CHECK: @dylib_global1 = external dllimport local_unnamed_addr global i32
// CHECK: @dylib_global2 = external dllimport local_unnamed_addr global i32
// CHECK: @static_global1 = external local_unnamed_addr global i32
// CHECK: @static_global2 = external local_unnamed_addr global i32
// CHECK: @dylib_global1 = external dllimport local_unnamed_addr global [4 x i8]
// CHECK: @dylib_global2 = external dllimport local_unnamed_addr global [4 x i8]
// CHECK: @static_global1 = external local_unnamed_addr global [4 x i8]
// CHECK: @static_global2 = external local_unnamed_addr global [4 x i8]

// CHECK: declare dllimport noundef i32 @dylib_func1(i32 noundef)
// CHECK: declare dllimport noundef i32 @dylib_func2(i32 noundef)
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-llvm/static-relocation-model-msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extern crate extern_decl;
// The `access_extern()` symbol is from this compilation unit, thus we expect
// it to be marked `dso_local` as well, given the static relocation model.
//
// CHECK: @extern_static = external dso_local local_unnamed_addr global i8
// CHECK: @extern_static = external dso_local local_unnamed_addr global [1 x i8]
// CHECK: define dso_local noundef i8 @access_extern() {{.*}}
// CHECK: declare dso_local noundef i8 @extern_fn() {{.*}}

Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-llvm/thread-local.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use std::cell::Cell;

thread_local!(static A: Cell<u32> = const { Cell::new(1) });

// CHECK: [[TLS_AUX:@.+]] = external thread_local{{.*}} global i64
// CHECK: [[TLS:@.+]] = internal thread_local{{.*}} global
// CHECK: [[TLS_AUX:@.+]] = external thread_local{{.*}} global [8 x i8]

// CHECK-LABEL: @get
#[no_mangle]
Expand Down
Loading