Rollup merge of #111382 - Zalathar:ffi, r=cuviper

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.
rust-lang · May 11, 2023 · 39761b0 · 39761b0
2 parents aa9adf4 + 9addf06
commit 39761b0
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 17 deletions.
diff --git 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
@@ -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,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 {

diff --git 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,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.
@@ -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);