#141 Delay AMO rd write until store succeeds

Overview

Do not commit the AMO destination register before the store succeeds.

AMO_OP wrote rd before confirming that mmu_store() succeeded. If the store raised a fault, such as a COW/page fault, the destination register could be committed even though the guest kernel would handle the fault and retry the same instruction.

This patch delays the rd write until after mmu_store() returns and vm->error is known to be clear.

Problem Statement

Guest GDB can hang for:

gdb -q -batch -ex "set debuginfod enabled off" -ex run --args /bin/true

AI-agent investigation reduced the hang to GDB's default worker-thread path and identified that glibc's malloc at fork unlock path was failing to release its lock:

amoswap.w.rl a5,a5,(a4)

At that point, a4 points at glibc malloc's list_lock, a5 is the value to store (0, meaning unlocked), and the current lock value is 1 (locked). The instruction swaps the value in a5 into the lock and returns the old lock value back in a5.

before: list_lock = 1, a5 = 0

On the broken path, single-stepping showed that a5 still receives the old lock value, but the store did not clear the lock:

broken after: list_lock = 1, a5 = 1
(normal after: list_lock = 0, a5 = 1)

The underlying issue is the AMO implementation:

#define AMO_OP(STORED_EXPR)                                   \
...
set_dest_idx(vm, decoded_rd(decoded), value);
mmu_store(vm, addr, RV_MEM_SW, (STORED_EXPR), false);
if (vm->error)
    return;
...

mmu_store() can fault, for example on a COW/page fault after fork(). Therefore, the old implementation can corrupt that register before the fault is handled

Fix

Compute the store value first, perform the store, check for an error, and only then commit rd:

uint32_t stored = (STORED_EXPR);
mmu_store(vm, addr, RV_MEM_SW, stored, false);
if (vm->error)
    return;
set_dest_idx(vm, decoded_rd(decoded), value);

Testing

scripts/buildroot-enable-gdb.sh is a testing helper that appends the target GDB options needed to Buildroot config.

#!/usr/bin/env bash
set -euo pipefail

CONFIG_FILE="${1:-configs/buildroot.config}"

cat >>"$CONFIG_FILE" <<'EOF'

# Target-side GDB reproduction support.
BR2_TOOLCHAIN_BUILDROOT_CXX=y
BR2_INSTALL_LIBSTDCPP=y
BR2_ENABLE_DEBUG=y
# BR2_STRIP_strip is not set
BR2_PACKAGE_GDB=y
BR2_PACKAGE_GDB_DEBUGGER=y
BR2_PACKAGE_GDB_SERVER=y
BR2_PACKAGE_HOST_GDB=y
EOF

echo "Appended target-side GDB reproduction options to $CONFIG_FILE"

Build a GDB-enabled guest:

chmod +x ./scripts/buildroot-enable-gdb.sh
./scripts/buildroot-enable-gdb.sh
./scripts/build-image.sh --all --no-ext4
./scripts/rootfs_ext4.sh rootfs.cpio ext4.img 256
make semu -j$(nproc)

Boot:

./semu -k Image -c 1 -b minimal.dtb -H -d ext4.img

Inside the guest:

gdb -q -batch -ex "set debuginfod enabled off" -ex run --args /bin/true
echo GDB_TRUE_EXIT:$?

Expected behavior:

Before this patch: hangs
After this patch: exits normally with GDB_TRUE_EXIT:0

---

Summary by cubic

Delay the AMO destination register write until after mmu_store() succeeds to prevent committing rd on a fault. Fixes a guest GDB hang triggered by amoswap.w.rl in glibc’s malloc lock path.

• Bug Fixes
  • In AMO_OP, compute the store value, call mmu_store, check vm->error, then write rd.
  • Prevents rd corruption on COW/page faults, restoring correct swap semantics and normal GDB exit.