[LLDB][GUI] Add Breakpoints window

[OmarEmaraDev]

Summary

This patch adds a breakpoints window that lists all breakpoints and
breakpoints locations. The window is implemented as a tree, where the
first level is the breakpoints and the second level is breakpoints
locations.

The tree delegate was hardcoded to only draw when there is a process,
which is not necessary for breakpoints, so the relevant logic was
abstracted in the TreeDelegateShouldDraw method.

[OmarEmaraDev]

I faced a bit of difficulty in this patch, so it is not exactly complete. One issue is that breakpoint descriptions can be very long, I tried to implemented multiline items to workaround this, but that didn't work out and was a waste of time. For now, I just tried to keep descriptions as small as possible. One thing I want to try now is to simply add another tree level and add all necessary information there, similar to verbose breakpoint description.
This is also missing the breakpoint operators, like enabling and disabling breakpoints. Since the window doesn't delegate key handing to the items, this will have to be implemented first.

[OmarEmaraDev]

@clayborg What do you think? Should we add another tree level with more details or do something else?

By the way, it would be nice to have D107182 commited because it is needed by the search form.

[clayborg]

lldb/source/Core/IOHandlerCursesGUI.cpp

4360–4362

Remove {} for single statement if statements per LLVM guidelines

4476

I would show the address first, then the dump of the symbol context. You can ask the address from the location to dump itself as a load address and fallback to module + file address. Check out the Address::Dump function:

bool Address::Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, DumpStyle fallback_style, uint32_t addr_byte_size) const;

To get the raw address first in the string, you can first call the dump with:

// Emit the breakpoint location ID
stream.Format("{0} ", breakpoint_location->GetID())
// Emit the the address as a load address if the process is running, or fallback to <module>[<file-addr>] format if no process is running
address.Dump(&stream, process,  DumpStyleLoadAddress, DumpStyleModuleWithFileAddress, process->GetAddressByteSize());

Then you can add a space and then call this again with:

stream.PutChar(' ');
// Dump the resolved address description
address.Dump(&stream, process,  DumpStyleResolvedDescription, DumpStyleInvalid, process->GetAddressByteSize());

I believe that "DumpStyleResolvedDescription" will dump something verify similar to what you are dumping here with the "symbol_context.DumpStopContext(...)" call.

4482

Breakpoint locations have integer IDs. We should show these as the first item in the string.

Take a look at the output of "breakpoint list" or "breakpoint list --verbose" for ideas on how to format the string.

4485

We could allow locations to be expanded and then show the details of the resolved address like in the "breakpoint list --verbose" output:

(lldb) breakpoint list --verbose
Current breakpoints:
1: name = 'main'
    1.1: 
      module = /Users/gclayton/Documents/src/args/build/a.out
      compile unit = main.cpp
      function = main
      location = /Users/gclayton/Documents/src/args/main.cpp:5:3
      address = a.out[0x0000000100003f3a]
      resolved = false
      hardware = false
      hit count = 0

We could make a child for each of the lines in this output. I believe the about output is from calling Address::Dump(...) with a DumpStyle of "DumpStyleDetailedSymbolContext" (see above details on how to call Address::Dump(...)). You can dump this to a "StringStream" and then get the std::string back from it and split the string using newlines if you like that idea. I like the idea of getting full details on a breakpoint location by being able to expand it.

4509

Breakpoints have integer IDs. We should show these as the first item in the string. Not sure on the formatting of this number, maybe just the raw number followed by a space or possibly with square brackets and a space.

Take a look at the output of "breakpoint list" or "breakpoint list --verbose" for ideas on how to format the string.

6769

We would switch this one to 't' for backTrace and then let the breakpoint view take the 'b' key?

6780

switch to 'b' and mentioned above?

[clayborg]

In D107386#2928910, @OmarEmaraDev wrote:

By the way, it would be nice to have D107182 commited because it is needed by the search form.

Just committed it!

[OmarEmaraDev]

@clayborg With you suggestions it currently looks like this. The concern I have is that lines are now 36 characters longer, and some important information are now truncated. The breakpoint and breakpoint locations IDs seems to be just the index of the objects in the tree, are they not? In this case, they seem redundant to me. Similarly, the address takes a lot of space and is included in the next level of details, so it might not be as important to include in this level either.

I am having some difficulties adding the last level of details. My initial approach was to add and compute a StringList containing all the details to the BreakpointLocationTreeDelegate. Then I added a StringTreeDelegate that have the string from the string list in its user data which it then simply draws. The problem, I realized, is that the BreakpointLocationTreeDelegate is shared between all Breakpoint Location items, so the list is overwritten by different elements. What I did was instance multiple BreakpointLocationTreeDelegate for each item, but that tuned more involved that I thought, so I am not sure what the best approach would be now. What do you think?

[jingham]

In D107386#2932037, @OmarEmaraDev wrote:

@clayborg With you suggestions it currently looks like this. The concern I have is that lines are now 36 characters longer, and some important information are now truncated. The breakpoint and breakpoint locations IDs seems to be just the index of the objects in the tree, are they not? In this case, they seem redundant to me.

The breakpoint ID & loc ID are how you would refer to the breakpoint or location in any console commands, and also to go from the breakpoint ID displayed in a stop notification back to the entry in the breakpoints list. Those end up being fairly common operations.

You also can't just count nodes to figure out the breakpoint ID from the tree, since we don't fill in holes in the numbering when you delete a breakpoint.

The same is true of locations. For instance, if you set a breakpoint on "foo" and we find "foo" in library A, then we will add a location N to the breakpoint. Later, if A is unloaded, that location will be deleted, leaving a hole at N in the location numbering.

I don't have a strong opinion about the addresses, but I think you do need the breakpoint & location ID's in the UI somewhere.

Similarly, the address takes a lot of space and is included in the next level of details, so it might not be as important to include in this level either.

I am having some difficulties adding the last level of details. My initial approach was to add and compute a StringList containing all the details to the BreakpointLocationTreeDelegate. Then I added a StringTreeDelegate that have the string from the string list in its user data which it then simply draws. The problem, I realized, is that the BreakpointLocationTreeDelegate is shared between all Breakpoint Location items, so the list is overwritten by different elements. What I did was instance multiple BreakpointLocationTreeDelegate for each item, but that tuned more involved that I thought, so I am not sure what the best approach would be now. What do you think?

[clayborg]

I like the new UI! I think you are right in that we don't need the address on the breakpoint location line if we can expand it and see it in the location children, so lets remove it from there. Other than that it looks good. The only other suggestion I would have is if we should display "1.1" and "1.2" for the breakpoint locations. This will match the output of "breakpoint list" a bit better.

[OmarEmaraDev]

• Address review

[OmarEmaraDev]

@jingham I see. Thanks!

Here is how it looks now:

[clayborg]

Just fix the possible crash when SetText gets a NULL string pointer, and this is good to go.

We also need to get the "is_pad(...)" fix in before this can be submitted right?

lldb/source/Core/IOHandlerCursesGUI.cpp

3975

This will crash if you pass in NULL. Test "text" before assignment and call m_text.clear() if it is NULL

[OmarEmaraDev]

• Fix possible crash in SetText

[OmarEmaraDev]

@clayborg This is unrelated to D107761, so it should be safe to commit.

lldb/source/Core/IOHandlerCursesGUI.cpp

 //===-- IOHandlerCursesGUI.cpp --------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/IOHandlerCursesGUI.h"
 #include "lldb/Host/Config.h"
 
 #if LLDB_ENABLE_CURSES
 #if CURSES_HAVE_NCURSES_CURSES_H
 #include <ncurses/curses.h>
 #include <ncurses/panel.h>
 #else
 #include <curses.h>

 
 #if LLDB_ENABLE_CURSES
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/Target.h"
public:
   virtual ~TreeDelegate() = default;
 
   virtual void TreeDelegateDrawTreeItem(TreeItem &item, Window &window) = 0;
   virtual void TreeDelegateGenerateChildren(TreeItem &item) = 0;
   virtual void TreeDelegateUpdateSelection(TreeItem &root, int &selection_index,
                                            TreeItem *&selected_item) {
     return;
   }
-  virtual bool TreeDelegateItemSelected(
-      TreeItem &item) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem &item) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr<TreeDelegate> TreeDelegateSP;
 
 class TreeItem {
 public:
   TreeItem(TreeItem *parent, TreeDelegate &delegate, bool might_have_children)
       : m_parent(parent), m_delegate(delegate), m_user_data(nullptr),
public:
   void *GetUserData() const { return m_user_data; }
 
   void SetUserData(void *user_data) { m_user_data = user_data; }
 
   uint64_t GetIdentifier() const { return m_identifier; }
 
   void SetIdentifier(uint64_t identifier) { m_identifier = identifier; }
 
+  const std::string &GetText() const { return m_text; }
+
+  void SetText(const char *text) {
+    if (text == nullptr) {
+      m_text.clear();
+      return;
+    }
+    m_text = text;
+  }
+
   void SetMightHaveChildren(bool b) { m_might_have_children = b; }
 
 protected:
   TreeItem *m_parent;
   TreeDelegate &m_delegate;
   void *m_user_data;
   uint64_t m_identifier;
+  std::string m_text;
   int m_row_idx; // Zero based visible row index, -1 if not visible or for the
                  // root item
   std::vector<TreeItem> m_children;
   bool m_might_have_children;
   bool m_is_expanded;
 };
 
 class TreeWindowDelegate : public WindowDelegate {
 public:
   TreeWindowDelegate(Debugger &debugger, const TreeDelegateSP &delegate_sp)
       : m_debugger(debugger), m_delegate_sp(delegate_sp),
         m_root(nullptr, *delegate_sp, true), m_selected_item(nullptr),
         m_num_rows(0), m_selected_row_idx(0), m_first_visible_row(0),
         m_min_x(0), m_min_y(0), m_max_x(0), m_max_y(0) {}
 
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window &window, bool force) override {
-    ExecutionContext exe_ctx(
-        m_debugger.GetCommandInterpreter().GetExecutionContext());
-    Process *process = exe_ctx.GetProcessPtr();
-
-    bool display_content = false;
-    if (process) {
-      StateType state = process->GetState();
-      if (StateIsStoppedState(state, true)) {
-        // We are stopped, so it is ok to
-        display_content = true;
-      } else if (StateIsRunningState(state)) {
-        return true; // Don't do any updating when we are running
-      }
-    }
-
     m_min_x = 2;
     m_min_y = 1;
     m_max_x = window.GetWidth() - 1;
     m_max_y = window.GetHeight() - 1;
 
     window.Erase();
     window.DrawTitleBox(window.GetName());
 
-    if (display_content) {
-      const int num_visible_rows = NumVisibleRows();
-      m_num_rows = 0;
-      m_root.CalculateRowIndexes(m_num_rows);
-      m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
-                                                 m_selected_item);
+    if (!m_delegate_sp->TreeDelegateShouldDraw()) {
+      m_selected_item = nullptr;
+      return true;
+    }
+
 
-      // If we unexpanded while having something selected our total number of
-      // rows is less than the num visible rows, then make sure we show all the
-      // rows by setting the first visible row accordingly.
-      if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-        m_first_visible_row = 0;
 
-      // Make sure the selected row is always visible
-      if (m_selected_row_idx < m_first_visible_row)
-        m_first_visible_row = m_selected_row_idx;
-      else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-        m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
 
-      int row_idx = 0;
-      int num_rows_left = num_visible_rows;
-      m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-                  num_rows_left);
-      // Get the selected row
-      m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-    } else {
-      m_selected_item = nullptr;
-    }
 
     return true; // Drawing handled
   }
 
   const char *WindowDelegateGetHelpText() override {
     return "Thread window keyboard shortcuts:";
   }
 
protected:
   int m_selected_row_idx;
   int m_first_visible_row;
   int m_min_x;
   int m_min_y;
   int m_max_x;
   int m_max_y;
 };
 
+// A tree delegate that just draws the text member of the tree item, it doesn't
+// have any children or actions.
+class TextTreeDelegate : public TreeDelegate {
+public:
+  TextTreeDelegate() : TreeDelegate() {}
+
+  ~TextTreeDelegate() override = default;
+
+  void TreeDelegateDrawTreeItem(TreeItem &item, Window &window) override {
+    window.PutCStringTruncated(1, item.GetText().c_str());
+  }
+
+  void TreeDelegateGenerateChildren(TreeItem &item) override {}
+
+  bool TreeDelegateItemSelected(TreeItem &item) override { return false; }
+};
+
 class FrameTreeDelegate : public TreeDelegate {
 public:
   FrameTreeDelegate() : TreeDelegate() {
     FormatEntity::Parse(
         "frame #${frame.index}: {${function.name}${function.pc-offset}}}",
         m_format);
   }
 
public:
   ~ThreadsTreeDelegate() override = default;
 
   ProcessSP GetProcess() {
     return m_debugger.GetCommandInterpreter()
         .GetExecutionContext()
         .GetProcessSP();
   }
 
+  bool TreeDelegateShouldDraw() override {
+    ProcessSP process = GetProcess();
+    if (!process)
+      return false;
+
+    if (StateIsRunningState(process->GetState()))
+      return false;
+
+    return true;
+  }
+
   void TreeDelegateDrawTreeItem(TreeItem &item, Window &window) override {
     ProcessSP process_sp = GetProcess();
     if (process_sp && process_sp->IsAlive()) {
       StreamString strm;
       ExecutionContext exe_ctx(process_sp);
       if (FormatEntity::Format(m_format, strm, nullptr, &exe_ctx, nullptr,
                                nullptr, false, false)) {
         int right_pad = 1;

 protected:
   std::shared_ptr<ThreadTreeDelegate> m_thread_delegate_sp;
   Debugger &m_debugger;
   uint32_t m_stop_id;
   bool m_update_selection;
   FormatEntity::Entry m_format;
 };
 
+class BreakpointLocationTreeDelegate : public TreeDelegate {
+public:
+  BreakpointLocationTreeDelegate(Debugger &debugger)
+      : TreeDelegate(), m_debugger(debugger) {}
+
+  ~BreakpointLocationTreeDelegate() override = default;
+
+  Process *GetProcess() {
+    ExecutionContext exe_ctx(
+        m_debugger.GetCommandInterpreter().GetExecutionContext());
+    return exe_ctx.GetProcessPtr();
+  }
+
+  BreakpointLocationSP GetBreakpointLocation(const TreeItem &item) {
+    Breakpoint *breakpoint = (Breakpoint *)item.GetUserData();
+    return breakpoint->GetLocationAtIndex(item.GetIdentifier());
+  }
+
+  void TreeDelegateDrawTreeItem(TreeItem &item, Window &window) override {
+    BreakpointLocationSP breakpoint_location = GetBreakpointLocation(item);
+    Process *process = GetProcess();
+    StreamString stream;
+    stream.Printf("%i.%i: ", breakpoint_location->GetBreakpoint().GetID(),
+                  breakpoint_location->GetID());
+    Address address = breakpoint_location->GetAddress();
+    address.Dump(&stream, process, Address::DumpStyleResolvedDescription,
+                 Address::DumpStyleInvalid);
+    window.PutCStringTruncated(1, stream.GetString().str().c_str());
+  }
+
+  StringList ComputeDetailsList(BreakpointLocationSP breakpoint_location) {
+    StringList details;
+
+    Address address = breakpoint_location->GetAddress();
+    SymbolContext symbol_context;
+    address.CalculateSymbolContext(&symbol_context);
+
+    if (symbol_context.module_sp) {
+      StreamString module_stream;
+      module_stream.PutCString("module = ");
+      symbol_context.module_sp->GetFileSpec().Dump(
+          module_stream.AsRawOstream());
+      details.AppendString(module_stream.GetString());
+    }
+
+    if (symbol_context.comp_unit != nullptr) {
+      StreamString compile_unit_stream;
+      compile_unit_stream.PutCString("compile unit = ");
+      symbol_context.comp_unit->GetPrimaryFile().GetFilename().Dump(
+          &compile_unit_stream);
+      details.AppendString(compile_unit_stream.GetString());
+
+      if (symbol_context.function != nullptr) {
+        StreamString function_stream;
+        function_stream.PutCString("function = ");
+        function_stream.PutCString(
+            symbol_context.function->GetName().AsCString("<unknown>"));
+        details.AppendString(function_stream.GetString());
+      }
+
+      if (symbol_context.line_entry.line > 0) {
+        StreamString location_stream;
+        location_stream.PutCString("location = ");
+        symbol_context.line_entry.DumpStopContext(&location_stream, true);
+        details.AppendString(location_stream.GetString());
+      }
+
+    } else {
+      if (symbol_context.symbol) {
+        StreamString symbol_stream;
+        if (breakpoint_location->IsReExported())
+          symbol_stream.PutCString("re-exported target = ");
+        else
+          symbol_stream.PutCString("symbol = ");
+        symbol_stream.PutCString(
+            symbol_context.symbol->GetName().AsCString("<unknown>"));
+        details.AppendString(symbol_stream.GetString());
+      }
+    }
+
+    Process *process = GetProcess();
+
+    StreamString address_stream;
+    address.Dump(&address_stream, process, Address::DumpStyleLoadAddress,
+                 Address::DumpStyleModuleWithFileAddress);
+    details.AppendString(address_stream.GetString());
+
+    BreakpointSiteSP breakpoint_site = breakpoint_location->GetBreakpointSite();
+    if (breakpoint_location->IsIndirect() && breakpoint_site) {
+      Address resolved_address;
+      resolved_address.SetLoadAddress(breakpoint_site->GetLoadAddress(),
+                                      &breakpoint_location->GetTarget());
+      Symbol *resolved_symbol = resolved_address.CalculateSymbolContextSymbol();
+      if (resolved_symbol) {
+        StreamString indirect_target_stream;
+        indirect_target_stream.PutCString("indirect target = ");
+        indirect_target_stream.PutCString(
+            resolved_symbol->GetName().GetCString());
+        details.AppendString(indirect_target_stream.GetString());
+      }
+    }
+
+    bool is_resolved = breakpoint_location->IsResolved();
+    StreamString resolved_stream;
+    resolved_stream.Printf("resolved = %s", is_resolved ? "true" : "false");
+    details.AppendString(resolved_stream.GetString());
+
+    bool is_hardware = is_resolved && breakpoint_site->IsHardware();
+    StreamString hardware_stream;
+    hardware_stream.Printf("hardware = %s", is_hardware ? "true" : "false");
+    details.AppendString(hardware_stream.GetString());
+
+    StreamString hit_count_stream;
+    hit_count_stream.Printf("hit count = %-4u",
+                            breakpoint_location->GetHitCount());
+    details.AppendString(hit_count_stream.GetString());
+
+    return details;
+  }
+
+  void TreeDelegateGenerateChildren(TreeItem &item) override {
+    BreakpointLocationSP breakpoint_location = GetBreakpointLocation(item);
+    StringList details = ComputeDetailsList(breakpoint_location);
+
+    if (!m_string_delegate_sp)
+      m_string_delegate_sp = std::make_shared<TextTreeDelegate>();
+    TreeItem details_tree_item(&item, *m_string_delegate_sp, false);
+
+    item.Resize(details.GetSize(), details_tree_item);
+    for (size_t i = 0; i < details.GetSize(); i++) {
+      item[i].SetText(details.GetStringAtIndex(i));
+    }
+  }
+
+  bool TreeDelegateItemSelected(TreeItem &item) override { return false; }
+
+protected:
+  Debugger &m_debugger;
+  std::shared_ptr<TextTreeDelegate> m_string_delegate_sp;
+};
+
+class BreakpointTreeDelegate : public TreeDelegate {
+public:
+  BreakpointTreeDelegate(Debugger &debugger)
+      : TreeDelegate(), m_debugger(debugger),
+        m_breakpoint_location_delegate_sp() {}
+
+  ~BreakpointTreeDelegate() override = default;
+
+  BreakpointSP GetBreakpoint(const TreeItem &item) {
+    TargetSP target = m_debugger.GetSelectedTarget();
+    BreakpointList &breakpoints = target->GetBreakpointList(false);
+    return breakpoints.GetBreakpointAtIndex(item.GetIdentifier());
+  }
+
+  void TreeDelegateDrawTreeItem(TreeItem &item, Window &window) override {
+    BreakpointSP breakpoint = GetBreakpoint(item);
+    StreamString stream;
+    stream.Format("{0}: ", breakpoint->GetID());
+    breakpoint->GetResolverDescription(&stream);
+    breakpoint->GetFilterDescription(&stream);
+    window.PutCStringTruncated(1, stream.GetString().str().c_str());
+  }
+
+  void TreeDelegateGenerateChildren(TreeItem &item) override {
+    BreakpointSP breakpoint = GetBreakpoint(item);
+
+    if (!m_breakpoint_location_delegate_sp)
+      m_breakpoint_location_delegate_sp =
+          std::make_shared<BreakpointLocationTreeDelegate>(m_debugger);
+    TreeItem breakpoint_location_tree_item(
+        &item, *m_breakpoint_location_delegate_sp, true);
+
+    item.Resize(breakpoint->GetNumLocations(), breakpoint_location_tree_item);
+    for (size_t i = 0; i < breakpoint->GetNumLocations(); i++) {
+      item[i].SetIdentifier(i);
+      item[i].SetUserData(breakpoint.get());
+    }
+  }
+
+  bool TreeDelegateItemSelected(TreeItem &item) override { return false; }
+
+protected:
+  Debugger &m_debugger;
+  std::shared_ptr<BreakpointLocationTreeDelegate>
+      m_breakpoint_location_delegate_sp;
+};
+
+class BreakpointsTreeDelegate : public TreeDelegate {
+public:
+  BreakpointsTreeDelegate(Debugger &debugger)
+      : TreeDelegate(), m_debugger(debugger), m_breakpoint_delegate_sp() {}
+
+  ~BreakpointsTreeDelegate() override = default;
+
+  bool TreeDelegateShouldDraw() override {
+    TargetSP target = m_debugger.GetSelectedTarget();
+    if (!target)
+      return false;
+
+    return true;
+  }
+
+  void TreeDelegateDrawTreeItem(TreeItem &item, Window &window) override {
+    window.PutCString("Breakpoints");
+  }
+
+  void TreeDelegateGenerateChildren(TreeItem &item) override {
+    TargetSP target = m_debugger.GetSelectedTarget();
+
+    BreakpointList &breakpoints = target->GetBreakpointList(false);
+    std::unique_lock<std::recursive_mutex> lock;
+    breakpoints.GetListMutex(lock);
+
+    if (!m_breakpoint_delegate_sp)
+      m_breakpoint_delegate_sp =
+          std::make_shared<BreakpointTreeDelegate>(m_debugger);
+    TreeItem breakpoint_tree_item(&item, *m_breakpoint_delegate_sp, true);
+
+    item.Resize(breakpoints.GetSize(), breakpoint_tree_item);
+    for (size_t i = 0; i < breakpoints.GetSize(); i++) {
+      item[i].SetIdentifier(i);
+    }
+  }
+
+  bool TreeDelegateItemSelected(TreeItem &item) override { return false; }
+
+  bool TreeDelegateExpandRootByDefault() override { return true; }
+
+protected:
+  Debugger &m_debugger;
+  std::shared_ptr<BreakpointTreeDelegate> m_breakpoint_delegate_sp;
+};
+
 class ValueObjectListDelegate : public WindowDelegate {
 public:
   ValueObjectListDelegate() : m_rows() {}
 
   ValueObjectListDelegate(ValueObjectList &valobj_list)
       : m_rows(), m_selected_row(nullptr), m_selected_row_idx(0),
         m_first_visible_row(0), m_num_rows(0), m_max_x(0), m_max_y(0) {
     SetValues(valobj_list);
enum {
     eMenuID_ThreadStepOver,
     eMenuID_ThreadStepOut,
 
     eMenuID_View,
     eMenuID_ViewBacktrace,
     eMenuID_ViewRegisters,
     eMenuID_ViewSource,
     eMenuID_ViewVariables,
+    eMenuID_ViewBreakpoints,
 
     eMenuID_Help,
     eMenuID_HelpGUIHelp
   };
 
   ApplicationDelegate(Application &app, Debugger &debugger)
       : WindowDelegate(), MenuDelegate(), m_app(app), m_debugger(debugger) {}
 
case eMenuID_Process: {
               MenuSP(new Menu(thread_menu_title.GetString().str().c_str(),
                               nullptr, menu_char, thread_sp->GetID())));
         }
       } else if (submenus.size() > 7) {
         // Remove the separator and any other thread submenu items that were
         // previously added
         submenus.erase(submenus.begin() + 7, submenus.end());
       }
-      // Since we are adding and removing items we need to recalculate the name
-      // lengths
+      // Since we are adding and removing items we need to recalculate the
+      // name lengths
       menu.RecalculateNameLengths();
     }
       return MenuActionResult::Handled;
 
     case eMenuID_ViewVariables: {
       WindowSP main_window_sp = m_app.GetMainWindow();
       WindowSP source_window_sp = main_window_sp->FindSubWindow("Source");
       WindowSP variables_window_sp = main_window_sp->FindSubWindow("Variables");
case eMenuID_ViewRegisters: {
             main_window_sp->CreateSubWindow("Registers", new_regs_rect, false);
         new_window_sp->SetDelegate(
             WindowDelegateSP(new RegistersWindowDelegate(m_debugger)));
       }
       touchwin(stdscr);
     }
       return MenuActionResult::Handled;
 
+    case eMenuID_ViewBreakpoints: {
+      WindowSP main_window_sp = m_app.GetMainWindow();
+      WindowSP threads_window_sp = main_window_sp->FindSubWindow("Threads");
+      WindowSP breakpoints_window_sp =
+          main_window_sp->FindSubWindow("Breakpoints");
+      const Rect threads_bounds = threads_window_sp->GetBounds();
+
+      // If a breakpoints window already exists, remove it and give the area
+      // it used to occupy to the threads window. If it doesn't exist, split
+      // the threads window horizontally into two windows where the top window
+      // is the threads window and the bottom window is a newly added
+      // breakpoints window.
+      if (breakpoints_window_sp) {
+        threads_window_sp->Resize(threads_bounds.size.width,
+                                  threads_bounds.size.height +
+                                      breakpoints_window_sp->GetHeight());
+        main_window_sp->RemoveSubWindow(breakpoints_window_sp.get());
+      } else {
+        Rect new_threads_bounds, breakpoints_bounds;
+        threads_bounds.HorizontalSplitPercentage(0.70, new_threads_bounds,
+                                                 breakpoints_bounds);
+        threads_window_sp->SetBounds(new_threads_bounds);
+        breakpoints_window_sp = main_window_sp->CreateSubWindow(
+            "Breakpoints", breakpoints_bounds, false);
+        TreeDelegateSP breakpoints_delegate_sp(
+            new BreakpointsTreeDelegate(m_debugger));
+        breakpoints_window_sp->SetDelegate(WindowDelegateSP(
+            new TreeWindowDelegate(m_debugger, breakpoints_delegate_sp)));
+      }
+      touchwin(stdscr);
+      return MenuActionResult::Handled;
+    }
+
     case eMenuID_HelpGUIHelp:
       m_app.GetMainWindow()->CreateHelpSubwindow();
       return MenuActionResult::Handled;
 
     default:
       break;
     }
 
if (update_location) {
           m_pc_line = m_sc.line_entry.line;
           if (m_pc_line != UINT32_MAX)
             --m_pc_line; // Convert to zero based line number...
           // Update the selected line if the stop ID changed...
           if (context_changed)
             m_selected_line = m_pc_line;
 
           if (m_file_sp && m_file_sp->GetFileSpec() == m_sc.line_entry.file) {
-            // Same file, nothing to do, we should either have the lines or not
-            // (source file missing)
+            // Same file, nothing to do, we should either have the lines or
+            // not (source file missing)
             if (m_selected_line >= static_cast<size_t>(m_first_visible_line)) {
               if (m_selected_line >= m_first_visible_line + num_visible_lines)
                 m_first_visible_line = m_selected_line - 10;
             } else {
               if (m_selected_line > 10)
                 m_first_visible_line = m_selected_line - 10;
               else
                 m_first_visible_line = 0;
if (num_source_lines > 0) {
 
       for (size_t i = 0; i < num_visible_lines; ++i) {
         const uint32_t curr_line = m_first_visible_line + i;
         if (curr_line < num_source_lines) {
           const int line_y = m_min_y + i;
           window.MoveCursor(1, line_y);
           const bool is_pc_line = curr_line == m_pc_line;
           const bool line_is_selected = m_selected_line == curr_line;
-          // Highlight the line as the PC line first, then if the selected line
-          // isn't the same as the PC line, highlight it differently
+          // Highlight the line as the PC line first, then if the selected
+          // line isn't the same as the PC line, highlight it differently
           attr_t highlight_attr = 0;
           attr_t bp_attr = 0;
           if (is_pc_line)
             highlight_attr = pc_highlight_attr;
           else if (line_is_selected)
             highlight_attr = selected_highlight_attr;
 
           if (bp_lines.find(curr_line + 1) != bp_lines.end())
if (num_source_lines > 0) {
           Instruction *inst = insts.GetInstructionAtIndex(inst_idx).get();
           if (!inst)
             break;
 
           const int line_y = m_min_y + i;
           window.MoveCursor(1, line_y);
           const bool is_pc_line = frame_sp && inst_idx == pc_idx;
           const bool line_is_selected = m_selected_line == inst_idx;
-          // Highlight the line as the PC line first, then if the selected line
-          // isn't the same as the PC line, highlight it differently
+          // Highlight the line as the PC line first, then if the selected
+          // line isn't the same as the PC line, highlight it differently
           attr_t highlight_attr = 0;
           attr_t bp_attr = 0;
           if (is_pc_line)
             highlight_attr = pc_highlight_attr;
           else if (line_is_selected)
             highlight_attr = selected_highlight_attr;
 
           if (bp_file_addrs.find(inst->GetAddress().GetFileAddress()) !=
thread_menu_sp->AddSubmenu(
         MenuSP(new Menu("Step Over", nullptr, 'v',
                         ApplicationDelegate::eMenuID_ThreadStepOver)));
     thread_menu_sp->AddSubmenu(MenuSP(new Menu(
         "Step Out", nullptr, 'o', ApplicationDelegate::eMenuID_ThreadStepOut)));
 
     MenuSP view_menu_sp(
         new Menu("View", "F5", KEY_F(5), ApplicationDelegate::eMenuID_View));
     view_menu_sp->AddSubmenu(
-        MenuSP(new Menu("Backtrace", nullptr, 'b',
+        MenuSP(new Menu("Backtrace", nullptr, 't',
                         ApplicationDelegate::eMenuID_ViewBacktrace)));
     view_menu_sp->AddSubmenu(
         MenuSP(new Menu("Registers", nullptr, 'r',
                         ApplicationDelegate::eMenuID_ViewRegisters)));
     view_menu_sp->AddSubmenu(MenuSP(new Menu(
         "Source", nullptr, 's', ApplicationDelegate::eMenuID_ViewSource)));
     view_menu_sp->AddSubmenu(
         MenuSP(new Menu("Variables", nullptr, 'v',
                         ApplicationDelegate::eMenuID_ViewVariables)));
+    view_menu_sp->AddSubmenu(
+        MenuSP(new Menu("Breakpoints", nullptr, 'b',
+                        ApplicationDelegate::eMenuID_ViewBreakpoints)));
 
     MenuSP help_menu_sp(
         new Menu("Help", "F6", KEY_F(6), ApplicationDelegate::eMenuID_Help));
     help_menu_sp->AddSubmenu(MenuSP(new Menu(
         "GUI Help", nullptr, 'g', ApplicationDelegate::eMenuID_HelpGUIHelp)));
 
     m_app_ap->Initialize();
     WindowSP &main_window_sp = m_app_ap->GetMainWindow();
if (!m_app_ap) {
     variables_window_sp->SetDelegate(
         WindowDelegateSP(new FrameVariablesWindowDelegate(m_debugger)));
     TreeDelegateSP thread_delegate_sp(new ThreadsTreeDelegate(m_debugger));
     threads_window_sp->SetDelegate(WindowDelegateSP(
         new TreeWindowDelegate(m_debugger, thread_delegate_sp)));
     status_window_sp->SetDelegate(
         WindowDelegateSP(new StatusBarWindowDelegate(m_debugger)));
 
-    // Show the main help window once the first time the curses GUI is launched
+    // Show the main help window once the first time the curses GUI is
+    // launched
     static bool g_showed_help = false;
     if (!g_showed_help) {
       g_showed_help = true;
       main_window_sp->CreateHelpSubwindow();
     }
 
     // All colors with black background.
     init_pair(1, COLOR_BLACK, COLOR_BLACK);