Skip to content

Fix runtime error: left shift #4912

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

Merged
merged 1 commit into from
Dec 23, 2021

Conversation

batizdaniel
Copy link
Contributor

This patch fixes #4703

JerryScript-DCO-1.0-Signed-off-by: Daniel Batiz daniel.batiz@h-lab.eu

@dbatyai
Copy link
Member

dbatyai commented Dec 22, 2021

I'm not a big fan of doing a clz call for every left shift operation, even if its handled by a builtin. It can have a significant performance overhead.

According to the C99 standard when the left value has an unsigned type the result will always be defined.

If E1 has an unsigned type, the value of the result is E1 × 2^E2, reduced modulo
one more than the maximum value representable in the result type

If the goal is to avoid undefined behavior, we could just cast the left operand to unsigned, and the result would be the same for non-negative values.

For negative values it's not defined what the result should be, neither the C99, not the ecma standard provide anything specific. Looking at implementations however it seems that the same E1 × 2^E2 rule is used for negative values as well.

So if the left value is negative, we could mask the low 31 bits, do an unsigned left shift to avoid undefined behavior, and if the result is non-zero, set the sign bit on the result.

IMO this approach would yield the same results as previously, avoids undefined behavior, and should also be much faster.

@batizdaniel
Copy link
Contributor Author

I was not sure, what is the correct way to fix this. Your approach is much better. Fixed.

This patch fixes jerryscript-project#4703
This patch fixes jerryscript-project#4702

JerryScript-DCO-1.0-Signed-off-by: Daniel Batiz daniel.batiz@h-lab.eu
@rerobika rerobika merged commit a6ab5e9 into jerryscript-project:master Dec 23, 2021
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.

ubsan: jerryscript/jerry-core/vm/vm.c:3731:77: runtime error: left shift of 33554431 by 22 places cannot be represented in type 'int'
3 participants