Skip to content

Fix ASAN error for mempool test#3124

Open
shadidashmiz wants to merge 1 commit intodevelopfrom
mempoolASAN
Open

Fix ASAN error for mempool test#3124
shadidashmiz wants to merge 1 commit intodevelopfrom
mempoolASAN

Conversation

@shadidashmiz
Copy link
Contributor

  • some attributes are 8 bytes but passed pointer was 4 bytes

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

- some attributes are 8 bytes but passed pointer was 4 bytes

Signed-off-by: sdashmiz <shadi.dashmiz@amd.com>
@shadidashmiz shadidashmiz requested a review from a team as a code owner February 6, 2026 16:12
Copilot AI review requested due to automatic review settings February 6, 2026 16:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to fix an AddressSanitizer (ASAN) error in the hipMemPoolSetAttribute performance test by changing the variable type from int to uint64_t. The ASAN error occurred because some attributes require 8-byte values but a 4-byte pointer was being passed.

Changes:

  • Changed the value variable type from int to uint64_t in the MemPoolSetAttributeBenchmark operator

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +35 to 38
uint64_t value{0};

TIMED_SECTION(kTimerTypeCpu) { HIP_CHECK(hipMemPoolSetAttribute(mem_pool, attribute, &value)); }

Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The fix changes the value type to uint64_t, but this is only correct for hipMemPoolAttrReleaseThreshold. The other three attributes being tested (hipMemPoolReuseFollowEventDependencies, hipMemPoolReuseAllowOpportunistic, hipMemPoolReuseAllowInternalDependencies) expect value type int according to the API documentation. This will cause incorrect behavior or potential memory safety issues when testing those attributes. The code should use different variable types based on the attribute being tested, similar to how it's done in hipMemPoolApi.cc lines 71-80 where uint64_t is used for hipMemPoolAttrUsedMemHigh and int is used for hipMemPoolReuseFollowEventDependencies.

Suggested change
uint64_t value{0};
TIMED_SECTION(kTimerTypeCpu) { HIP_CHECK(hipMemPoolSetAttribute(mem_pool, attribute, &value)); }
if (attribute == hipMemPoolAttrReleaseThreshold) {
uint64_t value{0};
TIMED_SECTION(kTimerTypeCpu) {
HIP_CHECK(hipMemPoolSetAttribute(mem_pool, attribute, &value));
}
} else {
int value{0};
TIMED_SECTION(kTimerTypeCpu) {
HIP_CHECK(hipMemPoolSetAttribute(mem_pool, attribute, &value));
}
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant