fjpanag commented on code in PR #7285:
URL: https://github.com/apache/incubator-nuttx/pull/7285#discussion_r993441377
##########
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:
> I agree that right shift of negative values is undefined by C standard,
but most compilers implement it to be compatible with integer division by 2
I agree that most systems behave in a similar fashion, but *technically* it
is still UB.
It is not a horrible bug in itself, but as UBSan calls itself it causes
infinite recursion, ending up in a system crash.
> Why are you saying that if value is -1 then ulong_val is
0xffffffffffffffff? Are you testing on 32 bit or 64 bit platform?
I mean that for 64 bit platform it should be true, but not for 32 bit
platform
Yes I am running it on a 64-bit system, using the simulator.
Unfortunately, due to the overhead of UBSan and of the test code (that
triggers this UB), I cannot run this on actual hardware.
I am having a look on your solution right now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
