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

Cast STORE64H/STORE64L argument to ulong64 #537

Closed
wants to merge 1 commit into from
Closed

Conversation

mkj
Copy link
Collaborator

@mkj mkj commented May 29, 2020

Avoids undefined behaviour with right shift greater than 32 bits. (c99 6.5.7 "If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined."

I haven't reproduced it here but it was hit using STORE64H outside of libtomcrypt on BCM4706 MIPS32r2 with GCC 4.2.4 mkj/dropbear#99

The patch doesn't seem to affect code generation with -DLTC_NO_ASM on x64.

Avoids undefined behaviour with right shift greater than 32 bits
@sjaeckel
Copy link
Member

sjaeckel commented Sep 6, 2020

IMO this cast belongs in the calling code and not in the STORE macro.

Those macro's clearly say that they store a 64bit Variable and if you pass a 32bit in there you have to cast it.

@karel-m your opinion?

@karel-m
Copy link
Member

karel-m commented Apr 11, 2021

I do not have strong opinion on this. But I slightly lean towards leaving the cast to the calling code as by casting in these macros we may hide some potential issues on the calling side (which now throws at least some warning).

@sjaeckel
Copy link
Member

sjaeckel commented Mar 5, 2024

Closing this, since @karel-m and me basically have the same opinion on this.

Please re-open if you have further questions/comments.

@sjaeckel sjaeckel closed this Mar 5, 2024
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.

None yet

3 participants