Skip to content

Commit 3ca0193

Browse files
[hrx] Retain executable per kernel handle to fix run_chain use-after-free (#3545)
1 parent a5b4788 commit 3ca0193

4 files changed

Lines changed: 112 additions & 4 deletions

File tree

python/utils/hostruntime/hrxruntime/_bindings.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ def decl(fn, restype, argtypes):
347347
_status_t,
348348
[_handle, ctypes.c_char_p, ctypes.POINTER(ctypes.c_uint32)],
349349
)
350+
self.hrx_executable_retain = decl("hrx_executable_retain", None, [_handle])
350351
self.hrx_executable_release = decl("hrx_executable_release", None, [_handle])
351352

352353
# Dispatch / sync

python/utils/hostruntime/hrxruntime/context.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,10 @@ def lookup_export(self, exe, name: str) -> int:
279279
)
280280
return ordv.value
281281

282+
def retain_executable(self, exe):
283+
if exe:
284+
lib.hrx_executable_retain(exe)
285+
282286
def release_executable(self, exe):
283287
if exe:
284288
lib.hrx_executable_release(exe)

python/utils/hostruntime/hrxruntime/hostruntime.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,31 @@ class HRXKernelHandle(KernelHandle):
106106
"""Handle for a loaded HRX executable (one XADX export)."""
107107

108108
def __init__(
109-
self, executable, export_ordinal, kernel_name, xclbin_path, insts_path
109+
self, executable, export_ordinal, kernel_name, xclbin_path, insts_path, ctx=None
110110
):
111111
self.executable = executable
112112
self.export_ordinal = export_ordinal
113113
self.kernel_name = kernel_name
114114
self.xclbin_path = xclbin_path
115115
self.insts_path = insts_path
116+
# Own an independent libhrx reference to the executable. The executable
117+
# cache holds only a single reference and drops it on LRU eviction; a
118+
# live handle (e.g. every step of a batched run_chain, kept in the
119+
# sequence callable for the whole dispatch) must not be left dangling
120+
# when an unrelated load evicts its cache entry. Balanced in __del__.
121+
self._ctx = ctx
122+
if ctx is not None and executable:
123+
ctx.retain_executable(executable)
124+
125+
def __del__(self):
126+
ctx = getattr(self, "_ctx", None)
127+
exe = getattr(self, "executable", None)
128+
if ctx is not None and exe:
129+
try:
130+
ctx.release_executable(exe)
131+
except Exception:
132+
pass
133+
self.executable = None
116134

117135

118136
class HRXKernelResult(KernelResult):
@@ -216,7 +234,9 @@ def load(self, npu_kernel, **kwargs) -> HRXKernelHandle:
216234
xclbin_path, insts_path, kernel_name = self._resolve_kernel(npu_kernel)
217235
exe, ordv = self._build_executable(xclbin_path, insts_path, kernel_name)
218236
self._executables.append(exe)
219-
return HRXKernelHandle(exe, ordv, kernel_name, xclbin_path, insts_path)
237+
return HRXKernelHandle(
238+
exe, ordv, kernel_name, xclbin_path, insts_path, ctx=self._ctx
239+
)
220240

221241
def _prepare_bindings(self, args):
222242
"""Validate/sync a run's args and return its HRX dispatch bindings.
@@ -453,7 +473,9 @@ def load(self, npu_kernel, **kwargs) -> HRXKernelHandle:
453473
if key in self._exe_cache:
454474
self._exe_cache.move_to_end(key)
455475
exe, ordv = self._exe_cache[key]
456-
return HRXKernelHandle(exe, ordv, kernel_name, xclbin_path, insts_path)
476+
return HRXKernelHandle(
477+
exe, ordv, kernel_name, xclbin_path, insts_path, ctx=self._ctx
478+
)
457479

458480
exe, ordv = self._build_executable(xclbin_path, insts_path, kernel_name)
459481

@@ -462,7 +484,9 @@ def load(self, npu_kernel, **kwargs) -> HRXKernelHandle:
462484
self._release_executable(old_exe)
463485
self._exe_cache[key] = (exe, ordv)
464486

465-
return HRXKernelHandle(exe, ordv, kernel_name, xclbin_path, insts_path)
487+
return HRXKernelHandle(
488+
exe, ordv, kernel_name, xclbin_path, insts_path, ctx=self._ctx
489+
)
466490

467491
def cleanup(self) -> None:
468492
"""Release cached executables, then any tracked by the base runtime."""

test/python/npu-hrx/test_chain_hrx.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
)
4040
from aie.iron.controlflow import range_
4141
from aie.utils.npukernel import NPUKernel
42+
from aie.utils.hostruntime.hrxruntime.hostruntime import CachedHRXRuntime
4243

4344
_TILE = 16
4445
_SIZE = 1024
@@ -76,6 +77,38 @@ def add_one(input_buf: In, output_buf: Out, *, N: CompileTime[int]):
7677
return _add_one_design(input_buf, output_buf, N=N)
7778

7879

80+
def _add_two_design(input_buf: In, output_buf: Out, N: CompileTime[int]):
81+
"""Add 2 to every element -- a second, distinct executable (see add_one)."""
82+
tile_ty = np.ndarray[(_TILE,), np.dtype[np.int32]]
83+
tensor_ty = np.ndarray[(N,), np.dtype[np.int32]]
84+
85+
of_in = ObjectFifo(tile_ty, name="in")
86+
of_out = ObjectFifo(tile_ty, name="out")
87+
88+
def core_body(of_in, of_out):
89+
for _ in range_(N // _TILE):
90+
elem_in = of_in.acquire(1)
91+
elem_out = of_out.acquire(1)
92+
for i in range_(_TILE):
93+
elem_out[i] = elem_in[i] + 2
94+
of_in.release(1)
95+
of_out.release(1)
96+
97+
worker = Worker(core_body, fn_args=[of_in.cons(), of_out.prod()])
98+
99+
def sequence(a, b, in_h, out_h):
100+
in_h.fill(a)
101+
out_h.drain(b, wait=True)
102+
103+
rt = Runtime(sequence, [tensor_ty, tensor_ty, of_in.prod(), of_out.cons()])
104+
return Program(iron.get_current_device(), rt, workers=[worker]).resolve_program()
105+
106+
107+
@compileconfig
108+
def add_two(input_buf: In, output_buf: Out, *, N: CompileTime[int]):
109+
return _add_two_design(input_buf, output_buf, N=N)
110+
111+
79112
def _hrx_runtime():
80113
"""The default NPU runtime, which is the HRX runtime under NPU_RUNTIME=hrx.
81114
@@ -143,3 +176,49 @@ def test_deep_chain(hrx_kernel):
143176
for k, st in enumerate(stages):
144177
st.to("cpu")
145178
np.testing.assert_array_equal(st.numpy(), base + (k + 1))
179+
180+
181+
def test_chain_survives_executable_eviction():
182+
"""A run_chain handle must outlive eviction of its executable cache entry.
183+
184+
The executable cache holds a single libhrx reference per executable and
185+
drops it on LRU eviction. A chain keeps every step's handle live for the
186+
whole batched dispatch, so if a later load evicts an earlier step's entry
187+
the handle must still own the executable -- otherwise the dispatch touches a
188+
freed executable (hrx_stream_dispatch: base_executable == NULL). We force
189+
the eviction by shrinking the cache to one entry and loading two distinct
190+
executables, then chaining across both. Without the per-handle retain this
191+
dispatches a freed executable and fails; with it the chain runs correctly.
192+
193+
Uses a fresh ``CachedHRXRuntime`` rather than the process-wide
194+
``DefaultNPURuntime`` singleton so the cache starts empty and eviction is
195+
deterministic (a shared runtime could already hold entries, making the load
196+
a cache hit or evicting an unrelated entry).
197+
"""
198+
rt = CachedHRXRuntime()
199+
200+
xa, ia = add_one.specialize(N=_SIZE).compile()
201+
xb, ib = add_two.specialize(N=_SIZE).compile()
202+
ka = NPUKernel(str(xa), str(ia), kernel_name="MLIR_AIE")
203+
kb = NPUKernel(str(xb), str(ib), kernel_name="MLIR_AIE")
204+
205+
rt._cache_size = 1
206+
try:
207+
h_add1 = rt.load(ka) # cache: {add_one}
208+
h_add2 = rt.load(kb) # size==1 -> evicts + releases add_one's executable
209+
# h_add1 now references an executable the cache no longer keeps alive.
210+
211+
base = np.arange(1, _SIZE + 1, dtype=np.int32)
212+
a1 = iron.tensor(base, dtype=np.int32, device="npu")
213+
c1 = iron.zeros(_SIZE, dtype=np.int32, device="npu")
214+
a2 = iron.tensor(base, dtype=np.int32, device="npu")
215+
c2 = iron.zeros(_SIZE, dtype=np.int32, device="npu")
216+
217+
rt.run_chain([(h_add1, [a1, c1]), (h_add2, [a2, c2])])
218+
219+
c1.to("cpu")
220+
c2.to("cpu")
221+
np.testing.assert_array_equal(c1.numpy(), base + 1)
222+
np.testing.assert_array_equal(c2.numpy(), base + 2)
223+
finally:
224+
rt.cleanup()

0 commit comments

Comments
 (0)