Fix use-after-free in SortDimensionsMemoryLayout rewrite patterns#2307
Merged
ppetrovi-amd merged 14 commits intodevelopfrom Apr 13, 2026
Merged
Fix use-after-free in SortDimensionsMemoryLayout rewrite patterns#2307ppetrovi-amd merged 14 commits intodevelopfrom
ppetrovi-amd merged 14 commits intodevelopfrom
Conversation
ad9c3ac to
584637c
Compare
umangyadav
reviewed
Mar 23, 2026
| // 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]}}. |
Member
There was a problem hiding this comment.
FYI : @Mr-Anyone can you also update your test to match this ? I've seen it failing because it matched more than 50% elements.
umangyadav
approved these changes
Mar 23, 2026
67f1abc to
9e51a06
Compare
9e51a06 to
1b92fe6
Compare
Mr-Anyone
approved these changes
Mar 25, 2026
* 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.
1b92fe6 to
64d5d94
Compare
justinrosner
approved these changes
Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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