Skip to content

Fix use-after-free in SortDimensionsMemoryLayout rewrite patterns#2307

Merged
ppetrovi-amd merged 14 commits intodevelopfrom
fix/airadsw-62-use-after-free-sort-dimensions
Apr 13, 2026
Merged

Fix use-after-free in SortDimensionsMemoryLayout rewrite patterns#2307
ppetrovi-amd merged 14 commits intodevelopfrom
fix/airadsw-62-use-after-free-sort-dimensions

Conversation

@ppetrovi-amd
Copy link
Copy Markdown
Contributor

@ppetrovi-amd ppetrovi-amd commented Mar 19, 2026

Motivation

Fix a use-after-free bug in GemmRewritePattern::matchAndRewrite in SortDimensionsMemoryLayout.cpp that causes an invalid pointer read crash during MLIR compilation. Under normal conditions the bug is masked because the freed memory still contains valid-looking data, but with heap verification tools enabled the invalid access is detected and crashes the process. The same use-after-free pattern was also present in ConvRewritePattern::matchAndRewrite and is fixed in this PR as well.

Technical Details

GemmRewritePattern::matchAndRewrite accessed the perf_config attribute in op after replaceOpWithNewOp had already erased it. ConvRewritePattern::matchAndRewrite had the same issue with both perf_config and output_layout.

Test Plan

Existing check-rocmlir tests should cover this pass, including mlir/test/Dialect/Rock/lowering_sort_dimensions_memory_layout.mlir

Test Result

Submission Checklist

@ppetrovi-amd ppetrovi-amd requested a review from causten as a code owner March 19, 2026 17:08
@ppetrovi-amd ppetrovi-amd force-pushed the fix/airadsw-62-use-after-free-sort-dimensions branch from ad9c3ac to 584637c Compare March 20, 2026 19:59
// in this example means that half of the memory is uninitialized. We expect
// at least 2304/4608 correct; uninitialized positions may also coincidentally
// match, so the count can be slightly higher (at least 50% correct is expected).
// CHECK: relDiff = 0 : {{[0-9]+}}/4608 ({{[5-9][0-9]}}.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FYI : @Mr-Anyone can you also update your test to match this ? I've seen it failing because it matched more than 50% elements.

@ppetrovi-amd ppetrovi-amd force-pushed the fix/airadsw-62-use-after-free-sort-dimensions branch from 67f1abc to 9e51a06 Compare March 25, 2026 09:58
@ppetrovi-amd ppetrovi-amd force-pushed the fix/airadsw-62-use-after-free-sort-dimensions branch from 9e51a06 to 1b92fe6 Compare March 25, 2026 12:01
* Save perf_config and output_layout attributes before replaceOpWithNewOp
  erases the original op in ConvRewritePattern and GemmRewritePattern.
* Fix CHECK in mixr-non-contiguous-strides.mlir to accept any count
  >= 50% since uninitialized memory positions may coincidentally match.
@ppetrovi-amd ppetrovi-amd force-pushed the fix/airadsw-62-use-after-free-sort-dimensions branch from 1b92fe6 to 64d5d94 Compare March 26, 2026 14:35
@ppetrovi-amd ppetrovi-amd merged commit b7bde90 into develop Apr 13, 2026
8 of 15 checks passed
@ppetrovi-amd ppetrovi-amd deleted the fix/airadsw-62-use-after-free-sort-dimensions branch April 13, 2026 22:07
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.

4 participants