[Lldb-commits] [PATCH] D131244: [LLDB] Missing break in a switch statement alters the execution flow.

[Slava Gurevich via Phabricator via lldb-commits]

fixathon marked an inline comment as done.
fixathon added inline comments.


================
Comment at: lldb/source/Plugins/Process/Utility/ARMUtils.h:49
   shift_t = SRType_Invalid;
   return UINT32_MAX;
 }
----------------
DavidSpickett wrote:
> This is now dead code?
This will execute for the default case after its break.


================
Comment at: lldb/source/Plugins/Process/Utility/ARMUtils.h:29
+    // assert(0 && "Invalid shift type");
+    break;
   case 0:
----------------
DavidSpickett wrote:
> Looking at the codepaths and the reason this function exists, it's either called with a 2 bit number which is the "type" field of an instruction, or it's not called because the type is known to be SRType_RRX already.
> 
> In the enum ARM_ShifterType SRType_RRX would be 4, which wouldn't fit the 2 bit expectation.
> 
> So I think we can safely re-enable this assert and move the two unreachable lines up here. I didn't get any test failures doing so and I don't see a codepath that could hit it.
> 
> (but it would be fairly easy for future changes to make this mistake since it's not very obvious from the types, so an assert is good here)
Added the assert back


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131244