Skip to content

Commit

Permalink
coverage: Set up MC/DC bitmaps without additional unsafe code
Browse files Browse the repository at this point in the history
Because this now always takes place at the start of the function, we can just
use the normal `alloca` method and then initialize each bitmap immediately.

This patch also moves bitmap setup out of the `mcdc_parameters` method, because
there is no longer any particular reason for it to be there.
  • Loading branch information
Zalathar committed Apr 30, 2024
1 parent 52d608b commit 0b3a479
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 29 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ use rustc_macros::{Decodable_Generic, Encodable_Generic};
use std::iter::Step;

mod layout;
#[cfg(test)]
mod tests;

pub use layout::LayoutCalculator;

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_abi/src/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
use super::*;

#[test]
fn align_constants() {
assert_eq!(Align::ONE, Align::from_bytes(1).unwrap());
assert_eq!(Align::EIGHT, Align::from_bytes(8).unwrap());
}
36 changes: 12 additions & 24 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_data_structures::small_c_str::SmallCStr;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
use rustc_middle::ty::layout::{
FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout,
FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers, TyAndLayout,
};
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_sanitizers::{cfi, kcfi};
Expand All @@ -27,7 +27,6 @@ use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange};
use rustc_target::spec::{HasTargetSpec, SanitizerSet, Target};
use smallvec::SmallVec;
use std::borrow::Cow;
use std::ffi::CString;
use std::iter;
use std::ops::Deref;
use std::ptr;
Expand Down Expand Up @@ -1705,13 +1704,21 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
kcfi_bundle
}

/// Emits a call to `llvm.instrprof.mcdc.parameters`.
///
/// This doesn't produce any code directly, but is used as input by
/// the LLVM pass that handles coverage instrumentation.
///
/// (See clang's [`CodeGenPGO::emitMCDCParameters`] for comparison.)
///
/// [`CodeGenPGO::emitMCDCParameters`]:
/// https://github.com/rust-lang/llvm-project/blob/5399a24/clang/lib/CodeGen/CodeGenPGO.cpp#L1124
pub(crate) fn mcdc_parameters(
&mut self,
fn_name: &'ll Value,
hash: &'ll Value,
bitmap_bytes: &'ll Value,
max_decision_depth: u32,
) -> Vec<&'ll Value> {
) {
debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes);

assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later");
Expand All @@ -1724,8 +1731,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
let args = &[fn_name, hash, bitmap_bytes];
let args = self.check_call("call", llty, llfn, args);

let mut cond_bitmaps = vec![];

unsafe {
let _ = llvm::LLVMRustBuildCall(
self.llbuilder,
Expand All @@ -1736,23 +1741,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
[].as_ptr(),
0 as c_uint,
);
// Create condition bitmap named `mcdc.addr`.
for i in 0..=max_decision_depth {
let mut bx = Builder::with_cx(self.cx);
bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn()));

let name = CString::new(format!("mcdc.addr.{i}")).unwrap();
let cond_bitmap = {
let alloca =
llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), name.as_ptr());
llvm::LLVMSetAlignment(alloca, 4);
alloca
};
bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi);
cond_bitmaps.push(cond_bitmap);
}
}
cond_bitmaps
}

pub(crate) fn mcdc_tvbitmap_update(
Expand Down Expand Up @@ -1794,8 +1783,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
0 as c_uint,
);
}
let i32_align = self.tcx().data_layout.i32_align.abi;
self.store(self.const_i32(0), mcdc_temp, i32_align);
self.store(self.const_i32(0), mcdc_temp, self.tcx.data_layout.i32_align.abi);
}

pub(crate) fn mcdc_condbitmap_update(
Expand Down
20 changes: 15 additions & 5 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc_middle::bug;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::ty::layout::HasTyCtxt;
use rustc_middle::ty::Instance;
use rustc_target::abi::Align;
use rustc_target::abi::{Align, Size};

use std::cell::RefCell;

Expand Down Expand Up @@ -106,15 +106,25 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
let fn_name = self.get_pgo_func_name_var(instance);
let hash = self.const_u64(function_coverage_info.function_source_hash);
let bitmap_bytes = self.const_u32(function_coverage_info.mcdc_bitmap_bytes);
let max_decision_depth = function_coverage_info.mcdc_max_decision_depth;
let cond_bitmap =
self.mcdc_parameters(fn_name, hash, bitmap_bytes, max_decision_depth as u32);
self.mcdc_parameters(fn_name, hash, bitmap_bytes);

// Create pointers named `mcdc.addr.{i}` to stack-allocated condition bitmaps.
let mut cond_bitmaps = vec![];
for i in 0..=function_coverage_info.mcdc_max_decision_depth {
// MC/DC intrinsics will perform loads/stores that use the ABI default
// alignment for i32, so our variable declaration should match.
let align = self.tcx.data_layout.i32_align.abi;
let cond_bitmap = self.alloca(Size::from_bytes(4), align);
llvm::set_value_name(cond_bitmap, format!("mcdc.addr.{i}").as_bytes());
self.store(self.const_i32(0), cond_bitmap, align);
cond_bitmaps.push(cond_bitmap);
}

self.coverage_context()
.expect("always present when coverage is enabled")
.mcdc_condition_bitmap_map
.borrow_mut()
.insert(instance, cond_bitmap);
.insert(instance, cond_bitmaps);
}

#[instrument(level = "debug", skip(self))]
Expand Down

0 comments on commit 0b3a479

Please sign in to comment.