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

UB in negate in mulh/mulhsu #1546

Merged
merged 1 commit into from
Jan 18, 2025

Conversation

nibrunieAtSi5
Copy link
Contributor

@nibrunieAtSi5 nibrunieAtSi5 commented Dec 21, 2023

follow-up to #1539.

Ensuring No negation on -2^63(int64_t) is performed during mulh/mulhsu

riscv/arith.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

Ping me again when it’s ready to merge.

@aswaterman
Copy link
Collaborator

@nibrunieAtSi5 Returning to this eight months later, can you rebase onto master and then squash the commits together?

@nibrunie nibrunie force-pushed the patch-2 branch 2 times, most recently from 6ddf910 to fefc853 Compare August 11, 2024 09:44
@nibrunie
Copy link

@jerryz123 could you review this PR since you reviewed #1539 ?

compiler link to reproduce the possible UB before this PR: https://godbolt.org/z/bn9xh3G1s (using -fsanitize=undefined)
issue disappear after the change: https://godbolt.org/z/svWMn9bn5

@nibrunieAtSi5 nibrunieAtSi5 marked this pull request as ready for review January 18, 2025 17:39
Ensuring No negation on -2^63(int64_t) is performed during mulh/mulhsu

Signed-off-by: Nicolas Brunie <82109999+nibrunieAtSi5@users.noreply.github.com>

Update riscv/arith.h

Signed-off-by: Nicolas Brunie <82109999+nibrunieAtSi5@users.noreply.github.com>
Copy link
Collaborator

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

I think you need to change it from Draft to Ready (I can't do this from my phone).

Edit: nvm

@jerryz123 jerryz123 merged commit 58da6a2 into riscv-software-src:master Jan 18, 2025
3 checks passed
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