[Lldb-commits] [PATCH] D130795: [LLDB][NFC][Reliability] Fixes for int overflow and uninitialized state

[Slava Gurevich via Phabricator via lldb-commits]

fixathon marked 2 inline comments as done.
fixathon added inline comments.


================
Comment at: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp:67
+  return static_cast<uint64_t>(size == 8 ? 0x2 : size - 1)
+         << (18 + 4 * wp_index);
 }
----------------
shafik wrote:
> So we know that `wp_index` is never greater than 11?
For this platform, based on the code and the hardcoded constant the range of wp_index is limited to 0-3 (inclusive)


================
Comment at: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp:77
   // wp.: 3333222211110000xxxxxxEE33221100
   return GetBothEnableBitMask(wp_index) | (0xF << (16 + 4 * wp_index));
 }
----------------
shafik wrote:
> You did not update `0xF` to `0xFULL`  like you did the constants above. Was that on purpose?
Not on purpose. It's interesting this instance wasn't flagged by coverity likely because 
max wp_index value is 3 and 0xF << 28 is still within the 32-bit limit of sizeof(**int**)  the default type for 0xF literal. Then, the signed int will get converted to uint64_t for the bitwise-OR. It's a bit iffy due to the signed type, but should work.

Agreed it would  be best be explicit and consistent and mark it as 0xFull. I could include this it into the next batch of reliability diffs next week.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130795/new/

https://reviews.llvm.org/D130795