In case of MIPS, we never set breakpoint in the delay slot. Hence while doing stepping, the delay slot instruction will be skipped.
Jim, can you take a look at this and see if this could be fixed in a nicer way? I would prefer to not see anything related to delay slots in a test. Can we abstract this better? Maybe ask the SBInstruction() if you can set a breakpoint on it instead of speaking in delay slot terms? Or maybe we ask the SBInstructionList to count the number of instructions between two addresses? Maybe something like:
uint32_t SBInstructionList::GetInstructionCount(const SBAddress &start, const SBAddress &end);
Or improve the test so it doesn't rely on instruction count??
The substance seems fine.
I'm not sure I would guess what GetInstructionsCount with canSetBreakpoint == true would do without reading the code. You could fix this with a more explicit parameter name, but I can't think of a good name that's short enough not to be awful, something like: excludeInstructionsWhereYouCantSetBreakpoints would get it but yuck... Better to just add a header comment explaining what that parameter actually does.
It's also odd to have the SBInstructionList hold the logic determining whether you can set a breakpoint on an instruction. That should really be in SBInstruction, or even better should be in the lldb_private::Instruction class, since this seems like a generally useful bit of knowledge, and then routed through SBInstruction.
Couple tiny tweaks to the comment, and this is good to go.
|36–38 ↗||(On Diff #96891)|
Also grammar. Like:
Returns the number of instructions between the start and end address. If canSetBreakpoint is true then the count will be the number of instructions on which a breakpoint can be set.