FreeBSD ARM support for software single step.

[dmikulin]

Summary

Implementation of software single step for FreeBSD on ARM.
The code is largely based on the Linux implementation of the same functionality.

I ran lldb test on an rpi2 board running FreeBSD 11.RC3. With the change the pass rate improves from the current 174
tests to 419. I used amd64 as a reference architecture where 517 tests pass on FreeBSD.

This is my first attempt to submit a patch for lldb, so I may need some guidance to work through the process.
How do reviewers get assigned for code reviews?

[dmikulin]

Fixed indentation.

[labath]

I'd love to see freebsd switch to lldb-server based debugging. Then we could make sure this code is actually reused instead of copied around (e.g. we are right now preparing a patch to improve this, which you could have gotten for free if the code was shared). </drive-by>

[dmikulin]

Are you saying someone is working on switching FreeBSD to lldb-server debugging? Can I get my hands on this patch?

[labath]

No, I'm saying someone *should*. :P

Ed looked into that at some point but, I don't think get got too far with it. Adding @emaste, who should probably review this.

[emaste]

In D25756#574258, @labath wrote:

No, I'm saying someone *should*. :P

Ed looked into that at some point but, I don't think get got too far with it. Adding @emaste, who should probably review this.

Yes, it needs to be done. Maybe now that NetBSD support is progressing a few of us can work together to make it happen.

[emaste]

On a quick look this seems OK. I'll try to test/review in detail.

source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp

925 ↗

(On Diff #75090)

typo here

[dmikulin]

I'm not familiar enough with the code yet to pick up the work on moving FreeBSD to lldb-server. Any pointers to revision numbers, internal discussions, etc, related to the Linux lldb-server switch? Also, if there's any unfinished FreeBSD work, it would help me get started. Thanks!

[labath]

I don't know of any good source of documentation for this, but I am willing to answer any questions you might have about this. Basically, the idea of lldb-server is that you use the same code path for remote debugging as you to for local. So all the debugging is done in a separate process called lldb-server, with which the client then communicates when it needs something done.

If we want to get lldb-server working on freebsd, I would propose a plan like this:

• rename NativeProcessLinux into something more generic (NativeProcessPosix ?), make a new NativeProcessLinux, which inherits from that, but contains no code.
• get NativeProcessPosix building on FreeBSD. Now, you will likely get compile errors as we may be using some OS interfaces that don't exist on freebsd. We will need to figure out how we can either: a) use interfaces which are available on both platforms; b) abstract the functionality so it can implemented in the derived classes. The biggest offender here will be ptrace (at one point we used signalfd(), but that is gone now), as it operates a bit differently on the two OSs. For the implementation of the FreeBSD part of it, you can use the existing ProcessFreeBSD classes for inspiration. All the other
• get lldb-server test suite passing on FreeBSD (these are all the tests in tools/lldb-server), which test pure server functionality)
• switch lldb client to use lldb-server instead of ProcessFreeBSD
• get the rest of lldb tests passing
• remove ProcessFreeBSD

When we were getting lldb-server working for linux, it really helped having osx as a reference, because it already used the same flow. I imagine that it could be quite helpful for you as well, if you could get a linux machine/VM to be able to compare how the code behaves on two platforms, but I think it should be possible to do it without it.

So, if you want to get started, I propose you do a preliminary invetigation -- set the build system so that in builds NativeProcessLinux on FreeBSD, and see how much code do you have to delete, so that it compiles. This should give us an idea of the scope of the work.

good luck :)

[dmikulin]

Thanks Pavel! I'll start working on it. Do you know when lldb-server Linux changes were committed? I want to use those patches as a template, but it's hard to locate them digging through thousands of commit log entries...

Ed, you mentioned NetBSD work. Do you know where they are in their implementation? Anything I can do to help? Or should I start from scratch?

[emaste]

On a quick look this seems OK. I'll try to test/review in detail.

In D25756#576642, @dmikulin wrote:

Thanks Pavel! I'll start working on it. Do you know when lldb-server Linux changes were committed? I want to use those patches as a template, but it's hard to locate them digging through thousands of commit log entries...

I recalled discussing this when the Linux changes were going in and wanted to point you at a mailing list archive, but it seems it was only in private mail. I'll try to send you the relevant threads.

Have a look at rL212069 for starters.

Ed, you mentioned NetBSD work. Do you know where they are in their implementation? Anything I can do to help? Or should I start from scratch?

I hope @krytarowski can comment on that.

It's probably best to move this discussion to a new thread on lldb-dev.

[krytarowski]

NetBSD is still focused on amd64-only port.

I start a funded project November 1st on getting x86 process plugin to work. More details here:

http://blog.netbsd.org/tnf/entry/funded_contract_2016_2017

Estimated time is 4 months full-time.

However there every help is appreciated and donations to TNF (The NetBSD Foundation) make it possible.

[krytarowski]

@emaste What's the status of remote debugging on FreeBSD?

[krytarowski]

I just scrolled the discussion so it's not really started. Thanks.

My plan is to work on ptrace(2) for about a month.

Then I will take the FreeBSD code as it is and port to NetBSD trying to make it functional. This is difficult part as there are everywhere OS specific interfaces and behavior. I hope to get it to some usable point after a month.

Then ,I will start working on remote debugging, as my previous patch for process tracing (it was buggy) was rejected without this property. I planned to merge my previous work with master as I was forced to keep catching up after the project changes and it was time expensive, and move my focus on proper platform support.

So far all my efforts were voluntary and NetBSD build bot is paid from my private resources. Now I will be able to spend on in full-time about 4 months.

So to make it short, hopefully I can join you in remote capabilities after New Year (according to my estimations).

[emaste]

source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp

551–557 ↗

(On Diff #75090)

according to LLVM style this should be promoted out of an else block because of the return above (use early exits / don't use else after a return).

As an aside it appears 0xe7fddefe is ARM-recommended breakpoint opcode and Linux is the outlier. So the generic Platform::GetSoftwareBreakpointTrapOpcode ought to use 0xFE,0xDE,0xFF,0xE7, with PlatformLinux::GetSoftwareBreakpointTrapOpcode having the override?

source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp

957 ↗

(On Diff #75090)

Few typo / nits:

only fills in
s/regsiter/register/
in some cases

1063 ↗

(On Diff #75090)

We should really be returning an Error from this fn instead, no?

1101–1104 ↗

(On Diff #75090)

Hope you don't mind a minor rewording of the comment:

The emulated instruction failed and it did not change the PC. Advance the PC by the size of the current opcode, as all PC-modifying instructions should be successfully emulated. The failure was most likely caused by an unsupported instruction.

[dmikulin]

Addressed review comments.
Haven't had a chance to re-test it on an arm board.

[dmikulin]

@emaste , any chance to get this committed soon?

lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp

if (bp_loc_sp) {
         addr_class = eAddressClassCodeAlternateISA;
     }
 
     if (addr_class == eAddressClassCodeAlternateISA) {
       // TODO: Enable when FreeBSD supports thumb breakpoints.
       // FreeBSD kernel as of 10.x, does not support thumb breakpoints
       return 0;
     }
+
+    static const uint8_t g_arm_breakpoint_opcode[] = {0xFE, 0xDE, 0xFF, 0xE7};
+    size_t trap_opcode_size = sizeof(g_arm_breakpoint_opcode);
+    assert(bp_site);
+    if (bp_site->SetTrapOpcode(g_arm_breakpoint_opcode, trap_opcode_size))
+      return trap_opcode_size;
   }
     LLVM_FALLTHROUGH;
   default:
     return Platform::GetSoftwareBreakpointTrapOpcode(target, bp_site);
   }
 }
 
 void PlatformFreeBSD::CalculateTrapHandlerSymbolNames() {

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h

public:
   /// for.
   bool AddThreadForInitialStopIfNeeded(lldb::tid_t stop_tid);
 
   bool WaitingForInitialStop(lldb::tid_t stop_tid);
 
   virtual FreeBSDThread *CreateNewFreeBSDThread(lldb_private::Process &process,
                                                 lldb::tid_t tid);
 
+  static bool SingleStepBreakpointHit(
+      void *baton, lldb_private::StoppointCallbackContext *context,
+      lldb::user_id_t break_id, lldb::user_id_t break_loc_id);
+
+  lldb_private::Error SetupSoftwareSingleStepping(lldb::tid_t tid);
+
+  lldb_private::Error SetSoftwareSingleStepBreakpoint(lldb::tid_t tid,
+                                                      lldb::addr_t addr);
+
+  bool IsSoftwareStepBreakpoint(lldb::tid_t tid);
+
+  bool SupportHardwareSingleStepping() const;
+
+  typedef std::vector<lldb::tid_t> tid_collection;
+  tid_collection &GetStepTids() { return m_step_tids; }
+
 protected:
+  static const size_t MAX_TRAP_OPCODE_SIZE = 8;
+
   /// Target byte order.
   lldb::ByteOrder m_byte_order;
 
   /// Process monitor;
   ProcessMonitor *m_monitor;
 
   /// The module we are executing.
   lldb_private::Module *m_module;
protected:
 
   typedef std::set<lldb::tid_t> ThreadStopSet;
   /// Every thread begins with a stop signal. This keeps track
   /// of the threads for which we have received the stop signal.
   ThreadStopSet m_seen_initial_stop;
 
   friend class FreeBSDThread;
 
-  typedef std::vector<lldb::tid_t> tid_collection;
   tid_collection m_suspend_tids;
   tid_collection m_run_tids;
   tid_collection m_step_tids;
+  std::map<lldb::tid_t, lldb::break_id_t> m_threads_stepping_with_breakpoint;
 
   int m_resume_signo;
 };
 
 #endif // liblldb_ProcessFreeBSD_H_

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp

 //===-- ProcessFreeBSD.cpp ----------------------------------------*- C++
 //-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C Includes
 #include <errno.h>
 
 // C++ Includes
 #include <mutex>
+#include <unordered_map>
 
 // Other libraries and framework includes
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/State.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/Target.h"

 
 Error ProcessFreeBSD::DoResume() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
 
   SetPrivateState(eStateRunning);
 
   std::lock_guard<std::recursive_mutex> guard(m_thread_list.GetMutex());
   bool do_step = false;
+  bool software_single_step = !SupportHardwareSingleStepping();
 
   for (tid_collection::const_iterator t_pos = m_run_tids.begin(),
                                       t_end = m_run_tids.end();
        t_pos != t_end; ++t_pos) {
     m_monitor->ThreadSuspend(*t_pos, false);
   }
   for (tid_collection::const_iterator t_pos = m_step_tids.begin(),
                                       t_end = m_step_tids.end();
        t_pos != t_end; ++t_pos) {
     m_monitor->ThreadSuspend(*t_pos, false);
     do_step = true;
+    if (software_single_step) {
+      Error error = SetupSoftwareSingleStepping(*t_pos);
+      if (error.Fail())
+        return error;
+    }
   }
   for (tid_collection::const_iterator t_pos = m_suspend_tids.begin(),
                                       t_end = m_suspend_tids.end();
        t_pos != t_end; ++t_pos) {
     m_monitor->ThreadSuspend(*t_pos, true);
     // XXX Cannot PT_CONTINUE properly with suspended threads.
     do_step = true;
   }
 
   if (log)
     log->Printf("process %" PRIu64 " resuming (%s)", GetID(),
                 do_step ? "step" : "continue");
-  if (do_step)
+  if (do_step && !software_single_step)
     m_monitor->SingleStep(GetID(), m_resume_signo);
   else
     m_monitor->Resume(GetID(), m_resume_signo);
 
   return Error();
 }
 
 bool ProcessFreeBSD::UpdateThreadList(ThreadList &old_thread_list,
const DataBufferSP ProcessFreeBSD::GetAuxvData() {
 
   // Somewhat unexpected - the process is not running locally or we don't have a
   // platform.
   assert(
       false &&
       "no platform or not the host - how did we get here with ProcessFreeBSD?");
   return DataBufferSP();
 }
+
+struct EmulatorBaton {
+  ProcessFreeBSD *m_process;
+  RegisterContext *m_reg_context;
+
+  // eRegisterKindDWARF -> RegisterValue
+  std::unordered_map<uint32_t, RegisterValue> m_register_values;
+
+  EmulatorBaton(ProcessFreeBSD *process, RegisterContext *reg_context)
+      : m_process(process), m_reg_context(reg_context) {}
+};
+
+static size_t ReadMemoryCallback(EmulateInstruction *instruction, void *baton,
+                                 const EmulateInstruction::Context &context,
+                                 lldb::addr_t addr, void *dst, size_t length) {
+  EmulatorBaton *emulator_baton = static_cast<EmulatorBaton *>(baton);
+
+  Error error;
+  size_t bytes_read =
+      emulator_baton->m_process->DoReadMemory(addr, dst, length, error);
+  if (!error.Success())
+    bytes_read = 0;
+  return bytes_read;
+}
+
+static bool ReadRegisterCallback(EmulateInstruction *instruction, void *baton,
+                                 const RegisterInfo *reg_info,
+                                 RegisterValue &reg_value) {
+  EmulatorBaton *emulator_baton = static_cast<EmulatorBaton *>(baton);
+
+  auto it = emulator_baton->m_register_values.find(
+      reg_info->kinds[eRegisterKindDWARF]);
+  if (it != emulator_baton->m_register_values.end()) {
+    reg_value = it->second;
+    return true;
+  }
+
+  // The emulator only fills in the dwarf register numbers (and in some cases
+  // the generic register numbers). Get the full register info from the
+  // register context based on the dwarf register numbers.
+  const RegisterInfo *full_reg_info =
+      emulator_baton->m_reg_context->GetRegisterInfo(
+          eRegisterKindDWARF, reg_info->kinds[eRegisterKindDWARF]);
+
+  bool error =
+      emulator_baton->m_reg_context->ReadRegister(full_reg_info, reg_value);
+  return error;
+}
+
+static bool WriteRegisterCallback(EmulateInstruction *instruction, void *baton,
+                                  const EmulateInstruction::Context &context,
+                                  const RegisterInfo *reg_info,
+                                  const RegisterValue &reg_value) {
+  EmulatorBaton *emulator_baton = static_cast<EmulatorBaton *>(baton);
+  emulator_baton->m_register_values[reg_info->kinds[eRegisterKindDWARF]] =
+      reg_value;
+  return true;
+}
+
+static size_t WriteMemoryCallback(EmulateInstruction *instruction, void *baton,
+                                  const EmulateInstruction::Context &context,
+                                  lldb::addr_t addr, const void *dst,
+                                  size_t length) {
+  return length;
+}
+
+bool ProcessFreeBSD::SingleStepBreakpointHit(
+    void *baton, lldb_private::StoppointCallbackContext *context,
+    lldb::user_id_t break_id, lldb::user_id_t break_loc_id) {
+  return false;
+}
+
+Error ProcessFreeBSD::SetSoftwareSingleStepBreakpoint(lldb::tid_t tid,
+                                                      lldb::addr_t addr) {
+  Error error;
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
+  if (log) {
+    log->Printf("ProcessFreeBSD::%s addr = 0x%" PRIx64, __FUNCTION__, addr);
+    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64, __FUNCTION__, addr);
+  }
+
+  // Validate the address.
+  if (addr == LLDB_INVALID_ADDRESS)
+    return Error("ProcessFreeBSD::%s invalid load address specified.",
+                 __FUNCTION__);
+
+  Breakpoint *const sw_step_break =
+      m_process->GetTarget().CreateBreakpoint(addr, true, false).get();
+  sw_step_break->SetCallback(SingleStepBreakpointHit, this, true);
+  sw_step_break->SetBreakpointKind("software-signle-step");
+
+  if (log)
+    log->Printf("ProcessFreeBSD::%s addr = 0x%" PRIx64 " -- SUCCESS",
+                __FUNCTION__, addr);
+
+  m_threads_stepping_with_breakpoint.insert({tid, sw_step_break->GetID()});
+  return Error();
+}
+
+bool ProcessFreeBSD::IsSoftwareStepBreakpoint(lldb::tid_t tid) {
+  ThreadSP thread = GetThreadList().FindThreadByID(tid);
+  if (!thread)
+    return false;
+
+  assert(thread->GetRegisterContext());
+  lldb::addr_t stop_pc = thread->GetRegisterContext()->GetPC();
+
+  const auto &iter = m_threads_stepping_with_breakpoint.find(tid);
+  if (iter == m_threads_stepping_with_breakpoint.end())
+    return false;
+
+  lldb::break_id_t bp_id = iter->second;
+  BreakpointSP bp = GetTarget().GetBreakpointByID(bp_id);
+  if (!bp)
+    return false;
+
+  BreakpointLocationSP bp_loc = bp->FindLocationByAddress(stop_pc);
+  if (!bp_loc)
+    return false;
+
+  GetTarget().RemoveBreakpointByID(bp_id);
+  m_threads_stepping_with_breakpoint.erase(tid);
+  return true;
+}
+
+bool ProcessFreeBSD::SupportHardwareSingleStepping() const {
+  lldb_private::ArchSpec arch = GetTarget().GetArchitecture();
+  if (arch.GetMachine() == llvm::Triple::arm ||
+      arch.GetMachine() == llvm::Triple::mips64 ||
+      arch.GetMachine() == llvm::Triple::mips64el ||
+      arch.GetMachine() == llvm::Triple::mips ||
+      arch.GetMachine() == llvm::Triple::mipsel)
+    return false;
+  return true;
+}
+
+Error ProcessFreeBSD::SetupSoftwareSingleStepping(lldb::tid_t tid) {
+  std::unique_ptr<EmulateInstruction> emulator_ap(
+      EmulateInstruction::FindPlugin(GetTarget().GetArchitecture(),
+                                     eInstructionTypePCModifying, nullptr));
+
+  if (emulator_ap == nullptr)
+    return Error("Instruction emulator not found!");
+
+  FreeBSDThread *thread = static_cast<FreeBSDThread *>(
+      m_thread_list.FindThreadByID(tid, false).get());
+  if (thread == NULL)
+    return Error("Thread not found not found!");
+
+  lldb::RegisterContextSP register_context_sp = thread->GetRegisterContext();
+
+  EmulatorBaton baton(this, register_context_sp.get());
+  emulator_ap->SetBaton(&baton);
+  emulator_ap->SetReadMemCallback(&ReadMemoryCallback);
+  emulator_ap->SetReadRegCallback(&ReadRegisterCallback);
+  emulator_ap->SetWriteMemCallback(&WriteMemoryCallback);
+  emulator_ap->SetWriteRegCallback(&WriteRegisterCallback);
+
+  if (!emulator_ap->ReadInstruction())
+    return Error("Read instruction failed!");
+
+  bool emulation_result =
+      emulator_ap->EvaluateInstruction(eEmulateInstructionOptionAutoAdvancePC);
+  const RegisterInfo *reg_info_pc = register_context_sp->GetRegisterInfo(
+      eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
+  auto pc_it =
+      baton.m_register_values.find(reg_info_pc->kinds[eRegisterKindDWARF]);
+
+  lldb::addr_t next_pc;
+  if (emulation_result) {
+    assert(pc_it != baton.m_register_values.end() &&
+           "Emulation was successful but PC wasn't updated");
+    next_pc = pc_it->second.GetAsUInt64();
+  } else if (pc_it == baton.m_register_values.end()) {
+    // Emulate instruction failed and it haven't changed PC. Advance PC
+    // with the size of the current opcode because the emulation of all
+    // PC modifying instruction should be successful. The failure most
+    // likely caused by a not supported instruction which don't modify PC.
+    next_pc =
+        register_context_sp->GetPC() + emulator_ap->GetOpcode().GetByteSize();
+  } else {
+    // The instruction emulation failed after it modified the PC. It is an
+    // unknown error where we can't continue because the next instruction is
+    // modifying the PC but we don't  know how.
+    return Error("Instruction emulation failed unexpectedly");
+  }
+
+  SetSoftwareSingleStepBreakpoint(tid, next_pc);
+  return Error();
+}

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

if (log)
       log->Printf("ProcessMonitor::%s() received trace event, tid = %" PRIu64
                   "  : si_code = %d",
                   __FUNCTION__, tid, info->si_code);
     message = ProcessMessage::Trace(tid);
     break;
 
   case SI_KERNEL:
   case TRAP_BRKPT:
+    if (monitor->m_process->IsSoftwareStepBreakpoint(tid)) {
+      if (log)
+        log->Printf("ProcessMonitor::%s() received sw single step breakpoint "
+                    "event, tid = %" PRIu64,
+                    __FUNCTION__, tid);
+      message = ProcessMessage::Trace(tid);
+    } else {
       if (log)
         log->Printf(
             "ProcessMonitor::%s() received breakpoint event, tid = %" PRIu64,
             __FUNCTION__, tid);
       message = ProcessMessage::Break(tid);
+    }
     break;
   }
 
   return message;
 }
 
 ProcessMessage ProcessMonitor::MonitorSignal(ProcessMonitor *monitor,
                                              const siginfo_t *info,