fix: do not hijack a developer's own GPU debugging session

mfc.sh test and mfc.sh bench are developer commands, not only CI entry points, and the agent was enabled purely on the library being reachable -- so it switched on for local runs on any ROCm machine. mfc.sh run is untouched and unaffected.

Two ways that was wrong, both silent. Setting HSA_TOOLS_LIB behind someone collecting a GPU core dump gives them "Failed to enable debug interface" and no dump, because the agent and ROCr core dumps are mutually exclusive -- the same path an attached rocgdb trips. And the OFFLOAD_TRACK_* values overwrote whatever the caller had chosen.

An explicit setting is now authoritative: the agent is skipped when HSA_TOOLS_LIB or HSA_ENABLE_DEBUG is already set, and the other two are defaults rather than overrides. Same rule in the case-optimization script.

Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy

Author: sbryngelson

.github/scripts/run_case_optimization.sh

@@ -107,9 +107,13 @@ for case in "${benchmarks[@]}"; do
     # address with nothing to act on. Both variables are inert until a fault;
     # the debug agent is what gives CCE a faulting kernel at all, and is set
     # only where its library is actually reachable.
-    export OFFLOAD_TRACK_ALLOCATION_TRACES=true
-    export OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES=8
-    if [ -n "${ROCM_PATH:-}" ] && [ -f "$ROCM_PATH/lib/librocm-debug-agent.so.2" ]; then
+    export OFFLOAD_TRACK_ALLOCATION_TRACES="${OFFLOAD_TRACK_ALLOCATION_TRACES:-true}"
+    export OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES="${OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES:-8}"
+    # Skipped when the caller already chose a tool, or is collecting a GPU core
+    # dump -- the agent is mutually exclusive with one, so loading it anyway
+    # would leave them with no dump and no reason why.
+    if [ -z "${HSA_TOOLS_LIB:-}" ] && [ -z "${HSA_ENABLE_DEBUG:-}" ] \
+       && [ -n "${ROCM_PATH:-}" ] && [ -f "$ROCM_PATH/lib/librocm-debug-agent.so.2" ]; then
         export HSA_TOOLS_LIB=librocm-debug-agent.so.2
     fi
 

toolchain/mfc/gpu_diagnostics.py

@@ -82,15 +82,21 @@ def fault_diagnostic_env(base: dict) -> dict:
     Returns a new dict: these run in worker threads, and mutating a shared
     environment would leak settings into every concurrent case.
     """
-    env = {
-        **base,
+    env = dict(base)
+
+    # Never clobber a setting the caller made. `mfc.sh test` and `mfc.sh bench`
+    # are developer commands, not just CI entry points, so anyone debugging by
+    # hand has to be able to choose their own values and have them survive.
+    defaults = {
         # Says whether the faulting address was ever a real host allocation,
         # separating an overrun of a known array from a wild pointer.
         "OFFLOAD_TRACK_ALLOCATION_TRACES": "true",
         # Host stack traces for the most recent kernel launches. The runtime
         # advertises this itself in the fault message ("0 now, up to 8").
         "OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES": "8",
     }
+    for name, value in defaults.items():
+        env.setdefault(name, value)
 
     # The only thing that gives CCE a faulting kernel. Measured on Frontier
     # under --gpu acc: it prints "Disassembly for function
@@ -122,8 +128,20 @@ def fault_diagnostic_env(base: dict) -> dict:
     # own fault report. Worst realistic case is one working diagnostic replacing
     # another strictly more detailed one; if a real AFAR fault shows otherwise,
     # gate this on the lane.
-    agent = rocm_debug_agent_path()
-    if agent is not None:
+    # Two ways the caller can say "stay out of my way", both of which mean a
+    # human is already debugging this run by hand:
+    #
+    #   HSA_TOOLS_LIB already set -- they chose a tool; do not replace it.
+    #   HSA_ENABLE_DEBUG set      -- they are collecting a GPU core dump, and
+    #                                the agent is mutually exclusive with one.
+    #                                Loading it anyway yields "Failed to enable
+    #                                debug interface" and no dump, with the
+    #                                cause being something the harness did
+    #                                behind them.
+    #
+    # The same reasoning covers an attached rocgdb, which trips the same
+    # already-attached path.
+    if "HSA_TOOLS_LIB" not in env and not env.get("HSA_ENABLE_DEBUG") and rocm_debug_agent_path() is not None:
         env["HSA_TOOLS_LIB"] = ROCM_DEBUG_AGENT
 
     return env

toolchain/mfc/test/test_gpu_fault_diagnostics.py

@@ -504,3 +504,53 @@ def test_a_missing_agent_report_on_a_gpu_fault_is_called_out():
 
     assert "rocm_debug_agent_path() is not None" in src
     assert "format has changed" in src
+
+
+def test_a_core_dump_session_is_not_hijacked(monkeypatch):
+    """`mfc.sh test` is a developer command, not only a CI entry point.
+
+    The debug agent and ROCr core dumps are mutually exclusive -- measured. So
+    setting the agent behind someone who has asked for a dump gives them
+    "Failed to enable debug interface" and no dump, caused by the harness
+    rather than by anything they did.
+    """
+    from mfc.gpu_diagnostics import fault_diagnostic_env
+
+    monkeypatch.setenv("ROCM_PATH", tmp_agent_dir())
+
+    assert "HSA_TOOLS_LIB" in fault_diagnostic_env({})
+    assert "HSA_TOOLS_LIB" not in fault_diagnostic_env({"HSA_ENABLE_DEBUG": "1"})
+
+
+def test_an_explicit_tool_choice_is_not_replaced(monkeypatch):
+    from mfc.gpu_diagnostics import fault_diagnostic_env
+
+    monkeypatch.setenv("ROCM_PATH", tmp_agent_dir())
+
+    env = fault_diagnostic_env({"HSA_TOOLS_LIB": "libmy-own-tool.so"})
+
+    assert env["HSA_TOOLS_LIB"] == "libmy-own-tool.so"
+
+
+def test_explicit_offload_settings_survive():
+    """A developer tuning these by hand must not have them silently reset."""
+    from mfc.gpu_diagnostics import fault_diagnostic_env
+
+    env = fault_diagnostic_env(
+        {
+            "OFFLOAD_TRACK_ALLOCATION_TRACES": "false",
+            "OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES": "2",
+        }
+    )
+
+    assert env["OFFLOAD_TRACK_ALLOCATION_TRACES"] == "false"
+    assert env["OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES"] == "2"
+
+
+def test_the_defaults_still_apply_when_nothing_was_chosen():
+    from mfc.gpu_diagnostics import fault_diagnostic_env
+
+    env = fault_diagnostic_env({})
+
+    assert env["OFFLOAD_TRACK_ALLOCATION_TRACES"] == "true"
+    assert env["OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES"] == "8"