Skip to content

Commit 7465121

Browse files
oli-obkoli-obk
committed
Document some safety constraints and use more safe wrappers
1 parent 2776bdf commit 7465121

File tree

11 files changed

+117
-128
lines changed

11 files changed

+117
-128
lines changed

‎compiler/rustc_codegen_llvm/src/allocator.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ pub(crate) unsafe fn codegen(
8181
llvm::set_visibility(ll_g, llvm::Visibility::from_generic(tcx.sess.default_visibility()));
8282
let val = tcx.sess.opts.unstable_opts.oom.should_panic();
8383
let llval = llvm::LLVMConstInt(i8, val as u64, False);
84-
llvm::LLVMSetInitializer(ll_g, llval);
84+
llvm::set_initializer(ll_g, llval);
8585

8686
let name = NO_ALLOC_SHIM_IS_UNSTABLE;
8787
let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_c_char_ptr(), name.len(), i8);
8888
llvm::set_visibility(ll_g, llvm::Visibility::from_generic(tcx.sess.default_visibility()));
8989
let llval = llvm::LLVMConstInt(i8, 0, False);
90-
llvm::LLVMSetInitializer(ll_g, llval);
90+
llvm::set_initializer(ll_g, llval);
9191
}
9292

9393
if tcx.sess.opts.debuginfo != DebugInfo::None {

‎compiler/rustc_codegen_llvm/src/back/archive.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rustc_codegen_ssa::back::archive::{
1111
use rustc_session::Session;
1212

1313
use crate::llvm::archive_ro::{ArchiveRO, Child};
14-
use crate::llvm::{self, ArchiveKind};
14+
use crate::llvm::{self, ArchiveKind, last_error};
1515

1616
/// Helper for adding many files to an archive.
1717
#[must_use = "must call build() to finish building the archive"]
@@ -169,6 +169,8 @@ impl<'a> LlvmArchiveBuilder<'a> {
169169
.unwrap_or_else(|kind| self.sess.dcx().emit_fatal(UnknownArchiveKind { kind }));
170170

171171
let mut additions = mem::take(&mut self.additions);
172+
// Values in the `members` list below will contain pointers to the strings allocated here.
173+
// So they need to get dropped after all elements of `members` get freed.
172174
let mut strings = Vec::new();
173175
let mut members = Vec::new();
174176

@@ -229,12 +231,7 @@ impl<'a> LlvmArchiveBuilder<'a> {
229231
self.sess.target.arch == "arm64ec",
230232
);
231233
let ret = if r.into_result().is_err() {
232-
let err = llvm::LLVMRustGetLastError();
233-
let msg = if err.is_null() {
234-
"failed to write archive".into()
235-
} else {
236-
String::from_utf8_lossy(CStr::from_ptr(err).to_bytes())
237-
};
234+
let msg = last_error().unwrap_or_else(|| "failed to write archive".into());
238235
Err(io::Error::new(io::ErrorKind::Other, msg))
239236
} else {
240237
Ok(!members.is_empty())

‎compiler/rustc_codegen_llvm/src/back/write.rs

+95-105
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ pub(crate) fn llvm_err<'a>(dcx: DiagCtxtHandle<'_>, err: LlvmError<'a>) -> Fatal
5151
}
5252
}
5353

54-
fn write_output_file<'ll>(
54+
unsafe fn write_output_file<'ll>(
5555
dcx: DiagCtxtHandle<'_>,
5656
target: &'ll llvm::TargetMachine,
57-
pm: &llvm::PassManager<'ll>,
57+
pm: *mut llvm::PassManager<'ll>,
5858
m: &'ll llvm::Module,
5959
output: &Path,
6060
dwo_output: Option<&Path>,
@@ -63,39 +63,39 @@ fn write_output_file<'ll>(
6363
verify_llvm_ir: bool,
6464
) -> Result<(), FatalError> {
6565
debug!("write_output_file output={:?} dwo_output={:?}", output, dwo_output);
66-
unsafe {
67-
let output_c = path_to_c_string(output);
68-
let dwo_output_c;
69-
let dwo_output_ptr = if let Some(dwo_output) = dwo_output {
70-
dwo_output_c = path_to_c_string(dwo_output);
71-
dwo_output_c.as_ptr()
72-
} else {
73-
std::ptr::null()
74-
};
75-
let result = llvm::LLVMRustWriteOutputFile(
66+
let output_c = path_to_c_string(output);
67+
let dwo_output_c;
68+
let dwo_output_ptr = if let Some(dwo_output) = dwo_output {
69+
dwo_output_c = path_to_c_string(dwo_output);
70+
dwo_output_c.as_ptr()
71+
} else {
72+
std::ptr::null()
73+
};
74+
let result = unsafe {
75+
llvm::LLVMRustWriteOutputFile(
7676
target,
7777
pm,
7878
m,
7979
output_c.as_ptr(),
8080
dwo_output_ptr,
8181
file_type,
8282
verify_llvm_ir,
83-
);
83+
)
84+
};
8485

85-
// Record artifact sizes for self-profiling
86-
if result == llvm::LLVMRustResult::Success {
87-
let artifact_kind = match file_type {
88-
llvm::FileType::ObjectFile => "object_file",
89-
llvm::FileType::AssemblyFile => "assembly_file",
90-
};
91-
record_artifact_size(self_profiler_ref, artifact_kind, output);
92-
if let Some(dwo_file) = dwo_output {
93-
record_artifact_size(self_profiler_ref, "dwo_file", dwo_file);
94-
}
86+
// Record artifact sizes for self-profiling
87+
if result == llvm::LLVMRustResult::Success {
88+
let artifact_kind = match file_type {
89+
llvm::FileType::ObjectFile => "object_file",
90+
llvm::FileType::AssemblyFile => "assembly_file",
91+
};
92+
record_artifact_size(self_profiler_ref, artifact_kind, output);
93+
if let Some(dwo_file) = dwo_output {
94+
record_artifact_size(self_profiler_ref, "dwo_file", dwo_file);
9595
}
96-
97-
result.into_result().map_err(|()| llvm_err(dcx, LlvmError::WriteOutput { path: output }))
9896
}
97+
98+
result.into_result().map_err(|()| llvm_err(dcx, LlvmError::WriteOutput { path: output }))
9999
}
100100

101101
pub(crate) fn create_informational_target_machine(
@@ -325,13 +325,17 @@ pub(crate) fn save_temp_bitcode(
325325
if !cgcx.save_temps {
326326
return;
327327
}
328+
let ext = format!("{name}.bc");
329+
let cgu = Some(&module.name[..]);
330+
let path = cgcx.output_filenames.temp_path_ext(&ext, cgu);
331+
write_bitcode_to_file(module, &path)
332+
}
333+
334+
fn write_bitcode_to_file(module: &ModuleCodegen<ModuleLlvm>, path: &Path) {
328335
unsafe {
329-
let ext = format!("{name}.bc");
330-
let cgu = Some(&module.name[..]);
331-
let path = cgcx.output_filenames.temp_path_ext(&ext, cgu);
332-
let cstr = path_to_c_string(&path);
336+
let path = path_to_c_string(&path);
333337
let llmod = module.module_llvm.llmod();
334-
llvm::LLVMWriteBitcodeToFile(llmod, cstr.as_ptr());
338+
llvm::LLVMWriteBitcodeToFile(llmod, path.as_ptr());
335339
}
336340
}
337341

@@ -661,7 +665,6 @@ pub(crate) unsafe fn optimize(
661665
) -> Result<(), FatalError> {
662666
let _timer = cgcx.prof.generic_activity_with_arg("LLVM_module_optimize", &*module.name);
663667

664-
let llmod = module.module_llvm.llmod();
665668
let llcx = &*module.module_llvm.llcx;
666669
let _handlers = DiagnosticHandlers::new(cgcx, dcx, llcx, module, CodegenDiagnosticsStage::Opt);
667670

@@ -670,8 +673,7 @@ pub(crate) unsafe fn optimize(
670673

671674
if config.emit_no_opt_bc {
672675
let out = cgcx.output_filenames.temp_path_ext("no-opt.bc", module_name);
673-
let out = path_to_c_string(&out);
674-
unsafe { llvm::LLVMWriteBitcodeToFile(llmod, out.as_ptr()) };
676+
write_bitcode_to_file(module, &out)
675677
}
676678

677679
// FIXME(ZuseZ4): support SanitizeHWAddress and prevent illegal/unsupported opts
@@ -748,18 +750,18 @@ pub(crate) unsafe fn codegen(
748750
// files for an LLVM module.
749751
//
750752
// Apparently each of these pass managers is a one-shot kind of
751-
// thing, so we create a new one for each type of output. The
752-
// pass manager passed to the closure should be ensured to not
753-
// escape the closure itself, and the manager should only be
754-
// used once.
755-
unsafe fn with_codegen<'ll, F, R>(
753+
// thing, so we create a new one for each type of output.
754+
// The pass manager must be consumed by `LLVMRustWriteOutputFile`,
755+
// which also takes care of freeing its memory. For details of why it's
756+
// this way, see the C++ code of `LLVMRustWriteOutputFile`
757+
fn with_codegen<'ll, F, R>(
756758
tm: &'ll llvm::TargetMachine,
757759
llmod: &'ll llvm::Module,
758760
no_builtins: bool,
759761
f: F,
760762
) -> R
761763
where
762-
F: FnOnce(&'ll mut PassManager<'ll>) -> R,
764+
F: FnOnce(*mut PassManager<'ll>) -> R,
763765
{
764766
unsafe {
765767
let cpm = llvm::LLVMCreatePassManager();
@@ -890,21 +892,19 @@ pub(crate) unsafe fn codegen(
890892
} else {
891893
llmod
892894
};
893-
unsafe {
894-
with_codegen(tm, llmod, config.no_builtins, |cpm| {
895-
write_output_file(
896-
dcx,
897-
tm,
898-
cpm,
899-
llmod,
900-
&path,
901-
None,
902-
llvm::FileType::AssemblyFile,
903-
&cgcx.prof,
904-
config.verify_llvm_ir,
905-
)
906-
})?;
907-
}
895+
with_codegen(tm, llmod, config.no_builtins, |cpm| unsafe {
896+
write_output_file(
897+
dcx,
898+
tm,
899+
cpm,
900+
llmod,
901+
&path,
902+
None,
903+
llvm::FileType::AssemblyFile,
904+
&cgcx.prof,
905+
config.verify_llvm_ir,
906+
)
907+
})?;
908908
}
909909

910910
match config.emit_obj {
@@ -928,21 +928,19 @@ pub(crate) unsafe fn codegen(
928928
(_, SplitDwarfKind::Split) => Some(dwo_out.as_path()),
929929
};
930930

931-
unsafe {
932-
with_codegen(tm, llmod, config.no_builtins, |cpm| {
933-
write_output_file(
934-
dcx,
935-
tm,
936-
cpm,
937-
llmod,
938-
&obj_out,
939-
dwo_out,
940-
llvm::FileType::ObjectFile,
941-
&cgcx.prof,
942-
config.verify_llvm_ir,
943-
)
944-
})?;
945-
}
931+
with_codegen(tm, llmod, config.no_builtins, |cpm| unsafe {
932+
write_output_file(
933+
dcx,
934+
tm,
935+
cpm,
936+
llmod,
937+
&obj_out,
938+
dwo_out,
939+
llvm::FileType::ObjectFile,
940+
&cgcx.prof,
941+
config.verify_llvm_ir,
942+
)
943+
})?;
946944
}
947945

948946
EmitObj::Bitcode => {
@@ -1069,24 +1067,18 @@ unsafe fn embed_bitcode(
10691067
{
10701068
// We don't need custom section flags, create LLVM globals.
10711069
let llconst = common::bytes_in_context(llcx, bitcode);
1072-
let llglobal = llvm::LLVMAddGlobal(
1073-
llmod,
1074-
common::val_ty(llconst),
1075-
c"rustc.embedded.module".as_ptr(),
1076-
);
1077-
llvm::LLVMSetInitializer(llglobal, llconst);
1070+
let llglobal =
1071+
llvm::add_global(llmod, common::val_ty(llconst), c"rustc.embedded.module");
1072+
llvm::set_initializer(llglobal, llconst);
10781073

10791074
llvm::set_section(llglobal, bitcode_section_name(cgcx));
10801075
llvm::set_linkage(llglobal, llvm::Linkage::PrivateLinkage);
10811076
llvm::LLVMSetGlobalConstant(llglobal, llvm::True);
10821077

10831078
let llconst = common::bytes_in_context(llcx, cmdline.as_bytes());
1084-
let llglobal = llvm::LLVMAddGlobal(
1085-
llmod,
1086-
common::val_ty(llconst),
1087-
c"rustc.embedded.cmdline".as_ptr(),
1088-
);
1089-
llvm::LLVMSetInitializer(llglobal, llconst);
1079+
let llglobal =
1080+
llvm::add_global(llmod, common::val_ty(llconst), c"rustc.embedded.cmdline");
1081+
llvm::set_initializer(llglobal, llconst);
10901082
let section = if cgcx.target_is_like_osx {
10911083
c"__LLVM,__cmdline"
10921084
} else if cgcx.target_is_like_aix {
@@ -1126,31 +1118,29 @@ fn create_msvc_imps(
11261118
// underscores added in front).
11271119
let prefix = if cgcx.target_arch == "x86" { "\x01__imp__" } else { "\x01__imp_" };
11281120

1129-
unsafe {
1130-
let ptr_ty = Type::ptr_llcx(llcx);
1131-
let globals = base::iter_globals(llmod)
1132-
.filter(|&val| {
1133-
llvm::get_linkage(val) == llvm::Linkage::ExternalLinkage
1134-
&& llvm::LLVMIsDeclaration(val) == 0
1135-
})
1136-
.filter_map(|val| {
1137-
// Exclude some symbols that we know are not Rust symbols.
1138-
let name = llvm::get_value_name(val);
1139-
if ignored(name) { None } else { Some((val, name)) }
1140-
})
1141-
.map(move |(val, name)| {
1142-
let mut imp_name = prefix.as_bytes().to_vec();
1143-
imp_name.extend(name);
1144-
let imp_name = CString::new(imp_name).unwrap();
1145-
(imp_name, val)
1146-
})
1147-
.collect::<Vec<_>>();
1121+
let ptr_ty = Type::ptr_llcx(llcx);
1122+
let globals = base::iter_globals(llmod)
1123+
.filter(|&val| {
1124+
llvm::get_linkage(val) == llvm::Linkage::ExternalLinkage && llvm::is_declaration(val)
1125+
})
1126+
.filter_map(|val| {
1127+
// Exclude some symbols that we know are not Rust symbols.
1128+
let name = llvm::get_value_name(val);
1129+
if ignored(name) { None } else { Some((val, name)) }
1130+
})
1131+
.map(move |(val, name)| {
1132+
let mut imp_name = prefix.as_bytes().to_vec();
1133+
imp_name.extend(name);
1134+
let imp_name = CString::new(imp_name).unwrap();
1135+
(imp_name, val)
1136+
})
1137+
.collect::<Vec<_>>();
11481138

1149-
for (imp_name, val) in globals {
1150-
let imp = llvm::LLVMAddGlobal(llmod, ptr_ty, imp_name.as_ptr());
1151-
llvm::LLVMSetInitializer(imp, val);
1152-
llvm::set_linkage(imp, llvm::Linkage::ExternalLinkage);
1153-
}
1139+
for (imp_name, val) in globals {
1140+
let imp = llvm::add_global(llmod, ptr_ty, &imp_name);
1141+
1142+
llvm::set_initializer(imp, val);
1143+
llvm::set_linkage(imp, llvm::Linkage::ExternalLinkage);
11541144
}
11551145

11561146
// Use this function to exclude certain symbols from `__imp` generation.

‎compiler/rustc_codegen_llvm/src/common.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,8 @@ impl<'ll, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
215215
let g = self.define_global(&sym, self.val_ty(sc)).unwrap_or_else(|| {
216216
bug!("symbol `{}` is already defined", sym);
217217
});
218+
llvm::set_initializer(g, sc);
218219
unsafe {
219-
llvm::LLVMSetInitializer(g, sc);
220220
llvm::LLVMSetGlobalConstant(g, True);
221221
llvm::LLVMSetUnnamedAddress(g, llvm::UnnamedAddr::Global);
222222
}

‎compiler/rustc_codegen_llvm/src/consts.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ fn check_and_apply_linkage<'ll, 'tcx>(
191191
})
192192
});
193193
llvm::set_linkage(g2, llvm::Linkage::InternalLinkage);
194-
unsafe { llvm::LLVMSetInitializer(g2, g1) };
194+
llvm::set_initializer(g2, g1);
195195
g2
196196
} else if cx.tcx.sess.target.arch == "x86"
197197
&& common::is_mingw_gnu_toolchain(&cx.tcx.sess.target)
@@ -227,7 +227,7 @@ impl<'ll> CodegenCx<'ll, '_> {
227227
}
228228
_ => self.define_private_global(self.val_ty(cv)),
229229
};
230-
unsafe { llvm::LLVMSetInitializer(gv, cv) };
230+
llvm::set_initializer(gv, cv);
231231
set_global_alignment(self, gv, align);
232232
llvm::SetUnnamedAddress(gv, llvm::UnnamedAddr::Global);
233233
gv
@@ -416,7 +416,7 @@ impl<'ll> CodegenCx<'ll, '_> {
416416
new_g
417417
};
418418
set_global_alignment(self, g, alloc.align);
419-
llvm::LLVMSetInitializer(g, v);
419+
llvm::set_initializer(g, v);
420420

421421
if self.should_assume_dso_local(g, true) {
422422
llvm::LLVMRustSetDSOLocal(g, true);

‎compiler/rustc_codegen_llvm/src/context.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -593,12 +593,10 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
593593
pub(crate) fn create_used_variable_impl(&self, name: &'static CStr, values: &[&'ll Value]) {
594594
let array = self.const_array(self.type_ptr(), values);
595595

596-
unsafe {
597-
let g = llvm::LLVMAddGlobal(self.llmod, self.val_ty(array), name.as_ptr());
598-
llvm::LLVMSetInitializer(g, array);
599-
llvm::set_linkage(g, llvm::Linkage::AppendingLinkage);
600-
llvm::set_section(g, c"llvm.metadata");
601-
}
596+
let g = llvm::add_global(self.llmod, self.val_ty(array), name);
597+
llvm::set_initializer(g, array);
598+
llvm::set_linkage(g, llvm::Linkage::AppendingLinkage);
599+
llvm::set_section(g, c"llvm.metadata");
602600
}
603601

604602
pub(crate) fn get_metadata_value(&self, metadata: &'ll Metadata) -> &'ll Value {

0 commit comments

Comments
 (0)