Skip to content

Commit c73a3f1

Browse files
committed
[lldb] [mostly NFC] Large WP foundation: WatchpointResources (llvm#68845)
This patch is rearranging code a bit to add WatchpointResources to Process. A WatchpointResource is meant to represent a hardware watchpoint register in the inferior process. It has an address, a size, a type, and a list of Watchpoints that are using this WatchpointResource. This current patch doesn't add any of the features of WatchpointResources that make them interesting -- a user asking to watch a 24 byte object could watch this with three 8 byte WatchpointResources. Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at 0x1003, these must both be served by a single WatchpointResource on that doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint registers were used to track these separately, one of them may not be hit. Or if you have one Watchpoint on a variable with a condition set, and another Watchpoint on that same variable with a command defined or different condition, or ignorecount, both of those Watchpoints need to evaluate their criteria/commands when their WatchpointResource has been hit. There's a bit of code movement to rearrange things in the direction I'll need for implementing this feature, so I want to start with reviewing & landing this mostly NFC patch and we can focus on the algorithmic choices about how WatchpointResources are shared and handled as they're triggeed, separately. This patch also stops printing "Watchpoint <n> hit: old value: <x>, new vlaue: <y>" for Read watchpoints. I could make an argument for print "Watchpoint <n> hit: current value <x>" but the current output doesn't make any sense, and the user can print the value if they are particularly interested. Read watchpoints are used primarily to understand what code is reading a variable. This patch adds more fallbacks for how to print the objects being watched if we have types, instead of assuming they are all integral values, so a struct will print its elements. As large watchpoints are added, we'll be doing a lot more of those. To track the WatchpointSP in the WatchpointResources, I changed the internal API which took a WatchpointSP and devolved it to a Watchpoint*, which meant touching several different Process files. I removed the watchpoint code in ProcessKDP which only reported that watchpoints aren't supported, the base class does that already. I haven't yet changed how we receive a watchpoint to identify the WatchpointResource responsible for the trigger, and identify all Watchpoints that are using this Resource to evaluate their conditions etc. This is the same work that a BreakpointSite needs to do when it has been tiggered, where multiple Breakpoints may be at the same address. There is not yet any printing of the Resources that a Watchpoint is implemented in terms of ("watchpoint list", or SBWatchpoint::GetDescription). "watchpoint set var" and "watchpoint set expression" take a size argument which was previously 1, 2, 4, or 8 (an enum). I've changed this to an unsigned int. Most hardware implementations can only watch 1, 2, 4, 8 byte ranges, but with Resources we'll allow a user to ask for different sized watchpoints and set them in hardware-expressble terms soon. I've annotated areas where I know there is work still needed with LWP_TODO that I'll be working on once this is landed. I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS. https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116 (cherry picked from commit fc6b725)
1 parent 812dad5 commit c73a3f1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1426
-759
lines changed

lldb/include/lldb/Breakpoint/BreakpointSite.h

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
4545
// display any breakpoint opcodes.
4646
};
4747

48+
typedef lldb::break_id_t SiteID;
49+
typedef lldb::break_id_t ConstituentID;
50+
4851
~BreakpointSite() override;
4952

5053
// This section manages the breakpoint traps
@@ -77,8 +80,8 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
7780
/// Tells whether the current breakpoint site is enabled or not
7881
///
7982
/// This is a low-level enable bit for the breakpoint sites. If a
80-
/// breakpoint site has no enabled owners, it should just get removed. This
81-
/// enable/disable is for the low-level target code to enable and disable
83+
/// breakpoint site has no enabled constituents, it should just get removed.
84+
/// This enable/disable is for the low-level target code to enable and disable
8285
/// breakpoint sites when single stepping, etc.
8386
bool IsEnabled() const;
8487

@@ -101,44 +104,46 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
101104
/// Standard Dump method
102105
void Dump(Stream *s) const override;
103106

104-
/// The "Owners" are the breakpoint locations that share this breakpoint
105-
/// site. The method adds the \a owner to this breakpoint site's owner list.
107+
/// The "Constituents" are the breakpoint locations that share this breakpoint
108+
/// site. The method adds the \a constituent to this breakpoint site's
109+
/// constituent list.
106110
///
107-
/// \param[in] owner
108-
/// \a owner is the Breakpoint Location to add.
109-
void AddOwner(const lldb::BreakpointLocationSP &owner);
111+
/// \param[in] constituent
112+
/// \a constituent is the Breakpoint Location to add.
113+
void AddConstituent(const lldb::BreakpointLocationSP &constituent);
110114

111115
/// This method returns the number of breakpoint locations currently located
112116
/// at this breakpoint site.
113117
///
114118
/// \return
115-
/// The number of owners.
116-
size_t GetNumberOfOwners();
119+
/// The number of constituents.
120+
size_t GetNumberOfConstituents();
117121

118122
/// This method returns the breakpoint location at index \a index located at
119-
/// this breakpoint site. The owners are listed ordinally from 0 to
120-
/// GetNumberOfOwners() - 1 so you can use this method to iterate over the
121-
/// owners
123+
/// this breakpoint site. The constituents are listed ordinally from 0 to
124+
/// GetNumberOfConstituents() - 1 so you can use this method to iterate over
125+
/// the constituents
122126
///
123127
/// \param[in] idx
124-
/// The index in the list of owners for which you wish the owner location.
128+
/// The index in the list of constituents for which you wish the
129+
/// constituent location.
125130
///
126131
/// \return
127132
/// A shared pointer to the breakpoint location at that index.
128-
lldb::BreakpointLocationSP GetOwnerAtIndex(size_t idx);
133+
lldb::BreakpointLocationSP GetConstituentAtIndex(size_t idx);
129134

130-
/// This method copies the breakpoint site's owners into a new collection.
131-
/// It does this while the owners mutex is locked.
135+
/// This method copies the breakpoint site's constituents into a new
136+
/// collection. It does this while the constituents mutex is locked.
132137
///
133138
/// \param[out] out_collection
134-
/// The BreakpointLocationCollection into which to put the owners
139+
/// The BreakpointLocationCollection into which to put the constituents
135140
/// of this breakpoint site.
136141
///
137142
/// \return
138143
/// The number of elements copied into out_collection.
139-
size_t CopyOwnersList(BreakpointLocationCollection &out_collection);
144+
size_t CopyConstituentsList(BreakpointLocationCollection &out_collection);
140145

141-
/// Check whether the owners of this breakpoint site have any thread
146+
/// Check whether the constituents of this breakpoint site have any thread
142147
/// specifiers, and if yes, is \a thread contained in any of these
143148
/// specifiers.
144149
///
@@ -151,7 +156,7 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
151156
bool ValidForThisThread(Thread &thread);
152157

153158
/// Print a description of this breakpoint site to the stream \a s.
154-
/// GetDescription tells you about the breakpoint site's owners. Use
159+
/// GetDescription tells you about the breakpoint site's constituents. Use
155160
/// BreakpointSite::Dump(Stream *) to get information about the breakpoint
156161
/// site itself.
157162
///
@@ -203,9 +208,10 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
203208

204209
void BumpHitCounts();
205210

206-
/// The method removes the owner at \a break_loc_id from this breakpoint
211+
/// The method removes the constituent at \a break_loc_id from this breakpoint
207212
/// list.
208-
size_t RemoveOwner(lldb::break_id_t break_id, lldb::break_id_t break_loc_id);
213+
size_t RemoveConstituent(lldb::break_id_t break_id,
214+
lldb::break_id_t break_loc_id);
209215

210216
BreakpointSite::Type m_type; ///< The type of this breakpoint site.
211217
uint8_t m_saved_opcode[8]; ///< The saved opcode bytes if this breakpoint site
@@ -215,20 +221,20 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
215221
bool
216222
m_enabled; ///< Boolean indicating if this breakpoint site enabled or not.
217223

218-
// Consider adding an optimization where if there is only one owner, we don't
219-
// store a list. The usual case will be only one owner...
220-
BreakpointLocationCollection m_owners; ///< This has the BreakpointLocations
221-
///that share this breakpoint site.
222-
std::recursive_mutex
223-
m_owners_mutex; ///< This mutex protects the owners collection.
224+
// Consider adding an optimization where if there is only one constituent, we
225+
// don't store a list. The usual case will be only one constituent...
226+
BreakpointLocationCollection
227+
m_constituents; ///< This has the BreakpointLocations
228+
/// that share this breakpoint site.
229+
std::recursive_mutex m_constituents_mutex; ///< This mutex protects the
230+
///< constituents collection.
224231

225232
static lldb::break_id_t GetNextID();
226233

227234
// Only the Process can create breakpoint sites in
228235
// Process::CreateBreakpointSite (lldb::BreakpointLocationSP &, bool).
229-
BreakpointSite(BreakpointSiteList *list,
230-
const lldb::BreakpointLocationSP &owner, lldb::addr_t m_addr,
231-
bool use_hardware);
236+
BreakpointSite(const lldb::BreakpointLocationSP &constituent,
237+
lldb::addr_t m_addr, bool use_hardware);
232238

233239
BreakpointSite(const BreakpointSite &) = delete;
234240
const BreakpointSite &operator=(const BreakpointSite &) = delete;

lldb/include/lldb/Breakpoint/BreakpointSiteList.h

Lines changed: 0 additions & 173 deletions
This file was deleted.

0 commit comments

Comments
 (0)