[Lldb-commits] [PATCH] D15708: Advance the return-address breakpoint location to the end of the next source line, or the next branching instruction, when stepping over a func call

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Mon Jan 4 16:32:22 PST 2016


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





More information about the lldb-commits mailing list