fjpanag commented on code in PR #7285:
URL: https://github.com/apache/incubator-nuttx/pull/7285#discussion_r993229457
##########
mm/ubsan/ubsan.c:
##########
@@ -160,7 +160,7 @@ static int64_t get_signed_val(FAR struct type_descriptor
*type,
unsigned extra_bits = sizeof(int64_t) * 8 - type_bit_width(type);
uintptr_t ulong_val = (uintptr_t)val;
- return ((int64_t)ulong_val) << extra_bits >> extra_bits;
+ return (int64_t)(ulong_val << extra_bits >> extra_bits);
Review Comment:
> The signed vs unsigned shift has essential difference in populating a sign
bit, so 0xffffffff << 1 >> 1 (for 32 bits) will be 0x7fffffff for unsigned and
0xffffffff for signed.
This is not true. Shifting negative numbers is undefined. This is why UBSan
complains in the first place.
The change must be kept to ensure that only unsigned (or positive) numbers
are shifted.
One solution that I can think of is to manually move the sign bit to the
correct place.
Another solution would be to get rid of the shifts, and construct a mask to
clear the bits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
