Skip to content

[TEST] Add ASan/LeakSanitizer harness and fix memory bugs it found - #7831

Open
Niz13 wants to merge 1 commit into
mainfrom
niz/asan-leak-check-5029
Open

[TEST] Add ASan/LeakSanitizer harness and fix memory bugs it found#7831
Niz13 wants to merge 1 commit into
mainfrom
niz/asan-leak-check-5029

Conversation

@Niz13

@Niz13 Niz13 commented Aug 24, 2026

Copy link
Copy Markdown

Implements the host-side memory-leak check requested in #5029.

Harness

  • scripts/test-triton.sh --asan runs the host-side C++ (ctest) and MLIR lit suites under LeakSanitizer. Sets ASAN_OPTIONS=allow_user_poisoning=0 to neutralize a false use-after-poison caused by linking ASan-instrumented Triton against the non-instrumented prebuilt LLVM (does not weaken leak/redzone detection), and guards that triton-opt is actually ASan-instrumented.
  • scripts/asan/lsan.supp suppresses only benign LLVM/MLIR globals — nothing under third_party/intel.

Bugs found & fixed

  1. Heap-use-after-free in StrideVersioning — the pass called forOp.erase() inside a pre-order walk(), so the walker then dereferenced the freed op. Fixed with collect-then-mutate: gather candidate loops during the walk, version them afterwards (safe — only non-nested top-level loops are candidates).
  2. Six orphaned-ModuleOp leaks in Analysis unit-test fixtures — ModuleOp::create() returned raw and was never freed. Fixed by owning modules in a SmallVector<OwningOpRef<ModuleOp>> member declared after the MLIRContext.

Result (under LeakSanitizer)

Phase Before After
ctest 116 failing (leaks) 453/453, 0 leaks
lit 1 failing (UAF) 446/446, 0 UAF

🤖 Generated with Claude Code

Implements the host-side memory-leak check requested in issue #5029.

- scripts/test-triton.sh gains `--asan`: runs the host-side ctest and
  lit suites under LeakSanitizer. Sets allow_user_poisoning=0 to
  neutralize the false use-after-poison from linking instrumented
  Triton against the non-instrumented prebuilt LLVM, guards that
  triton-opt is ASan-instrumented, and wires in the suppressions file.
- scripts/asan/lsan.supp suppresses only benign LLVM/MLIR globals
  (never anything under third_party/intel).

Fixes found by the harness:
- StrideVersioning erased a loop from inside a pre-order walk(), so the
  walker dereferenced the freed op (heap-use-after-free). Switch to
  collect-then-mutate: gather candidate loops during the walk, version
  them afterwards.
- Six Analysis unit-test fixtures returned a raw ModuleOp from
  ModuleOp::create() and never freed it. Own each module in a
  SmallVector<OwningOpRef<ModuleOp>> member declared after the
  MLIRContext.

Host-side ctest (453) and lit (446) run clean under LeakSanitizer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
// ModuleOp::create() are owned by nothing, so without this they leak (caught
// by LeakSanitizer). Declared after `ctx` so the modules are erased before
// the context is destroyed.
SmallVector<OwningOpRef<ModuleOp>> modules;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just change methods/functions signature to return OwningOpRef<ModuleOp> (like in lib\Dialect\TritonGPU\Transforms\WarpSpecialization\OptimizePartitionWarps.cpp) and avoid using separate variables to manage ModuleOp's lifecycle?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants