Skip to content

Commit

Permalink
Rollup merge of #111382 - Zalathar:ffi, r=cuviper
Browse files Browse the repository at this point in the history
Isolate coverage FFI type layouts from their underlying LLVM C++ types

I noticed that several of the types used to send coverage information through FFI are not properly isolated from the layout of their corresponding C++ types in the LLVM API.

This PR adds more explicitly-defined FFI struct/enum types in `CoverageMappingWrapper.cpp`, so that Rust source files in `rustc_codegen_ssa` and `rustc_codegen_llvm` aren't directly exposed to LLVM C++ types.
  • Loading branch information
matthiaskrgr authored May 11, 2023
2 parents aa9adf4 + 9addf06 commit 39761b0
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 17 deletions.
8 changes: 6 additions & 2 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,9 @@ pub type InlineAsmDiagHandlerTy = unsafe extern "C" fn(&SMDiagnostic, *const c_v
pub mod coverageinfo {
use super::coverage_map;

/// Aligns with [llvm::coverage::CounterMappingRegion::RegionKind](https://github.com/rust-lang/llvm-project/blob/rustc/13.0-2021-09-30/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L209-L230)
/// Corresponds to enum `llvm::coverage::CounterMappingRegion::RegionKind`.
///
/// Must match the layout of `LLVMRustCounterMappingRegionKind`.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub enum RegionKind {
Expand Down Expand Up @@ -714,7 +716,9 @@ pub mod coverageinfo {
/// array", encoded separately), and source location (start and end positions of the represented
/// code region).
///
/// Matches LLVMRustCounterMappingRegion.
/// Corresponds to struct `llvm::coverage::CounterMappingRegion`.
///
/// Must match the layout of `LLVMRustCounterMappingRegion`.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct CounterMappingRegion {
Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_middle::mir::coverage::{CounterValueReference, MappedExpressionIndex};

/// Aligns with [llvm::coverage::Counter::CounterKind](https://github.com/rust-lang/llvm-project/blob/rustc/13.0-2021-09-30/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L95)
/// Must match the layout of `LLVMRustCounterKind`.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub enum CounterKind {
Expand All @@ -17,8 +17,10 @@ pub enum CounterKind {
/// `instrprof.increment()`)
/// * For `CounterKind::Expression`, `id` is the index into the coverage map's array of
/// counter expressions.
/// Aligns with [llvm::coverage::Counter](https://github.com/rust-lang/llvm-project/blob/rustc/13.0-2021-09-30/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L102-L103)
/// Important: The Rust struct layout (order and types of fields) must match its C++ counterpart.
///
/// Corresponds to struct `llvm::coverage::Counter`.
///
/// Must match the layout of `LLVMRustCounter`.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct Counter {
Expand Down Expand Up @@ -59,17 +61,19 @@ impl Counter {
}
}

/// Aligns with [llvm::coverage::CounterExpression::ExprKind](https://github.com/rust-lang/llvm-project/blob/rustc/13.0-2021-09-30/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L150)
/// Corresponds to enum `llvm::coverage::CounterExpression::ExprKind`.
///
/// Must match the layout of `LLVMRustCounterExprKind`.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub enum ExprKind {
Subtract = 0,
Add = 1,
}

/// Aligns with [llvm::coverage::CounterExpression](https://github.com/rust-lang/llvm-project/blob/rustc/13.0-2021-09-30/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L151-L152)
/// Important: The Rust struct layout (order and types of fields) must match its C++
/// counterpart.
/// Corresponds to struct `llvm::coverage::CounterExpression`.
///
/// Must match the layout of `LLVMRustCounterExpression`.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct CounterExpression {
Expand Down
109 changes: 101 additions & 8 deletions compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,100 @@

using namespace llvm;

// FFI equivalent of enum `llvm::coverage::Counter::CounterKind`
// https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L97-L99
enum class LLVMRustCounterKind {
Zero = 0,
CounterValueReference = 1,
Expression = 2,
};

// FFI equivalent of struct `llvm::coverage::Counter`
// https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L94-L149
struct LLVMRustCounter {
LLVMRustCounterKind CounterKind;
uint32_t ID;
};

static coverage::Counter fromRust(LLVMRustCounter Counter) {
switch (Counter.CounterKind) {
case LLVMRustCounterKind::Zero:
return coverage::Counter::getZero();
case LLVMRustCounterKind::CounterValueReference:
return coverage::Counter::getCounter(Counter.ID);
case LLVMRustCounterKind::Expression:
return coverage::Counter::getExpression(Counter.ID);
}
report_fatal_error("Bad LLVMRustCounterKind!");
}

// FFI equivalent of enum `llvm::coverage::CounterMappingRegion::RegionKind`
// https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L213-L234
enum class LLVMRustCounterMappingRegionKind {
CodeRegion = 0,
ExpansionRegion = 1,
SkippedRegion = 2,
GapRegion = 3,
BranchRegion = 4,
};

static coverage::CounterMappingRegion::RegionKind
fromRust(LLVMRustCounterMappingRegionKind Kind) {
switch (Kind) {
case LLVMRustCounterMappingRegionKind::CodeRegion:
return coverage::CounterMappingRegion::CodeRegion;
case LLVMRustCounterMappingRegionKind::ExpansionRegion:
return coverage::CounterMappingRegion::ExpansionRegion;
case LLVMRustCounterMappingRegionKind::SkippedRegion:
return coverage::CounterMappingRegion::SkippedRegion;
case LLVMRustCounterMappingRegionKind::GapRegion:
return coverage::CounterMappingRegion::GapRegion;
case LLVMRustCounterMappingRegionKind::BranchRegion:
return coverage::CounterMappingRegion::BranchRegion;
}
report_fatal_error("Bad LLVMRustCounterMappingRegionKind!");
}

// FFI equivalent of struct `llvm::coverage::CounterMappingRegion`
// https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L211-L304
struct LLVMRustCounterMappingRegion {
coverage::Counter Count;
coverage::Counter FalseCount;
LLVMRustCounter Count;
LLVMRustCounter FalseCount;
uint32_t FileID;
uint32_t ExpandedFileID;
uint32_t LineStart;
uint32_t ColumnStart;
uint32_t LineEnd;
uint32_t ColumnEnd;
coverage::CounterMappingRegion::RegionKind Kind;
LLVMRustCounterMappingRegionKind Kind;
};

// FFI equivalent of enum `llvm::coverage::CounterExpression::ExprKind`
// https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L154
enum class LLVMRustCounterExprKind {
Subtract = 0,
Add = 1,
};

// FFI equivalent of struct `llvm::coverage::CounterExpression`
// https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L151-L160
struct LLVMRustCounterExpression {
LLVMRustCounterExprKind Kind;
LLVMRustCounter LHS;
LLVMRustCounter RHS;
};

static coverage::CounterExpression::ExprKind
fromRust(LLVMRustCounterExprKind Kind) {
switch (Kind) {
case LLVMRustCounterExprKind::Subtract:
return coverage::CounterExpression::Subtract;
case LLVMRustCounterExprKind::Add:
return coverage::CounterExpression::Add;
}
report_fatal_error("Bad LLVMRustCounterExprKind!");
}

extern "C" void LLVMRustCoverageWriteFilenamesSectionToBuffer(
const char* const Filenames[],
size_t FilenamesLen,
Expand All @@ -37,9 +119,9 @@ extern "C" void LLVMRustCoverageWriteFilenamesSectionToBuffer(
extern "C" void LLVMRustCoverageWriteMappingToBuffer(
const unsigned *VirtualFileMappingIDs,
unsigned NumVirtualFileMappingIDs,
const coverage::CounterExpression *Expressions,
const LLVMRustCounterExpression *RustExpressions,
unsigned NumExpressions,
LLVMRustCounterMappingRegion *RustMappingRegions,
const LLVMRustCounterMappingRegion *RustMappingRegions,
unsigned NumMappingRegions,
RustStringRef BufferOut) {
// Convert from FFI representation to LLVM representation.
Expand All @@ -48,13 +130,24 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer(
for (const auto &Region : ArrayRef<LLVMRustCounterMappingRegion>(
RustMappingRegions, NumMappingRegions)) {
MappingRegions.emplace_back(
Region.Count, Region.FalseCount, Region.FileID, Region.ExpandedFileID,
fromRust(Region.Count), fromRust(Region.FalseCount),
Region.FileID, Region.ExpandedFileID,
Region.LineStart, Region.ColumnStart, Region.LineEnd, Region.ColumnEnd,
Region.Kind);
fromRust(Region.Kind));
}

std::vector<coverage::CounterExpression> Expressions;
Expressions.reserve(NumExpressions);
for (const auto &Expression :
ArrayRef<LLVMRustCounterExpression>(RustExpressions, NumExpressions)) {
Expressions.emplace_back(fromRust(Expression.Kind),
fromRust(Expression.LHS),
fromRust(Expression.RHS));
}

auto CoverageMappingWriter = coverage::CoverageMappingWriter(
ArrayRef<unsigned>(VirtualFileMappingIDs, NumVirtualFileMappingIDs),
ArrayRef<coverage::CounterExpression>(Expressions, NumExpressions),
Expressions,
MappingRegions);
RawRustStringOstream OS(BufferOut);
CoverageMappingWriter.write(OS);
Expand Down

0 comments on commit 39761b0

Please sign in to comment.