Skip to content

Commit e574fef

Browse files
oli-obkoli-obk
committed
Shrink some unsafe blocks in cg_llvm
1 parent d3d51b4 commit e574fef

File tree

4 files changed

+137
-139
lines changed

4 files changed

+137
-139
lines changed

‎compiler/rustc_codegen_llvm/src/builder.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -637,17 +637,16 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
637637
} else if place.layout.is_llvm_immediate() {
638638
let mut const_llval = None;
639639
let llty = place.layout.llvm_type(self);
640-
unsafe {
641-
if let Some(global) = llvm::LLVMIsAGlobalVariable(place.val.llval) {
642-
if llvm::LLVMIsGlobalConstant(global) == llvm::True {
643-
if let Some(init) = llvm::LLVMGetInitializer(global) {
644-
if self.val_ty(init) == llty {
645-
const_llval = Some(init);
646-
}
640+
if let Some(global) = llvm::LLVMIsAGlobalVariable(place.val.llval) {
641+
if llvm::LLVMIsGlobalConstant(global) == llvm::True {
642+
if let Some(init) = llvm::LLVMGetInitializer(global) {
643+
if self.val_ty(init) == llty {
644+
const_llval = Some(init);
647645
}
648646
}
649647
}
650648
}
649+
651650
let llval = const_llval.unwrap_or_else(|| {
652651
let load = self.load(llty, place.val.llval, place.val.align);
653652
if let abi::BackendRepr::Scalar(scalar) = place.layout.backend_repr {

‎compiler/rustc_codegen_llvm/src/consts.rs‎

Lines changed: 125 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -396,149 +396,148 @@ impl<'ll> CodegenCx<'ll, '_> {
396396
}
397397

398398
fn codegen_static_item(&mut self, def_id: DefId) {
399-
unsafe {
400-
assert!(
401-
llvm::LLVMGetInitializer(
402-
self.instances.borrow().get(&Instance::mono(self.tcx, def_id)).unwrap()
403-
)
404-
.is_none()
405-
);
406-
let attrs = self.tcx.codegen_fn_attrs(def_id);
399+
assert!(
400+
llvm::LLVMGetInitializer(
401+
self.instances.borrow().get(&Instance::mono(self.tcx, def_id)).unwrap()
402+
)
403+
.is_none()
404+
);
405+
let attrs = self.tcx.codegen_fn_attrs(def_id);
407406

408-
let Ok((v, alloc)) = codegen_static_initializer(self, def_id) else {
409-
// Error has already been reported
410-
return;
411-
};
412-
let alloc = alloc.inner();
407+
let Ok((v, alloc)) = codegen_static_initializer(self, def_id) else {
408+
// Error has already been reported
409+
return;
410+
};
411+
let alloc = alloc.inner();
413412

414-
let val_llty = self.val_ty(v);
413+
let val_llty = self.val_ty(v);
415414

416-
let g = self.get_static_inner(def_id, val_llty);
417-
let llty = self.get_type_of_global(g);
415+
let g = self.get_static_inner(def_id, val_llty);
416+
let llty = self.get_type_of_global(g);
418417

419-
let g = if val_llty == llty {
420-
g
421-
} else {
422-
// codegen_static_initializer creates the global value just from the
423-
// `Allocation` data by generating one big struct value that is just
424-
// all the bytes and pointers after each other. This will almost never
425-
// match the type that the static was declared with. Unfortunately
426-
// we can't just LLVMConstBitCast our way out of it because that has very
427-
// specific rules on what can be cast. So instead of adding a new way to
428-
// generate static initializers that match the static's type, we picked
429-
// the easier option and retroactively change the type of the static item itself.
430-
let name = llvm::get_value_name(g);
431-
llvm::set_value_name(g, b"");
432-
433-
let linkage = llvm::get_linkage(g);
434-
let visibility = llvm::get_visibility(g);
435-
436-
let new_g = llvm::LLVMRustGetOrInsertGlobal(
437-
self.llmod,
438-
name.as_c_char_ptr(),
439-
name.len(),
440-
val_llty,
441-
);
442-
443-
llvm::set_linkage(new_g, linkage);
444-
llvm::set_visibility(new_g, visibility);
445-
446-
// The old global has had its name removed but is returned by
447-
// get_static since it is in the instance cache. Provide an
448-
// alternative lookup that points to the new global so that
449-
// global_asm! can compute the correct mangled symbol name
450-
// for the global.
451-
self.renamed_statics.borrow_mut().insert(def_id, new_g);
452-
453-
// To avoid breaking any invariants, we leave around the old
454-
// global for the moment; we'll replace all references to it
455-
// with the new global later. (See base::codegen_backend.)
456-
self.statics_to_rauw.borrow_mut().push((g, new_g));
457-
new_g
458-
};
459-
set_global_alignment(self, g, alloc.align);
460-
llvm::set_initializer(g, v);
461-
462-
self.assume_dso_local(g, true);
463-
464-
// Forward the allocation's mutability (picked by the const interner) to LLVM.
465-
if alloc.mutability.is_not() {
466-
llvm::set_global_constant(g, true);
467-
}
418+
let g = if val_llty == llty {
419+
g
420+
} else {
421+
// codegen_static_initializer creates the global value just from the
422+
// `Allocation` data by generating one big struct value that is just
423+
// all the bytes and pointers after each other. This will almost never
424+
// match the type that the static was declared with. Unfortunately
425+
// we can't just LLVMConstBitCast our way out of it because that has very
426+
// specific rules on what can be cast. So instead of adding a new way to
427+
// generate static initializers that match the static's type, we picked
428+
// the easier option and retroactively change the type of the static item itself.
429+
let name = String::from_utf8(llvm::get_value_name(g))
430+
.expect("we declare our statics with a utf8-valid name");
431+
llvm::set_value_name(g, b"");
432+
433+
let linkage = llvm::get_linkage(g);
434+
let visibility = llvm::get_visibility(g);
435+
436+
let new_g = self.declare_global(&name, val_llty);
437+
438+
llvm::set_linkage(new_g, linkage);
439+
llvm::set_visibility(new_g, visibility);
440+
441+
// The old global has had its name removed but is returned by
442+
// get_static since it is in the instance cache. Provide an
443+
// alternative lookup that points to the new global so that
444+
// global_asm! can compute the correct mangled symbol name
445+
// for the global.
446+
self.renamed_statics.borrow_mut().insert(def_id, new_g);
447+
448+
// To avoid breaking any invariants, we leave around the old
449+
// global for the moment; we'll replace all references to it
450+
// with the new global later. (See base::codegen_backend.)
451+
self.statics_to_rauw.borrow_mut().push((g, new_g));
452+
new_g
453+
};
454+
set_global_alignment(self, g, alloc.align);
455+
llvm::set_initializer(g, v);
468456

469-
debuginfo::build_global_var_di_node(self, def_id, g);
457+
self.assume_dso_local(g, true);
470458

471-
if attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL) {
472-
llvm::set_thread_local_mode(g, self.tls_model);
473-
}
459+
// Forward the allocation's mutability (picked by the const interner) to LLVM.
460+
if alloc.mutability.is_not() {
461+
llvm::set_global_constant(g, true);
462+
}
474463

475-
// Wasm statics with custom link sections get special treatment as they
476-
// go into custom sections of the wasm executable. The exception to this
477-
// is the `.init_array` section which are treated specially by the wasm linker.
478-
if self.tcx.sess.target.is_like_wasm
479-
&& attrs
480-
.link_section
481-
.map(|link_section| !link_section.as_str().starts_with(".init_array"))
482-
.unwrap_or(true)
483-
{
484-
if let Some(section) = attrs.link_section {
485-
let section = llvm::LLVMMDStringInContext2(
464+
debuginfo::build_global_var_di_node(self, def_id, g);
465+
466+
if attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL) {
467+
llvm::set_thread_local_mode(g, self.tls_model);
468+
}
469+
470+
// Wasm statics with custom link sections get special treatment as they
471+
// go into custom sections of the wasm executable. The exception to this
472+
// is the `.init_array` section which are treated specially by the wasm linker.
473+
if self.tcx.sess.target.is_like_wasm
474+
&& attrs
475+
.link_section
476+
.map(|link_section| !link_section.as_str().starts_with(".init_array"))
477+
.unwrap_or(true)
478+
{
479+
if let Some(section) = attrs.link_section {
480+
let section = unsafe {
481+
llvm::LLVMMDStringInContext2(
486482
self.llcx,
487483
section.as_str().as_c_char_ptr(),
488484
section.as_str().len(),
489-
);
490-
assert!(alloc.provenance().ptrs().is_empty());
491-
492-
// The `inspect` method is okay here because we checked for provenance, and
493-
// because we are doing this access to inspect the final interpreter state (not
494-
// as part of the interpreter execution).
495-
let bytes =
496-
alloc.inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.len());
497-
let alloc =
498-
llvm::LLVMMDStringInContext2(self.llcx, bytes.as_c_char_ptr(), bytes.len());
499-
let data = [section, alloc];
500-
let meta = llvm::LLVMMDNodeInContext2(self.llcx, data.as_ptr(), data.len());
501-
let val = self.get_metadata_value(meta);
485+
)
486+
};
487+
assert!(alloc.provenance().ptrs().is_empty());
488+
489+
// The `inspect` method is okay here because we checked for provenance, and
490+
// because we are doing this access to inspect the final interpreter state (not
491+
// as part of the interpreter execution).
492+
let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.len());
493+
let alloc = unsafe {
494+
llvm::LLVMMDStringInContext2(self.llcx, bytes.as_c_char_ptr(), bytes.len())
495+
};
496+
let data = [section, alloc];
497+
let meta =
498+
unsafe { llvm::LLVMMDNodeInContext2(self.llcx, data.as_ptr(), data.len()) };
499+
let val = self.get_metadata_value(meta);
500+
unsafe {
502501
llvm::LLVMAddNamedMetadataOperand(
503502
self.llmod,
504503
c"wasm.custom_sections".as_ptr(),
505504
val,
506-
);
507-
}
508-
} else {
509-
base::set_link_section(g, attrs);
505+
)
506+
};
510507
}
508+
} else {
509+
base::set_link_section(g, attrs);
510+
}
511511

512-
base::set_variable_sanitizer_attrs(g, attrs);
513-
514-
if attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER) {
515-
// `USED` and `USED_LINKER` can't be used together.
516-
assert!(!attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER));
517-
518-
// The semantics of #[used] in Rust only require the symbol to make it into the
519-
// object file. It is explicitly allowed for the linker to strip the symbol if it
520-
// is dead, which means we are allowed to use `llvm.compiler.used` instead of
521-
// `llvm.used` here.
522-
//
523-
// Additionally, https://reviews.llvm.org/D97448 in LLVM 13 started emitting unique
524-
// sections with SHF_GNU_RETAIN flag for llvm.used symbols, which may trigger bugs
525-
// in the handling of `.init_array` (the static constructor list) in versions of
526-
// the gold linker (prior to the one released with binutils 2.36).
527-
//
528-
// That said, we only ever emit these when `#[used(compiler)]` is explicitly
529-
// requested. This is to avoid similar breakage on other targets, in particular
530-
// MachO targets have *their* static constructor lists broken if `llvm.compiler.used`
531-
// is emitted rather than `llvm.used`. However, that check happens when assigning
532-
// the `CodegenFnAttrFlags` in the `codegen_fn_attrs` query, so we don't need to
533-
// take care of it here.
534-
self.add_compiler_used_global(g);
535-
}
536-
if attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) {
537-
// `USED` and `USED_LINKER` can't be used together.
538-
assert!(!attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER));
512+
base::set_variable_sanitizer_attrs(g, attrs);
513+
514+
if attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER) {
515+
// `USED` and `USED_LINKER` can't be used together.
516+
assert!(!attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER));
517+
518+
// The semantics of #[used] in Rust only require the symbol to make it into the
519+
// object file. It is explicitly allowed for the linker to strip the symbol if it
520+
// is dead, which means we are allowed to use `llvm.compiler.used` instead of
521+
// `llvm.used` here.
522+
//
523+
// Additionally, https://reviews.llvm.org/D97448 in LLVM 13 started emitting unique
524+
// sections with SHF_GNU_RETAIN flag for llvm.used symbols, which may trigger bugs
525+
// in the handling of `.init_array` (the static constructor list) in versions of
526+
// the gold linker (prior to the one released with binutils 2.36).
527+
//
528+
// That said, we only ever emit these when `#[used(compiler)]` is explicitly
529+
// requested. This is to avoid similar breakage on other targets, in particular
530+
// MachO targets have *their* static constructor lists broken if `llvm.compiler.used`
531+
// is emitted rather than `llvm.used`. However, that check happens when assigning
532+
// the `CodegenFnAttrFlags` in the `codegen_fn_attrs` query, so we don't need to
533+
// take care of it here.
534+
self.add_compiler_used_global(g);
535+
}
536+
if attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) {
537+
// `USED` and `USED_LINKER` can't be used together.
538+
assert!(!attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER));
539539

540-
self.add_used_global(g);
541-
}
540+
self.add_used_global(g);
542541
}
543542
}
544543

‎compiler/rustc_codegen_llvm/src/llvm/ffi.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,17 +1168,17 @@ unsafe extern "C" {
11681168
pub(crate) fn LLVMGlobalGetValueType(Global: &Value) -> &Type;
11691169

11701170
// Operations on global variables
1171-
pub(crate) fn LLVMIsAGlobalVariable(GlobalVar: &Value) -> Option<&Value>;
1171+
pub(crate) safe fn LLVMIsAGlobalVariable(GlobalVar: &Value) -> Option<&Value>;
11721172
pub(crate) fn LLVMAddGlobal<'a>(M: &'a Module, Ty: &'a Type, Name: *const c_char) -> &'a Value;
11731173
pub(crate) fn LLVMGetNamedGlobal(M: &Module, Name: *const c_char) -> Option<&Value>;
11741174
pub(crate) fn LLVMGetFirstGlobal(M: &Module) -> Option<&Value>;
11751175
pub(crate) fn LLVMGetNextGlobal(GlobalVar: &Value) -> Option<&Value>;
11761176
pub(crate) fn LLVMDeleteGlobal(GlobalVar: &Value);
1177-
pub(crate) fn LLVMGetInitializer(GlobalVar: &Value) -> Option<&Value>;
1177+
pub(crate) safe fn LLVMGetInitializer(GlobalVar: &Value) -> Option<&Value>;
11781178
pub(crate) fn LLVMSetInitializer<'a>(GlobalVar: &'a Value, ConstantVal: &'a Value);
1179-
pub(crate) fn LLVMIsThreadLocal(GlobalVar: &Value) -> Bool;
1179+
pub(crate) safe fn LLVMIsThreadLocal(GlobalVar: &Value) -> Bool;
11801180
pub(crate) fn LLVMSetThreadLocalMode(GlobalVar: &Value, Mode: ThreadLocalMode);
1181-
pub(crate) fn LLVMIsGlobalConstant(GlobalVar: &Value) -> Bool;
1181+
pub(crate) safe fn LLVMIsGlobalConstant(GlobalVar: &Value) -> Bool;
11821182
pub(crate) safe fn LLVMSetGlobalConstant(GlobalVar: &Value, IsConstant: Bool);
11831183
pub(crate) safe fn LLVMSetTailCall(CallInst: &Value, IsTailCall: Bool);
11841184

‎compiler/rustc_codegen_llvm/src/mono_item.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ impl CodegenCx<'_, '_> {
131131
}
132132

133133
// Thread-local variables generally don't support copy relocations.
134-
let is_thread_local_var = unsafe { llvm::LLVMIsAGlobalVariable(llval) }
135-
.is_some_and(|v| unsafe { llvm::LLVMIsThreadLocal(v) } == llvm::True);
134+
let is_thread_local_var = llvm::LLVMIsAGlobalVariable(llval)
135+
.is_some_and(|v| llvm::LLVMIsThreadLocal(v) == llvm::True);
136136
if is_thread_local_var {
137137
return false;
138138
}

0 commit comments

Comments
 (0)