fjpanag commented on code in PR #7285:
URL: https://github.com/apache/incubator-nuttx/pull/7285#discussion_r994365145


##########
mm/ubsan/ubsan.c:
##########
@@ -158,9 +158,11 @@ static int64_t get_signed_val(FAR struct type_descriptor 
*type,
   if (is_inline_int(type))
     {
       unsigned extra_bits = sizeof(int64_t) * 8 - type_bit_width(type);
-      uintptr_t ulong_val = (uintptr_t)val;
+      uint64_t mask = (1llu << extra_bits) - 1;
+      uint64_t ret = (uint64_t)val & mask;
 
-      return ((int64_t)ulong_val) << extra_bits >> extra_bits;
+      return (int64_t)(((ret & (1llu << (extra_bits - 1))) != 0) ?
+             ret | ~mask : ret);

Review Comment:
   @pkarashchenko  I tried your new code, but in practice I don't see any 
difference. Both yield correct results.
   
   As I see, no matter the original type of the value, it is always encoded as 
a 64-bit value.  
   For example, consider the following:
   
   ```
   int16_t a = -1;
   int16_t b = a << 2;
   ```
   
   The above will trigger UBSan, but the variable `val` in function 
`get_signed_val()` will contain the value `0xFFFFFFFFFFFFFFFF`.
   
   *(As said, I can only test on simulator / 64-bit)*
   
   I trust you on this, and I will create a new PR with the suggested fix, but 
in practice both work, at least for me.
   



-- 
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

Reply via email to