Skip to content

Commit

Permalink
Rollup merge of rust-lang#132260 - Zalathar:type-safe-cast, r=compile…
Browse files Browse the repository at this point in the history
…r-errors

cg_llvm: Use a type-safe helper to cast `&str` and `&[u8]` to `*const c_char`

In `rustc_codegen_llvm` there are many uses of `.as_ptr().cast()` to convert a string or byte-slice to `*const c_char`, which then gets passed through FFI.

This works, but is fragile, because there's nothing constraining the pointer cast to actually be from `u8` to `c_char`. If the original value changes to something else that has an `as_ptr` method, or the context changes to expect something other than `c_char`, the cast will silently do the wrong thing.

By making the cast more explicit via a helper method, we can be sure that it will either perform the intended cast, or fail at compile time.
  • Loading branch information
workingjubilee authored Oct 28, 2024
2 parents 9d1f9ab + 4bd84b2 commit ec388ee
Show file tree
Hide file tree
Showing 17 changed files with 110 additions and 87 deletions.
9 changes: 5 additions & 4 deletions compiler/rustc_codegen_llvm/src/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_middle::bug;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::{DebugInfo, OomStrategy};

use crate::common::AsCCharPtr;
use crate::llvm::{self, Context, False, Module, True, Type};
use crate::{ModuleLlvm, attributes, debuginfo};

Expand Down Expand Up @@ -76,14 +77,14 @@ pub(crate) unsafe fn codegen(
unsafe {
// __rust_alloc_error_handler_should_panic
let name = OomStrategy::SYMBOL;
let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_ptr().cast(), name.len(), i8);
let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_c_char_ptr(), name.len(), i8);
llvm::set_visibility(ll_g, llvm::Visibility::from_generic(tcx.sess.default_visibility()));
let val = tcx.sess.opts.unstable_opts.oom.should_panic();
let llval = llvm::LLVMConstInt(i8, val as u64, False);
llvm::LLVMSetInitializer(ll_g, llval);

let name = NO_ALLOC_SHIM_IS_UNSTABLE;
let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_ptr().cast(), name.len(), i8);
let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_c_char_ptr(), name.len(), i8);
llvm::set_visibility(ll_g, llvm::Visibility::from_generic(tcx.sess.default_visibility()));
let llval = llvm::LLVMConstInt(i8, 0, False);
llvm::LLVMSetInitializer(ll_g, llval);
Expand Down Expand Up @@ -115,7 +116,7 @@ fn create_wrapper_function(
);
let llfn = llvm::LLVMRustGetOrInsertFunction(
llmod,
from_name.as_ptr().cast(),
from_name.as_c_char_ptr(),
from_name.len(),
ty,
);
Expand All @@ -137,7 +138,7 @@ fn create_wrapper_function(
}

let callee =
llvm::LLVMRustGetOrInsertFunction(llmod, to_name.as_ptr().cast(), to_name.len(), ty);
llvm::LLVMRustGetOrInsertFunction(llmod, to_name.as_c_char_ptr(), to_name.len(), ty);
if let Some(no_return) = no_return {
// -> ! DIFlagNoReturn
attributes::apply_to_llfn(callee, llvm::AttributePlace::Function, &[no_return]);
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_codegen_llvm/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use smallvec::SmallVec;
use tracing::debug;

use crate::builder::Builder;
use crate::common::Funclet;
use crate::common::{AsCCharPtr, Funclet};
use crate::context::CodegenCx;
use crate::type_::Type;
use crate::type_of::LayoutLlvmExt;
Expand Down Expand Up @@ -420,7 +420,7 @@ impl<'tcx> AsmCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
unsafe {
llvm::LLVMAppendModuleInlineAsm(
self.llmod,
template_str.as_ptr().cast(),
template_str.as_c_char_ptr(),
template_str.len(),
);
}
Expand Down Expand Up @@ -458,14 +458,14 @@ pub(crate) fn inline_asm_call<'ll>(
let fty = bx.cx.type_func(&argtys, output);
unsafe {
// Ask LLVM to verify that the constraints are well-formed.
let constraints_ok = llvm::LLVMRustInlineAsmVerify(fty, cons.as_ptr().cast(), cons.len());
let constraints_ok = llvm::LLVMRustInlineAsmVerify(fty, cons.as_c_char_ptr(), cons.len());
debug!("constraint verification result: {:?}", constraints_ok);
if constraints_ok {
let v = llvm::LLVMRustInlineAsm(
fty,
asm.as_ptr().cast(),
asm.as_c_char_ptr(),
asm.len(),
cons.as_ptr().cast(),
cons.as_c_char_ptr(),
cons.len(),
volatile,
alignstack,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_llvm/src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use tracing::{debug, info};
use crate::back::write::{
self, CodegenDiagnosticsStage, DiagnosticHandlers, bitcode_section_name, save_temp_bitcode,
};
use crate::common::AsCCharPtr;
use crate::errors::{
DynamicLinkingWithLTO, LlvmError, LtoBitcodeFromRlib, LtoDisallowed, LtoDylib, LtoProcMacro,
};
Expand Down Expand Up @@ -604,7 +605,7 @@ pub(crate) fn run_pass_manager(
unsafe {
if !llvm::LLVMRustHasModuleFlag(
module.module_llvm.llmod(),
"LTOPostLink".as_ptr().cast(),
"LTOPostLink".as_c_char_ptr(),
11,
) {
llvm::LLVMRustAddModuleFlagU32(
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::back::owned_target_machine::OwnedTargetMachine;
use crate::back::profiling::{
LlvmSelfProfiler, selfprofile_after_pass_callback, selfprofile_before_pass_callback,
};
use crate::common::AsCCharPtr;
use crate::errors::{
CopyBitcode, FromLlvmDiag, FromLlvmOptimizationDiag, LlvmError, UnknownCompression,
WithLlvmError, WriteBytecode,
Expand Down Expand Up @@ -596,9 +597,9 @@ pub(crate) unsafe fn llvm_optimize(
llvm_selfprofiler,
selfprofile_before_pass_callback,
selfprofile_after_pass_callback,
extra_passes.as_ptr().cast(),
extra_passes.as_c_char_ptr(),
extra_passes.len(),
llvm_plugins.as_ptr().cast(),
llvm_plugins.as_c_char_ptr(),
llvm_plugins.len(),
)
};
Expand Down Expand Up @@ -1042,7 +1043,7 @@ unsafe fn embed_bitcode(
llvm::LLVMSetInitializer(llglobal, llconst);

let section = bitcode_section_name(cgcx);
llvm::LLVMSetSection(llglobal, section.as_ptr().cast());
llvm::LLVMSetSection(llglobal, section.as_c_char_ptr());
llvm::set_linkage(llglobal, llvm::Linkage::PrivateLinkage);
llvm::LLVMSetGlobalConstant(llglobal, llvm::True);

Expand All @@ -1066,9 +1067,9 @@ unsafe fn embed_bitcode(
// We need custom section flags, so emit module-level inline assembly.
let section_flags = if cgcx.is_pe_coff { "n" } else { "e" };
let asm = create_section_with_flags_asm(".llvmbc", section_flags, bitcode);
llvm::LLVMAppendModuleInlineAsm(llmod, asm.as_ptr().cast(), asm.len());
llvm::LLVMAppendModuleInlineAsm(llmod, asm.as_c_char_ptr(), asm.len());
let asm = create_section_with_flags_asm(".llvmcmd", section_flags, cmdline.as_bytes());
llvm::LLVMAppendModuleInlineAsm(llmod, asm.as_ptr().cast(), asm.len());
llvm::LLVMAppendModuleInlineAsm(llmod, asm.as_c_char_ptr(), asm.len());
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,3 +392,21 @@ pub(crate) fn get_dllimport<'tcx>(
tcx.native_library(id)
.and_then(|lib| lib.dll_imports.iter().find(|di| di.name.as_str() == name))
}

/// Extension trait for explicit casts to `*const c_char`.
pub(crate) trait AsCCharPtr {
/// Equivalent to `self.as_ptr().cast()`, but only casts to `*const c_char`.
fn as_c_char_ptr(&self) -> *const c_char;
}

impl AsCCharPtr for str {
fn as_c_char_ptr(&self) -> *const c_char {
self.as_ptr().cast()
}
}

impl AsCCharPtr for [u8] {
fn as_c_char_ptr(&self) -> *const c_char {
self.as_ptr().cast()
}
}
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc_target::abi::{
};
use tracing::{debug, instrument, trace};

use crate::common::CodegenCx;
use crate::common::{AsCCharPtr, CodegenCx};
use crate::errors::{
InvalidMinimumAlignmentNotPowerOfTwo, InvalidMinimumAlignmentTooLarge, SymbolAlreadyDefined,
};
Expand Down Expand Up @@ -400,7 +400,7 @@ impl<'ll> CodegenCx<'ll, '_> {

let new_g = llvm::LLVMRustGetOrInsertGlobal(
self.llmod,
name.as_ptr().cast(),
name.as_c_char_ptr(),
name.len(),
val_llty,
);
Expand Down Expand Up @@ -451,7 +451,7 @@ impl<'ll> CodegenCx<'ll, '_> {
if let Some(section) = attrs.link_section {
let section = llvm::LLVMMDStringInContext2(
self.llcx,
section.as_str().as_ptr().cast(),
section.as_str().as_c_char_ptr(),
section.as_str().len(),
);
assert!(alloc.provenance().ptrs().is_empty());
Expand All @@ -462,7 +462,7 @@ impl<'ll> CodegenCx<'ll, '_> {
let bytes =
alloc.inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.len());
let alloc =
llvm::LLVMMDStringInContext2(self.llcx, bytes.as_ptr().cast(), bytes.len());
llvm::LLVMMDStringInContext2(self.llcx, bytes.as_c_char_ptr(), bytes.len());
let data = [section, alloc];
let meta = llvm::LLVMMDNodeInContext2(self.llcx, data.as_ptr(), data.len());
let val = llvm::LLVMMetadataAsValue(self.llcx, meta);
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use smallvec::SmallVec;

use crate::back::write::to_llvm_code_model;
use crate::callee::get_fn;
use crate::common::AsCCharPtr;
use crate::debuginfo::metadata::apply_vcall_visibility_metadata;
use crate::llvm::{Metadata, MetadataType};
use crate::type_::Type;
Expand Down Expand Up @@ -231,7 +232,7 @@ pub(crate) unsafe fn create_module<'ll>(
// If we're normalizing integers with CFI, ensure LLVM generated functions do the same.
// See https://github.com/llvm/llvm-project/pull/104826
if sess.is_sanitizer_cfi_normalize_integers_enabled() {
let cfi_normalize_integers = c"cfi-normalize-integers".as_ptr().cast();
let cfi_normalize_integers = c"cfi-normalize-integers".as_ptr();
unsafe {
llvm::LLVMRustAddModuleFlagU32(
llmod,
Expand Down Expand Up @@ -268,7 +269,7 @@ pub(crate) unsafe fn create_module<'ll>(
let pfe =
PatchableFunctionEntry::from_config(sess.opts.unstable_opts.patchable_function_entry);
if pfe.prefix() > 0 {
let kcfi_offset = c"kcfi-offset".as_ptr().cast();
let kcfi_offset = c"kcfi-offset".as_ptr();
unsafe {
llvm::LLVMRustAddModuleFlagU32(
llmod,
Expand Down Expand Up @@ -429,7 +430,7 @@ pub(crate) unsafe fn create_module<'ll>(
let name_metadata = unsafe {
llvm::LLVMMDStringInContext2(
llcx,
rustc_producer.as_ptr().cast(),
rustc_producer.as_c_char_ptr(),
rustc_producer.as_bytes().len(),
)
};
Expand All @@ -453,7 +454,7 @@ pub(crate) unsafe fn create_module<'ll>(
llmod,
llvm::LLVMModFlagBehavior::Error,
c"target-abi".as_ptr(),
llvm_abiname.as_ptr().cast(),
llvm_abiname.as_c_char_ptr(),
llvm_abiname.len(),
);
}
Expand All @@ -474,7 +475,7 @@ pub(crate) unsafe fn create_module<'ll>(
// We already checked this during option parsing
_ => unreachable!(),
};
unsafe { llvm::LLVMRustAddModuleFlagU32(llmod, behavior, key.as_ptr().cast(), *value) }
unsafe { llvm::LLVMRustAddModuleFlagU32(llmod, behavior, key.as_c_char_ptr(), *value) }
}

llmod
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_target::abi::Size;
use tracing::{debug, instrument};

use crate::builder::Builder;
use crate::common::CodegenCx;
use crate::common::{AsCCharPtr, CodegenCx};
use crate::coverageinfo::map_data::FunctionCoverageCollector;
use crate::llvm;

Expand Down Expand Up @@ -236,7 +236,7 @@ fn create_pgo_func_name_var<'ll, 'tcx>(
unsafe {
llvm::LLVMRustCoverageCreatePGOFuncNameVar(
llfn,
mangled_fn_name.as_ptr().cast(),
mangled_fn_name.as_c_char_ptr(),
mangled_fn_name.len(),
)
}
Expand All @@ -248,7 +248,7 @@ pub(crate) fn write_filenames_section_to_buffer<'a>(
) {
let (pointers, lengths) = filenames
.into_iter()
.map(|s: &str| (s.as_ptr().cast(), s.len()))
.map(|s: &str| (s.as_c_char_ptr(), s.len()))
.unzip::<_, _, Vec<_>, Vec<_>>();

unsafe {
Expand Down Expand Up @@ -291,7 +291,7 @@ pub(crate) fn write_mapping_to_buffer(
}

pub(crate) fn hash_bytes(bytes: &[u8]) -> u64 {
unsafe { llvm::LLVMRustCoverageHashByteArray(bytes.as_ptr().cast(), bytes.len()) }
unsafe { llvm::LLVMRustCoverageHashByteArray(bytes.as_c_char_ptr(), bytes.len()) }
}

pub(crate) fn mapping_version() -> u32 {
Expand Down
Loading

0 comments on commit ec388ee

Please sign in to comment.