search

jerryscript-project / jerryscript

Ultra-lightweight JavaScript engine for the Internet of Things.
https://jerryscript.net
Apache License 2.0
6.92k stars 670 forks source link

Fix runtime error: left shift #4912

Closed batizdaniel closed 2 years ago

batizdaniel commented 2 years ago

This patch fixes #4703

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

dbatyai commented 2 years ago

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 commented 2 years ago

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