From 7cab196e7cccb68454e0ee5e667628387ad06463 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 8 May 2023 13:57:20 +1000 Subject: [PATCH 1/2] Isolate coverage FFI type layouts from their underlying LLVM C++ types --- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 8 +- .../rustc_codegen_ssa/src/coverageinfo/ffi.rs | 18 +-- .../llvm-wrapper/CoverageMappingWrapper.cpp | 107 ++++++++++++++++-- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 61365e6dc4b..aefd5b2a13c 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -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 { @@ -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 { diff --git a/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs index e288760a02b..1791ce4b315 100644 --- a/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs @@ -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 { @@ -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 { @@ -59,7 +61,9 @@ 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 { @@ -67,9 +71,9 @@ pub enum ExprKind { 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 { diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 03e6d2149e9..38fcd0527da 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -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, @@ -37,7 +119,7 @@ extern "C" void LLVMRustCoverageWriteFilenamesSectionToBuffer( extern "C" void LLVMRustCoverageWriteMappingToBuffer( const unsigned *VirtualFileMappingIDs, unsigned NumVirtualFileMappingIDs, - const coverage::CounterExpression *Expressions, + const LLVMRustCounterExpression *RustExpressions, unsigned NumExpressions, LLVMRustCounterMappingRegion *RustMappingRegions, unsigned NumMappingRegions, @@ -48,13 +130,24 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer( for (const auto &Region : ArrayRef( 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 Expressions; + Expressions.reserve(NumExpressions); + for (const auto &Expression : + ArrayRef(RustExpressions, NumExpressions)) { + Expressions.emplace_back(fromRust(Expression.Kind), + fromRust(Expression.LHS), + fromRust(Expression.RHS)); + } + auto CoverageMappingWriter = coverage::CoverageMappingWriter( ArrayRef(VirtualFileMappingIDs, NumVirtualFileMappingIDs), - ArrayRef(Expressions, NumExpressions), + Expressions, MappingRegions); RawRustStringOstream OS(BufferOut); CoverageMappingWriter.write(OS); From 9addf0651c2fc9174824ea7e28c2651c780ae968 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 9 May 2023 18:38:47 +1000 Subject: [PATCH 2/2] Correctly mark parameter `RustMappingRegions` as pointer-to-`const` The regions don't need to be mutable because we pass a copy of them to LLVM instead, and this matches the `*const` in the Rust-side signature. --- compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 38fcd0527da..87906dee4d3 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -121,7 +121,7 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer( unsigned NumVirtualFileMappingIDs, const LLVMRustCounterExpression *RustExpressions, unsigned NumExpressions, - LLVMRustCounterMappingRegion *RustMappingRegions, + const LLVMRustCounterMappingRegion *RustMappingRegions, unsigned NumMappingRegions, RustStringRef BufferOut) { // Convert from FFI representation to LLVM representation.