[Lldb-commits] [PATCH] D25926: Don't set a software stepping breakpoint at 0 on arm or mips.

[Pavel Labath via lldb-commits]

labath added a comment.

Could you make two more changes to this patch?



================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:1365
     error = SetSoftwareBreakpoint(next_pc, 4);
-  else {
+    // If setting the breakpoint fails because next_pc is out of
+    // the address space, ignore it and let the debugee segfault.
----------------
You can put the code behind this switch on the architecture below, so we don't duplicate it needlessly -- I don't know of any arch which uses software single stepping and is not arm or mips, so probably the else branch is dead code, but even if such arch existed, this should be correct for it as well. (Also, probably the whole switch should be a bit deduplicated but that's for a different change).


================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:1378
 
   m_threads_stepping_with_breakpoint.insert({thread.GetID(), next_pc});
 
----------------
I don't think the thread should be added to this list if setting the breakpoint failed, as we will then just try to remove (and fail) the non-existing breakpoint in SignalIfAllThreadsStopped().


https://reviews.llvm.org/D25926