jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
I don't so much mind that you couldn't reuse
AdvanceAddressToNextBranchInstruction, we wouldn't be using the "get the
disassembly" part of it, which is the biggest bit, since that's already done in
GetInstructionsForAddress which also handles the cache of disassembled
instructions. If the disassembly part of this stepping ever shows up as a big
time sink, then we can move the cache of instruction fragments to the process,
and get it from there.
The only bit I'm concerned with is why you needed to call Clear on the
disassembler's instruction list. That seems odd to me.
================
Comment at: include/lldb/Target/Process.h:3182-3183
@@ +3181,4 @@
+ /// problems with the disassembly or getting the instructions,
+ /// The original default_stop_addr will be returned.
+ //------------------------------------------------------------------
+ Address
----------------
"The" should be "the".
================
Comment at: include/lldb/Target/Thread.h:912-913
@@ -902,2 +911,4 @@
+ uint32_t frame_idx,
+ bool private_step_out);
//------------------------------------------------------------------
----------------
Can you call this something that says what it does, like
"continue_to_next_branch"? private_step_out seems too generic.
Also, should this be defaulted to false?
================
Comment at: source/Target/Process.cpp:6578-6579
@@ +6577,4 @@
+ {
+ disassembler_sp->GetInstructionList().Clear(); // ThreadPlanStepRange
claims there is a retain cycle
+ }
+
----------------
I don't understand this comment, and this seems like an odd thing to have to
do. The disassembler is going to get destroyed when we leave this function.
Why should we have to clear its instruction list manually?
Repository:
rL LLVM
http://reviews.llvm.org/D15708
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
