llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (jimingham) <details> <summary>Changes</summary> This patch adds the notion of "Facade" locations which can be reported from a ScriptedResolver instead of the actual underlying breakpoint location for the breakpoint. Also add a "was_hit" method to the scripted resolver that allows the breakpoint to say which of these "Facade" locations was hit, and "get_location_description" to provide a description for the facade locations. I apologize in advance for the size of the patch. Almost all of what's here was necessary to (a) make the feature testable and (b) not break any of the current behavior. The motivation for this feature is given in the "Providing Facade Locations" section that I added to the python-reference.rst so I won't repeat it here. rdar://152112327 --- Patch is 77.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/158128.diff 36 Files Affected: - (modified) lldb/bindings/python/python-swigsafecast.swig (+4) - (modified) lldb/bindings/python/python-wrapper.swig (+24) - (modified) lldb/docs/use/python-reference.rst (+61) - (modified) lldb/include/lldb/API/SBBreakpoint.h (+8-1) - (modified) lldb/include/lldb/API/SBBreakpointLocation.h (+1) - (modified) lldb/include/lldb/API/SBFrame.h (+2-1) - (modified) lldb/include/lldb/Breakpoint/Breakpoint.h (+53-5) - (modified) lldb/include/lldb/Breakpoint/BreakpointLocation.h (+60-8) - (modified) lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h (+1-1) - (modified) lldb/include/lldb/Breakpoint/BreakpointLocationList.h (+2-1) - (modified) lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h (+6) - (modified) lldb/include/lldb/Breakpoint/BreakpointSite.h (+2-1) - (modified) lldb/include/lldb/Breakpoint/StopPointSiteList.h (-24) - (modified) lldb/include/lldb/Breakpoint/StoppointSite.h (+4-1) - (modified) lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h (+10) - (modified) lldb/include/lldb/Interpreter/ScriptInterpreter.h (+6) - (modified) lldb/source/API/SBBreakpoint.cpp (+10) - (modified) lldb/source/Breakpoint/Breakpoint.cpp (+74-14) - (modified) lldb/source/Breakpoint/BreakpointLocation.cpp (+136-41) - (modified) lldb/source/Breakpoint/BreakpointLocationCollection.cpp (+14-2) - (modified) lldb/source/Breakpoint/BreakpointLocationList.cpp (+6-5) - (modified) lldb/source/Breakpoint/BreakpointResolverScripted.cpp (+23-1) - (modified) lldb/source/Breakpoint/BreakpointSite.cpp (+3-2) - (modified) lldb/source/Interpreter/ScriptInterpreter.cpp (+13) - (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedBreakpointPythonInterface.cpp (+26) - (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedBreakpointPythonInterface.h (+6) - (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp (+47) - (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h (+27) - (modified) lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h (+3) - (modified) lldb/source/Target/StopInfo.cpp (+47-25) - (modified) lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py (-1) - (added) lldb/test/API/functionalities/breakpoint/scripted_bkpt/was_hit/Makefile (+4) - (added) lldb/test/API/functionalities/breakpoint/scripted_bkpt/was_hit/TestWasHit.py (+85) - (added) lldb/test/API/functionalities/breakpoint/scripted_bkpt/was_hit/bkpt_resolver.py (+45) - (added) lldb/test/API/functionalities/breakpoint/scripted_bkpt/was_hit/main.c (+17) - (modified) lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp (+10) ``````````diff diff --git a/lldb/bindings/python/python-swigsafecast.swig b/lldb/bindings/python/python-swigsafecast.swig index 4721dfdc17e6a..3ea24f1a31414 100644 --- a/lldb/bindings/python/python-swigsafecast.swig +++ b/lldb/bindings/python/python-swigsafecast.swig @@ -142,5 +142,9 @@ PythonObject SWIGBridge::ToSWIGWrapper( return ToSWIGHelper(module_spec_sb.release(), SWIGTYPE_p_lldb__SBModuleSpec); } +PythonObject SWIGBridge::ToSWIGWrapper(lldb::DescriptionLevel level) { + return PythonInteger((int64_t) level); +} + } // namespace python } // namespace lldb_private diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig index 2c30d536a753d..64b7dc8381073 100644 --- a/lldb/bindings/python/python-wrapper.swig +++ b/lldb/bindings/python/python-wrapper.swig @@ -422,6 +422,30 @@ void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBBreakpoint(PyObject * return sb_ptr; } +void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBFrame(PyObject * data) { + lldb::SBFrame *sb_ptr = nullptr; + + int valid_cast = + SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBFrame, 0); + + if (valid_cast == -1) + return NULL; + + return sb_ptr; +} + +void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBBreakpointLocation(PyObject * data) { + lldb::SBBreakpointLocation *sb_ptr = nullptr; + + int valid_cast = + SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBBreakpointLocation, 0); + + if (valid_cast == -1) + return NULL; + + return sb_ptr; +} + void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBAttachInfo(PyObject * data) { lldb::SBAttachInfo *sb_ptr = nullptr; diff --git a/lldb/docs/use/python-reference.rst b/lldb/docs/use/python-reference.rst index 4292714c9c208..5f40526a7308e 100644 --- a/lldb/docs/use/python-reference.rst +++ b/lldb/docs/use/python-reference.rst @@ -420,6 +420,67 @@ of Modules and the list of CompileUnits that will make up the SearchFilter. If you pass in empty lists, the breakpoint will use the default "search everywhere,accept everything" filter. +Providing Facade Locations: + +The breakpoint resolver interface also allows you to present a separate set +of locations for the breakpoint than the ones that actually implement the +breakpoint in the target. + +An example use case for this is if you are providing a debugging interface for a +library that implements an interpreter for a language lldb can't debug. But +while debugging that library at the level of the implementation language (e.g. C/C++, etc) +you would like to offer the ability to "stop when a line in a source language +file is executed". + +You can do this if you know where new lines of code are dispatched in the +interpreter. You would set a breakpoint there, and then look at the state +when that breakpoint is hit to see if it is dispatching the source file and +line that were requested, and stop appropriately. + +Facade breakpoint locations are intended to make a more natural presentation +of that sort of feature. The idea is that you would make a custom breakpoint +resolver that sets actual locations in the places of interest in the interpreter. + +Then your resolver would add "facade locations" that represent the places in the +interpreted code that you want the breakpoint to stop at, using SBBreakpoint::AddFacadeLocation. +When lldb describes the breakpoint, it will only show the Facade locations. +Since facade breakpoint location's description is customizable, you can make these +locations more descriptive. And when the "real" location is hit, lldb will call the +"was_hit" method of your resolver. That will return the facade location you +consider to have been hit this time around, or if you return None, the breakpoint +will be considered not to have been hit. + +Note, this feature is also useful if you don't intend to present facade +locations since it essentially provides a scripted breakpoint condition. Every +time one of the locations in your breakpoint is hit, you can run the code in +your "was_hit" to determine whether to consider the breakpoint hit or not, and +return the location you were passed in if you want it to be a hit, and None if not. + +The Facade location adds these optional affordances to the Resolver class: + ++------------------------------+----------------------------------------+------------------------------------------------------------------------------------------------------------------+ +| Name | Arguments | Description | ++------------------------------+----------------------------------------+------------------------------------------------------------------------------------------------------------------+ +| ``was_hit`` | ``frame``:`lldb.SBFrame` | This will get called when one of the "real" locations set by your resolver is hit | +| | ``bp_loc``:`lldb.SBBreakpointLocation` | | +| | | | +| | | ``frame`` is the stack frame that hit this location. | +| | | | +| | | | +| | | ``bp_loc`` is the real location that was hit. | +| | | | +| | | Return either the facade location that you want to consider hit on this stop, or None if you don't consider | +| | | any of your facade locations to have been hit. | ++------------------------------+----------------------------------------+------------------------------------------------------------------------------------------------------------------+ +| ``get_location_description`` | ``bp_loc``:`lldb.SBBreakpointLocation` | Use this to provide a helpful description of each facade location. | +| | ``desc_level``:`lldb.DescriptionLevel` | | +| | | ``bp_loc`` is the facade location to describe. | +| | | | +| | | | +| | | ``desc_level`` is the level of description requested. The Brief description is printed when the location is | +| | | hit. Full is printed for `break list` and Verbose for `break list -v`. | ++------------------------------+----------------------------------------+------------------------------------------------------------------------------------------------------------------+ + Using the python API' to create custom stepping logic ----------------------------------------------------- diff --git a/lldb/include/lldb/API/SBBreakpoint.h b/lldb/include/lldb/API/SBBreakpoint.h index 18ed3e7226d3b..3bf87a1b7cc92 100644 --- a/lldb/include/lldb/API/SBBreakpoint.h +++ b/lldb/include/lldb/API/SBBreakpoint.h @@ -153,9 +153,16 @@ class LLDB_API SBBreakpoint { /// fails, e.g. when there aren't enough hardware resources available. lldb::SBError SetIsHardware(bool is_hardware); - // Can only be called from a ScriptedBreakpointResolver... + /// Adds a location to the breakpoint at the address passed in. + /// Can only be called from a ScriptedBreakpointResolver... SBError AddLocation(SBAddress &address); + /// Add a "Facade location" to the breakpoint. This returns the Facade + /// Location that was added, which you can then use in + /// get_location_description and was_hit in your breakpoint resolver. + /// Can only be called from a ScriptedBreakpointResolver. + SBBreakpointLocation + AddFacadeLocation(); SBStructuredData SerializeToStructuredData(); diff --git a/lldb/include/lldb/API/SBBreakpointLocation.h b/lldb/include/lldb/API/SBBreakpointLocation.h index fa823e2b518ac..deda4970cd0ed 100644 --- a/lldb/include/lldb/API/SBBreakpointLocation.h +++ b/lldb/include/lldb/API/SBBreakpointLocation.h @@ -24,6 +24,7 @@ class SWIGBridge; namespace lldb { class LLDB_API SBBreakpointLocation { + friend class lldb_private::ScriptInterpreter; public: SBBreakpointLocation(); diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h index e4bbcd5ddcd9c..4abb44b4bb0e5 100644 --- a/lldb/include/lldb/API/SBFrame.h +++ b/lldb/include/lldb/API/SBFrame.h @@ -225,7 +225,8 @@ class LLDB_API SBFrame { friend class SBInstruction; friend class SBThread; friend class SBValue; - + + friend class lldb_private::ScriptInterpreter; friend class lldb_private::python::SWIGBridge; friend class lldb_private::lua::SWIGBridge; diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index 26a5e901a0d7e..2bef6a919d878 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -248,6 +248,23 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, /// Returns a pointer to the new location. lldb::BreakpointLocationSP AddLocation(const Address &addr, bool *new_location = nullptr); + /// Add a `facade` location to the breakpoint's collection of facade locations. + /// This is only meant to be called by the breakpoint's resolver. + /// Facade locations are placeholders that a scripted breakpoint can use to + /// represent the stop locations provided by the breakpoint. The scripted + /// breakpoint should record the id of the facade location, and provide + /// the description of the location in the GetDescription method + /// To emulate hitting a facade location, the breakpoint's WasHit should + /// return the ID of the facade that was "hit". + /// + /// \param[out] new_location + /// Set to \b true if a new location was created, to \b false if there + /// already was a location at this Address. + /// \return + /// Returns a pointer to the new location. + lldb::BreakpointLocationSP AddFacadeLocation(); + + lldb::BreakpointLocationSP GetFacadeLocationByID(lldb::break_id_t); /// Find a breakpoint location by Address. /// @@ -268,27 +285,36 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, /// there is no breakpoint location at that address. lldb::break_id_t FindLocationIDByAddress(const Address &addr); - /// Find a breakpoint location for a given breakpoint location ID. + /// Find a breakpoint location for a given breakpoint location ID. If there + /// are Facade Locations in the breakpoint, the facade locations will be + /// searched instead of the "real" ones. /// /// \param[in] bp_loc_id /// The ID specifying the location. + /// + /// \param[in] use_facade + /// If \b true, then prefer facade locations over "real" ones if they exist. + /// /// \return /// Returns a shared pointer to the location with ID \a bp_loc_id. The /// pointer /// in the shared pointer will be nullptr if there is no location with that /// ID. - lldb::BreakpointLocationSP FindLocationByID(lldb::break_id_t bp_loc_id); + lldb::BreakpointLocationSP FindLocationByID(lldb::break_id_t bp_loc_id, bool use_facade = true); /// Get breakpoint locations by index. /// /// \param[in] index /// The location index. /// + /// \param[in] use_facade + /// If \b true, then prefer facade locations over "real" ones if they exist. + /// /// \return /// Returns a shared pointer to the location with index \a /// index. The shared pointer might contain nullptr if \a index is /// greater than then number of actual locations. - lldb::BreakpointLocationSP GetLocationAtIndex(size_t index); + lldb::BreakpointLocationSP GetLocationAtIndex(size_t index, bool use_facade = true); /// Removes all invalid breakpoint locations. /// @@ -409,9 +435,12 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, /// Return the number of breakpoint locations that have resolved to actual /// breakpoint sites. /// + /// \param[in] use_facade + /// If \b true, then prefer facade locations over "real" ones if they exist. + /// /// \return /// The number locations resolved breakpoint sites. - size_t GetNumResolvedLocations() const; + size_t GetNumResolvedLocations(bool use_facade = true) const; /// Return whether this breakpoint has any resolved locations. /// @@ -421,9 +450,12 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, /// Return the number of breakpoint locations. /// + /// \param[in] use_facade + /// If \b true, then prefer facade locations over "real" ones if they exist. + /// /// \return /// The number breakpoint locations. - size_t GetNumLocations() const; + size_t GetNumLocations(bool use_facade = true) const; /// Put a description of this breakpoint into the stream \a s. /// @@ -529,6 +561,20 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, m_name_list.erase(name_to_remove); } + enum TypeDisplay { + eDisplayFacade = 1, + eDisplayReal = 1 << 1, + eDisplayHeader = 1 << 2 + }; + + void GetDescriptionForType(Stream *s, lldb::DescriptionLevel level, + uint8_t display_type, bool show_locations); + + bool HasFacadeLocations() { + return m_facade_locations.GetSize() != 0; + } + + public: bool MatchesName(const char *name) { return m_name_list.find(name) != m_name_list.end(); @@ -657,6 +703,8 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, BreakpointOptions m_options; // Settable breakpoint options BreakpointLocationList m_locations; // The list of locations currently found for this breakpoint. + BreakpointLocationCollection m_facade_locations; + std::string m_kind_description; bool m_resolve_indirect_symbols; diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h index ab2e5e170559d..c4fa9bec51aae 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h @@ -38,6 +38,12 @@ namespace lldb_private { class BreakpointLocation : public std::enable_shared_from_this<BreakpointLocation> { + friend class BreakpointSite; + friend class BreakpointLocationList; + friend class Breakpoint; + friend class Process; + friend class StopInfoBreakpoint; + public: ~BreakpointLocation(); @@ -55,16 +61,39 @@ class BreakpointLocation Target &GetTarget(); + /// This is a programmatic version of a breakpoint "condition". When a + /// breakpoint is hit, WasHit will get called before the synchronous ShouldStop + /// callback is run, and if it returns an empty BreakpointLocationSP, lldb will + /// act as if that breakpoint wasn't hit. + /// + /// \param[in] context + /// The context at the stop point + /// + /// \return + /// This will return the breakpoint location that was hit on this stop. + /// If there was no facade location this will be the original location. + /// If the shared pointer is empty, then we'll treat it as if the + /// breakpoint was not hit. + lldb::BreakpointLocationSP WasHit(StoppointCallbackContext *context); + /// Determines whether we should stop due to a hit at this breakpoint /// location. /// /// Side Effects: This may evaluate the breakpoint condition, and run the /// callback. So this command may do a considerable amount of work. /// + /// \param[in] context + /// The context at the stop point + /// + /// \param[out] facade_loc_sp + /// If this stop should be attributed not to the location that was hit, but + /// to a facade location, it will be returned in this facade_loc_sp. + /// /// \return /// \b true if this breakpoint location thinks we should stop, /// \b false otherwise. - bool ShouldStop(StoppointCallbackContext *context); + bool ShouldStop(StoppointCallbackContext *context, + lldb::BreakpointLocationSP &facade_loc_sp); // The next section deals with various breakpoint options. @@ -292,11 +321,6 @@ class BreakpointLocation } protected: - friend class BreakpointSite; - friend class BreakpointLocationList; - friend class Process; - friend class StopInfoBreakpoint; - /// Set the breakpoint site for this location to \a bp_site_sp. /// /// \param[in] bp_site_sp @@ -346,9 +370,11 @@ class BreakpointLocation // Constructors and Destructors // // Only the Breakpoint can make breakpoint locations, and it owns them. - /// Constructor. /// + /// \param[in] loc_id + /// The location id of the new location. + /// /// \param[in] owner /// A back pointer to the breakpoint that owns this location. /// @@ -359,10 +385,25 @@ class BreakpointLocation /// The thread for which this breakpoint location is valid, or /// LLDB_INVALID_THREAD_ID if it is valid for all threads. /// - BreakpointLocation(lldb::break_id_t bid, Breakpoint &owner, + BreakpointLocation(lldb::break_id_t loc_id, Breakpoint &owner, const Address &addr, lldb::tid_t tid, bool check_for_resolver = true); + /// This is the constructor for locations with no address. Currently this is + /// just used for Facade locations. + /// + /// \param[in] loc_id + /// The location id of the new location. + /// + /// \param[in] owner + /// A back pointer to the breakpoint that owns this location. + /// + /// +public: + BreakpointLocation(lldb::break_id_t loc_id, Breakpoint &owner); + bool IsValid() const { return m_is_valid; } + bool IsFacade() const { return m_is_facade; } +private: // Data members: bool m_should_resolve_indirect_functions; bool m_is_reexported; @@ -390,6 +431,17 @@ class BreakpointLocation /// location was given somewhere in the virtual inlined call stack since the /// Address always resolves to the lowest entry in the stack. std::optional<LineEntry> m_preferred_line_entry; + bool m_is_valid = true; /// Because Facade locations don't have sites + /// we can't use the presence of the site to mean + /// this breakpoint is valid, but must manage + /// the state directly. + bool m_is_facade = false; /// Facade locations aren't directly triggered + /// and don't have a breakpoint site. They are + /// a useful ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/158128 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
