diff --git a/.github/workflows/store-order.yml b/.github/workflows/store-order.yml new file mode 100644 index 0000000..ff1a268 --- /dev/null +++ b/.github/workflows/store-order.yml @@ -0,0 +1,237 @@ +# Temporary, and not to be merged: asks whether the libdispatch crash on the +# Linux arm64 rows (#11) is a store reordering. +# +# The reading being tested: when a worker takes Swift's global concurrent +# queue off a root queue, `_dispatch_queue_class_invoke` writes +# DISPATCH_OBJECT_LISTLESS into the queue's `do_next` with a plain store, and +# if the queue cannot run (its width is used up) clears ENQUEUED in `dq_state` +# with an acquire-only compare-and-swap. Nothing orders the store before the +# swap, so another thread can see ENQUEUED clear, enqueue the queue again — +# writing NULL into `do_next` and linking it — and then have the late +# LISTLESS land on top. The next two workers to drain that root queue take +# LISTLESS for the next item and the second one dies on it. +# +# Two questions, one job family each: +# +# - Litmus: does this hardware make that reordering at all? StoreOrder/litmus.c +# is the pattern alone, with two controls that must stay at zero. +# - Reproduce: is it this reordering that crashes the reproducer? The same +# libdispatch the toolchain ships, rebuilt three ways from one source — as +# it is, with `dmb ishst` after the store (orders it before the swap), and +# with `dmb ishld` in the same place (costs about as much and orders +# nothing that matters here) — each under the reproducer, beside the +# toolchain's own library. + +name: Store order + +# Only a change to this file or to what it runs starts it, and only from a +# pull request; the paths are none that swift.yml or apple-platforms.yml +# listen to. +on: + pull_request: + paths: + - .github/workflows/store-order.yml + - Reproducer/** + - StoreOrder/** + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + SWIFT: "6.4" + # The libdispatch sources of that release, which the toolchain's + # libdispatch.so was built from. + DISPATCH_REF: swift-6.4.0-RELEASE + +jobs: + litmus: + name: Litmus + runs-on: ubuntu-24.04-arm + timeout-minutes: 60 + steps: + - uses: actions/checkout@v7 + + # Outline atomics, as libdispatch is built: the swaps go through the + # same `__aarch64_cas8_*` helpers, which pick `casa`/`casl`/`casal` on + # this CPU. + - name: Build + run: | + clang -O2 -Wall -Wextra -moutline-atomics -pthread StoreOrder/litmus.c -o "$RUNNER_TEMP/litmus" + nm "$RUNNER_TEMP/litmus" | grep -E "__aarch64_cas8_(acq|rel|acq_rel)$" + grep -m1 -E "^(model name|CPU part)" /proc/cpuinfo || true + lscpu | grep -E "Model name|Vendor" || true + + # The pattern gets longer than the controls: it is the one whose rate + # matters. + - name: Run + run: | + echo "| order | layout | LISTLESS at the end | seconds |" >> "$GITHUB_STEP_SUMMARY" + echo "| --- | --- | --- | --- |" >> "$GITHUB_STEP_SUMMARY" + for config in "acq same 600" "acq split 600" "acqrel same 300" "acqrel split 300" "ishst same 300" "ishst split 300"; do + set -- $config + line=$("$RUNNER_TEMP/litmus" "$1" "$2" "$3" | tee /dev/stderr | tail -n 1) + echo "| $1 | $2 | ${line#*: x ended as LISTLESS in } |" >> "$GITHUB_STEP_SUMMARY" + done + + dispatch: + name: Build libdispatch + runs-on: ubuntu-24.04-arm + timeout-minutes: 30 + steps: + - uses: actions/checkout@v7 + + # As swift.yml installs it; its clang builds libdispatch, as the + # toolchain's own build does. + - name: Install Swift toolchain + run: | + curl -fL "https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz" -o "$RUNNER_TEMP/swiftly.tar.gz" + tar -C "$RUNNER_TEMP" -zxf "$RUNNER_TEMP/swiftly.tar.gz" + "$RUNNER_TEMP/swiftly" init --assume-yes --skip-install + . "$HOME/.local/share/swiftly/env.sh" + swiftly install --use "$SWIFT" --post-install-file "$RUNNER_TEMP/swiftly-post-install.sh" + if [ -f "$RUNNER_TEMP/swiftly-post-install.sh" ]; then + sudo bash "$RUNNER_TEMP/swiftly-post-install.sh" + fi + echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" + + - name: Build three variants + run: | + sudo apt-get install -y -qq ninja-build + resources=$(swift -print-target-info | jq -r .paths.runtimeResourcePath) + toolchain_bin=$(realpath "$resources/../../bin") + git clone --depth 1 --branch "$DISPATCH_REF" https://github.com/swiftlang/swift-corelibs-libdispatch.git "$RUNNER_TEMP/dispatch" + git -C "$RUNNER_TEMP/dispatch" apply "$GITHUB_WORKSPACE/StoreOrder/listless-probe.patch" + for variant in rebuilt:"" ishst:-DPROBE_LISTLESS_DMB_ISHST ishld:-DPROBE_LISTLESS_DMB_ISHLD; do + name=${variant%%:*} flags=${variant#*:} + cmake -S "$RUNNER_TEMP/dispatch" -B "$RUNNER_TEMP/build-$name" -G Ninja \ + -DCMAKE_C_COMPILER="$toolchain_bin/clang" -DCMAKE_CXX_COMPILER="$toolchain_bin/clang++" \ + -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF -DENABLE_SWIFT=OFF \ + -DCMAKE_C_FLAGS="$flags" -DCMAKE_CXX_FLAGS="$flags" + ninja -C "$RUNNER_TEMP/build-$name" dispatch + mkdir -p "dist/$name" + cp "$RUNNER_TEMP/build-$name/libdispatch.so" "dist/$name/" + done + + # Each variant must be what its name says: the barrier right after the + # LISTLESS store in `_dispatch_lane_invoke`, or no barrier there. + - name: Check the barriers + run: | + for name in rebuilt ishst ishld; do + echo "== $name" + objdump -d --no-show-raw-insn --disassemble=_dispatch_lane_invoke "dist/$name/libdispatch.so" \ + | grep -A 1 -E "str\s+x[0-9]+, \[x[0-9]+, #16\]" | head -n 2 | tee "$RUNNER_TEMP/after-$name" + done + ! grep -q dmb "$RUNNER_TEMP/after-rebuilt" + grep -q "dmb\sishst" "$RUNNER_TEMP/after-ishst" + grep -q "dmb\sishld" "$RUNNER_TEMP/after-ishld" + + - uses: actions/upload-artifact@v7 + with: + name: libdispatch + path: dist/ + + reproduce: + name: Reproducer · ${{ matrix.variant }} · ${{ matrix.copy }} + needs: dispatch + runs-on: ubuntu-24.04-arm + timeout-minutes: 60 + # BUDGET is how long a job goes on starting runs, in seconds; ENOUGH stops + # a job that fails for some other reason. + env: + BUDGET: 2100 + ENOUGH: 20 + SWIFT_BACKTRACE: enable=yes,interactive=no,color=no + strategy: + fail-fast: false + # At #11's rate a budget holds about three crashes. `ishst` gets the + # most copies because its answer is a zero, and a zero needs room: four + # budgets would have held about twelve. `rebuilt` gets as many because + # the first round gave it one crash in 664 runs beside the toolchain's + # seven, and whether the rebuild stands in for the toolchain's library + # is what the other variants are measured against. + matrix: + include: + - { variant: stock, copy: a } + - { variant: stock, copy: b } + - { variant: stock, copy: c } + - { variant: rebuilt, copy: a } + - { variant: rebuilt, copy: b } + - { variant: rebuilt, copy: c } + - { variant: rebuilt, copy: d } + - { variant: ishst, copy: a } + - { variant: ishst, copy: b } + - { variant: ishst, copy: c } + - { variant: ishst, copy: d } + - { variant: ishld, copy: a } + - { variant: ishld, copy: b } + - { variant: ishld, copy: c } + steps: + - uses: actions/checkout@v7 + + - name: Install Swift toolchain + run: | + curl -fL "https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz" -o "$RUNNER_TEMP/swiftly.tar.gz" + tar -C "$RUNNER_TEMP" -zxf "$RUNNER_TEMP/swiftly.tar.gz" + "$RUNNER_TEMP/swiftly" init --assume-yes --skip-install + . "$HOME/.local/share/swiftly/env.sh" + swiftly install --use "$SWIFT" --post-install-file "$RUNNER_TEMP/swiftly-post-install.sh" + if [ -f "$RUNNER_TEMP/swiftly-post-install.sh" ]; then + sudo bash "$RUNNER_TEMP/swiftly-post-install.sh" + fi + echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" + + - uses: actions/download-artifact@v8 + with: + name: libdispatch + path: dist + + - name: swift build + run: | + swift build -c release --package-path Reproducer + echo "reproducer=$(swift build -c release --package-path Reproducer --show-bin-path)/reproducer" >> "$GITHUB_ENV" + + # The toolchain's libraries find libdispatch.so through RUNPATH, which + # LD_LIBRARY_PATH comes before. The loader is asked which one it took. + # + # The toolchain's own directory follows the variant's. A rebuilt + # library's RUNPATH names the build job's tree, which is not on this + # runner, so without it swift-backtrace — which inherits the variable + # and loads the rebuilt library too — finds no libBlocksRuntime.so, and + # a crash is reported without its backtrace. + - name: Choose libdispatch + run: | + toolchain_lib=$(swift -print-target-info | jq -r '.paths.runtimeLibraryPaths[0]') + if [ "${{ matrix.variant }}" = stock ]; then + expected=$toolchain_lib/libdispatch.so + else + echo "LD_LIBRARY_PATH=$GITHUB_WORKSPACE/dist/${{ matrix.variant }}:$toolchain_lib" >> "$GITHUB_ENV" + export LD_LIBRARY_PATH="$GITHUB_WORKSPACE/dist/${{ matrix.variant }}:$toolchain_lib" + expected=$GITHUB_WORKSPACE/dist/${{ matrix.variant }}/libdispatch.so + fi + loaded=$(LD_DEBUG=libs "$reproducer" 2>&1 | sed -n 's/.*calling init: \(.*libdispatch\.so\)$/\1/p' | head -n 1) + echo "expected $expected" + echo "loaded $loaded" + [ "$(realpath "$loaded")" = "$(realpath "$expected")" ] + readelf -n "$loaded" | grep "Build ID" + + # A crash that is not the one in #11 is counted apart, and shown. + - name: Run the program until the budget is spent + run: | + runs=0 listless=0 other=0 + while [ $SECONDS -lt "$BUDGET" ] && [ $other -lt "$ENOUGH" ]; do + runs=$((runs + 1)) + if ! "$reproducer" > "$RUNNER_TEMP/run.log" 2>&1; then + if grep -q "0xffffffff89abcdff" "$RUNNER_TEMP/run.log"; then + listless=$((listless + 1)) + else + other=$((other + 1)) + fi + echo "::group::run $runs failed at ${SECONDS}s" + grep -A 6 -E "Program crashed|crashed:|Fatal error" "$RUNNER_TEMP/run.log" || tail -n 40 "$RUNNER_TEMP/run.log" | cut -c 1-300 + echo "::endgroup::" + fi + done + echo "| ${{ matrix.variant }} | ${{ matrix.copy }} | $listless of $runs | $other | ${SECONDS}s |" >> "$GITHUB_STEP_SUMMARY" + echo "$listless LISTLESS crashes and $other other failures in $runs runs, ${SECONDS}s" + [ $other -eq 0 ] diff --git a/Reproducer/Package.swift b/Reproducer/Package.swift new file mode 100644 index 0000000..1e64bde --- /dev/null +++ b/Reproducer/Package.swift @@ -0,0 +1,13 @@ +// swift-tools-version: 6.0 + +import PackageDescription + +// Depends on nothing: the point of it is that the crash needs none of +// swift-synchronization-kit. +let package = Package( + name: "Reproducer", + platforms: [.macOS(.v13)], + targets: [ + .executableTarget(name: "reproducer", path: "Sources", swiftSettings: [.swiftLanguageMode(.v5)]) + ] +) diff --git a/Reproducer/Sources/main.swift b/Reproducer/Sources/main.swift new file mode 100644 index 0000000..89f4fb0 --- /dev/null +++ b/Reproducer/Sources/main.swift @@ -0,0 +1,61 @@ +// What `AsyncMutexPerformanceTests`' actor cases do, with XCTest and the +// package taken away: detached tasks take turns on one actor, yielding after +// each turn, while the main thread waits for them on a semaphore. +// +// On Linux arm64 a libdispatch worker dies about once in a hundred of these: +// +// *** Program crashed: Bad pointer dereference at 0xffffffff89abcdff *** +// 0 _dispatch_worker_thread + 540 in libdispatch.so +// +// which is DISPATCH_OBJECT_LISTLESS + 0x10: the root queue's head is an item +// marked as being on no list. + +import Dispatch + +actor Box { + private let cycle = (0 ..< 1024).map { ($0 &* 5 &+ 1) % 1024 } + private var writes = 0 + + func step(from index: Int) -> Int { + writes &+= 1 + return cycle[index] + } + + var count: Int { + writes + } +} + +func sample(tasks: Int, iterations: Int) { + let box = Box() + let finished = DispatchSemaphore(value: 0) + + for task in 0 ..< tasks { + Task.detached(priority: .userInitiated) { + var index = task + for _ in 0 ..< iterations { + index = await box.step(from: index) + await Task.yield() + } + finished.signal() + } + } + for _ in 0 ..< tasks { + finished.wait() + } + + let checked = DispatchSemaphore(value: 0) + Task.detached { + let writes = await box.count + precondition(writes == tasks * iterations, "the workload did not run") + checked.signal() + } + checked.wait() +} + +// The four cases, ten samples of each, as the measurements take them. +for (tasks, iterations) in [(64, 2_000), (8, 10_000), (1, 100_000), (512, 250)] { + for _ in 0 ..< 10 { + sample(tasks: tasks, iterations: iterations) + } +} diff --git a/StoreOrder/listless-probe.patch b/StoreOrder/listless-probe.patch new file mode 100644 index 0000000..301fb5f --- /dev/null +++ b/StoreOrder/listless-probe.patch @@ -0,0 +1,20 @@ +Puts a barrier right after the LISTLESS store in _dispatch_queue_class_invoke, +one of two kinds, chosen at build time. PROBE_LISTLESS_DMB_ISHST orders the +store before the drain lock's acquire-only compare-and-swap; PROBE_LISTLESS_DMB_ISHLD +sits in the same place and does not. Neither defined, the source is as released. +Applies to swift-corelibs-libdispatch at swift-6.4.0-RELEASE. + +--- a/src/inline_internal.h ++++ b/src/inline_internal.h +@@ -1790,6 +1790,11 @@ _dispatch_queue_class_invoke(dispatch_queue_class_t dqu, + + if (!(flags & (DISPATCH_INVOKE_STEALING | DISPATCH_INVOKE_WLH))) { + dq->do_next = DISPATCH_OBJECT_LISTLESS; ++#if defined(PROBE_LISTLESS_DMB_ISHST) ++ __asm__ __volatile__("dmb ishst" ::: "memory"); ++#elif defined(PROBE_LISTLESS_DMB_ISHLD) ++ __asm__ __volatile__("dmb ishld" ::: "memory"); ++#endif + _dispatch_trace_item_pop(_dispatch_queue_get_current(), dq); + } + flags |= const_restrict_flags; diff --git a/StoreOrder/litmus.c b/StoreOrder/litmus.c new file mode 100644 index 0000000..1e6531d --- /dev/null +++ b/StoreOrder/litmus.c @@ -0,0 +1,208 @@ +// Whether a plain store can become visible after a later acquire-only +// compare-and-swap, in the shape libdispatch gives it when a worker takes a +// lane off a root queue and finds it cannot run it: +// +// invoker pusher +// x = LISTLESS (plain str) spin until y == 0 +// CAS y: 1 -> 0 (acquire) CAS y: 0 -> 1 (release) +// x = NULL (plain str) +// +// The pusher writes x only after it has seen the invoker's CAS, and the +// invoker wrote x before that CAS. If x ends as LISTLESS, the invoker's store +// reached memory after the pusher's: the reordering in question. +// +// Usage: litmus +// order: acq CAS with acquire only, as libdispatch has it +// acqrel CAS with acquire and release (control, must be 0) +// ishst acquire CAS after `dmb ishst` (control, must be 0) +// layout: same x and y in one cache line, at a lane's offsets +// split x and y in two cache lines, the pusher reading both + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SLOTS (1u << 14) +#define LISTLESS 1 +#define CLEARED 2 + +typedef struct { + _Alignas(128) uint64_t line0[16]; + uint64_t line1[16]; +} slot_t; + +enum order { ORDER_ACQ, ORDER_ACQREL, ORDER_ISHST }; + +typedef struct { + slot_t *slots; + enum order order; + bool split; + int cpu[2]; + _Alignas(128) volatile uint64_t batch; + _Alignas(128) volatile uint64_t invoker_done; + _Alignas(128) volatile uint64_t pusher_done; + _Alignas(128) volatile bool stop; + uint64_t trials, observed; +} pair_t; + +static uint64_t *x_of(pair_t *p, uint32_t i) { + return p->split ? &p->slots[i].line0[0] : &p->slots[i].line0[2]; // +0x10 +} +static uint64_t *y_of(pair_t *p, uint32_t i) { + return p->split ? &p->slots[i].line1[0] : &p->slots[i].line0[7]; // +0x38 +} +static uint64_t *done_of(pair_t *p, uint32_t i) { + return &p->slots[i].line1[8]; +} + +static void pin(int cpu) { + cpu_set_t set; + CPU_ZERO(&set); + CPU_SET(cpu, &set); + if (pthread_setaffinity_np(pthread_self(), sizeof set, &set) != 0) { + fprintf(stderr, "cannot pin to cpu %d\n", cpu); + exit(2); + } +} + +static void reset(pair_t *p) { + for (uint32_t i = 0; i < SLOTS; i++) { + memset(&p->slots[i], 0, sizeof p->slots[i]); + *y_of(p, i) = 1; + } + __atomic_thread_fence(__ATOMIC_SEQ_CST); +} + +static void *invoker(void *arg) { + pair_t *p = arg; + pin(p->cpu[0]); + for (uint64_t b = 1;; b++) { + while (p->batch < b) { + if (p->stop) return NULL; + } + for (uint32_t i = 0; i < SLOTS; i++) { + // Keep the pusher close behind: it is spinning on this slot's y + // by the time x is written. + if (i > 0) { + while (__atomic_load_n(done_of(p, i - 1), __ATOMIC_ACQUIRE) == 0) {} + } + uint64_t expected = 1; + __atomic_store_n(x_of(p, i), LISTLESS, __ATOMIC_RELAXED); + switch (p->order) { + case ORDER_ACQ: + __atomic_compare_exchange_n(y_of(p, i), &expected, 0, false, + __ATOMIC_ACQUIRE, __ATOMIC_RELAXED); + break; + case ORDER_ACQREL: + __atomic_compare_exchange_n(y_of(p, i), &expected, 0, false, + __ATOMIC_ACQ_REL, __ATOMIC_ACQUIRE); + break; + case ORDER_ISHST: + __asm__ __volatile__("dmb ishst" ::: "memory"); + __atomic_compare_exchange_n(y_of(p, i), &expected, 0, false, + __ATOMIC_ACQUIRE, __ATOMIC_RELAXED); + break; + } + } + __atomic_store_n(&p->invoker_done, b, __ATOMIC_RELEASE); + } +} + +static void *pusher(void *arg) { + pair_t *p = arg; + pin(p->cpu[1]); + for (uint64_t b = 1;; b++) { + while (p->batch < b) { + if (p->stop) return NULL; + } + for (uint32_t i = 0; i < SLOTS; i++) { + uint64_t *x = x_of(p, i), *y = y_of(p, i); + while (__atomic_load_n(y, __ATOMIC_RELAXED) != 0) { + // In the split layout, keep x's line shared here too, as the + // other threads touching a lane keep it. + if (p->split) (void)__atomic_load_n(x, __ATOMIC_RELAXED); + } + uint64_t expected = 0; + if (__atomic_compare_exchange_n(y, &expected, 1, false, + __ATOMIC_RELEASE, __ATOMIC_RELAXED)) { + __atomic_store_n(x, CLEARED, __ATOMIC_RELAXED); + } + __atomic_store_n(done_of(p, i), 1, __ATOMIC_RELEASE); + } + __atomic_store_n(&p->pusher_done, b, __ATOMIC_RELEASE); + } +} + +static double now(void) { + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + return ts.tv_sec + ts.tv_nsec / 1e9; +} + +int main(int argc, char **argv) { + if (argc != 4) { + fprintf(stderr, "usage: %s acq|acqrel|ishst same|split seconds\n", argv[0]); + return 2; + } + enum order order; + if (!strcmp(argv[1], "acq")) order = ORDER_ACQ; + else if (!strcmp(argv[1], "acqrel")) order = ORDER_ACQREL; + else if (!strcmp(argv[1], "ishst")) order = ORDER_ISHST; + else return 2; + bool split = !strcmp(argv[2], "split"); + double seconds = atof(argv[3]); + + // One pair per two CPUs, all running at once. + int cpus = (int)sysconf(_SC_NPROCESSORS_ONLN); + int pairs = cpus / 2; + pair_t *pair = aligned_alloc(128, sizeof(pair_t) * pairs); + pthread_t threads[2 * pairs]; + for (int k = 0; k < pairs; k++) { + memset(&pair[k], 0, sizeof pair[k]); + pair[k].slots = aligned_alloc(128, sizeof(slot_t) * SLOTS); + pair[k].order = order; + pair[k].split = split; + pair[k].cpu[0] = 2 * k; + pair[k].cpu[1] = 2 * k + 1; + reset(&pair[k]); + pthread_create(&threads[2 * k], NULL, invoker, &pair[k]); + pthread_create(&threads[2 * k + 1], NULL, pusher, &pair[k]); + } + + double start = now(); + for (uint64_t b = 1; now() - start < seconds; b++) { + for (int k = 0; k < pairs; k++) __atomic_store_n(&pair[k].batch, b, __ATOMIC_RELEASE); + for (int k = 0; k < pairs; k++) { + while (__atomic_load_n(&pair[k].invoker_done, __ATOMIC_ACQUIRE) < b || + __atomic_load_n(&pair[k].pusher_done, __ATOMIC_ACQUIRE) < b) {} + for (uint32_t i = 0; i < SLOTS; i++) { + uint64_t x = *x_of(&pair[k], i); + if (x == LISTLESS) pair[k].observed++; + else if (x != CLEARED) { + fprintf(stderr, "slot %u ended as %llu\n", i, (unsigned long long)x); + return 3; + } + } + pair[k].trials += SLOTS; + reset(&pair[k]); + } + } + uint64_t trials = 0, observed = 0; + for (int k = 0; k < pairs; k++) { + pair[k].stop = true; + trials += pair[k].trials; + observed += pair[k].observed; + printf("pair %d (cpu %d,%d): %llu of %llu\n", k, pair[k].cpu[0], pair[k].cpu[1], + (unsigned long long)pair[k].observed, (unsigned long long)pair[k].trials); + } + printf("%s %s: x ended as LISTLESS in %llu of %llu trials in %.0fs\n", + argv[1], argv[2], (unsigned long long)observed, (unsigned long long)trials, now() - start); + return 0; +}