Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Negative TIDs in Trace #1002

Closed
wants to merge 1 commit into from

Conversation

sraikund16
Copy link
Contributor

Summary:
Right now we take the TIDs returned from CUPTI and convert them from int64_t to int32_t which results in negative tids. Because Perfetto uses int32_t in its protobuf files (according to google/perfetto#886) we can't just convert to uint32_t because perfetto will just convert values back into their negative counterparts. To fix this, we convert to only positive values in int32_t by finding the absolute value of the original int32_t representation of the tid. We also make a special case for INT_MIN to map to 0 so that it is apparent how we handle overflow.

We add this sanitization to all tid outputs for hygiene purposes instead of just CUPTI output.

Differential Revision: D64490195

Summary:
Right now we take the TIDs returned from CUPTI and convert them from int64_t to int32_t which results in negative tids. Because Perfetto uses int32_t in its protobuf files (according to google/perfetto#886) we can't just convert to uint32_t because perfetto will just convert values back into their negative counterparts. To fix this, we convert to only positive values in int32_t by finding the absolute value of the original int32_t representation of the tid. We also make a special case for INT_MIN to map to 0 so that it is apparent how we handle overflow.

We add this sanitization to all tid outputs for hygiene purposes instead of just CUPTI output.

Differential Revision: D64490195
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64490195

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in eb1713f.

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.

4 participants