https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/198845
>From 0fca1f605f992b90e7c85b22682a92f816ab2397 Mon Sep 17 00:00:00 2001 From: Jim Ingham <[email protected]> Date: Tue, 19 May 2026 14:02:20 -0700 Subject: [PATCH 1/4] Add a pre-filter to the breakpoint resolver overrides. That way if a override only wants to override, say, file and line breakpoints, it can say that when it registers itself, and we won't do the work of creating the resolver and checking with it if the breakpoint wasn't of the requested type. --- lldb/include/lldb/API/SBTarget.h | 5 +- .../lldb/Breakpoint/BreakpointResolver.h | 14 ++++- .../ScriptedBreakpointOverrideResolver.h | 7 ++- .../Interpreter/CommandOptionArgumentTable.h | 16 +++++ lldb/include/lldb/Target/Target.h | 29 +++++---- lldb/include/lldb/lldb-enumerations.h | 16 +++++ lldb/source/API/SBTarget.cpp | 3 +- lldb/source/Breakpoint/BreakpointResolver.cpp | 44 +++++++++++++ .../Commands/CommandObjectBreakpoint.cpp | 20 +++++- lldb/source/Commands/Options.td | 5 ++ lldb/source/Target/Target.cpp | 36 +++++++++-- .../TestOverridesResolver.py | 44 +++++++++++-- .../overrides_resolver/bkpt_resolver.py | 61 ++++++++++++++++++- 13 files changed, 262 insertions(+), 38 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index d598c44dd7332..559e0c0f59338 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -639,9 +639,10 @@ class LLDB_API SBTarget { lldb::SBError &error); /// Adds a breakpoint override implemented by class_name. Returns the ID - /// of the new override or LLDB_INVALID_INDEX64 on error. + /// of the new override or LLDB_INVALID_INDEX64 on error. The type_mask + /// is composed of elements from the lldb::BreakpointResolverType enum. uint64_t AddBreakpointOverride(const char *class_name, - const char *description, + const char *description, uint64_t type_mask, SBStructuredData &args_data, SBError &status); bool RemoveBreakpointOverride(uint64_t id); diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index 064e370d08a40..f27854812cb92 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -154,18 +154,28 @@ class BreakpointResolver : public Searcher { /// for any other purpose, as the values may change as LLDB evolves. unsigned getResolverID() const { return SubclassID; } + /// This checks whether the resolver's type matches the enum + /// lldb::eBreakpointResolverType. + bool ResolverTyInMask(uint64_t mask); + enum ResolverTy GetResolverTy() { if (SubclassID > ResolverTy::LastKnownResolverType) return ResolverTy::UnknownResolver; return (enum ResolverTy)SubclassID; } - + + uint64_t MaskForResolverTy(); + + /// Returns true if this resolver is in the mask (made of elements of + /// BreakpointResolverType. + bool ResolverInMask(uint64_t mask); + static std::string DescribeMask(uint64_t mask); const char *GetResolverName() { return ResolverTyToName(GetResolverTy()); } static const char *ResolverTyToName(enum ResolverTy); static ResolverTy NameToResolverTy(llvm::StringRef name); - + virtual lldb::BreakpointResolverSP CopyForBreakpoint(lldb::BreakpointSP &breakpoint) = 0; diff --git a/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h b/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h index 2800fafeec8b1..17b872d7e1bdb 100644 --- a/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h +++ b/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h @@ -20,16 +20,17 @@ class ScriptedBreakpointResolverOverride public: ScriptedBreakpointResolverOverride(Target &target, const std::string &description, + uint64_t type_mask, const std::string &class_name, StructuredDataImpl &args_data) - : Target::BreakpointResolverOverride(target, description), + : Target::BreakpointResolverOverride(target, description, type_mask), m_args_data(args_data), m_class_name(class_name) {} Target::BreakpointResolverOverrideUP CopyIntoNewTarget(Target &target) override { return Target::BreakpointResolverOverrideUP( - new ScriptedBreakpointResolverOverride(target, m_desc, m_class_name, - m_args_data)); + new ScriptedBreakpointResolverOverride(target, m_desc, m_type_mask, + m_class_name, m_args_data)); } lldb::BreakpointResolverSP diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index bdb23bf314aae..996e109b56627 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -183,6 +183,21 @@ static constexpr OptionEnumValueElement g_plugin_domain_values[] = { {lldb::ePluginDomainKindTarget, "target", "Apply to the current target."}, }; +static constexpr OptionEnumValueElement g_resolver_mask_values[] = { + {lldb::eResolverFileAndLine, "file_and_line", + "Matches file and line breakpoint resolvers."}, + {lldb::eResolverAddress, "address", + "Matches address breakpoint resolvers."}, + {lldb::eResolverName, "name", + "Matches symbol name breakpoint resolvers."}, + {lldb::eResolverFileRegex, "source", + "Matches source text pattern resolvers."}, + {lldb::eResolverPython, "scripted", + "Matches scripted breakpoint resolvers."}, + {lldb::eResolverException, "exception", + "Matches language exception breakpoint resolvers."}, +}; + static constexpr OptionEnumValueElement g_completion_type[] = { {lldb::eNoCompletion, "none", "No completion."}, {lldb::eSourceFileCompletion, "source-file", "Completes to a source file."}, @@ -349,6 +364,7 @@ static constexpr CommandObject::ArgumentTableEntry g_argument_table[] = { { lldb::eArgTypeExceptionStage, "exception-stage", lldb::CompletionType::eNoCompletion, g_exception_stage, { nullptr, false }, "Specify at which stage of the exception raise to stop." }, { lldb::eArgTypeNameMatchStyle, "match-style", lldb::CompletionType::eNoCompletion, g_name_match_style, { nullptr, false }, "Specify the kind of match to use when looking up names." }, { lldb::eArgTypePluginDomain, "plugin-domain", lldb::CompletionType::eNoCompletion, g_plugin_domain_values, { nullptr, false }, "The domain to apply the plugin operation to." }, + { lldb::eArgTypeBreakpointResolverMask, "resolver-mask", lldb::CompletionType::eNoCompletion, g_resolver_mask_values, { nullptr, false }, "Specify the breakpoint resolver type your override will handle. Can be specified more than once to specify a mask of resolver types." }, // clang-format on }; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 7a88184849d6e..bb032b2347e4f 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1008,16 +1008,19 @@ class Target : public std::enable_shared_from_this<Target>, // be the one we use. If no overrides return an override resolver, we'll use // the original one. - // This is the abstract version of the override. Particular implementations - // e.g. the scripted override will derive from this. + /// This is the abstract version of the override. Particular implementations + /// e.g. the scripted override resolve instantiate actual versions of the class. + /// The constructor takes the target this resolver is registered in, a + /// description for the override and a mask of the resolver types this + /// overrides, made of elements of the BreakpointResolverType enum. class BreakpointResolverOverride; using BreakpointResolverOverrideUP = std::unique_ptr<BreakpointResolverOverride>; class BreakpointResolverOverride { public: - BreakpointResolverOverride(Target &target, const std::string &description) - : m_target(target), m_desc(description) {} + BreakpointResolverOverride(Target &target, const std::string &description, uint64_t type_mask) + : m_target(target), m_desc(description), m_type_mask(type_mask) {} virtual BreakpointResolverOverrideUP CopyIntoNewTarget(Target &target) = 0; @@ -1027,10 +1030,12 @@ class Target : public std::enable_shared_from_this<Target>, // Return whether constructing this resolver was successful. virtual llvm::Error Validate() = 0; const std::string &GetDescription() { return m_desc; } - + uint64_t GetTypeMask() { return m_type_mask; } + std::string DescribeTypeMask(); protected: Target &m_target; std::string m_desc; + uint64_t m_type_mask = 0; }; /// Add a breakpoint override resolver. This version can't fail. @@ -1045,6 +1050,7 @@ class Target : public std::enable_shared_from_this<Target>, /// Add a breakpoint override resolver. Return the ID or an error: llvm::Expected<lldb::user_id_t> AddBreakpointResolverOverride(llvm::StringRef class_name, + uint64_t type_mask, StructuredData::DictionarySP args_data_sp, llvm::StringRef description); @@ -1055,22 +1061,15 @@ class Target : public std::enable_shared_from_this<Target>, void ClearBreakpointResolverOverrides() { m_breakpoint_overrides.clear(); } - lldb::BreakpointResolverSP - CheckBreakpointOverrides(lldb::BreakpointResolverSP original_sp) { - for (auto const &elem : m_breakpoint_overrides) { - if (lldb::BreakpointResolverSP overriden_sp = - elem.second->CheckForOverride(*this, original_sp)) - return overriden_sp; - } - return {}; - } + lldb::BreakpointResolverSP CheckBreakpointOverrides(lldb::BreakpointResolverSP original_sp); /// Describe the breakpoint overrides. If ixds is empty, list all. Otherwise /// list the overrides whose ids match the ones given in idxs. The matched /// elements are removed from the list, so any elements remaining in idxs are /// indexes that are not breakpoint override indexes. void DescribeBreakpointOverrides(Stream &stream, - std::vector<lldb::user_id_t> &idxs); + std::vector<lldb::user_id_t> &idxs, + uint32_t terminal_width, bool use_color); // The flag 'end_to_end', default to true, signifies that the operation is // performed end to end, for both the debugger and the debuggee. diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index a40788fca89e5..b78a3c8cf7053 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -691,6 +691,7 @@ enum CommandArgumentType { eArgTypeExceptionStage, eArgTypeNameMatchStyle, eArgTypePluginDomain, + eArgTypeBreakpointResolverMask, eArgTypeLastArg // Always keep this entry as the last entry in this // enumeration!! }; @@ -1479,6 +1480,21 @@ enum BinaryInformationLevel { eBinaryInformationLevelFull }; +/// This reflects the BreakpointResolver::ResolverTy, but this is a convenient +/// enum for making a mask to pass to RegisterOverrideResolver. It has to be +/// kept in sync with the ResolverTy. + +enum BreakpointResolverType { + eResolverUnknown = 0, + eResolverFileAndLine = (1 << 0), + eResolverAddress = (1 << 1), + eResolverName = (1 << 2), + eResolverFileRegex = (1 << 3), + eResolverPython = (1 << 4), + eResolverException = (1 << 5), + eResolverLastKnown = eResolverException, +}; + } // namespace lldb #endif // LLDB_LLDB_ENUMERATIONS_H diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 72286336102d2..63ae509498b22 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -683,6 +683,7 @@ size_t SBTarget::ReadMemory(const SBAddress addr, void *buf, size_t size, uint64_t SBTarget::AddBreakpointOverride(const char *class_name, const char *description, + uint64_t type_mask, SBStructuredData &args_data, SBError &error) { if (!class_name || class_name[0] == '\0') { @@ -703,7 +704,7 @@ uint64_t SBTarget::AddBreakpointOverride(const char *class_name, llvm::Expected<lldb::user_id_t> id_or_err = target_sp->AddBreakpointResolverOverride( - class_name, args_dict, + class_name, type_mask, args_dict, description ? description : "<No Description>"); if (id_or_err) return *id_or_err; diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp index 4ac40501a5df5..c289afbb865aa 100644 --- a/lldb/source/Breakpoint/BreakpointResolver.cpp +++ b/lldb/source/Breakpoint/BreakpointResolver.cpp @@ -39,6 +39,7 @@ const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address", "SymbolName", "SourceRegex", "Python", "Exception", "Unknown"}; + const char *BreakpointResolver::g_option_names[static_cast<uint32_t>( BreakpointResolver::OptionNames::LastOptionName)] = { @@ -63,6 +64,49 @@ BreakpointResolver::NameToResolverTy(llvm::StringRef name) { return UnknownResolver; } +bool BreakpointResolver::ResolverTyInMask(uint64_t mask) { + if (mask == eResolverUnknown) + return false; + + return !((mask & MaskForResolverTy()) == 0); +} + +uint64_t BreakpointResolver::MaskForResolverTy() { + ResolverTy thisID = GetResolverTy(); + + if (thisID == FileLineResolver) + return eResolverFileAndLine; + if (thisID == AddressResolver) + return eResolverAddress; + if (thisID == NameResolver) + return eResolverName; + if (thisID == FileRegexResolver) + return eResolverFileRegex; + if (thisID == PythonResolver) + return eResolverPython; + if (thisID == ExceptionResolver) + return eResolverException; + return eResolverUnknown; +} + +std::string BreakpointResolver::DescribeMask(uint64_t mask) { + std::string result; + if (mask & eResolverFileAndLine) + result.push_back('F'); + if (mask & eResolverAddress) + result.push_back('A'); + if (mask & eResolverName) + result.push_back('N'); + if (mask & eResolverFileRegex) + result.push_back('S'); + if (mask & eResolverPython) + result.push_back('P'); + if (mask & eResolverException) + result.push_back('E'); + return result; + +} + BreakpointResolver::BreakpointResolver(const BreakpointSP &bkpt, const unsigned char resolverTy, lldb::addr_t offset, diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index f11af5fff2362..35ac264073098 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -3632,6 +3632,14 @@ class CommandObjectBreakpointOverrideAdd : public CommandObjectParsed { case 'd': m_description.assign(std::string(option_arg)); break; + case 'm': { + uint64_t this_mask = (uint64_t) OptionArgParser::ToOptionEnum( + option_arg, g_breakpoint_override_add_options[option_idx].enum_values, + eResolverUnknown, error); + if (error.Success()) + m_mask |= this_mask; + } + break; default: llvm_unreachable("Unimplemented option"); } @@ -3641,6 +3649,7 @@ class CommandObjectBreakpointOverrideAdd : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_description.clear(); + m_mask = 0; } llvm::ArrayRef<OptionDefinition> GetDefinitions() override { @@ -3650,6 +3659,7 @@ class CommandObjectBreakpointOverrideAdd : public CommandObjectParsed { // Instance variables to hold the values for command options. std::string m_description; + uint64_t m_mask; }; Options *GetOptions() override { return &m_all_options; } @@ -3657,8 +3667,11 @@ class CommandObjectBreakpointOverrideAdd : public CommandObjectParsed { void DoExecute(Args &command, CommandReturnObject &result) override { Target *target = m_dummy_options.m_use_dummy ? &GetDummyTarget() : GetTarget(); + // If no mask was provided, then the default is file and line: + if (m_options.m_mask == 0) + m_options.m_mask = eResolverFileAndLine; llvm::Expected<lldb::user_id_t> id = target->AddBreakpointResolverOverride( - m_python_class_options.GetName(), + m_python_class_options.GetName(), m_options.m_mask, m_python_class_options.GetStructuredData(), m_options.m_description); if (id) { result.AppendMessageWithFormatv("{0}", *id); @@ -3765,7 +3778,10 @@ class CommandObjectBreakpointOverrideList : public CommandObjectParsed { } } } - target->DescribeBreakpointOverrides(result.GetOutputStream(), idxs); + target->DescribeBreakpointOverrides(result.GetOutputStream(), idxs, + GetCommandInterpreter().GetDebugger().GetTerminalWidth(), + GetCommandInterpreter().GetDebugger().GetUseColor()); + if (idxs.empty()) { result.SetStatus(eReturnStatusSuccessFinishResult); } else { diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 1f525c07f852a..35a9528bffbbd 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -488,6 +488,11 @@ let Command = "breakpoint add scripted" in { let Command = "breakpoint override add" in { def breakpoint_override_add_description : Option<"description", "d">, Desc<"Description string for this override resolver">, Arg<"HelpText">; + def breakpoint_override_add_mask : Option<"resolver-type-mask", "m">, + Desc<"Specifies which breakpoint resolver type the override operates on. " + "The option can be repeated, lldb will 'and' the options together. " + "If no options are specified, the default value is 'file_and_line'">, + EnumArg<"BreakpointResolverMask">; } let Command = "breakpoint clear" in { diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index f654f267b9f56..146f9ec43fe9f 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -62,6 +62,7 @@ #include "lldb/Target/Thread.h" #include "lldb/Target/ThreadSpec.h" #include "lldb/Target/UnixSignals.h" +#include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/Event.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/LLDBAssert.h" @@ -953,6 +954,7 @@ void Target::GetBreakpointNames(std::vector<std::string> &names) { llvm::Expected<lldb::user_id_t> Target::AddBreakpointResolverOverride(llvm::StringRef class_name, + uint64_t type_mask, StructuredData::DictionarySP args_data_sp, llvm::StringRef description) { if (class_name.empty()) @@ -963,7 +965,7 @@ Target::AddBreakpointResolverOverride(llvm::StringRef class_name, BreakpointResolverOverrideUP new_override_up( new ScriptedBreakpointResolverOverride(*this, std::string(description), - std::string(class_name), impl)); + type_mask, std::string(class_name), impl)); llvm::Error error = new_override_up->Validate(); if (error) return error; @@ -971,8 +973,12 @@ Target::AddBreakpointResolverOverride(llvm::StringRef class_name, return AddBreakpointResolverOverride(std::move(new_override_up)); } +std::string Target::BreakpointResolverOverride::DescribeTypeMask() { + return BreakpointResolver::DescribeMask(m_type_mask); +} + void Target::DescribeBreakpointOverrides(Stream &stream, - std::vector<lldb::user_id_t> &idxs) { + std::vector<lldb::user_id_t> &idxs, uint32_t output_width, bool use_color) { if (m_breakpoint_overrides.size() == 0) { stream << "No overrides.\n"; return; @@ -984,12 +990,17 @@ void Target::DescribeBreakpointOverrides(Stream &stream, auto idx_pos = llvm::find(idxs, elem.first); if (empty || idx_pos != idxs.end()) { if (print_first) { - // FIXME: Is there some good way to flow the description? - stream << "ID Description\n"; - stream << "---- -----------\n"; + + ansi::OutputWordWrappedLines(stream, "ID Mask Description\n", + output_width, use_color); + ansi::OutputWordWrappedLines(stream, "---- ------ -----------\n", + output_width, use_color); print_first = false; } - stream.Format("{0,4} {1}\n", elem.first, elem.second->GetDescription()); + auto content = llvm::formatv("{0,4} {1,6} {2}\n", elem.first, + elem.second->DescribeTypeMask(), + elem.second->GetDescription()).str(); + ansi::OutputWordWrappedLines(stream, content, output_width, use_color); if (!empty) idxs.erase(idx_pos); } @@ -1000,6 +1011,19 @@ bool Target::ProcessIsValid() { return (m_process_sp && m_process_sp->IsAlive()); } +lldb::BreakpointResolverSP +Target::CheckBreakpointOverrides(lldb::BreakpointResolverSP original_sp) { + for (auto const &elem : m_breakpoint_overrides) { + if (!original_sp->ResolverTyInMask(elem.second->GetTypeMask())) + continue; + lldb::BreakpointResolverSP overriden_sp; + overriden_sp = elem.second->CheckForOverride(*this, original_sp); + if (overriden_sp) + return overriden_sp; + } + return {}; +} + static bool CheckIfWatchpointsSupported(Target *target, Status &error) { std::optional<uint32_t> num_supported_hardware_watchpoints = target->GetProcessSP()->GetWatchpointSlotCount(); diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py index dda9822f550d1..0c637b6c3bf67 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py @@ -38,11 +38,22 @@ def import_resolver_script(self): command = "command script import " + script_name self.runCmd(command) - def add_override(self, use_cmd, help_text, class_name, key, value): + def add_override(self, use_cmd, help_text, class_name, mask, key, value): if use_cmd: result = lldb.SBCommandReturnObject() + # Convert the mask to command arguments. I just do the ones I + # am using in the test. + mask_args = "" + mask_str = "" + if mask & lldb.eResolverFileAndLine: + mask_args += "-m file_and_line " + mask_str += "F" + if mask & lldb.eResolverName: + mask_args += "-m name " + mask_str += "N" + self.ci.HandleCommand( - f"breakpoint override add -P {class_name} -k {key} -v {value} -d '{help_text}'", + f"breakpoint override add -P {class_name} -k {key} -v {value} -d '{help_text}' {mask_args}", result, ) self.assertCommandReturn(result, "breakpoint override worked") @@ -53,12 +64,12 @@ def add_override(self, use_cmd, help_text, class_name, key, value): extra_args.SetFromJSON(json_str) error = lldb.SBError() override_id = target.AddBreakpointOverride( - class_name, help_text, extra_args, error + class_name, help_text, mask, extra_args, error ) self.assertError(error, "Made the override successfully") # Check the override listing, make sure our new entry is present: - self.expect("breakpoint override list", substrs=[str(override_id), help_text]) + self.expect("breakpoint override list", substrs=[str(override_id), mask_str, help_text]) return override_id @@ -73,6 +84,7 @@ def do_test(self, use_cmd): use_cmd, trivial_help, "bkpt_resolver.TrivialExample", + lldb.eResolverName, "test_key", "test_value", ) @@ -82,6 +94,7 @@ def do_test(self, use_cmd): use_cmd, useful_help, "bkpt_resolver.OverrideExample", + lldb.eResolverFileAndLine, "symbol", "stop_here_instead", ) @@ -107,11 +120,14 @@ def do_test(self, use_cmd): ) # Now make a breakpoint by file and line: - # FIXME: Use source_line to find this line number: bkpt = target.BreakpointCreateByLocation( "main.c", line_number("main.c", "I am in the stop symbol") ) self.assertEqual(bkpt.GetNumLocations(), 1, "We make one location") + # Make sure that the override was called but trivial was not: + self.expect("checker override", startstr="1") + self.expect("checker trivial", startstr="0") + # Now continue and we'll hit this breakpoint but not in the # right place: (target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run( @@ -127,10 +143,15 @@ def do_test(self, use_cmd): func_name, alternate_location, "Stopped at overridden location" ) - # Now set a source name breakpoint, that should not get overridden, and + # Now set a symbol name breakpoint, that should not get overridden, and # when we continue we should hit it: name_bkpt = target.BreakpointCreateByName("change_him") self.assertGreater(name_bkpt.GetNumLocations(), 0, "Found locations") + # Now we've made one by name and one file and line breakpoint so both + # override functions should have been called. + self.expect("checker trivial", startstr="1") + self.expect("checker override", startstr="1") + threads = lldbutil.continue_to_breakpoint(process, name_bkpt) self.assertEqual(len(threads), 1, "Hit our name breakpoint") func_name = threads[0].frames[0].name @@ -157,8 +178,19 @@ def do_test(self, use_cmd): new_bkpt = target.BreakpointCreateByLocation( "main.c", line_number("main.c", "return 0") ) + # Neither override should have been called for this breakpoint + # so the counts should still be at 1 each: + self.expect("checker trivial", startstr="1") + self.expect("checker override", startstr="1") + self.assertEqual(new_bkpt.num_locations, 1, "Made breakpoint") threads = lldbutil.continue_to_breakpoint(process, new_bkpt) self.assertEqual(len(threads), 1, "Hit our new breakpoint") func_name = threads[0].frames[0].name self.assertEqual(func_name, "main", "Stopped in unchanged location") + + # Finally, make sure neither of the overrides was called with a + # type that was not part of the mask. + self.expect("checker trivial_not_name", startstr="0") + self.expect("checker override_not_file", startstr="0") + diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py index 181e2100444f8..68c1d5b61fa04 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py @@ -1,5 +1,41 @@ import lldb +# These count how many times the resolver override was called. +# Used to check that the masks actually work. + +override_count = 0 +trivial_count = 0 +override_not_file = 0 +trivial_not_name = 0 + +class CheckerCommand: + def __init__(self, debugger, internal_dict): + self.debugger = debugger + def get_short_help(self): + return "A command the checks how many times the resolvers were called" + def __call__(self, debugger, command, exe_ctx, result): + global override_count + global trivial_count + global override_not_file + global trivial_not_name + + result.SetStatus(lldb.eReturnStatusSuccessFinishResult) + if command == "trivial": + result.AppendMessage(str(trivial_count)) + return + if command == "override": + result.AppendMessage(str(override_count)) + return + if command == "override_not_file": + result.AppendMessage(str(override_not_file)) + return + if command == "trivial_not_name": + result.AppendMessage(str(trivial_not_name)) + return + + result.AppendError(f"unknown check type: {command}") + + class OverrideExample: def __init__( @@ -34,12 +70,20 @@ def set_breakpoint(self, bkpt: lldb.SBBreakpoint): def overrides_resolver( self, target: lldb.SBTarget, initial_resolver: lldb.SBStructuredData ): + global override_count + global override_not_file + + override_count += 1 + strm = lldb.SBStream() + initial_resolver.GetAsJSON(strm) type = initial_resolver.GetValueForKey("Type").GetStringValue(1000) if type == "FileAndLine": return True - return False + else: + override_not_file += 1 + return False class TrivialExample: @@ -63,5 +107,20 @@ def set_breakpoint(self, bkpt: lldb.SBBreakpoint): def overrides_resolver( self, target: lldb.SBTarget, initial_resolver: lldb.SBStructuredData ): + global trivial_count + global trivial_not_name + trivial_count += 1 + + strm = lldb.SBStream() + + initial_resolver.GetAsJSON(strm) + type = initial_resolver.GetValueForKey("Type").GetStringValue(1000) + if type != "SymbolName": + trivial_not_name += 1 + """Trivial - overrides nothing""" return False + +def __lldb_init_module(debugger, dict): + print(f"About to run: command script add -c {__name__}.CheckerCommand checker") + debugger.HandleCommand(f"command script add -c {__name__}.CheckerCommand checker") >From fd6a41982126310d0d0ae63e86d11e70b9b3dbc4 Mon Sep 17 00:00:00 2001 From: Jim Ingham <[email protected]> Date: Wed, 20 May 2026 14:26:07 -0700 Subject: [PATCH 2/4] Formatting --- .../lldb/Breakpoint/BreakpointResolver.h | 8 +++---- .../ScriptedBreakpointOverrideResolver.h | 2 +- .../Interpreter/CommandOptionArgumentTable.h | 3 +-- lldb/include/lldb/Target/Target.h | 16 +++++++------ lldb/source/Breakpoint/BreakpointResolver.cpp | 8 +++---- .../Commands/CommandObjectBreakpoint.cpp | 19 ++++++++------- lldb/source/Target/Target.cpp | 24 ++++++++++--------- .../TestOverridesResolver.py | 11 +++++---- .../overrides_resolver/bkpt_resolver.py | 21 +++++++++------- 9 files changed, 59 insertions(+), 53 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index f27854812cb92..c0b45cd62e30a 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -163,10 +163,10 @@ class BreakpointResolver : public Searcher { return ResolverTy::UnknownResolver; return (enum ResolverTy)SubclassID; } - + uint64_t MaskForResolverTy(); - - /// Returns true if this resolver is in the mask (made of elements of + + /// Returns true if this resolver is in the mask (made of elements of /// BreakpointResolverType. bool ResolverInMask(uint64_t mask); static std::string DescribeMask(uint64_t mask); @@ -175,7 +175,7 @@ class BreakpointResolver : public Searcher { static const char *ResolverTyToName(enum ResolverTy); static ResolverTy NameToResolverTy(llvm::StringRef name); - + virtual lldb::BreakpointResolverSP CopyForBreakpoint(lldb::BreakpointSP &breakpoint) = 0; diff --git a/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h b/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h index 17b872d7e1bdb..f787421b41303 100644 --- a/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h +++ b/lldb/include/lldb/Breakpoint/ScriptedBreakpointOverrideResolver.h @@ -30,7 +30,7 @@ class ScriptedBreakpointResolverOverride CopyIntoNewTarget(Target &target) override { return Target::BreakpointResolverOverrideUP( new ScriptedBreakpointResolverOverride(target, m_desc, m_type_mask, - m_class_name, m_args_data)); + m_class_name, m_args_data)); } lldb::BreakpointResolverSP diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index 996e109b56627..abb056d9d51df 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -188,8 +188,7 @@ static constexpr OptionEnumValueElement g_resolver_mask_values[] = { "Matches file and line breakpoint resolvers."}, {lldb::eResolverAddress, "address", "Matches address breakpoint resolvers."}, - {lldb::eResolverName, "name", - "Matches symbol name breakpoint resolvers."}, + {lldb::eResolverName, "name", "Matches symbol name breakpoint resolvers."}, {lldb::eResolverFileRegex, "source", "Matches source text pattern resolvers."}, {lldb::eResolverPython, "scripted", diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index bb032b2347e4f..281dee2a64695 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1009,17 +1009,18 @@ class Target : public std::enable_shared_from_this<Target>, // the original one. /// This is the abstract version of the override. Particular implementations - /// e.g. the scripted override resolve instantiate actual versions of the class. - /// The constructor takes the target this resolver is registered in, a + /// e.g. the scripted override resolve instantiate actual versions of the + /// class. The constructor takes the target this resolver is registered in, a /// description for the override and a mask of the resolver types this - /// overrides, made of elements of the BreakpointResolverType enum. + /// overrides, made of elements of the BreakpointResolverType enum. class BreakpointResolverOverride; using BreakpointResolverOverrideUP = std::unique_ptr<BreakpointResolverOverride>; class BreakpointResolverOverride { public: - BreakpointResolverOverride(Target &target, const std::string &description, uint64_t type_mask) + BreakpointResolverOverride(Target &target, const std::string &description, + uint64_t type_mask) : m_target(target), m_desc(description), m_type_mask(type_mask) {} virtual BreakpointResolverOverrideUP CopyIntoNewTarget(Target &target) = 0; @@ -1032,6 +1033,7 @@ class Target : public std::enable_shared_from_this<Target>, const std::string &GetDescription() { return m_desc; } uint64_t GetTypeMask() { return m_type_mask; } std::string DescribeTypeMask(); + protected: Target &m_target; std::string m_desc; @@ -1049,8 +1051,7 @@ class Target : public std::enable_shared_from_this<Target>, /// Add a breakpoint override resolver. Return the ID or an error: llvm::Expected<lldb::user_id_t> - AddBreakpointResolverOverride(llvm::StringRef class_name, - uint64_t type_mask, + AddBreakpointResolverOverride(llvm::StringRef class_name, uint64_t type_mask, StructuredData::DictionarySP args_data_sp, llvm::StringRef description); @@ -1061,7 +1062,8 @@ class Target : public std::enable_shared_from_this<Target>, void ClearBreakpointResolverOverrides() { m_breakpoint_overrides.clear(); } - lldb::BreakpointResolverSP CheckBreakpointOverrides(lldb::BreakpointResolverSP original_sp); + lldb::BreakpointResolverSP + CheckBreakpointOverrides(lldb::BreakpointResolverSP original_sp); /// Describe the breakpoint overrides. If ixds is empty, list all. Otherwise /// list the overrides whose ids match the ones given in idxs. The matched diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp index c289afbb865aa..1c2b678083554 100644 --- a/lldb/source/Breakpoint/BreakpointResolver.cpp +++ b/lldb/source/Breakpoint/BreakpointResolver.cpp @@ -39,7 +39,6 @@ const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address", "SymbolName", "SourceRegex", "Python", "Exception", "Unknown"}; - const char *BreakpointResolver::g_option_names[static_cast<uint32_t>( BreakpointResolver::OptionNames::LastOptionName)] = { @@ -95,16 +94,15 @@ std::string BreakpointResolver::DescribeMask(uint64_t mask) { result.push_back('F'); if (mask & eResolverAddress) result.push_back('A'); - if (mask & eResolverName) + if (mask & eResolverName) result.push_back('N'); if (mask & eResolverFileRegex) result.push_back('S'); - if (mask & eResolverPython) + if (mask & eResolverPython) result.push_back('P'); - if (mask & eResolverException) + if (mask & eResolverException) result.push_back('E'); return result; - } BreakpointResolver::BreakpointResolver(const BreakpointSP &bkpt, diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index 35ac264073098..5a083c1927f9b 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -3633,13 +3633,13 @@ class CommandObjectBreakpointOverrideAdd : public CommandObjectParsed { m_description.assign(std::string(option_arg)); break; case 'm': { - uint64_t this_mask = (uint64_t) OptionArgParser::ToOptionEnum( - option_arg, g_breakpoint_override_add_options[option_idx].enum_values, + uint64_t this_mask = (uint64_t)OptionArgParser::ToOptionEnum( + option_arg, + g_breakpoint_override_add_options[option_idx].enum_values, eResolverUnknown, error); - if (error.Success()) - m_mask |= this_mask; - } - break; + if (error.Success()) + m_mask |= this_mask; + } break; default: llvm_unreachable("Unimplemented option"); } @@ -3778,9 +3778,10 @@ class CommandObjectBreakpointOverrideList : public CommandObjectParsed { } } } - target->DescribeBreakpointOverrides(result.GetOutputStream(), idxs, - GetCommandInterpreter().GetDebugger().GetTerminalWidth(), - GetCommandInterpreter().GetDebugger().GetUseColor()); + target->DescribeBreakpointOverrides( + result.GetOutputStream(), idxs, + GetCommandInterpreter().GetDebugger().GetTerminalWidth(), + GetCommandInterpreter().GetDebugger().GetUseColor()); if (idxs.empty()) { result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 146f9ec43fe9f..e13ef8a830edb 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -952,11 +952,9 @@ void Target::GetBreakpointNames(std::vector<std::string> &names) { llvm::sort(names); } -llvm::Expected<lldb::user_id_t> -Target::AddBreakpointResolverOverride(llvm::StringRef class_name, - uint64_t type_mask, - StructuredData::DictionarySP args_data_sp, - llvm::StringRef description) { +llvm::Expected<lldb::user_id_t> Target::AddBreakpointResolverOverride( + llvm::StringRef class_name, uint64_t type_mask, + StructuredData::DictionarySP args_data_sp, llvm::StringRef description) { if (class_name.empty()) return LLDB_INVALID_INDEX64; @@ -965,7 +963,8 @@ Target::AddBreakpointResolverOverride(llvm::StringRef class_name, BreakpointResolverOverrideUP new_override_up( new ScriptedBreakpointResolverOverride(*this, std::string(description), - type_mask, std::string(class_name), impl)); + type_mask, std::string(class_name), + impl)); llvm::Error error = new_override_up->Validate(); if (error) return error; @@ -978,7 +977,9 @@ std::string Target::BreakpointResolverOverride::DescribeTypeMask() { } void Target::DescribeBreakpointOverrides(Stream &stream, - std::vector<lldb::user_id_t> &idxs, uint32_t output_width, bool use_color) { + std::vector<lldb::user_id_t> &idxs, + uint32_t output_width, + bool use_color) { if (m_breakpoint_overrides.size() == 0) { stream << "No overrides.\n"; return; @@ -992,14 +993,15 @@ void Target::DescribeBreakpointOverrides(Stream &stream, if (print_first) { ansi::OutputWordWrappedLines(stream, "ID Mask Description\n", - output_width, use_color); + output_width, use_color); ansi::OutputWordWrappedLines(stream, "---- ------ -----------\n", - output_width, use_color); + output_width, use_color); print_first = false; } auto content = llvm::formatv("{0,4} {1,6} {2}\n", elem.first, - elem.second->DescribeTypeMask(), - elem.second->GetDescription()).str(); + elem.second->DescribeTypeMask(), + elem.second->GetDescription()) + .str(); ansi::OutputWordWrappedLines(stream, content, output_width, use_color); if (!empty) idxs.erase(idx_pos); diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py index 0c637b6c3bf67..a9d7e7c456439 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py @@ -69,7 +69,9 @@ def add_override(self, use_cmd, help_text, class_name, mask, key, value): self.assertError(error, "Made the override successfully") # Check the override listing, make sure our new entry is present: - self.expect("breakpoint override list", substrs=[str(override_id), mask_str, help_text]) + self.expect( + "breakpoint override list", substrs=[str(override_id), mask_str, help_text] + ) return override_id @@ -127,7 +129,7 @@ def do_test(self, use_cmd): # Make sure that the override was called but trivial was not: self.expect("checker override", startstr="1") self.expect("checker trivial", startstr="0") - + # Now continue and we'll hit this breakpoint but not in the # right place: (target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run( @@ -151,7 +153,7 @@ def do_test(self, use_cmd): # override functions should have been called. self.expect("checker trivial", startstr="1") self.expect("checker override", startstr="1") - + threads = lldbutil.continue_to_breakpoint(process, name_bkpt) self.assertEqual(len(threads), 1, "Hit our name breakpoint") func_name = threads[0].frames[0].name @@ -182,7 +184,7 @@ def do_test(self, use_cmd): # so the counts should still be at 1 each: self.expect("checker trivial", startstr="1") self.expect("checker override", startstr="1") - + self.assertEqual(new_bkpt.num_locations, 1, "Made breakpoint") threads = lldbutil.continue_to_breakpoint(process, new_bkpt) self.assertEqual(len(threads), 1, "Hit our new breakpoint") @@ -193,4 +195,3 @@ def do_test(self, use_cmd): # type that was not part of the mask. self.expect("checker trivial_not_name", startstr="0") self.expect("checker override_not_file", startstr="0") - diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py index 68c1d5b61fa04..b25074bc75084 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py @@ -8,17 +8,20 @@ override_not_file = 0 trivial_not_name = 0 + class CheckerCommand: def __init__(self, debugger, internal_dict): self.debugger = debugger + def get_short_help(self): return "A command the checks how many times the resolvers were called" + def __call__(self, debugger, command, exe_ctx, result): global override_count global trivial_count global override_not_file global trivial_not_name - + result.SetStatus(lldb.eReturnStatusSuccessFinishResult) if command == "trivial": result.AppendMessage(str(trivial_count)) @@ -32,10 +35,9 @@ def __call__(self, debugger, command, exe_ctx, result): if command == "trivial_not_name": result.AppendMessage(str(trivial_not_name)) return - + result.AppendError(f"unknown check type: {command}") - - + class OverrideExample: def __init__( @@ -72,11 +74,11 @@ def overrides_resolver( ): global override_count global override_not_file - + override_count += 1 strm = lldb.SBStream() - + initial_resolver.GetAsJSON(strm) type = initial_resolver.GetValueForKey("Type").GetStringValue(1000) if type == "FileAndLine": @@ -110,9 +112,9 @@ def overrides_resolver( global trivial_count global trivial_not_name trivial_count += 1 - + strm = lldb.SBStream() - + initial_resolver.GetAsJSON(strm) type = initial_resolver.GetValueForKey("Type").GetStringValue(1000) if type != "SymbolName": @@ -121,6 +123,7 @@ def overrides_resolver( """Trivial - overrides nothing""" return False + def __lldb_init_module(debugger, dict): print(f"About to run: command script add -c {__name__}.CheckerCommand checker") - debugger.HandleCommand(f"command script add -c {__name__}.CheckerCommand checker") + debugger.HandleCommand(f"command script add -c {__name__}.CheckerCommand checker") >From 4e6d79d9491cb019740e49c891eda466a2269388 Mon Sep 17 00:00:00 2001 From: jimingham <[email protected]> Date: Wed, 1 Jul 2026 11:36:19 -0700 Subject: [PATCH 3/4] Apply suggestion from @JDevlieghere Co-authored-by: Jonas Devlieghere <[email protected]> --- lldb/include/lldb/Breakpoint/BreakpointResolver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index c0b45cd62e30a..d4886fc564e56 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -155,7 +155,7 @@ class BreakpointResolver : public Searcher { unsigned getResolverID() const { return SubclassID; } /// This checks whether the resolver's type matches the enum - /// lldb::eBreakpointResolverType. + /// lldb::BreakpointResolverType. bool ResolverTyInMask(uint64_t mask); enum ResolverTy GetResolverTy() { >From 7a9b2bd2d79b160de1675cfcea72c435e689913e Mon Sep 17 00:00:00 2001 From: Jim Ingham <[email protected]> Date: Thu, 2 Jul 2026 10:46:05 -0700 Subject: [PATCH 4/4] Respond to review comments. Added more up-front error checking for the resolver mask, and added tests for error behavior. Added a "BreakpointResolverAllMasks" define so that if you want to make an override for all resolvers, you can. --- .../lldb/Breakpoint/BreakpointResolver.h | 5 +- lldb/include/lldb/Target/Target.h | 4 +- lldb/include/lldb/lldb-enumerations.h | 6 +- lldb/source/Breakpoint/BreakpointResolver.cpp | 9 +- lldb/source/Target/Target.cpp | 8 +- .../TestOverridesResolver.py | 201 +++++++++++++----- .../overrides_resolver/bkpt_resolver.py | 3 +- 7 files changed, 171 insertions(+), 65 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index b4c8b224a4329..d7256212865f9 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -157,6 +157,8 @@ class BreakpointResolver : public Searcher { /// lldb::BreakpointResolverType. bool ResolverTyInMask(uint64_t mask); + static bool TypeMaskIsValid(uint64_t mask); + enum ResolverTy GetResolverTy() { if (SubclassID > ResolverTy::LastKnownResolverType) return ResolverTy::UnknownResolver; @@ -165,9 +167,6 @@ class BreakpointResolver : public Searcher { uint64_t MaskForResolverTy(); - /// Returns true if this resolver is in the mask (made of elements of - /// BreakpointResolverType. - bool ResolverInMask(uint64_t mask); static std::string DescribeMask(uint64_t mask); const char *GetResolverName() { return ResolverTyToName(GetResolverTy()); } diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index a258837c45776..31e2cd0ae5018 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1010,8 +1010,8 @@ class Target : public std::enable_shared_from_this<Target>, // be the one we use. If no overrides return an override resolver, we'll use // the original one. - /// This is the abstract version of the override. Particular implementations - /// e.g. the scripted override resolve instantiate actual versions of the + /// This is the abstract version of the override. Particular implementations, + /// e.g. the scripted override resolver, instantiate actual versions of the /// class. The constructor takes the target this resolver is registered in, a /// description for the override and a mask of the resolver types this /// overrides, made of elements of the BreakpointResolverType enum. diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index c824abee076db..ee0d6d27b8bef 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1587,7 +1587,7 @@ enum BinaryInformationLevel { /// enum for making a mask to pass to RegisterOverrideResolver. It has to be /// kept in sync with the ResolverTy. -enum BreakpointResolverType { +FLAGS_ENUM(BreakpointResolverType) { eResolverUnknown = 0, eResolverFileAndLine = (1 << 0), eResolverAddress = (1 << 1), @@ -1597,6 +1597,10 @@ enum BreakpointResolverType { eResolverException = (1 << 5), eResolverLastKnown = eResolverException, }; +constexpr unsigned BreakpointResolverAllResolversMask = eResolverFileAndLine | + eResolverAddress | eResolverName | eResolverFileRegex | eResolverPython | + eResolverException; + } // namespace lldb diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp index 1c2b678083554..e1b20fd4d78e0 100644 --- a/lldb/source/Breakpoint/BreakpointResolver.cpp +++ b/lldb/source/Breakpoint/BreakpointResolver.cpp @@ -67,7 +67,7 @@ bool BreakpointResolver::ResolverTyInMask(uint64_t mask) { if (mask == eResolverUnknown) return false; - return !((mask & MaskForResolverTy()) == 0); + return (mask & MaskForResolverTy()) != 0; } uint64_t BreakpointResolver::MaskForResolverTy() { @@ -105,6 +105,13 @@ std::string BreakpointResolver::DescribeMask(uint64_t mask) { return result; } +bool BreakpointResolver::TypeMaskIsValid(uint64_t mask) { + if (mask == 0) + return false; + return (mask & BreakpointResolverAllResolversMask) != 0; +} + + BreakpointResolver::BreakpointResolver(const BreakpointSP &bkpt, const unsigned char resolverTy, lldb::addr_t offset, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index c250ba7af33a7..4f55fc0ed0700 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -956,7 +956,13 @@ llvm::Expected<lldb::user_id_t> Target::AddBreakpointResolverOverride( llvm::StringRef class_name, uint64_t type_mask, StructuredData::DictionarySP args_data_sp, llvm::StringRef description) { if (class_name.empty()) - return LLDB_INVALID_INDEX64; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "empty class name"); + + if (!BreakpointResolver::TypeMaskIsValid(type_mask)) + return llvm::createStringErrorV(llvm::inconvertibleErrorCode(), + "invalid breakpoint type mask: {0}, should be composed of the " + "elements of the BreakpointResolverType enum.", type_mask); StructuredDataImpl impl; impl.SetObjectSP(args_data_sp); diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py index a9d7e7c456439..4072bbcc2328c 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/TestOverridesResolver.py @@ -22,12 +22,16 @@ def test_overrides_resolver_resolver_python(self): def test_overrides_resolver_resolver_cmd(self): """Use facade breakpoints to emulate hitting some locations""" self.build() - self.do_test(True) + self.do_test(False) + + def test_bad_values(self): + self.build() + self.do_test_bad_values() def make_target_and_import(self): - target = lldbutil.run_to_breakpoint_make_target(self) + self.target = lldbutil.run_to_breakpoint_make_target(self) self.import_resolver_script() - return target + return self.target def import_resolver_script(self): interp = self.dbg.GetCommandInterpreter() @@ -38,39 +42,54 @@ def import_resolver_script(self): command = "command script import " + script_name self.runCmd(command) - def add_override(self, use_cmd, help_text, class_name, mask, key, value): - if use_cmd: - result = lldb.SBCommandReturnObject() - # Convert the mask to command arguments. I just do the ones I - # am using in the test. - mask_args = "" - mask_str = "" - if mask & lldb.eResolverFileAndLine: - mask_args += "-m file_and_line " - mask_str += "F" - if mask & lldb.eResolverName: - mask_args += "-m name " - mask_str += "N" - - self.ci.HandleCommand( - f"breakpoint override add -P {class_name} -k {key} -v {value} -d '{help_text}' {mask_args}", - result, - ) + + def calculate_mask_desc(self, mask): + mask_str = "" + if mask & lldb.eResolverFileAndLine: + mask_str += "F" + if mask & lldb.eResolverName: + mask_str += "N" + return mask_str + + def add_override_python(self, help_text, class_name, mask, key, value, expect_error=False, error_string=""): + extra_args = lldb.SBStructuredData() + json_str = '{"' + str(key) + '":"' + str(value) + '"}' + extra_args.SetFromJSON(json_str) + error = lldb.SBError() + override_id = self.target.AddBreakpointOverride( + class_name, help_text, mask, extra_args, error + ) + if not expect_error: + self.assertSuccess(error, "Made the override successfully") + else: + self.assertFailure(error, error_string) + return 0 + + # Check the override listing, make sure our new entry is present: + mask_str = self.calculate_mask_desc(mask) + + self.expect( + "breakpoint override list", substrs=[str(override_id), mask_str, help_text] + ) + + return override_id + + def add_override_cmd(self, help_text, class_name, mask_args, mask_desc, key, value, expect_error=False, error_string=""): + result = lldb.SBCommandReturnObject() + self.ci.HandleCommand( + f"breakpoint override add -P {class_name} -k {key} -v {value} -d '{help_text}' {mask_args}", + result, + ) + if not expect_error: self.assertCommandReturn(result, "breakpoint override worked") override_id = int(result.GetOutput()) else: - extra_args = lldb.SBStructuredData() - json_str = '{"' + key + '":"' + value + '"}' - extra_args.SetFromJSON(json_str) - error = lldb.SBError() - override_id = target.AddBreakpointOverride( - class_name, help_text, mask, extra_args, error - ) - self.assertError(error, "Made the override successfully") + self.assertFalse(result.Succeeded(), "We expected this to fail") + return 0 # Check the override listing, make sure our new entry is present: self.expect( - "breakpoint override list", substrs=[str(override_id), mask_str, help_text] + "breakpoint override list", substrs=[str(override_id), mask_desc, help_text] ) return override_id @@ -78,28 +97,49 @@ def add_override(self, use_cmd, help_text, class_name, mask, key, value): def do_test(self, use_cmd): """This reads in a python file and sets a breakpoint using it.""" alternate_location = "stop_here_instead" - target = self.make_target_and_import() - # Add out trivial one first so we test more than one list element: + self.make_target_and_import() + # Add our trivial one first so we test more than one list element: trivial_help = "Trivial help text" - trivial_id = self.add_override( - use_cmd, - trivial_help, - "bkpt_resolver.TrivialExample", - lldb.eResolverName, - "test_key", - "test_value", - ) + trivial_id = 0 + useful_id = 0 + + if use_cmd: + trivial_id = self.add_override_cmd( + trivial_help, + "bkpt_resolver.TrivialExample", + "-m name", + "N", + "test_key", + "test_value", + ) - useful_help = "SOME HELP TEXT" - useful_id = self.add_override( - use_cmd, - useful_help, - "bkpt_resolver.OverrideExample", - lldb.eResolverFileAndLine, - "symbol", - "stop_here_instead", - ) + useful_help = "SOME HELP TEXT" + useful_id = self.add_override_cmd( + useful_help, + "bkpt_resolver.OverrideExample", + "-m file_and_line", + "F", + "symbol", + "stop_here_instead", + ) + else: + trivial_id = self.add_override_python( + trivial_help, + "bkpt_resolver.TrivialExample", + lldb.eResolverName, + "test_key", + "test_value", + ) + + useful_help = "SOME HELP TEXT" + useful_id = self.add_override_python( + useful_help, + "bkpt_resolver.OverrideExample", + lldb.eResolverFileAndLine, + "symbol", + "stop_here_instead", + ) # Now exercise the list command by id: self.expect( @@ -122,7 +162,7 @@ def do_test(self, use_cmd): ) # Now make a breakpoint by file and line: - bkpt = target.BreakpointCreateByLocation( + bkpt = self.target.BreakpointCreateByLocation( "main.c", line_number("main.c", "I am in the stop symbol") ) self.assertEqual(bkpt.GetNumLocations(), 1, "We make one location") @@ -132,8 +172,8 @@ def do_test(self, use_cmd): # Now continue and we'll hit this breakpoint but not in the # right place: - (target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run( - self, target, bkpt + (_, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run( + self, self.target, bkpt ) # This location should be bkpt_no.1: self.assertEqual( @@ -147,7 +187,7 @@ def do_test(self, use_cmd): # Now set a symbol name breakpoint, that should not get overridden, and # when we continue we should hit it: - name_bkpt = target.BreakpointCreateByName("change_him") + name_bkpt = self.target.BreakpointCreateByName("change_him") self.assertGreater(name_bkpt.GetNumLocations(), 0, "Found locations") # Now we've made one by name and one file and line breakpoint so both # override functions should have been called. @@ -165,7 +205,7 @@ def do_test(self, use_cmd): self.runCmd(f"breakpoint override delete {useful_id}") else: self.assertTrue( - target.DeleteBreakpointOverride(useful_id), "Delete the right one" + self.target.RemoveBreakpointOverride(useful_id), "Delete the right one" ) # Make sure it's gone from the listings: @@ -177,7 +217,7 @@ def do_test(self, use_cmd): # And that listing it is an error: self.expect(f"breakpoint override list {useful_id}", error=True) - new_bkpt = target.BreakpointCreateByLocation( + new_bkpt = self.target.BreakpointCreateByLocation( "main.c", line_number("main.c", "return 0") ) # Neither override should have been called for this breakpoint @@ -195,3 +235,54 @@ def do_test(self, use_cmd): # type that was not part of the mask. self.expect("checker trivial_not_name", startstr="0") self.expect("checker override_not_file", startstr="0") + + def do_test_bad_values(self): + self.make_target_and_import() + # Add our trivial one with a bad mask: + + trivial_help = "Trivial help text" + bad_type_mask = lldb.BreakpointResolverValidMask + 1 + trivial_id = self.add_override_python( + trivial_help, + "bkpt_resolver.TrivialExample", + bad_type_mask, + "test_key", + "test_value", + expect_error=True, + error_string="invalid breakpoint type mask: 64, should be composed of the elements of the BreakpointResolverType enum." + ) + + # Now try with an empty class name: + trivial_id = self.add_override_python( + trivial_help, + "", + lldb.eResolverName, + "test_key", + "test_value", + expect_error=True, + error_string="empty class name" + ) + # And with a class name that doesn't exist: + #trivial_id = self.add_override_python( + # trivial_help, + # "NoModuleOfThisName.NoClassOfThisName", + # lldb.eResolverName, + # "test_key", + # "test_value", + # expect_error=True, + # error_string="" + #) + + trivial_id = self.add_override_cmd( + trivial_help, + "bkpt_resolver.TrivialExample", + "-m no_such_resolver", + "", + "test_key", + "test_value", + expect_error=True, + error_string="" + ) + + + diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py index b25074bc75084..ff7600fa30e61 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/overrides_resolver/bkpt_resolver.py @@ -14,7 +14,7 @@ def __init__(self, debugger, internal_dict): self.debugger = debugger def get_short_help(self): - return "A command the checks how many times the resolvers were called" + return "A command that checks how many times the resolvers were called" def __call__(self, debugger, command, exe_ctx, result): global override_count @@ -125,5 +125,4 @@ def overrides_resolver( def __lldb_init_module(debugger, dict): - print(f"About to run: command script add -c {__name__}.CheckerCommand checker") debugger.HandleCommand(f"command script add -c {__name__}.CheckerCommand checker") _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
