test(coverage): union coverage across re-imported instances of a module

src/jsc/bindings/ZigSourceProvider.cpp

@@ -48,7 +48,6 @@ SourceOrigin toSourceOrigin(const String& sourceURL, bool isBuiltin)
     return SourceOrigin(WTF::URL::fileURLWithFileSystemPath(sourceURL));
 }
 
-extern "C" int ByteRangeMapping__getSourceID(void* mappings, BunString sourceURL);
 extern "C" void* ByteRangeMapping__find(BunString sourceURL);
 void* sourceMappingForSourceURL(const WTF::String& sourceURL)
 {
@@ -57,16 +56,6 @@ void* sourceMappingForSourceURL(const WTF::String& sourceURL)
 
 extern "C" void ByteRangeMapping__generate(BunString sourceURL, BunString code, int sourceID);
 
-JSC::SourceID sourceIDForSourceURL(const WTF::String& sourceURL)
-{
-    void* mappings = ByteRangeMapping__find(Bun::toString(sourceURL));
-    if (!mappings) {
-        return 0;
-    }
-
-    return ByteRangeMapping__getSourceID(mappings, Bun::toString(sourceURL));
-}
-
 extern "C" bool BunTest__shouldGenerateCodeCoverage(BunString sourceURL);
 extern "C" void Bun__addSourceProviderSourceMap(void* bun_vm, SourceProvider* opaque_source_provider, BunString* specifier);
 extern "C" void Bun__removeSourceProviderSourceMap(void* bun_vm, SourceProvider* opaque_source_provider, BunString* specifier);

src/jsc/bindings/ZigSourceProvider.h

@@ -21,7 +21,6 @@ namespace Zig {
 class GlobalObject;
 
 void forEachSourceProvider(WTF::Function<void(JSC::SourceID)>);
-JSC::SourceID sourceIDForSourceURL(const WTF::String& sourceURL);
 void* sourceMappingForSourceURL(const WTF::String& sourceURL);
 JSC::SourceOrigin toSourceOrigin(const String& sourceURL, bool isBuiltin);
 class SourceProvider final : public JSC::SourceProvider {

src/jsc/modules/BunJSCModule.h

@@ -49,8 +49,6 @@
 extern "C" char* mi_stats_get_json(size_t, char*);
 extern "C" char* mi_heap_dump_json(bool include_blocks, bool hash_addresses);
 
-#include <JavaScriptCore/ControlFlowProfiler.h>
-
 #if OS(DARWIN)
 #if ASSERT_ENABLED
 #if !__has_feature(address_sanitizer)
@@ -878,8 +876,7 @@ JSC_DEFINE_HOST_FUNCTION(functionDeserialize, (JSGlobalObject * globalObject, Ca
 }
 
 extern "C" JSC::EncodedJSValue ByteRangeMapping__findExecutedLines(
-    JSC::JSGlobalObject*, BunString sourceURL, BasicBlockRange* ranges,
-    size_t len, size_t functionOffset, bool ignoreSourceMap);
+    JSC::JSGlobalObject*, BunString sourceURL, bool ignoreSourceMap);
 
 JSC_DEFINE_HOST_FUNCTION(functionCodeCoverageForFile,
     (JSGlobalObject * globalObject,
@@ -892,41 +889,16 @@ JSC_DEFINE_HOST_FUNCTION(functionCodeCoverageForFile,
     RETURN_IF_EXCEPTION(throwScope, {});
     bool ignoreSourceMap = callFrame->argument(1).toBoolean(globalObject);
 
-    auto sourceID = Zig::sourceIDForSourceURL(fileName);
-    if (!sourceID) {
+    JSValue result = JSValue::decode(ByteRangeMapping__findExecutedLines(
+        globalObject, Bun::toString(fileName), ignoreSourceMap));
+    RETURN_IF_EXCEPTION(throwScope, {});
+    if (result.isNull()) {
         throwException(globalObject, throwScope,
             createError(globalObject, "No source for file"_s));
         return {};
     }
 
-    auto basicBlocks = vm.controlFlowProfiler()->getBasicBlocksForSourceIDWithoutFunctionRange(
-        sourceID, vm);
-
-    if (basicBlocks.isEmpty()) {
-        return JSC::JSValue::encode(
-            JSC::constructEmptyArray(globalObject, nullptr, 0));
-    }
-
-    size_t functionStartOffset = basicBlocks.size();
-
-    const Vector<std::tuple<bool, unsigned, unsigned>>& functionRanges = vm.functionHasExecutedCache()->getFunctionRanges(sourceID);
-
-    basicBlocks.reserveCapacity(functionRanges.size() + basicBlocks.size());
-
-    for (const auto& functionRange : functionRanges) {
-        BasicBlockRange range;
-        range.m_hasExecuted = std::get<0>(functionRange);
-        range.m_startOffset = static_cast<int>(std::get<1>(functionRange));
-        range.m_endOffset = static_cast<int>(std::get<2>(functionRange));
-        range.m_executionCount = range.m_hasExecuted
-            ? 1
-            : 0; // This is a hack. We don't actually count this.
-        basicBlocks.append(range);
-    }
-
-    return ByteRangeMapping__findExecutedLines(
-        globalObject, Bun::toString(fileName), basicBlocks.begin(),
-        basicBlocks.size(), functionStartOffset, ignoreSourceMap);
+    return JSValue::encode(result);
 }
 
 JSC_DEFINE_HOST_FUNCTION(functionEstimateDirectMemoryUsageOf, (JSGlobalObject * globalObject, CallFrame* callFrame))

src/sourcemap_jsc/CodeCoverage.rs

@@ -87,30 +87,24 @@ impl Report {
         // functionHasExecutedCache), so we must preserve write provenance.
         let vm = global_this.vm_ptr();
 
-        let mut result: Option<Report> = None;
-
-        let mut generator = Generator {
-            result: &mut result,
-            byte_range_mapping,
-        };
-
-        // SAFETY: `vm` is the live `*mut VM` owning `global_this`; Generator and the
-        // callback are kept alive for the duration of the FFI call;
-        // CodeCoverage__withBlocksAndFunctions invokes the callback synchronously.
-        let ok = unsafe {
-            CodeCoverage__withBlocksAndFunctions(
-                vm,
-                generator.byte_range_mapping.source_id,
-                (&raw mut generator).cast::<c_void>(),
-                ignore_sourcemap_,
-                Generator::do_,
-            )
-        };
-        if !ok {
+        let collector = byte_range_mapping.collect_blocks(vm, ignore_sourcemap_)?;
+        if collector.blocks.is_empty() {
             return None;
         }
 
-        result
+        // No ownership transfer here:
+        // `from_utf8_never_free` already detaches the lifetime by design, and
+        // `generate_report_from_blocks` only borrows `&self`, so no &/&mut overlap.
+        let source_url =
+            ZigStringSlice::from_utf8_never_free(byte_range_mapping.source_url.slice());
+        byte_range_mapping
+            .generate_report_from_blocks(
+                source_url,
+                &collector.blocks,
+                &collector.function_blocks,
+                ignore_sourcemap_,
+            )
+            .ok()
     }
 }
 
@@ -360,25 +354,26 @@ unsafe extern "C" {
         source_id: i32,
         ctx: *mut c_void,
         ignore_sourcemap: bool,
-        cb: extern "C" fn(*mut Generator, *const BasicBlockRange, usize, usize, bool),
+        cb: extern "C" fn(*mut BlockCollector, *const BasicBlockRange, usize, usize, bool),
     ) -> bool;
 }
 
-struct Generator<'a> {
-    byte_range_mapping: &'a mut ByteRangeMapping,
-    result: &'a mut Option<Report>,
+struct BlockCollector {
+    blocks: Vec<BasicBlockRange>,
+    function_blocks: Vec<BasicBlockRange>,
 }
 
-impl<'a> Generator<'a> {
-    extern "C" fn do_(
-        this: *mut Generator,
+impl BlockCollector {
+    extern "C" fn collect(
+        this: *mut BlockCollector,
         blocks_ptr: *const BasicBlockRange,
         blocks_len: usize,
         function_start_offset: usize,
-        ignore_sourcemap: bool,
+        _ignore_sourcemap: bool,
     ) {
-        // SAFETY: `this` was passed as &mut Generator to CodeCoverage__withBlocksAndFunctions
-        // and is valid for the duration of this synchronous callback.
+        // SAFETY: `this` was passed as &mut BlockCollector to
+        // CodeCoverage__withBlocksAndFunctions and is valid for the duration of this
+        // synchronous callback.
         let this = unsafe { &mut *this };
         // The C++ side (CodeCoverage.cpp) invokes this callback with `(nullptr, 0, 0)` when
         // basicBlocks is empty. `core::slice::from_raw_parts` requires a non-null, aligned
@@ -395,20 +390,34 @@ impl<'a> Generator<'a> {
             function_blocks = &function_blocks[1..];
         }
 
-        if blocks.is_empty() {
-            return;
-        }
+        this.blocks.extend_from_slice(blocks);
+        this.function_blocks.extend_from_slice(function_blocks);
+    }
+}
 
-        // No ownership transfer here:
-        // `from_utf8_never_free` already detaches the lifetime by design, and
-        // `generate_report_from_blocks` only borrows `&self`, so no &/&mut overlap.
-        let source_url =
-            ZigStringSlice::from_utf8_never_free(this.byte_range_mapping.source_url.slice());
-        *this.result = this
-            .byte_range_mapping
-            .generate_report_from_blocks(source_url, blocks, function_blocks, ignore_sourcemap)
-            .ok();
+/// Union coverage data for identical byte ranges collected from multiple
+/// instances of the same source: a range counts as executed if it executed in
+/// any instance, and hit counts are summed.
+fn merge_duplicate_ranges(ranges: &mut Vec<BasicBlockRange>) {
+    if ranges.len() < 2 {
+        return;
     }
+    ranges.sort_unstable_by_key(|r| (r.start_offset, r.end_offset));
+    let mut out: usize = 0;
+    for i in 1..ranges.len() {
+        if ranges[i].start_offset == ranges[out].start_offset
+            && ranges[i].end_offset == ranges[out].end_offset
+        {
+            ranges[out].has_executed |= ranges[i].has_executed;
+            ranges[out].execution_count = ranges[out]
+                .execution_count
+                .saturating_add(ranges[i].execution_count);
+        } else {
+            out += 1;
+            ranges[out] = ranges[i];
+        }
+    }
+    ranges.truncate(out + 1);
 }
 
 #[repr(C)]
@@ -422,7 +431,12 @@ pub struct BasicBlockRange {
 
 pub struct ByteRangeMapping {
     pub line_offset_table: line_offset_table::List,
+    /// The most recent JSC source ID for this source URL.
     pub source_id: i32,
+    /// Source IDs of earlier instances of the same URL (one per extra
+    /// evaluation, e.g. cache-busting query-string re-imports). Coverage is
+    /// unioned across all of them.
+    pub prior_source_ids: Vec<i32>,
     pub source_url: ZigStringSlice,
 }
 
@@ -480,6 +494,50 @@ impl ByteRangeMapping {
         thread_map_opt()
     }
 
+    /// Collect the profiler's basic-block and function ranges for every
+    /// instance of this source URL. A URL has multiple JSC source IDs when the
+    /// same file is evaluated more than once (e.g. cache-busting
+    /// `import("./mod?v=" + n)`); the per-instance data is unioned so earlier
+    /// instances' hits are not lost.
+    fn collect_blocks(&self, vm: *mut VM, ignore_sourcemap: bool) -> Option<BlockCollector> {
+        let mut collector = BlockCollector {
+            blocks: Vec::new(),
+            function_blocks: Vec::new(),
+        };
+
+        for source_id in self
+            .prior_source_ids
+            .iter()
+            .copied()
+            .chain(core::iter::once(self.source_id))
+        {
+            // SAFETY: `vm` is the caller's live `*mut VM`; the collector and the
+            // callback are kept alive for the duration of the FFI call;
+            // CodeCoverage__withBlocksAndFunctions invokes the callback synchronously.
+            let ok = unsafe {
+                CodeCoverage__withBlocksAndFunctions(
+                    vm,
+                    source_id,
+                    (&raw mut collector).cast::<c_void>(),
+                    ignore_sourcemap,
+                    BlockCollector::collect,
+                )
+            };
+            if !ok {
+                return None;
+            }
+        }
+
+        if !self.prior_source_ids.is_empty() {
+            // The source text is identical across instances, so ranges line up
+            // byte-for-byte; union the per-instance data per range.
+            merge_duplicate_ranges(&mut collector.blocks);
+            merge_duplicate_ranges(&mut collector.function_blocks);
+        }
+
+        Some(collector)
+    }
+
     pub fn generate_report_from_blocks(
         &self,
         source_url: ZigStringSlice,
@@ -829,6 +887,7 @@ impl ByteRangeMapping {
             line_offset_table: LineOffsetTable::generate(source_contents, 0)
                 .unwrap_or_else(|_| bun_alloc::out_of_memory()),
             source_id,
+            prior_source_ids: Vec::new(),
             source_url,
         }
     }
@@ -852,17 +911,21 @@ pub(crate) extern "C" fn ByteRangeMapping__generate(
     let hash = bun_wyhash::hash(slice.slice());
     let source_contents = source_contents_str.to_utf8();
 
-    let new_value = ByteRangeMapping::compute(source_contents.slice(), source_id, slice);
+    let mut new_value = ByteRangeMapping::compute(source_contents.slice(), source_id, slice);
+    // Re-evaluating the same URL (e.g. `import("./mod?v=" + n)`) creates a new
+    // JSC source ID; keep the old ones so report generation can union coverage
+    // across every instance instead of only the latest.
+    if let Some(old) = map.get_mut(&hash) {
+        new_value.prior_source_ids = core::mem::take(&mut old.prior_source_ids);
+        if old.source_id != source_id {
+            new_value.prior_source_ids.push(old.source_id);
+        }
+    }
     map.insert(hash, new_value);
     // `source_contents` drops here (matches `defer source_contents.deinit()`).
     // Note: `slice` ownership transferred into the new ByteRangeMapping.source_url.
 }
 
-#[unsafe(no_mangle)]
-pub(crate) extern "C" fn ByteRangeMapping__getSourceID(this: &ByteRangeMapping) -> i32 {
-    this.source_id
-}
-
 #[unsafe(no_mangle)]
 pub(crate) extern "C" fn ByteRangeMapping__find(
     path: bun_core::String,
@@ -881,9 +944,6 @@ pub(crate) extern "C" fn ByteRangeMapping__find(
 pub(crate) extern "C" fn ByteRangeMapping__findExecutedLines(
     global_this: &JSGlobalObject,
     source_url: bun_core::String,
-    blocks_ptr: NonNull<BasicBlockRange>,
-    blocks_len: usize,
-    function_start_offset: usize,
     ignore_sourcemap: bool,
 ) -> JSValue {
     let Some(this_ptr) = ByteRangeMapping__find(source_url.clone()) else {
@@ -892,18 +952,21 @@ pub(crate) extern "C" fn ByteRangeMapping__findExecutedLines(
     // SAFETY: pointer into the thread-local map, valid for this call.
     let this = unsafe { &*this_ptr.as_ptr() };
 
-    // SAFETY: blocks_ptr[0..blocks_len] is a valid contiguous C array from JSC.
-    let all = unsafe { core::slice::from_raw_parts(blocks_ptr.as_ptr(), blocks_len) };
-    let blocks: &[BasicBlockRange] = &all[0..function_start_offset];
-    let mut function_blocks: &[BasicBlockRange] = &all[function_start_offset..blocks_len];
-    if function_blocks.len() > 1 {
-        function_blocks = &function_blocks[1..];
+    let Some(collector) = this.collect_blocks(global_this.vm_ptr(), ignore_sourcemap) else {
+        return JSValue::NULL;
+    };
+    if collector.blocks.is_empty() {
+        return match JSValue::create_empty_array(global_this, 0) {
+            Ok(v) => v,
+            Err(_) => JSValue::ZERO,
+        };
     }
+
     let url_slice = source_url.to_utf8();
     let report = match this.generate_report_from_blocks(
         url_slice,
-        blocks,
-        function_blocks,
+        &collector.blocks,
+        &collector.function_blocks,
         ignore_sourcemap,
     ) {
         Ok(r) => r,