fix: record nothing for a process with single-byte coverage counters

AGENTS.md

@@ -9,11 +9,13 @@ A Swift package that test targets link to record which coverage counters each te
   - `Fixtures/Example` - with SwiftPM, xcodebuild on macOS and xcodebuild on an iOS simulator.
   - `Fixtures/Concurrency` - the overlapping, nested-trait and failure cases, in a package of their own since Swift Testing runs suites in parallel across the process.
   - `Fixtures/StaticArchive` - links the observer from a static archive, with and without `-ObjC`, into a bundle with a Swift and an Objective-C test class.
+  - `Fixtures/ByteCoverage` - an Objective-C bundle and the C code it tests, built with 8-byte counters and with single-byte ones (`-mllvm -enable-single-byte-coverage`), with and without local symbols, by `run.sh`; the observer records the first and nothing for the others.
   - `Fixtures/UITests` - an app, its UI tests and its hosted unit tests. It's a Tuist project (`Project.swift`), because a UI test needs an app target and SwiftPM can't build apps; `Launches.swift` reads `launches.tsv`, and `SkipListTests.swift` runs the `SkipList` scheme serially, in parallel and as a shard and reads the result bundle.
 
 ## Rules
 - Never crash or block the test process: every failure path returns and the tests run as if nothing were linked.
 - Never reset or write the counters.
+- Counters are read as 8-byte counts only: an image whose counters are anything else (`has_byte_counters()`) stops recording for the process rather than being read or left out.
 - Changing `records.bin`: update `write_record()`, `Records.swift`, the README and Tuist's reader together, and bump `kRecordVersion` when a record gains a field.
 - Changing `launches.tsv`: update `write_launch()`, `Launches.swift`, the README and Tuist's reader (`CoverageObserverOutput.parseLaunches`) together.
 - Changing the skip list format: update `load_skip_list()`, `SkipListTests.swift`, the README and Tuist's writer together.

Fixtures/ByteCoverage/ByteCoverageTests.m

@@ -0,0 +1,18 @@
+@import XCTest;
+#import "Classify.h"
+
+@interface ByteCoverageTests : XCTestCase
+@end
+
+@implementation ByteCoverageTests
+
+- (void)testClassify {
+    XCTAssertEqual(classify(-3), -1);
+    XCTAssertEqual(classify(0), 0);
+}
+
+- (void)testSum {
+    XCTAssertEqual(sum(4), 10);
+}
+
+@end

Fixtures/ByteCoverage/Classify.c

@@ -0,0 +1,13 @@
+#include "Classify.h"
+
+int classify(int value) {
+    if (value < 0) return -1;
+    if (value == 0) return 0;
+    return 1;
+}
+
+int sum(int count) {
+    int total = 0;
+    for (int i = 1; i <= count; i++) total += i;
+    return total;
+}

Fixtures/ByteCoverage/Classify.h

@@ -0,0 +1,2 @@
+int classify(int value);
+int sum(int count);

Fixtures/ByteCoverage/run.sh

@@ -0,0 +1,82 @@
+#!/bin/bash
+# Builds an Objective-C XCTest bundle, its tests and the C code they call instrumented for coverage,
+# with the observer linked in, and runs it: run.sh <build directory> <counters>, where <counters> is
+# `word` for clang's default 8-byte counters, `byte` for single-byte counters
+# (`-mllvm -enable-single-byte-coverage`), or `byte-stripped` for single-byte counters in a bundle
+# stripped of its local symbols, the profile version among them. Each single-byte bundle is padded
+# so that only one of the observer's checks can catch it: `byte` has a whole number of 8-byte
+# counters, so only its profile version tells, and `byte-stripped` hasn't.
+set -euo pipefail
+
+BUILD=$1
+COUNTERS=$2
+FIXTURE=$(cd "$(dirname "$0")" && pwd)
+ROOT=$(cd "$FIXTURE/../.." && pwd)
+SDK=$(xcrun --show-sdk-path)
+PLATFORM=$(xcrun --show-sdk-platform-path)
+FRAMEWORKS=$PLATFORM/Developer/Library/Frameworks
+BUNDLE=$BUILD/ByteCoverageTests.xctest
+BINARY=$BUNDLE/Contents/MacOS/ByteCoverageTests
+# The coverage flags, as positional parameters: macOS's bash 3.2 treats an empty array as unbound
+# under `set -u`.
+set -- -fprofile-instr-generate -fcoverage-mapping
+if [ "$COUNTERS" != word ]; then set -- "$@" -mllvm -enable-single-byte-coverage; fi
+
+link() {
+    xcrun clang -bundle -fobjc-arc -isysroot "$SDK" -F "$FRAMEWORKS" -fprofile-instr-generate \
+        "$BUILD"/*.o -o "$BINARY" -framework Foundation -framework XCTest \
+        -Xlinker -rpath -Xlinker "$FRAMEWORKS"
+}
+
+counters_size() {
+    xcrun size -m "$BINARY" | awk '/__llvm_prf_cnts/ { print $3 }'
+}
+
+rm -rf "$BUILD"
+mkdir -p "$BUNDLE/Contents/MacOS"
+xcrun clang -c -fobjc-arc -fmodules -isysroot "$SDK" \
+    -I "$ROOT/Sources/TestCoverageAttributionObserver/include" \
+    "$ROOT/Sources/TestCoverageAttributionObserver/TestCoverageAttributionObserver.m" -o "$BUILD/observer.o"
+xcrun clang -c -isysroot "$SDK" "$@" "$FIXTURE/Classify.c" -o "$BUILD/Classify.o"
+xcrun clang -c -fobjc-arc -fmodules -isysroot "$SDK" -F "$FRAMEWORKS" "$@" \
+    "$FIXTURE/ByteCoverageTests.m" -o "$BUILD/ByteCoverageTests.o"
+/usr/libexec/PlistBuddy \
+    -c "Add :CFBundleIdentifier string dev.tuist.ByteCoverageTests" \
+    -c "Add :CFBundleExecutable string ByteCoverageTests" \
+    -c "Add :CFBundlePackageType string BNDL" \
+    "$BUNDLE/Contents/Info.plist" > /dev/null
+link
+
+# A function without branches has one single-byte counter.
+SIZE=$(counters_size)
+case $COUNTERS in
+    byte) PADDING=$(((8 - SIZE % 8) % 8)) ;;
+    byte-stripped) PADDING=$((SIZE % 8 == 0 ? 1 : 0)) ;;
+    *) PADDING=0 ;;
+esac
+if [ "$PADDING" -gt 0 ]; then
+    for i in $(seq "$PADDING"); do echo "int padding$i(void) { return $i; }"; done > "$BUILD/Padding.c"
+    xcrun clang -c -isysroot "$SDK" "$@" "$BUILD/Padding.c" -o "$BUILD/Padding.o"
+    link
+fi
+SIZE=$(counters_size)
+case $COUNTERS in
+    byte)
+        if [ $((SIZE % 8)) -ne 0 ]; then
+            echo "The counters section ($SIZE bytes) isn't a multiple of 8 bytes" >&2
+            exit 1
+        fi
+        ;;
+    byte-stripped)
+        xcrun strip -x "$BINARY"
+        if nm "$BINARY" | grep -q ___llvm_profile_raw_version; then
+            echo "strip kept the profile version" >&2
+            exit 1
+        fi
+        if [ $((SIZE % 8)) -eq 0 ]; then
+            echo "The counters section ($SIZE bytes) is a multiple of 8 bytes" >&2
+            exit 1
+        fi
+        ;;
+esac
+LLVM_PROFILE_FILE="$BUILD/default.profraw" xcrun xctest "$BUNDLE"

README.md

@@ -178,6 +178,7 @@ The deltas are what turns counters into lines. In the image's coverage mapping (
 - **Swift Testing needs the trait.** Tests in suites without it aren't attributed or skipped, tests that run at the same time get no evidence, and skipping them needs Swift 6.3. See [Usage](#usage).
 - **Skipping some of a class's tests uses a private method.** XCTest has no public way to remove tests from a class's suite, so the package uses `XCTestSuite`'s private `-setTests:`, which keeps the suite and the class `setUp` it runs. If a future Xcode removes it, a class with some tests listed runs all of them; a class with all of its tests listed is still skipped. A class that overrides `defaultTestSuite` without calling `super` isn't filtered.
 - **UI tests record which app process each test launched, not the app's counters.** The app's coverage comes from the profile Xcode writes for each app process (`<uuid>-<pid>.profraw` beside the run's `Coverage.profdata`). The pid is read from `XCUIApplication`'s private `processID` property; if a future Xcode removes it, nothing is written. Processes the app starts itself, such as extensions, aren't listed.
+- **Single-byte counters aren't supported.** LLVM's single-byte coverage mode (`-mllvm -enable-single-byte-coverage`) gives an image counters of one byte, which start at `0xFF` and are cleared once their code runs, so they can't be read as the 8-byte counts the records hold. A process with such an image records nothing: the package finds the mode in the image's profile version (`__llvm_profile_raw_version`, looked up in its symbol table) or, in an image stripped of local symbols, in a counters section that isn't a whole number of 8-byte counters, and removes what it wrote for the process. A stripped image whose single-byte counters happen to fill whole 8-byte words isn't caught. Skipping is unaffected.
 - **Images loaded late aren't observed.** An image loaded after the first test starts, such as a framework the tests `dlopen`, is missing from the records.
 - **The package's own code shows up in the records.** The package is instrumented like the code under test, so its functions appear in the records, and a reader should ignore them.
 

Sources/TestCoverageAttributionObserver/TestCoverageAttributionObserver.m

@@ -4,7 +4,9 @@
 // by how much, while the tests run. It never calls the LLVM profile runtime, whose symbols are
 // hidden per image: it finds the `__llvm_prf_cnts` section of every instrumented image, copies
 // it when a scope starts, and when the scope ends records the counters that changed. Counters
-// are never zeroed, so Xcode's own coverage report is untouched.
+// are never zeroed, so Xcode's own coverage report is untouched. The counters are read as 8-byte
+// counts, so a process with an image of single-byte counters records nothing, see
+// has_byte_counters().
 //
 // Nothing happens unless $TEST_COVERAGE_ATTRIBUTION_DIR names where to write (through xcodebuild,
 // TEST_RUNNER_TEST_COVERAGE_ATTRIBUTION_DIR), so a test target can link it unconditionally.
@@ -37,6 +39,7 @@
 #include <limits.h>
 #include <mach-o/dyld.h>
 #include <mach-o/getsect.h>
+#include <mach-o/nlist.h>
 #include <objc/message.h>
 #include <objc/runtime.h>
 #include <pthread.h>
@@ -113,8 +116,54 @@ static bool dump_section(const struct mach_header_64 *h, const char *name, unsig
     return fclose(f) == 0 && written;
 }
 
+// The bit of LLVM's profile version that marks single-byte counters (VARIANT_MASK_BYTE_COVERAGE in
+// InstrProfData.inc).
+static const uint64_t kByteCoverage = 1ULL << 60;
+
+// The address of a symbol the image defines, local ones included, from its symbol table; NULL when
+// the table is stripped of it.
+static const void *image_symbol(const struct mach_header_64 *h, intptr_t slide, const char *name) {
+    const struct segment_command_64 *linkedit = NULL;
+    const struct symtab_command *symtab = NULL;
+    const uint8_t *command = (const uint8_t *)(h + 1);
+    for (uint32_t i = 0; i < h->ncmds; i++) {
+        const struct load_command *load = (const struct load_command *)command;
+        if (load->cmd == LC_SEGMENT_64) {
+            const struct segment_command_64 *segment = (const struct segment_command_64 *)load;
+            if (strncmp(segment->segname, SEG_LINKEDIT, sizeof segment->segname) == 0) linkedit = segment;
+        } else if (load->cmd == LC_SYMTAB) {
+            symtab = (const struct symtab_command *)load;
+        }
+        command += load->cmdsize;
+    }
+    if (!linkedit || !symtab) return NULL;
+    uintptr_t base = (uintptr_t)slide + linkedit->vmaddr - linkedit->fileoff;
+    const struct nlist_64 *symbols = (const struct nlist_64 *)(base + symtab->symoff);
+    const char *strings = (const char *)(base + symtab->stroff);
+    for (uint32_t i = 0; i < symtab->nsyms; i++) {
+        if ((symbols[i].n_type & (N_STAB | N_TYPE)) != N_SECT || symbols[i].n_un.n_strx >= symtab->strsize) continue;
+        if (strcmp(strings + symbols[i].n_un.n_strx, name) == 0) return (const void *)(symbols[i].n_value + slide);
+    }
+    return NULL;
+}
+
+// Whether the image's counters are single bytes, as LLVM's single-byte coverage mode
+// (`-mllvm -enable-single-byte-coverage`) makes them: each starts at 0xFF and is cleared once its
+// code runs, so read as 8-byte counters they would give wrong deltas, and the reader's function
+// table can't place them. The image's profile version says so, as it tells the profile runtime:
+// clang defines it in single-byte mode, and otherwise the profile runtime the image links does.
+// It's hidden, so it's looked up in the symbol table. A stripped image has no symbol to go by, but
+// a section that isn't a whole number of 8-byte counters still gives it away.
+static bool has_byte_counters(const struct mach_header_64 *h, intptr_t slide, unsigned long counters_size) {
+    if (counters_size % sizeof(uint64_t) != 0) return true;
+    const uint64_t *version = image_symbol(h, slide, "___llvm_profile_raw_version");
+    return version && (*version & kByteCoverage) != 0;
+}
+
 // Images loaded after the first scope (a framework the tests dlopen late) are not picked up;
-// their code is then missing from the evidence, which the per-target floor covers.
+// their code is then missing from the evidence, which the per-target floor covers. An image with
+// single-byte counters stops recording: leaving it out would give its code's tests evidence that
+// misses it, which reads as tests that didn't run it.
 static void discover_images(void) {
     if (images_discovered) return;
     images_discovered = true;
@@ -133,6 +182,10 @@ static void discover_images(void) {
         unsigned long data_size = 0;
         const uint8_t *data = section(h, "__llvm_prf_data", &data_size);
         if (!data) continue;
+        if (has_byte_counters(h, _dyld_get_image_vmaddr_slide(i), size)) {
+            ok = false;
+            break;
+        }
         instrumented_image *img = &images[images_count];
         img->counters = (uint64_t *)cnts;
         img->snapshot = malloc(size);

Tests/TestCoverageAttributionTests/AttributionTests.swift

@@ -340,3 +340,42 @@ extension AttributionTests {
         return try Data(contentsOf: URL(filePath: path))
     }
 }
+
+extension AttributionTests {
+    /// Single-byte counters (`-mllvm -enable-single-byte-coverage`) start at 0xFF and are cleared
+    /// once their code runs, so read as 8-byte counters they'd give wrong deltas. A process with such
+    /// an image records nothing: the observer removes what it wrote, whether the image's profile
+    /// version gives it away (`byte`) or, stripped of it, its counters section's size does
+    /// (`byte-stripped`), and the tests run as usual. With clang's default counters (`word`) the same
+    /// bundle records each test.
+    @Test(arguments: ["word", "byte", "byte-stripped"])
+    func recordsNothingForAnImageWithSingleByteCounters(counters: String) throws {
+        let scratch = FileManager.default.temporaryDirectory.appending(path: "TestCoverageAttribution-\(UUID().uuidString)")
+        defer { try? FileManager.default.removeItem(at: scratch) }
+        let output = scratch.appending(path: "output")
+        try FileManager.default.createDirectory(at: output, withIntermediateDirectories: true)
+        var environment = Self.cleanEnvironment
+        environment["TEST_COVERAGE_ATTRIBUTION_DIR"] = output.path()
+
+        try Self.execute(
+            Self.packageRoot.appending(path: "Fixtures/ByteCoverage/run.sh"),
+            [scratch.appending(path: "build").path(), counters],
+            environment: environment,
+        )
+
+        // The observer loaded: it made the process's directory.
+        let processes = try FileManager.default.contentsOfDirectory(at: output, includingPropertiesForKeys: nil)
+        try #require(processes.count == 1)
+        let files = try FileManager.default.contentsOfDirectory(atPath: processes[0].path())
+        let records = try Record.all(in: output)
+        if counters == "word" {
+            let classify = try #require(records.first { $0.kind == .xcTest && $0.name == "testClassify" })
+            let sum = try #require(records.first { $0.kind == .xcTest && $0.name == "testSum" })
+            #expect(classify.functions.contains("classify") && !classify.functions.contains("sum"))
+            #expect(sum.functions.contains("sum") && !sum.functions.contains("classify"))
+        } else {
+            #expect(records.isEmpty)
+            #expect(!files.contains("images.tsv") && !files.contains("records.bin"))
+        }
+    }
+}