Add the ability to pass extra args to a Python breakpoint command function

[jingham]

Summary

For example, it is pretty easy to write a breakpoint command that implements "stop when my caller is Foo", and it is pretty easy to write a breakpoint command that implements "stop when my caller is Bar". But there's no way to write a generic "stop when my caller is..." function, and then specify the caller when you add the command. With this patch, you can pass this data in a SBStructuredData dictionary. That will get stored in the PythonCommandBaton for the breakpoint, and passed to the implementation function (if it has the right signature) when the breakpoint is hit. Then in lldb, you can say:

(lldb) break com add -F caller_is -k caller_name -v Foo

More generally this will allow us to write reusable Python breakpoint commands.

[shafik]

lldb/include/lldb/API/SBBreakpointLocation.h

59

Could this be a const &

lldb/include/lldb/API/SBBreakpointName.h

89

Could this be a const &

[clayborg]

Looks good overall. A few questions:

• should we do a global "s/extra_args/args/" edit in this patch? Not sure what "extra_" is adding? Can we remove?
• Can the structured data not be a dictionary? Valid StructuredData could be "malloc" or "[0x1000, 0x2000]" or "true", etc. If the data is required to be a dictionary we should provide lldb::SBError support for the new API calls to ensure we can let the user know what they did wrong. Be good to test this as well.
• is it possible to update the args for a callback function? That would seem like a good thing you might want to do? And if so, to test?
  • If it is possible to update the structured data, and if the structured data can be anything (array, string, boolean, dictionary), we might want an option like: --json "..." to be able to set or update from the command line? The current -k -v options seem to imply it must be a dict and only one level deep?

lldb/include/lldb/API/SBBreakpointLocation.h

59

const doesn't mean anything for lldb::SB arguments. Since they contain shared pointers or unique pointers, so just leave as is.

lldb/include/lldb/API/SBBreakpointName.h

89

const doesn't mean anything for lldb::SB arguments. Since they contain shared pointers or unique pointers, so just leave as is.

lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h

26–30

clang-format? Indent seems off (it was before too I see).

[jingham]

In D68671#1704803, @clayborg wrote:

Looks good overall. A few questions:

• should we do a global "s/extra_args/args/" edit in this patch? Not sure what "extra_" is adding? Can we remove?

Since we still support the form that doesn't take extra_args, they do feel "extra" to me. I also like the "extra" because it helps make it clear these are just free bits of data that the callback system just passes along. After all, there are some fixed arguments to the call back that do have specific meanings.

Note, I called the equivalent feature "extra_args" for the scripted breakpoint resolvers (added last year) and scripted thread plans, so if we change it here, we need to change it in those places as well. If we're going to do that, I'll do it as a follow on patch because I don't want to mix those changes in with this patch. But as I said, I like the "extra".

• Can the structured data not be a dictionary? Valid StructuredData could be "malloc" or "[0x1000, 0x2000]" or "true", etc. If the data is required to be a dictionary we should provide lldb::SBError support for the new API calls to ensure we can let the user know what they did wrong. Be good to test this as well.

The only place where I treat the extra_args as a Dictionary is when I use as a way to tell whether the extra_args is empty, which I need to write the right wrapper function in GenerateBreakpointCommandCallbackData. That function is little annoying because it doesn't know whether it is calling a function or some text gotten from the I/O handler, so I can't count the function arguments there to decide which signature to use.

Let me see if I can make this cleaner so I don't have to rely on that. I really don't intend to limit fancier uses of the extra args.

Note, the command-line doesn't allow you to make anything but a dictionary (through the -k -v option pairs). I'm actually happy with that, it enforces some kind of structure on the breakpoint command callback that you write. You'll thank me when you want to add a second or third parameter...

Plus once you write something you are going to use frequently, you can do:

(lldb) command alias caller_break breakpoint command add -F MyCommands.break_when_parent -k func_name -v %1
(lldb) caller_break Caller -n Callee

That's how you would really want to use it.

• is it possible to update the args for a callback function? That would seem like a good thing you might want to do? And if so, to test?

There's no way to do this. "breakpoint command add" is a bit poorly named, since it really overwrites the current command rather than adding to it. But that's the way it has always worked. So only giving you the option to provide a new pair {python function, extra_args} is in keeping with its behavior.

People have asked for a "breakpoint command modify" that could chain python callbacks, or incrementally add command-line commands. That would also naturally allow you to change/add to the extra args in place. But that's a whole other piece of work that I don't want to take on here.

• If it is possible to update the structured data, and if the structured data can be anything (array, string, boolean, dictionary), we might want an option like: --json "..." to be able to set or update from the command line? The current -k -v options seem to imply it must be a dict and only one level deep?

I thought about adding --json when making the OptionGroupPythonClassWithDict. The completionist side of me was sorely tempted, especially since it would be quite easy to do. But the options for "break set" are already several pages long, so I don't want to add more options to it just "because I can".

For all the use cases I can think of, a simple one level dictionary will suffice. If you want to do something fancier, you can make an arbitrarily complicated SBStructuredData in Python, and then use SBBreakpoint.SetScriptCallbackFunction to pass it in. And if you want to do that something fancier a lot, you can make this into a python command.

One useful enhancement that wouldn't overburden "break set" would be to call ParseJSON on the -v option value, and if that succeeded add the resultant StructuredData::ObjectSP as the value. That would allow you to pass lists in for the value, which does seem useful. You can of course do this yourself by getting the string value out in your callback and passing it to SBStructuredData.ParseFromJSON(). So I don't think it's important to add this now. I'd also need to assure myself that this wouldn't surprise people by munging up legitimate string values.

But anyway, I'd prefer to do that as a follow on, since I'm running out of time to play with this this time round the wheel...

[clayborg]

The only place where I treat the extra_args as a Dictionary is when I use as a way to tell whether the extra_args is empty, which I need to write the right wrapper function in GenerateBreakpointCommandCallbackData. That function is little annoying because it doesn't know whether it is calling a function or some text gotten from the I/O handler, so I can't count the function arguments there to decide which signature to use.

Let me see if I can make this cleaner so I don't have to rely on that. I really don't intend to limit fancier uses of the extra args.

Let me know if you are going to look into making this cleaner for this patch. If so, I will hold off, else I can accept if you are out of time

[jingham]

I added ScriptInterpreter::GetNumArgumentsForCallable, so I can base all the decisions on how to call the function on whether I was passed a function with 3 or 4 arguments. That's cleaner. I also plumbed an error through the callback setting, so I can report an error if you pass me a function with the wrong number of arguments, or provide extra_args when you are passing a function that doesn't take 4 arguments.

I added a check in the extant test to ensure that a function that takes extra_args will work with empty extra_args, since there's no reason to disallow that.

I also added a test for the error handling when you pass a function with the wrong number of arguments.

Now we don't depend on the SBStructuredData handed across the SB API's being any particular structure.

[labath]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

In light of varargs functions (*args, **kwargs), which are fairly popular in python, the concept of "number of arguments of a callable" does not seem that well defined. The current implementation seems to return the number of fixed arguments, which might be fine, but I think this behavior should be documented. Also, it would be good to modernize this function signature -- have it take a StringRef, and return a Expected<unsigned (?)> -- ongoing work by @lawrence_danna will make it possible to return errors from the python interpreter, and this will make it possible to display those, instead of just guessing that this is because the callable was not found (it could in fact be because the named thing is not a callable, of because resolving the name produced an exception, ...).

[jingham]

Addressed Pavel's comments.

I explicitly want the number of fixed arguments, so I changed the function name to GetNumFixedArguments. The docs say explicitly that you have to make the function take either three or four fixed arguments, and that's what I have to check against. I don't want to know about varargs and optional named arguments.

I added the Expected, though I didn't plumb the change into PythonCallable::GetNumArguments. That's of marginal extra benefit, and orthogonal to the purposes of this patch.

Note also, the fact that I was now checking argument signatures when adding the command means you can no longer do:

(lldb) break command add -F myfunc.function
(lldb) command script import myfunc.py

which TestBreakpointCommand.py was doing. You have to make the Python function available BEFORE adding it to the breakpoint. I don't think this was an feature, and I don't see any harm in adding that requirement. But it is a change in behavior so I thought I'd bring it up.

[lawrence_danna]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

I just took a look at PythonCallable::GetNumArguments() and it's horribly broken.

It doesn't even work for the simplest test case I could think of.

auto builtins = PythonModule::Import("builtins");
ASSERT_THAT_EXPECTED(builtins, llvm::Succeeded());
auto hex = As<PythonCallable>(builtins.get().GetAttribute("hex"));
ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());  
EXPECT_EQ(hex.get().GetNumArguments().count, 1u);

we should really re-write it to use inspect.signature.

[labath]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

lol :)

I would actually say that we should try not to use this function(ality) wherever possible. Making decisions based on the number of arguments the thing you're about to call takes sounds weird. I don't want to get too involved in this, but I was designing this, I'd just say that if one tries to pass arguments to the callback then the callback MUST take three arguments (or we'll abort processing the breakpoint command). If he wants his function to be called both with arguments and without, he can just add a default value to the third argument. (And if his function takes two arguments, but he still tries to pass something... well, it's his own fault).

Anyway, feel free to ignore this comment, but I felt like I had to say something. :)

[lawrence_danna]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

I completely agree with Pavel. Inspecting a function signature before calling it is a big code smell in python. If there's a way to avoid doing that introspection, that would be better.

[jingham]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

Interesting. We use it for free functions (what you pass to the -F option of breakpoint command add) and for the init and call method in the little classes you can make up for scripted thread plans and for the class version of Python implemented command-line commands. We have tests for telling 3 vrs. 4 vrs. not enough or too many, and they all pass. So it seems to work in the cases we currently need it to work for...

"inspect.signature" is python 3 only, and the python 2 equivalent is deprecated. So it will take a little fancy footwork to use it in the transition period.

469

Unfortunately, we originally designed this interface to take three arguments, the frame pointer, the breakpoint location and the Python session dict. Then it became clear that it would be better to add this extra args argument (and in the case of Python based commands the ExecutionContext pointer). At that point we had three choices, abandon the improvement; switch to unconditionally passing the extra arguments, and break everybody's extant uses; or switch off of the number of arguments to decide whether the user had provided the old or new forms.

My feeling about lldb Python scripts/commands etc. that people use in the debugger is that a lot of users don't know how they work at all, they just got them from somebody else; and many more figured out how to write them for some specific purpose, and then pretty much forgot how they worked. So suddenly breaking all these bits of functionality will result in folks just deciding that this affordance is not reliable and not worth their time, which would be a shame.

So instead we accommodate both forms, which requires that we know which one the user provided. If you see a better way to do this, (and are willing to implement it because so far as I can see this method is going to work just fine) dig in, I'm not wedded to the particular approach. But I am not excited about penalizing our users because we didn't get the API design right the first time through.

[lawrence_danna]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

makes sense.

The only other way I can think of to solve it would be to have some indication in the break com add command of what signature it expects from the function. But that's really ugly too because now you're asking users to understand yet another option.

I put up https://reviews.llvm.org/D68995 this morning which adds inspect.signature support for pythons that have it.

Currently we have really gnarly ArgInfo tests like this:

if (argc.count == 5 || argc.is_bound_method || argc.has_varargs)
    pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg, dict);
else
    pfunc(debugger_arg, PythonString(args), cmd_retobj_arg, dict);

😖

I think if we replace count with max_positional_args we should be able to replace that kindof test with just

if (argc.max_positional_args < 5)
   old_version
else
   new_version

[labath]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

So, how about this: Put extra_args as the last argument, instead of inserting it in the middle (so the new signature becomes frame, bp_loc, dict, extra_args instead of frame, bp_loc, extra_args, dict. Then instead of

if (arg_info.count == 3)
    result = pfunc(frame_arg, bp_loc_arg, dict);
  else if (arg_info.count == 4) {
      lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
      PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
      result = pfunc(frame_arg, bp_loc_arg, args_arg, dict);

we do:

if (args_impl.was_specified())
  pfunc(frame_arg, bp_loc_arg, dict, args_arg)
else
  pfunc(frame_arg, bp_loc_arg, dict);

All existing scripts will not specify the extra arguments, so they will work as usual. New scripts which do pass extra arguments will have to use the new signature. New scripts can also put args = None in their python signature, so that they are callable both with and without arguments, should they so desire. (If we don't want to support the =None use case then we can even keep the arguments in the same order as in this patch.)

Is there some reason why that would not work?

[labath]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

The only other way I can think of to solve it would be to have some indication in the break com add command of what signature it expects from the function.

That's kind of what I'm getting at. I am hoping that the presence of the --key, --value options can be used as an indicator of that signature. (Though maybe I am misunderstanding something and I should shut up.)

[jingham]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

The is_bound_method test is cheesy. We didn't offer a "class with call" for Python based commands until after we added the ExecutionContext argument, so this check knows that if you are providing a class method, then you are probably also providing the correct number of arguments. max_positional_args seems a more explicit approach.

I think the varargs check is to allow you to write a command that takes the old three arguments and the ExecutionContext as a vararg, so the same Python function could work with an older lldb that didn't send the exe_ctx but take advantage of the better interface if it was present. After all, the fallback of using the currently selected "target/process/thread/frame" inside the function will mostly work.

For the affordances taking the "extra_args", like breakpoint commands and scripted breakpoints and the like, it's hard to see how you could have a reasonable fallback to "you didn't tell me what function to look for..." So for these I want to count the fixed arguments, I don't think it is necessary to allow them to be passed as varargs.

Thanks for fixing up the PythonObjects code, BTW!

[jingham]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

I did consider switching off of whether the user provided key & value arguments. But that would mean that you could not write Python breakpoint command functions that have default behaviors, but will do special things if the user provided --key and --value arguments. That seemed like a reasonable thing to support, so I switched to doing everything based on the actual signature we were provided.

[labath]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

Why not? If the last argument has a default value (def my_fancy_callback(frame, bp_loc, dict, extra_args = None):), then it should be callable both with three and four arguments, should it not? I think the =None part actually nicely documents that the callback is usable both with extra arguments, but also has a reasonable default behavior.

[jingham]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

It's already weird that we have to tell people to pass the session dict. Now we're saying you have to pass a defaulted arg after the dict; that seems even weirder. I don't actually think lots of people are going to want to write commands that take advantage of being passed extra arguments but want them to work with older lldb's. The case I want to support is that you are always going to get this extra args SBStructuredData, but it might be empty depending on whether the user added --key and --value, and your command can do what it wants based on the contents of the extra_args. That seems straightforwardly supported by requiring the arguments.

I'm not sure we are solving any actual problem here?

[labath]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

What I find weird is that the meaning of a positional argument changes depending on what comes after it. So if I define a function like def foo(a,b,c), then c will be a session dictionary. But if I define it as def foo(a,b,c,d): then the dict becomes d. I don't find the =None confusing, particularly as that means we could just have one function signature we advertise, and if the users write it that way then it will "happen" to work on older lldb's too. But then again, I've never written any breakpoint callbacks, so if you think that these concerns are unfounded, then feel free to go ahead.

[jingham]

lldb/include/lldb/Interpreter/ScriptInterpreter.h

469

Yeah, I don't think that is what you are told to do. For breakpoint commands, you are told to write:

def foo(frame, bp_no, dict):

in the old form, or if you want to receive extra args, you would write:

def foo(frame, bp_no, extra_args, dict)

and in either case the last - "dict" - argument is an internal detail. That seems straightforward.

Also, it is the way command implementation functions (with the added exe_ctx argument), and scripted thread plans (with a similar extra_args argument) work. It is also how the scripted breakpoint resolvers work, except there was never a version w/o the extra_args, because that's when I realized that was a much better way to go... So I do think I'm going to leave it this way.

[amccarth]

FYI: This broke the build for me.

lldb/include/lldb/Interpreter/ScriptInterpreter.h

323

This breaks some builds because it doesn't return a value.

[wallace]

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

21

I've been trying to track a regression in source maps between Android Studio versions and it seems that this test shouldn't have been disabled. :(

[jingham]

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

21

Yup, my bad. I sometimes disable tests this way when I'm only trying to run one test in the set a bunch of times. I must have forgotten to set it back. Thanks for catching this!

[wallace]

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

21

No worries!! It was fun, though

[jingham]

Just for history's sake, this change was committed (738af7a6241c9). I put in the Differential Revision line in the changelog, so I'm not sure why this didn't get marked as closed...

[JDevlieghere]

Apparently closing a revision requires it to be accepted first.

lldb/include/lldb/API/SBBreakpoint.h

public:
   void SetQueueName(const char *queue_name);
 
   const char *GetQueueName() const;
 
   void SetCallback(SBBreakpointHitCallback callback, void *baton);
 
   void SetScriptCallbackFunction(const char *callback_function_name);
 
+  SBError SetScriptCallbackFunction(const char *callback_function_name,
+                                 SBStructuredData &extra_args);
+
   void SetCommandLineCommands(SBStringList &commands);
 
   bool GetCommandLineCommands(SBStringList &commands);
 
   SBError SetScriptCallbackBody(const char *script_body_text);
 
   bool AddName(const char *new_name);
 

lldb/include/lldb/API/SBBreakpointLocation.h

public:
   const char *GetCondition();
    
   void SetAutoContinue(bool auto_continue);
 
   bool GetAutoContinue();
 
   void SetScriptCallbackFunction(const char *callback_function_name);
 
+  SBError SetScriptCallbackFunction(const char *callback_function_name,
+                                    lldb::SBStructuredData &extra_args);
+
   SBError SetScriptCallbackBody(const char *script_body_text);
   
-  void SetCommandLineCommands(SBStringList &commands);
+  void SetCommandLineCommands(lldb::SBStringList &commands);
 
-  bool GetCommandLineCommands(SBStringList &commands);
+  bool GetCommandLineCommands(lldb::SBStringList &commands);
  
   void SetThreadID(lldb::tid_t sb_thread_id);
 
   lldb::tid_t GetThreadID();
 
   void SetThreadIndex(uint32_t index);
 
   uint32_t GetThreadIndex() const;

lldb/include/lldb/API/SBBreakpointName.h

//                                        lldb::SBBreakpointLocation &location);
   void SetQueueName(const char *queue_name);
 
   const char *GetQueueName() const;
 
   void SetCallback(SBBreakpointHitCallback callback, void *baton);
 
   void SetScriptCallbackFunction(const char *callback_function_name);
 
-  void SetCommandLineCommands(SBStringList &commands);
+  SBError SetScriptCallbackFunction(const char *callback_function_name,
+                                    SBStructuredData &extra_args);
 
-  bool GetCommandLineCommands(SBStringList &commands);
+  void SetCommandLineCommands(lldb::SBStringList &commands);
+
+  bool GetCommandLineCommands(lldb::SBStringList &commands);
 
   SBError SetScriptCallbackBody(const char *script_body_text);
   
   const char *GetHelpString() const;
   void SetHelpString(const char *help_string);
   
   bool GetAllowList() const;
   void SetAllowList(bool value);

lldb/include/lldb/API/SBStructuredData.h

public:
   size_t GetStringValue(char *dst, size_t dst_len) const;
 
 protected:
   friend class SBTraceOptions;
   friend class SBDebugger;
   friend class SBTarget;
   friend class SBThread;
   friend class SBThreadPlan;
+  friend class SBBreakpoint;
+  friend class SBBreakpointLocation;
+  friend class SBBreakpointName;
 
   StructuredDataImplUP m_impl_up;
 };
 } // namespace lldb
 
 #endif /* SBStructuredData_h */

lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h

 // Use this Option group if you have a python class that implements some
 // Python extension point, and you pass a SBStructuredData to the class 
 // __init__ method.  
 // class_option specifies the class name
 // the key and value options are read in in pairs, and a 
 // StructuredData::Dictionary is constructed with those pairs.
 class OptionGroupPythonClassWithDict : public OptionGroup {
 public:
   OptionGroupPythonClassWithDict(const char *class_use,
+                                 bool is_class = true,
                                  int class_option = 'C',
                                  int key_option = 'k', 
-                      int value_option = 'v',
-                      const char *class_long_option = "python-class",
-                      const char *key_long_option = "python-class-key",
-                      const char *value_long_option = "python-class-value",
-                      bool required = false);
+                                 int value_option = 'v');
                       
   ~OptionGroupPythonClassWithDict() override;
 
   llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
     return llvm::ArrayRef<OptionDefinition>(m_option_definition);
   }
 
   Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
                         ExecutionContext *execution_context) override;
   Status SetOptionValue(uint32_t, const char *, ExecutionContext *) = delete;
 
   void OptionParsingStarting(ExecutionContext *execution_context) override;
   Status OptionParsingFinished(ExecutionContext *execution_context) override;
   
   const StructuredData::DictionarySP GetStructuredData() {
     return m_dict_sp;
   }
-  const std::string &GetClassName() {
-    return m_class_name;
+  const std::string &GetName() {
+    return m_name;
   }
 
 protected:
-  std::string m_class_name;
+  std::string m_name;
   std::string m_current_key;
   StructuredData::DictionarySP m_dict_sp;
   std::string m_class_usage_text, m_key_usage_text, m_value_usage_text;
-  OptionDefinition m_option_definition[3];
+  bool m_is_class;
+  OptionDefinition m_option_definition[4];
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_OptionGroupPythonClassWithDict_h_

lldb/include/lldb/Interpreter/ScriptInterpreter.h

public:
 
   virtual Status
   ExportFunctionDefinitionToInterpreter(StringList &function_def) {
     Status error;
     error.SetErrorString("not implemented");
     return error;
   }
 
-  virtual Status GenerateBreakpointCommandCallbackData(StringList &input,
-                                                       std::string &output) {
+  virtual Status GenerateBreakpointCommandCallbackData(
+      StringList &input,
+      std::string &output,
+      bool has_extra_args) {
     Status error;
     error.SetErrorString("not implemented");
     return error;
   }
 
   virtual bool GenerateWatchpointCommandCallbackData(StringList &input,
                                                      std::string &output) {
     return false;
public:
   virtual Status SetBreakpointCommandCallback(
       BreakpointOptions *bp_options,
       std::unique_ptr<BreakpointOptions::CommandData> &data_up) {
     Status error;
     error.SetErrorString("unimplemented");
     return error;
   }
 
-  void SetBreakpointCommandCallbackFunction(
+  Status SetBreakpointCommandCallbackFunction(
       std::vector<BreakpointOptions *> &bp_options_vec,
-      const char *function_name);
+      const char *function_name, 
+      StructuredData::ObjectSP extra_args_sp);
 
-  /// Set a one-liner as the callback for the breakpoint.
-  virtual void
-  SetBreakpointCommandCallbackFunction(BreakpointOptions *bp_options,
-                                       const char *function_name) {}
+  /// Set a script function as the callback for the breakpoint.
+  virtual Status
+  SetBreakpointCommandCallbackFunction(
+      BreakpointOptions *bp_options,
+      const char *function_name,
+      StructuredData::ObjectSP extra_args_sp) {}
 
   /// Set a one-liner as the callback for the watchpoint.
   virtual void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
                                             const char *oneliner) {}
 
   virtual bool GetScriptedSummary(const char *function_name,
                                   lldb::ValueObjectSP valobj,
                                   StructuredData::ObjectSP &callee_wrapper_sp,
public:
 
   virtual bool IsReservedWord(const char *word) { return false; }
 
   virtual std::unique_ptr<ScriptInterpreterLocker> AcquireInterpreterLock();
 
   const char *GetScriptInterpreterPtyName();
 
   int GetMasterFileDescriptor();
   
+  virtual llvm::Expected<size_t> 
+  GetNumFixedArgumentsForCallable(const llvm::StringRef &callable_name) { 
+    return llvm::createStringError(
+    llvm::inconvertibleErrorCode(), "Unimplemented function");
+  }
+
   static std::string LanguageToString(lldb::ScriptLanguage language);
 
   static lldb::ScriptLanguage StringToLanguage(const llvm::StringRef &string);
 
   lldb::ScriptLanguage GetLanguage() { return m_script_lang; }
 
 protected:
   Debugger &m_debugger;
   lldb::ScriptLanguage m_script_lang;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_ScriptInterpreter_h_

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

 
 
 class BreakpointCommandTestCase(TestBase):
 
     NO_DEBUG_INFO_TESTCASE = True
     mydir = TestBase.compute_mydir(__file__)
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
-    def test_breakpoint_command_sequence(self):
+    def not_test_breakpoint_command_sequence(self):
         """Test a sequence of breakpoint command add, list, and delete."""
         self.build()
         self.breakpoint_command_sequence()
 
     def test_script_parameters(self):
         """Test a sequence of breakpoint command add, list, and delete."""
         self.build()
         self.breakpoint_command_script_parameters()
def breakpoint_command_sequence(self):
         lldbutil.run_break_set_by_file_and_line(
             self, "./invalid/main.c", self.line,
             num_expected_locations=0, loc_exact=True)
         # Now add callbacks for the breakpoints just created.
         self.runCmd(
             "breakpoint command add -s command -o 'frame variable --show-types --scope' 1 4")
         self.runCmd(
             "breakpoint command add -s python -o 'import side_effect; side_effect.one_liner = \"one liner was here\"' 2")
+
+        import side_effect
+        self.runCmd("command script import --allow-reload ./bktptcmd.py")
+
         self.runCmd(
             "breakpoint command add --python-function bktptcmd.function 3")
 
         # Check that the breakpoint commands are correctly set.
 
         # The breakpoint list now only contains breakpoint 1.
         self.expect(
             "breakpoint list", "Breakpoints 1 & 2 created", substrs=[
def breakpoint_command_sequence(self):
                              "bktptcmd.function(frame, bp_loc, internal_dict)"])
 
         self.expect("breakpoint command list 4", "Breakpoint 4 command ok",
                     substrs=["Breakpoint commands:",
                              "frame variable --show-types --scope"])
 
         self.runCmd("breakpoint delete 4")
 
-        self.runCmd("command script import --allow-reload ./bktptcmd.py")
-
         # Next lets try some other breakpoint kinds.  First break with a regular expression
         # and then specify only one file.  The first time we should get two locations,
         # the second time only one:
 
         lldbutil.run_break_set_by_regexp(
             self, r"._MyFunction", num_expected_locations=2)
 
         lldbutil.run_break_set_by_regexp(

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py

 
 class PythonBreakpointCommandSettingTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
     @add_test_categories(['pyapi'])
     def test_step_out_python(self):
-        """Test stepping out using avoid-no-debug with dsyms."""
+        """Test stepping out using a python breakpoint command."""
         self.build()
         self.do_set_python_command_from_python()
 
+    def test_bkpt_cmd_bad_arguments(self):
+        """Test what happens when pass structured data to a command:"""
+        self.build()
+        self.do_bad_args_to_python_command()
+        
     def setUp(self):
         TestBase.setUp(self)
         self.main_source = "main.c"
         self.main_source_spec = lldb.SBFileSpec(self.main_source)
 
     def do_set_python_command_from_python(self):
         exe = self.getBuildArtifact("a.out")
         error = lldb.SBError()
 
         self.target = self.dbg.CreateTarget(exe)
         self.assertTrue(self.target, VALID_TARGET)
 
         body_bkpt = self.target.BreakpointCreateBySourceRegex(
             "Set break point at this line.", self.main_source_spec)
         self.assertTrue(body_bkpt, VALID_BREAKPOINT)
 
         func_bkpt = self.target.BreakpointCreateBySourceRegex(
             "Set break point at this line.", self.main_source_spec)
         self.assertTrue(func_bkpt, VALID_BREAKPOINT)
 
+        fancy_bkpt = self.target.BreakpointCreateBySourceRegex(
+            "Set break point at this line.", self.main_source_spec)
+        self.assertTrue(fancy_bkpt, VALID_BREAKPOINT)
+
+        fancier_bkpt = self.target.BreakpointCreateBySourceRegex(
+            "Set break point at this line.", self.main_source_spec)
+        self.assertTrue(fancier_bkpt, VALID_BREAKPOINT)
+
+        not_so_fancy_bkpt = self.target.BreakpointCreateBySourceRegex(
+            "Set break point at this line.", self.main_source_spec)
+        self.assertTrue(not_so_fancy_bkpt, VALID_BREAKPOINT)
+
         # Also test that setting a source regex breakpoint with an empty file
         # spec list sets it on all files:
         no_files_bkpt = self.target.BreakpointCreateBySourceRegex(
             "Set a breakpoint here", lldb.SBFileSpecList(), lldb.SBFileSpecList())
         self.assertTrue(no_files_bkpt, VALID_BREAKPOINT)
         num_locations = no_files_bkpt.GetNumLocations()
         self.assertTrue(
             num_locations >= 2,
def do_set_python_command_from_python(self):
         error = lldb.SBError()
         error = body_bkpt.SetScriptCallbackBody(
                 "import side_effect; side_effect.callback = 'callback was here'")
         self.assertTrue(
             error.Success(),
             "Failed to set the script callback body: %s." %
             (error.GetCString()))
 
-        self.dbg.HandleCommand(
-            "command script import --allow-reload ./bktptcmd.py")
+        self.expect("command script import --allow-reload ./bktptcmd.py")
+        
         func_bkpt.SetScriptCallbackFunction("bktptcmd.function")
 
+        extra_args = lldb.SBStructuredData()
+        stream = lldb.SBStream()
+        stream.Print('{"side_effect" : "I am fancy"}')
+        extra_args.SetFromJSON(stream)
+        error = fancy_bkpt.SetScriptCallbackFunction("bktptcmd.another_function", extra_args)
+        self.assertTrue(error.Success(), "Failed to add callback %s"%(error.GetCString()))
+        
+        stream.Clear()
+        stream.Print('{"side_effect" : "I am so much fancier"}')
+        extra_args.SetFromJSON(stream)
+        
+        # Fancier's callback is set up from the command line
+        id = fancier_bkpt.GetID()
+        self.expect("breakpoint command add -F bktptcmd.a_third_function -k side_effect -v 'I am fancier' %d"%(id))
+
+        # Not so fancy gets an empty extra_args:
+        empty_args = lldb.SBStructuredData()
+        error = not_so_fancy_bkpt.SetScriptCallbackFunction("bktptcmd.empty_extra_args", empty_args)
+        self.assertTrue(error.Success(), "Failed to add callback %s"%(error.GetCString()))
+        
         # Clear out canary variables
         side_effect.bktptcmd = None
         side_effect.callback = None
+        side_effect.fancy    = None
+        side_effect.fancier  = None
+        side_effect.not_so_fancy = None
         
         # Now launch the process, and do not stop at entry point.
         self.process = self.target.LaunchSimple(
             None, None, self.get_process_working_directory())
 
         self.assertTrue(self.process, PROCESS_IS_VALID)
 
         # Now finish, and make sure the return value is correct.
         threads = lldbutil.get_threads_stopped_at_breakpoint(
             self.process, body_bkpt)
         self.assertTrue(len(threads) == 1, "Stopped at inner breakpoint.")
         self.thread = threads[0]
 
         self.assertEquals("callback was here", side_effect.callback)
         self.assertEquals("function was here", side_effect.bktptcmd)
+        self.assertEquals("I am fancy", side_effect.fancy)
+        self.assertEquals("I am fancier", side_effect.fancier)
+        self.assertEquals("Not so fancy", side_effect.not_so_fancy)
+
+    def do_bad_args_to_python_command(self):
+        exe = self.getBuildArtifact("a.out")
+        error = lldb.SBError()
+
+        self.target = self.dbg.CreateTarget(exe)
+        self.assertTrue(self.target, VALID_TARGET)
+
+
+        self.expect("command script import --allow-reload ./bktptcmd.py")
+
+        bkpt = self.target.BreakpointCreateBySourceRegex(
+            "Set break point at this line.", self.main_source_spec)
+        self.assertTrue(bkpt, VALID_BREAKPOINT)
+
+        # Pass a breakpoint command function that doesn't take extra_args,
+        # but pass it extra args:
+        
+        extra_args = lldb.SBStructuredData()
+        stream = lldb.SBStream()
+        stream.Print('{"side_effect" : "I am fancy"}')
+        extra_args.SetFromJSON(stream)
+
+        error = bkpt.SetScriptCallbackFunction("bktptcmd.function", extra_args)
+        self.assertTrue(error.Fail(), "Can't pass extra args if the function doesn't take them")
+
+        error = bkpt.SetScriptCallbackFunction("bktptcmd.useless_function", extra_args)
+        self.assertTrue(error.Fail(), "Can't pass extra args if the function has wrong number of args.")
+
+        error = bkpt.SetScriptCallbackFunction("bktptcmd.nosuch_function", extra_args)
+        self.assertTrue(error.Fail(), "Can't pass extra args if the function doesn't exist.")
+        

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/bktptcmd.py

 from __future__ import print_function
 import side_effect
 
+def useless_function(first, second):
+    print("I have the wrong number of arguments.")
+
 def function(frame, bp_loc, dict):
     side_effect.bktptcmd = "function was here"
+
+def another_function(frame, bp_loc, extra_args, dict):
+    se_value = extra_args.GetValueForKey("side_effect")
+    se_string = se_value.GetStringValue(100)
+    side_effect.fancy = se_string
+
+def a_third_function(frame, bp_loc, extra_args, dict):
+    se_value = extra_args.GetValueForKey("side_effect")
+    se_string = se_value.GetStringValue(100)
+    side_effect.fancier = se_string
+
+def empty_extra_args(frame, bp_loc, extra_args, dict):
+    if extra_args.IsValid():
+        side_effect.not_so_fancy = "Extra args should not be valid"
+    side_effect.not_so_fancy = "Not so fancy"

lldb/scripts/Python/python-wrapper.swig

 // and is used when a script command is attached to a breakpoint for execution.
 
 SWIGEXPORT bool
 LLDBSwigPythonBreakpointCallbackFunction
 (
     const char *python_function_name,
     const char *session_dictionary_name,
     const lldb::StackFrameSP& frame_sp,
-    const lldb::BreakpointLocationSP& bp_loc_sp
+    const lldb::BreakpointLocationSP& bp_loc_sp,
+    lldb_private::StructuredDataImpl *args_impl
 )
 {
     using namespace lldb_private;
     lldb::SBFrame sb_frame (frame_sp);
     lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
 
     bool stop_at_breakpoint = true;
 
     PyErr_Cleaner py_err_cleaner(true);
     auto dict = PythonModule::MainModule().ResolveName<PythonDictionary>(session_dictionary_name);
     auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>(python_function_name, dict);
 
     if (!pfunc.IsAllocated())
         return stop_at_breakpoint;
 
     PythonObject frame_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_frame));
     PythonObject bp_loc_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_bp_loc));
-    PythonObject result = pfunc(frame_arg, bp_loc_arg, dict);
+
+    PythonObject result;
+    // If the called function doesn't take extra_args, drop them here:
+    auto arg_info = pfunc.GetNumArguments();
+    if (arg_info.count == 3)
+      result = pfunc(frame_arg, bp_loc_arg, dict);
+    else if (arg_info.count == 4) {
+        lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
+        PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
+        result = pfunc(frame_arg, bp_loc_arg, args_arg, dict);      
+    } else {
+      return stop_at_breakpoint;
+    }
 
     if (result.get() == Py_False)
         stop_at_breakpoint = false;
 
     return stop_at_breakpoint;
 }
 
 // This function is called by lldb_private::ScriptInterpreterPython::WatchpointCallbackFunction(...)

lldb/scripts/interface/SBBreakpoint.i

public:
     GetQueueName () const;
 
     %feature("docstring", "
     Set the name of the script function to be called when the breakpoint is hit.") SetScriptCallbackFunction;
     void
     SetScriptCallbackFunction (const char *callback_function_name);
 
     %feature("docstring", "
+    Set the name of the script function to be called when the breakpoint is hit.
+    To use this variant, the function should take (frame, bp_loc, extra_args, dict) and
+    when the breakpoint is hit the extra_args will be passed to the callback function.") SetScriptCallbackFunction;
+    SBError
+    SetScriptCallbackFunction (const char *callback_function_name,
+                               SBStructuredData &extra_args);
+
+    %feature("docstring", "
     Provide the body for the script function to be called when the breakpoint is hit.
     The body will be wrapped in a function, which be passed two arguments:
     'frame' - which holds the bottom-most SBFrame of the thread that hit the breakpoint
     'bpno'  - which is the SBBreakpointLocation to which the callback was attached.
 
     The error parameter is currently ignored, but will at some point hold the Python
     compilation diagnostics.
     Returns true if the body compiles successfully, false if not.") SetScriptCallbackBody;

lldb/scripts/interface/SBBreakpointLocation.i

public:
     const char *
     GetCondition ();
 
     bool GetAutoContinue();
 
     void SetAutoContinue(bool auto_continue);
 
     %feature("docstring", "
-    Set the callback to the given Python function name.") SetScriptCallbackFunction;
+    Set the callback to the given Python function name.
+    The function takes three arguments (frame, bp_loc, dict).") SetScriptCallbackFunction;
     void
     SetScriptCallbackFunction (const char *callback_function_name);
 
     %feature("docstring", "
+    Set the name of the script function to be called when the breakpoint is hit.
+    To use this variant, the function should take (frame, bp_loc, extra_args, dict) and
+    when the breakpoint is hit the extra_args will be passed to the callback function.") SetScriptCallbackFunction;
+    SBError
+    SetScriptCallbackFunction (const char *callback_function_name,
+                               SBStructuredData &extra_args);
+
+    %feature("docstring", "
     Provide the body for the script function to be called when the breakpoint location is hit.
     The body will be wrapped in a function, which be passed two arguments:
     'frame' - which holds the bottom-most SBFrame of the thread that hit the breakpoint
     'bpno'  - which is the SBBreakpointLocation to which the callback was attached.
 
     The error parameter is currently ignored, but will at some point hold the Python
     compilation diagnostics.
     Returns true if the body compiles successfully, false if not.") SetScriptCallbackBody;

lldb/scripts/interface/SBBreakpointName.i

public:
   const char *GetThreadName() const;
 
   void SetQueueName(const char *queue_name);
 
   const char *GetQueueName() const;
 
   void SetScriptCallbackFunction(const char *callback_function_name);
 
+  SBError
+  SetScriptCallbackFunction (const char *callback_function_name,
+                             SBStructuredData &extra_args);
+
   void SetCommandLineCommands(SBStringList &commands);
 
   bool GetCommandLineCommands(SBStringList &commands);
 
   SBError SetScriptCallbackBody(const char *script_body_text);
 
   const char *GetHelpString() const;
   void SetHelpString(const char *help_string);

lldb/source/API/SBBreakpoint.cpp

 //===-- SBBreakpoint.cpp ----------------------------------------*- C++ -*-===//
 //
 // 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/API/SBBreakpoint.h"
 #include "SBReproducerPrivate.h"
 #include "lldb/API/SBBreakpointLocation.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStringList.h"
+#include "lldb/API/SBStructuredData.h"
 #include "lldb/API/SBThread.h"
 
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Breakpoint/BreakpointResolverScripted.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/Stream.h"
if (bkpt_sp) {
     BatonSP baton_sp(new SBBreakpointCallbackBaton(callback, baton));
     bkpt_sp->SetCallback(SBBreakpointCallbackBaton
       ::PrivateBreakpointHitCallback, baton_sp,
                          false);
   }
 }
 
 void SBBreakpoint::SetScriptCallbackFunction(
-    const char *callback_function_name) {
-  LLDB_RECORD_METHOD(void, SBBreakpoint, SetScriptCallbackFunction,
-                     (const char *), callback_function_name);
+  SBStructuredData empty_args;
+  SetScriptCallbackFunction(callback_function_name, empty_args);
+}
 
+SBError SBBreakpoint::SetScriptCallbackFunction(
+    const char *callback_function_name,
+    SBStructuredData &extra_args) {
+  LLDB_RECORD_METHOD(SBError, SBBreakpoint, SetScriptCallbackFunction,
+  (const char *, SBStructuredData &), callback_function_name, extra_args);
+  SBError sb_error;
   BreakpointSP bkpt_sp = GetSP();
 
   if (bkpt_sp) {
+    Status error;
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
     BreakpointOptions *bp_options = bkpt_sp->GetOptions();
-    bkpt_sp->GetTarget()
+    error = bkpt_sp->GetTarget()
         .GetDebugger()
         .GetScriptInterpreter()
         ->SetBreakpointCommandCallbackFunction(bp_options,
-                                               callback_function_name);
-  }
+                                               callback_function_name,
+                                               extra_args.m_impl_up
+                                                   ->GetObjectSP());
+    sb_error.SetError(error);
+  } else
+    sb_error.SetErrorString("invalid breakpoint");
+  
+  return LLDB_RECORD_RESULT(sb_error);
 }
 
 SBError SBBreakpoint::SetScriptCallbackBody(const char *callback_body_text) {
   LLDB_RECORD_METHOD(lldb::SBError, SBBreakpoint, SetScriptCallbackBody,
                      (const char *), callback_body_text);
 
   BreakpointSP bkpt_sp = GetSP();
 
void RegisterMethods<SBBreakpoint>(Registry &R) {
   LLDB_REGISTER_METHOD(bool, SBBreakpoint, GetDescription,
                        (lldb::SBStream &));
   LLDB_REGISTER_METHOD(bool, SBBreakpoint, GetDescription,
                        (lldb::SBStream &, bool));
   LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, AddLocation,
                        (lldb::SBAddress &));
   LLDB_REGISTER_METHOD(void, SBBreakpoint, SetScriptCallbackFunction,
                        (const char *));
+  LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, SetScriptCallbackFunction,
+                       (const char *, SBStructuredData &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, SetScriptCallbackBody,
                        (const char *));
   LLDB_REGISTER_METHOD(bool, SBBreakpoint, AddName, (const char *));
   LLDB_REGISTER_METHOD(void, SBBreakpoint, RemoveName, (const char *));
   LLDB_REGISTER_METHOD(bool, SBBreakpoint, MatchesName, (const char *));
   LLDB_REGISTER_METHOD(void, SBBreakpoint, GetNames, (lldb::SBStringList &));
   LLDB_REGISTER_STATIC_METHOD(bool, SBBreakpoint, EventIsBreakpointEvent,
                               (const lldb::SBEvent &));

lldb/source/API/SBBreakpointLocation.cpp

 //===-- SBBreakpointLocation.cpp --------------------------------*- C++ -*-===//
 //
 // 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/API/SBBreakpointLocation.h"
 #include "SBReproducerPrivate.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBStructuredData.h"
 #include "lldb/API/SBStringList.h"
 
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
if (loc_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         loc_sp->GetTarget().GetAPIMutex());
     return loc_sp->IsAutoContinue();
   }
   return false;
 }
 
 void SBBreakpointLocation::SetScriptCallbackFunction(
-    const char *callback_function_name) {
-  LLDB_RECORD_METHOD(void, SBBreakpointLocation, SetScriptCallbackFunction,
-                     (const char *), callback_function_name);
+}
 
+SBError SBBreakpointLocation::SetScriptCallbackFunction(
+    const char *callback_function_name,
+    SBStructuredData &extra_args) {
+  LLDB_RECORD_METHOD(SBError, SBBreakpointLocation, SetScriptCallbackFunction,
+                     (const char *, SBStructuredData &), 
+                     callback_function_name, extra_args);
+  SBError sb_error;
   BreakpointLocationSP loc_sp = GetSP();
 
   if (loc_sp) {
+    Status error;
     std::lock_guard<std::recursive_mutex> guard(
         loc_sp->GetTarget().GetAPIMutex());
     BreakpointOptions *bp_options = loc_sp->GetLocationOptions();
-    loc_sp->GetBreakpoint()
+    error = loc_sp->GetBreakpoint()
         .GetTarget()
         .GetDebugger()
         .GetScriptInterpreter()
         ->SetBreakpointCommandCallbackFunction(bp_options,
-                                               callback_function_name);
-  }
+                                               callback_function_name,
+                                               extra_args.m_impl_up
+                                                   ->GetObjectSP());
+      sb_error.SetError(error);
+    } else
+      sb_error.SetErrorString("invalid breakpoint");
+    
+    return LLDB_RECORD_RESULT(sb_error);
 }
 
 SBError
 SBBreakpointLocation::SetScriptCallbackBody(const char *callback_body_text) {
   LLDB_RECORD_METHOD(lldb::SBError, SBBreakpointLocation, SetScriptCallbackBody,
                      (const char *), callback_body_text);
 
   BreakpointLocationSP loc_sp = GetSP();
LLDB_REGISTER_METHOD(void, SBBreakpointLocation, SetIgnoreCount,
                        (uint32_t));
   LLDB_REGISTER_METHOD(void, SBBreakpointLocation, SetCondition,
                        (const char *));
   LLDB_REGISTER_METHOD(const char *, SBBreakpointLocation, GetCondition, ());
   LLDB_REGISTER_METHOD(void, SBBreakpointLocation, SetAutoContinue, (bool));
   LLDB_REGISTER_METHOD(bool, SBBreakpointLocation, GetAutoContinue, ());
   LLDB_REGISTER_METHOD(void, SBBreakpointLocation, SetScriptCallbackFunction,
                        (const char *));
+  LLDB_REGISTER_METHOD(SBError, SBBreakpointLocation, SetScriptCallbackFunction,
+                       (const char *, SBStructuredData &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpointLocation,
                        SetScriptCallbackBody, (const char *));
   LLDB_REGISTER_METHOD(void, SBBreakpointLocation, SetCommandLineCommands,
                        (lldb::SBStringList &));
   LLDB_REGISTER_METHOD(bool, SBBreakpointLocation, GetCommandLineCommands,
                        (lldb::SBStringList &));
   LLDB_REGISTER_METHOD(void, SBBreakpointLocation, SetThreadID,
                        (lldb::tid_t));

lldb/source/API/SBBreakpointName.cpp

 //===-- SBBreakpointName.cpp ----------------------------------------*- C++ -*-===//
 //
 // 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/API/SBBreakpointName.h"
 #include "SBReproducerPrivate.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStringList.h"
+#include "lldb/API/SBStructuredData.h"
 #include "lldb/API/SBTarget.h"
 
 #include "lldb/Breakpoint/BreakpointName.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/Stream.h"
 
 #include "SBBreakpointOptionCommon.h"
 
void SBBreakpointName::SetCallback(SBBreakpointHitCallback callback,
   bp_name->GetOptions().SetCallback(SBBreakpointCallbackBaton
                                        ::PrivateBreakpointHitCallback,
                                     baton_sp,
                                     false);
   UpdateName(*bp_name);
 }
 
 void SBBreakpointName::SetScriptCallbackFunction(
-    const char *callback_function_name) {
-  LLDB_RECORD_METHOD(void, SBBreakpointName, SetScriptCallbackFunction,
-                     (const char *), callback_function_name);
+  SBStructuredData empty_args;
+  SetScriptCallbackFunction(callback_function_name, empty_args);
+}
 
+SBError SBBreakpointName::SetScriptCallbackFunction(
+    const char *callback_function_name, 
+    SBStructuredData &extra_args) {
+  LLDB_RECORD_METHOD(SBError, SBBreakpointName, SetScriptCallbackFunction,
+                     (const char *, SBStructuredData &), 
+                     callback_function_name, extra_args);
+  SBError sb_error;
   BreakpointName *bp_name = GetBreakpointName();
-  if (!bp_name)
-    return;
+  if (!bp_name) {
+    sb_error.SetErrorString("unrecognized breakpoint name");
+    return LLDB_RECORD_RESULT(sb_error);
+  }
 
   std::lock_guard<std::recursive_mutex> guard(
         m_impl_up->GetTarget()->GetAPIMutex());
 
   BreakpointOptions &bp_options = bp_name->GetOptions();
-  m_impl_up->GetTarget()
+  Status error;
+  error = m_impl_up->GetTarget()
       ->GetDebugger()
       .GetScriptInterpreter()
       ->SetBreakpointCommandCallbackFunction(&bp_options,
-                                             callback_function_name);
+                                             callback_function_name,
+                                             extra_args.m_impl_up
+                                                 ->GetObjectSP());
+  sb_error.SetError(error);
   UpdateName(*bp_name);
+  return LLDB_RECORD_RESULT(sb_error);
 }
 
 SBError
 SBBreakpointName::SetScriptCallbackBody(const char *callback_body_text) {
   LLDB_RECORD_METHOD(lldb::SBError, SBBreakpointName, SetScriptCallbackBody,
                      (const char *), callback_body_text);
 
   SBError sb_error;
LLDB_REGISTER_METHOD(bool, SBBreakpointName, GetCommandLineCommands,
                        (lldb::SBStringList &));
   LLDB_REGISTER_METHOD_CONST(const char *, SBBreakpointName, GetHelpString,
                              ());
   LLDB_REGISTER_METHOD(void, SBBreakpointName, SetHelpString, (const char *));
   LLDB_REGISTER_METHOD(bool, SBBreakpointName, GetDescription,
                        (lldb::SBStream &));
   LLDB_REGISTER_METHOD(void, SBBreakpointName, SetScriptCallbackFunction,
                        (const char *));
+  LLDB_REGISTER_METHOD(SBError, SBBreakpointName, SetScriptCallbackFunction,
+                       (const char *, SBStructuredData &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpointName, SetScriptCallbackBody,
                        (const char *));
   LLDB_REGISTER_METHOD_CONST(bool, SBBreakpointName, GetAllowList, ());
   LLDB_REGISTER_METHOD(void, SBBreakpointName, SetAllowList, (bool));
   LLDB_REGISTER_METHOD(bool, SBBreakpointName, GetAllowDelete, ());
   LLDB_REGISTER_METHOD(void, SBBreakpointName, SetAllowDelete, (bool));
   LLDB_REGISTER_METHOD(bool, SBBreakpointName, GetAllowDisable, ());
   LLDB_REGISTER_METHOD(void, SBBreakpointName, SetAllowDisable, (bool));
 }
 
 }
 }

lldb/source/Commands/CommandObjectBreakpoint.cpp

enum BreakpointSetType {
     eSetTypeScripted,
   };
 
   CommandObjectBreakpointSet(CommandInterpreter &interpreter)
       : CommandObjectParsed(
             interpreter, "breakpoint set",
             "Sets a breakpoint or set of breakpoints in the executable.",
             "breakpoint set <cmd-options>"),
-        m_bp_opts(), m_python_class_options("scripted breakpoint", 'P'),
+        m_bp_opts(), m_python_class_options("scripted breakpoint", true, 'P'),
         m_options() {
     // We're picking up all the normal options, commands and disable.
-    m_all_options.Append(&m_python_class_options, LLDB_OPT_SET_1,
+    m_all_options.Append(&m_python_class_options, LLDB_OPT_SET_1|LLDB_OPT_SET_2,
                          LLDB_OPT_SET_11);
     m_all_options.Append(&m_bp_opts,
                          LLDB_OPT_SET_1 | LLDB_OPT_SET_3 | LLDB_OPT_SET_4,
                          LLDB_OPT_SET_ALL);
     m_all_options.Append(&m_dummy_options, LLDB_OPT_SET_1, LLDB_OPT_SET_ALL);
     m_all_options.Append(&m_options);
     m_all_options.Finalize();
   }
bool DoExecute(Args &command, CommandReturnObject &result) override {
     //   expression)
     //   5).  -p -f               (setting a breakpoint by comparing a reg-exp
     //   to source text)
     //   6).  -E [-w -h]          (setting a breakpoint for exceptions for a
     //   given language.)
 
     BreakpointSetType break_type = eSetTypeInvalid;
 
-    if (!m_python_class_options.GetClassName().empty())
+    if (!m_python_class_options.GetName().empty())
       break_type = eSetTypeScripted;
     else if (m_options.m_line_num != 0)
       break_type = eSetTypeFileAndLine;
     else if (m_options.m_load_addr != LLDB_INVALID_ADDRESS)
       break_type = eSetTypeAddress;
     else if (!m_options.m_func_names.empty())
       break_type = eSetTypeFunctionName;
     else if (!m_options.m_func_regexp.empty())
case eSetTypeException: {
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
     } break;
     case eSetTypeScripted: {
 
       Status error;
       bp_sp = target.CreateScriptedBreakpoint(
-          m_python_class_options.GetClassName().c_str(), &(m_options.m_modules),
+          m_python_class_options.GetName().c_str(), &(m_options.m_modules),
           &(m_options.m_filenames), false, m_options.m_hardware,
           m_python_class_options.GetStructuredData(), &error);
       if (error.Fail()) {
         result.AppendErrorWithFormat(
             "Error setting extra exception arguments: %s",
             error.AsCString());
         target.RemoveBreakpointByID(bp_sp->GetID());
         result.SetStatus(eReturnStatusFailed);

lldb/source/Commands/CommandObjectBreakpointCommand.cpp

 #include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/State.h"
 
 #include "llvm/ADT/STLExtras.h"
 
 using namespace lldb;
 using namespace lldb_private;
CommandObjectBreakpointCommandAdd(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "add",
                             "Add LLDB commands to a breakpoint, to be executed "
                             "whenever the breakpoint is hit."
                             "  If no breakpoint is specified, adds the "
                             "commands to the last created breakpoint.",
                             nullptr),
         IOHandlerDelegateMultiline("DONE",
                                    IOHandlerDelegate::Completion::LLDBCommand),
-        m_options() {
+        m_options(), m_func_options("breakpoint command", false, 'F') {
     SetHelpLong(
         R"(
 General information about entering breakpoint commands
 ------------------------------------------------------
 
 )"
         "This command will prompt for commands to be executed when the specified \
 breakpoint is hit.  Each command is typed on its own line following the '> ' \

 returning None, or even omitting a return statement entirely, will cause \
 LLDB to stop."
         R"(
 
 )"
         "Final Note: A warning that no breakpoint command was generated when there \
 are no syntax errors may indicate that a function was declared but never called.");
 
+    m_all_options.Append(&m_options);
+    m_all_options.Append(&m_func_options, LLDB_OPT_SET_2 | LLDB_OPT_SET_3, 
+                         LLDB_OPT_SET_2);
+    m_all_options.Finalize();
+    
     CommandArgumentEntry arg;
     CommandArgumentData bp_id_arg;
 
     // Define the first (and only) variant of this arg.
     bp_id_arg.arg_type = eArgTypeBreakpointID;
     bp_id_arg.arg_repetition = eArgRepeatOptional;
 
     // There is only one variant this argument could be; put it into the
     // argument entry.
     arg.push_back(bp_id_arg);
 
     // Push the data for the first argument into the m_arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectBreakpointCommandAdd() override = default;
 
-  Options *GetOptions() override { return &m_options; }
+  Options *GetOptions() override { return &m_all_options; }
 
   void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFileSP());
     if (output_sp && interactive) {
       output_sp->PutCString(g_reader_instructions);
       output_sp->Flush();
     }
   }
for (auto bp_options : bp_options_vec) {
 
       cmd_data->user_source.AppendString(oneliner);
       cmd_data->stop_on_error = m_options.m_stop_on_error;
 
       bp_options->SetCommandDataCallback(cmd_data);
     }
   }
 
-  class CommandOptions : public Options {
+  class CommandOptions : public OptionGroup {
   public:
     CommandOptions()
-        : Options(), m_use_commands(false), m_use_script_language(false),
+        : OptionGroup(), m_use_commands(false), m_use_script_language(false),
           m_script_language(eScriptLanguageNone), m_use_one_liner(false),
-          m_one_liner(), m_function_name() {}
+          m_one_liner() {}
 
     ~CommandOptions() override = default;
 
     Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
                           ExecutionContext *execution_context) override {
       Status error;
-      const int short_option = m_getopt_table[option_idx].val;
+      const int short_option 
+          = g_breakpoint_command_add_options[option_idx].short_option;
 
       switch (short_option) {
       case 'o':
         m_use_one_liner = true;
         m_one_liner = option_arg;
         break;
 
       case 's':
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         m_stop_on_error =
             OptionArgParser::ToBoolean(option_arg, false, &success);
         if (!success)
           error.SetErrorStringWithFormat(
               "invalid value for stop-on-error: \"%s\"",
               option_arg.str().c_str());
       } break;
 
-      case 'F':
-        m_use_one_liner = false;
-        m_use_script_language = true;
-        m_function_name.assign(option_arg);
-        break;
-
       case 'D':
         m_use_dummy = true;
         break;
 
       default:
         llvm_unreachable("Unimplemented option");
       }
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_use_commands = true;
       m_use_script_language = false;
       m_script_language = eScriptLanguageNone;
 
       m_use_one_liner = false;
       m_stop_on_error = true;
       m_one_liner.clear();
-      m_function_name.clear();
       m_use_dummy = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_breakpoint_command_add_options);
     }
 
     // Instance variables to hold the values for command options.
 
     bool m_use_commands;
     bool m_use_script_language;
     lldb::ScriptLanguage m_script_language;
 
     // Instance variables to hold the values for one_liner options.
     bool m_use_one_liner;
     std::string m_one_liner;
     bool m_stop_on_error;
-    std::string m_function_name;
     bool m_use_dummy;
   };
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target &target = GetSelectedOrDummyTarget(m_options.m_use_dummy);
 
     const BreakpointList &breakpoints = target.GetBreakpointList();
     size_t num_breakpoints = breakpoints.GetSize();
 
     if (num_breakpoints == 0) {
       result.AppendError("No breakpoints exist to have commands added");
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
-    if (!m_options.m_use_script_language &&
-        !m_options.m_function_name.empty()) {
-      result.AppendError("need to enable scripting to have a function run as a "
-                         "breakpoint command");
-      result.SetStatus(eReturnStatusFailed);
-      return false;
+    if (!m_func_options.GetName().empty()) {
+      m_options.m_use_one_liner = false;
+      m_options.m_use_script_language = true;
     }
 
     BreakpointIDList valid_bp_ids;
     CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
         command, &target, result, &valid_bp_ids,
         BreakpointName::Permissions::PermissionKinds::listPerm);
 
     m_bp_options_vec.clear();
if (result.Succeeded()) {
       // to set or collect command callback.  Otherwise, call the methods
       // associated with this object.
       if (m_options.m_use_script_language) {
         ScriptInterpreter *script_interp = GetDebugger().GetScriptInterpreter();
         // Special handling for one-liner specified inline.
         if (m_options.m_use_one_liner) {
           script_interp->SetBreakpointCommandCallback(
               m_bp_options_vec, m_options.m_one_liner.c_str());
-        } else if (!m_options.m_function_name.empty()) {
-          script_interp->SetBreakpointCommandCallbackFunction(
-              m_bp_options_vec, m_options.m_function_name.c_str());
+        } else if (!m_func_options.GetName().empty()) {
+          Status error = script_interp->SetBreakpointCommandCallbackFunction(
+              m_bp_options_vec, m_func_options.GetName().c_str(),
+              m_func_options.GetStructuredData());
+            if (!error.Success())
+              result.SetError(error);
         } else {
           script_interp->CollectDataForBreakpointCommandCallback(
               m_bp_options_vec, result);
         }
       } else {
         // Special handling for one-liner specified inline.
         if (m_options.m_use_one_liner)
           SetBreakpointCommandCallback(m_bp_options_vec,
                                        m_options.m_one_liner.c_str());
         else
           CollectDataForBreakpointCommandCallback(m_bp_options_vec, result);
       }
     }
 
     return result.Succeeded();
   }
 
 private:
   CommandOptions m_options;
+  OptionGroupPythonClassWithDict m_func_options;
+  OptionGroupOptions m_all_options;
+
   std::vector<BreakpointOptions *> m_bp_options_vec; // This stores the
                                                      // breakpoint options that
                                                      // we are currently
   // collecting commands for.  In the CollectData... calls we need to hand this
   // off to the IOHandler, which may run asynchronously. So we have to have
   // some way to keep it alive, and not leak it. Making it an ivar of the
   // command object, which never goes away achieves this.  Note that if we were
   // able to run the same command concurrently in one interpreter we'd have to

lldb/source/Commands/CommandObjectThread.cpp

CommandObjectThreadStepWithTypeAndScope(CommandInterpreter &interpreter,
     // There is only one variant this argument could be; put it into the
     // argument entry.
     arg.push_back(thread_id_arg);
 
     // Push the data for the first argument into the m_arguments vector.
     m_arguments.push_back(arg);
     
     if (step_type == eStepTypeScripted) {
-      m_all_options.Append(&m_class_options, LLDB_OPT_SET_1, LLDB_OPT_SET_1);
+      m_all_options.Append(&m_class_options, 
+                           LLDB_OPT_SET_1 | LLDB_OPT_SET_2, 
+                           LLDB_OPT_SET_1);
     }
     m_all_options.Append(&m_options);
     m_all_options.Finalize();
   }
 
   ~CommandObjectThreadStepWithTypeAndScope() override = default;
 
   Options *GetOptions() override { 
if (command.GetArgumentCount() == 0) {
             "Thread index %u is out of range (valid values are 0 - %u).\n",
             step_thread_idx, num_threads);
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
     }
 
     if (m_step_type == eStepTypeScripted) {
-      if (m_class_options.GetClassName().empty()) {
+      if (m_class_options.GetName().empty()) {
         result.AppendErrorWithFormat("empty class name for scripted step.");
         result.SetStatus(eReturnStatusFailed);
         return false;
       } else if (!GetDebugger().GetScriptInterpreter()->CheckObjectExists(
-                     m_class_options.GetClassName().c_str())) {
+                     m_class_options.GetName().c_str())) {
         result.AppendErrorWithFormat(
             "class for scripted step: \"%s\" does not exist.",
-            m_class_options.GetClassName().c_str());
+            m_class_options.GetName().c_str());
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
     }
 
     if (m_options.m_end_line != LLDB_INVALID_LINE_NUMBER &&
         m_step_type != eStepTypeInto) {
       result.AppendErrorWithFormat(
if (m_step_type == eStepTypeInto) {
           true, abort_other_plans, bool_stop_other_threads, new_plan_status);
     } else if (m_step_type == eStepTypeOut) {
       new_plan_sp = thread->QueueThreadPlanForStepOut(
           abort_other_plans, nullptr, false, bool_stop_other_threads, eVoteYes,
           eVoteNoOpinion, thread->GetSelectedFrameIndex(), new_plan_status,
           m_options.m_step_out_avoid_no_debug);
     } else if (m_step_type == eStepTypeScripted) {
       new_plan_sp = thread->QueueThreadPlanForStepScripted(
-          abort_other_plans, m_class_options.GetClassName().c_str(), 
+          abort_other_plans, m_class_options.GetName().c_str(), 
           m_class_options.GetStructuredData(),
           bool_stop_other_threads, new_plan_status);
     } else {
       result.AppendError("step type is not supported");
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 

lldb/source/Commands/Options.td

def breakpoint_add_one_liner : Option<"one-liner", "o">, Group<1>,
     "sure to surround it with quotes.">;
   def breakpoint_add_stop_on_error : Option<"stop-on-error", "e">,
     Arg<"Boolean">, Desc<"Specify whether breakpoint command execution should "
     "terminate on error.">;
   def breakpoint_add_script_type : Option<"script-type", "s">,
     EnumArg<"None", "ScriptOptionEnum()">,
     Desc<"Specify the language for the commands - if none is specified, the "
     "lldb command interpreter will be used.">;
-  def breakpoint_add_python_function : Option<"python-function", "F">,
-    Group<2>, Arg<"PythonFunction">,
-    Desc<"Give the name of a Python function to run as command for this "
-    "breakpoint. Be sure to give a module name if appropriate.">;
   def breakpoint_add_dummy_breakpoints : Option<"dummy-breakpoints", "D">,
     Desc<"Sets Dummy breakpoints - i.e. breakpoints set before a file is "
     "provided, which prime new targets.">;
 }
 
 let Command = "breakpoint command delete" in {
   def breakpoint_command_delete_dummy_breakpoints :
     Option<"dummy-breakpoints", "D">, Group<1>,
def thread_step_scope_run_mode : Option<"run-mode", "m">, Group<1>,
     EnumArg<"RunMode", "TriRunningModes()">, Desc<"Determine how to run other "
     "threads while stepping the current thread.">;
   def thread_step_scope_step_over_regexp : Option<"step-over-regexp", "r">,
     Group<1>, Arg<"RegularExpression">, Desc<"A regular expression that defines"
     "function names to not to stop at when stepping in.">;
   def thread_step_scope_step_in_target : Option<"step-in-target", "t">,
     Group<1>, Arg<"FunctionName">, Desc<"The name of the directly called "
     "function step in should stop at when stepping into.">;
-  def thread_step_scope_python_class : Option<"python-class", "C">, Group<2>,
-    Arg<"PythonClass">, Desc<"The name of the class that will manage this step "
-    "- only supported for Scripted Step.">;
 }
 
 let Command = "thread until" in {
   def thread_until_frame : Option<"frame", "f">, Group<1>, Arg<"FrameIndex">,
     Desc<"Frame index for until operation - defaults to 0">;
   def thread_until_thread : Option<"thread", "t">, Group<1>, Arg<"ThreadIndex">,
     Desc<"Thread index for the thread for until operation">;
   def thread_until_run_mode : Option<"run-mode", "m">, Group<1>,

lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp

 
 #include "lldb/Host/OptionParser.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 OptionGroupPythonClassWithDict::OptionGroupPythonClassWithDict
     (const char *class_use,
+     bool is_class,
      int class_option,
      int key_option, 
-     int value_option,
-     const char *class_long_option,
-     const char *key_long_option,
-     const char *value_long_option,
-     bool required) {
-  m_key_usage_text.assign("The key for a key/value pair passed to the class"
-                            " that implements a ");
+     int value_option) : OptionGroup(), m_is_class(is_class) {
+  m_key_usage_text.assign("The key for a key/value pair passed to the "
+                          "implementation of a ");
   m_key_usage_text.append(class_use);
   m_key_usage_text.append(".  Pairs can be specified more than once.");
   
-  m_value_usage_text.assign("The value for a previous key in the pair passed to"
-                            " the class that implements a ");
+  m_value_usage_text.assign("The value for the previous key in the pair passed "
+                            "to the implementation of a ");
   m_value_usage_text.append(class_use);
   m_value_usage_text.append(".  Pairs can be specified more than once.");
   
-  m_class_usage_text.assign("The name of the class that will manage a ");
+  m_class_usage_text.assign("The name of the ");
+  m_class_usage_text.append(m_is_class ? "class" : "function");
+  m_class_usage_text.append(" that will manage a ");
   m_class_usage_text.append(class_use);
   m_class_usage_text.append(".");
   
   m_option_definition[0].usage_mask = LLDB_OPT_SET_1;
-  m_option_definition[0].required = required;
-  m_option_definition[0].long_option = class_long_option;
+  m_option_definition[0].required = true;
+  m_option_definition[0].long_option = "script-class";
   m_option_definition[0].short_option = class_option;
   m_option_definition[0].validator = nullptr;
   m_option_definition[0].option_has_arg = OptionParser::eRequiredArgument;
   m_option_definition[0].enum_values = {};
   m_option_definition[0].completion_type = 0;
   m_option_definition[0].argument_type = eArgTypePythonClass;
   m_option_definition[0].usage_text = m_class_usage_text.data();
 
-  m_option_definition[1].usage_mask = LLDB_OPT_SET_1;
-  m_option_definition[1].required = required;
-  m_option_definition[1].long_option = key_long_option;
+  m_option_definition[1].usage_mask = LLDB_OPT_SET_2;
+  m_option_definition[1].required = false;
+  m_option_definition[1].long_option = "structured-data-key";
   m_option_definition[1].short_option = key_option;
   m_option_definition[1].validator = nullptr;
   m_option_definition[1].option_has_arg = OptionParser::eRequiredArgument;
   m_option_definition[1].enum_values = {};
   m_option_definition[1].completion_type = 0;
   m_option_definition[1].argument_type = eArgTypeNone;
   m_option_definition[1].usage_text = m_key_usage_text.data();
 
-  m_option_definition[2].usage_mask = LLDB_OPT_SET_1;
-  m_option_definition[2].required = required;
-  m_option_definition[2].long_option = value_long_option;
+  m_option_definition[2].usage_mask = LLDB_OPT_SET_2;
+  m_option_definition[2].required = false;
+  m_option_definition[2].long_option = "structured-data-value";
   m_option_definition[2].short_option = value_option;
   m_option_definition[2].validator = nullptr;
   m_option_definition[2].option_has_arg = OptionParser::eRequiredArgument;
   m_option_definition[2].enum_values = {};
   m_option_definition[2].completion_type = 0;
   m_option_definition[2].argument_type = eArgTypeNone;
   m_option_definition[2].usage_text = m_value_usage_text.data();
+  
+  m_option_definition[3].usage_mask = LLDB_OPT_SET_3;
+  m_option_definition[3].required = true;
+  m_option_definition[3].long_option = "python-function";
+  m_option_definition[3].short_option = class_option;
+  m_option_definition[3].validator = nullptr;
+  m_option_definition[3].option_has_arg = OptionParser::eRequiredArgument;
+  m_option_definition[3].enum_values = {};
+  m_option_definition[3].completion_type = 0;
+  m_option_definition[3].argument_type = eArgTypePythonFunction;
+  m_option_definition[3].usage_text = m_class_usage_text.data();
+
 }
 
 OptionGroupPythonClassWithDict::~OptionGroupPythonClassWithDict() {}
 
 Status OptionGroupPythonClassWithDict::SetOptionValue(
     uint32_t option_idx,
     llvm::StringRef option_arg,
     ExecutionContext *execution_context) {
   Status error;
   switch (option_idx) {
-  case 0: {
-    m_class_name.assign(option_arg);
+  case 0:
+  case 3: {
+    m_name.assign(option_arg);
   } break;
   case 1: {
+      if (!m_dict_sp)
+        m_dict_sp = std::make_shared<StructuredData::Dictionary>();
       if (m_current_key.empty())
         m_current_key.assign(option_arg);
       else
         error.SetErrorStringWithFormat("Key: \"%s\" missing value.",
                                         m_current_key.c_str());
     
   } break;
   case 2: {
+      if (!m_dict_sp)
+        m_dict_sp = std::make_shared<StructuredData::Dictionary>();
       if (!m_current_key.empty()) {
           m_dict_sp->AddStringItem(m_current_key, option_arg);
           m_current_key.clear();
       }
       else
         error.SetErrorStringWithFormat("Value: \"%s\" missing matching key.",
                                        option_arg.str().c_str());
   } break;
   default:
     llvm_unreachable("Unimplemented option");
   }
   return error;
 }
 
 void OptionGroupPythonClassWithDict::OptionParsingStarting(
   ExecutionContext *execution_context) {
   m_current_key.erase();
-  m_dict_sp = std::make_shared<StructuredData::Dictionary>();
+  // Leave the dictionary shared pointer unset.  That way you can tell that
+  // the user didn't pass any -k -v pairs.  We want to be able to warn if these
+  // were passed when the function they passed won't use them.
+  m_dict_sp.reset();
 }
 
 Status OptionGroupPythonClassWithDict::OptionParsingFinished(
   ExecutionContext *execution_context) {
   Status error;
   // If we get here and there's contents in the m_current_key, somebody must
   // have provided a key but no value.
   if (!m_current_key.empty())
       error.SetErrorStringWithFormat("Key: \"%s\" missing value.",
                                      m_current_key.c_str());
   return error;
 }
 

lldb/source/Interpreter/ScriptInterpreter.cpp

Status ScriptInterpreter::SetBreakpointCommandCallback(
   for (BreakpointOptions *bp_options : bp_options_vec) {
     return_error = SetBreakpointCommandCallback(bp_options, callback_text);
     if (return_error.Success())
       break;
   }
   return return_error;
 }
 
-void ScriptInterpreter::SetBreakpointCommandCallbackFunction(
+Status ScriptInterpreter::SetBreakpointCommandCallbackFunction(
     std::vector<BreakpointOptions *> &bp_options_vec,
-    const char *function_name) {
+    const char *function_name,
+    StructuredData::ObjectSP extra_args_sp) {
+  Status error;
   for (BreakpointOptions *bp_options : bp_options_vec) {
-    SetBreakpointCommandCallbackFunction(bp_options, function_name);
+    error = SetBreakpointCommandCallbackFunction(bp_options, function_name, 
+                                         extra_args_sp);
+    if (!error.Success())
+      return error;
   }
+  return error;
 }
 
 std::unique_ptr<ScriptInterpreterLocker>
 ScriptInterpreter::AcquireInterpreterLock() {
   return std::unique_ptr<ScriptInterpreterLocker>(
       new ScriptInterpreterLocker());
 }

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

 #ifdef LLDB_DISABLE_PYTHON
 
 // Python is disabled in this build
 
 #else
 
 #include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Core/IOHandler.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/lldb-private.h"
 
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace lldb_private {
 /// Abstract interface for the Python script interpreter.
 class ScriptInterpreterPython : public ScriptInterpreter,
                                 public IOHandlerDelegateMultiline {
 public:
   class CommandDataPython : public BreakpointOptions::CommandData {
   public:
     CommandDataPython() : BreakpointOptions::CommandData() {
       interpreter = lldb::eScriptLanguagePython;
     }
+    CommandDataPython(StructuredData::ObjectSP extra_args_sp) :
+        BreakpointOptions::CommandData(),
+        m_extra_args_up(new StructuredDataImpl()) {
+        interpreter = lldb::eScriptLanguagePython;
+        m_extra_args_up->SetObjectSP(extra_args_sp);
+    }
+    lldb::StructuredDataImplUP m_extra_args_up;
   };
 
   ScriptInterpreterPython(Debugger &debugger)
       : ScriptInterpreter(debugger, lldb::eScriptLanguagePython),
         IOHandlerDelegateMultiline("DONE") {}
 
   static void Initialize();
   static void Terminate();

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

 #endif
 
 // These prototypes are the Pythonic implementations of the required callbacks.
 // Although these are scripting-language specific, their definition depends on
 // the public API.
 extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
     const lldb::StackFrameSP &sb_frame,
-    const lldb::BreakpointLocationSP &sb_bp_loc);
+    const lldb::BreakpointLocationSP &sb_bp_loc,
+    StructuredDataImpl *args_impl);
 
 extern "C" bool LLDBSwigPythonWatchpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
     const lldb::StackFrameSP &sb_frame, const lldb::WatchpointSP &sb_wp);
 
 extern "C" bool LLDBSwigPythonCallTypeScript(
     const char *python_function_name, void *session_dictionary,
     const lldb::ValueObjectSP &valobj_sp, void **pyfunct_wrapper,
for (auto bp_options : *bp_options_vec) {
       if (!bp_options)
         continue;
 
       auto data_up = std::make_unique<CommandDataPython>();
       if (!data_up)
         break;
       data_up->user_source.SplitIntoLines(data);
 
+      StructuredData::ObjectSP empty_args_sp;
       if (GenerateBreakpointCommandCallbackData(data_up->user_source,
-                                                data_up->script_source)
+                                                data_up->script_source,
+                                                false)
               .Success()) {
         auto baton_sp = std::make_shared<BreakpointOptions::CommandBaton>(
             std::move(data_up));
         bp_options->SetCallback(
             ScriptInterpreterPythonImpl::BreakpointCallbackFunction, baton_sp);
       } else if (!batch_mode) {
         StreamFileSP error_sp = io_handler.GetErrorStreamFileSP();
         if (error_sp) {
PythonDictionary &ScriptInterpreterPythonImpl::GetSysModuleDictionary() {
 
   PythonObject sys_module(PyRefType::Borrowed, PyImport_AddModule("sys"));
   if (sys_module.IsValid())
     m_sys_module_dict.Reset(PyRefType::Borrowed,
                             PyModule_GetDict(sys_module.get()));
   return m_sys_module_dict;
 }
 
+llvm::Expected<size_t>
+ScriptInterpreterPythonImpl::GetNumFixedArgumentsForCallable(
+    const llvm::StringRef &callable_name)
+{
+  if (callable_name.empty()) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "called with empty callable name.");
+  }
+  Locker py_lock(this, Locker::AcquireLock |
+                 Locker::InitSession |
+                 Locker::NoSTDIN);
+  auto dict = PythonModule::MainModule()
+      .ResolveName<PythonDictionary>(m_dictionary_name);
+  auto pfunc 
+     = PythonObject::ResolveNameWithDictionary<PythonCallable>(callable_name, 
+                                                               dict);
+  if (!pfunc.IsAllocated()) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "can't find callable: %s", callable_name.str().c_str());
+  }
+  PythonCallable::ArgInfo arg_info = pfunc.GetNumArguments();
+  return arg_info.count;
+}
+
 static std::string GenerateUniqueName(const char *base_name_wanted,
                                       uint32_t &functions_counter,
                                       const void *name_token = nullptr) {
   StreamString sstr;
 
   if (!base_name_wanted)
     return std::string();
 

 
 void ScriptInterpreterPythonImpl::CollectDataForWatchpointCommandCallback(
     WatchpointOptions *wp_options, CommandReturnObject &result) {
   m_active_io_handler = eIOHandlerWatchpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
       "    ", *this, true, wp_options);
 }
 
-void ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
-    BreakpointOptions *bp_options, const char *function_name) {
+Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
+    BreakpointOptions *bp_options, const char *function_name,
+    StructuredData::ObjectSP extra_args_sp) {
+  Status error;
   // For now just cons up a oneliner that calls the provided function.
   std::string oneliner("return ");
   oneliner += function_name;
+  
+  llvm::Expected<size_t> maybe_args 
+      = GetNumFixedArgumentsForCallable(function_name);
+  if (!maybe_args) {
+    error.SetErrorStringWithFormat("could not get num args: %s", 
+        llvm::toString(maybe_args.takeError()).c_str());
+    return error;
+  }
+  size_t num_args = *maybe_args;
+  
+  bool uses_extra_args = false;
+  if (num_args == 4) {
+    uses_extra_args = true;
+    oneliner += "(frame, bp_loc, extra_args, internal_dict)";
+  }
+  else if (num_args == 3) {
+    if (extra_args_sp) {
+      error.SetErrorString("cannot pass extra_args to a three argument callback"
+                          );
+      return error;
+    }
+    uses_extra_args = false;
     oneliner += "(frame, bp_loc, internal_dict)";
-  m_debugger.GetScriptInterpreter()->SetBreakpointCommandCallback(
-      bp_options, oneliner.c_str());
+  } else {
+    error.SetErrorStringWithFormat("expected 3 or 4 argument "
+                                   "function, %s has %d",
+                                   function_name, num_args);
+    return error;
+  }
+  
+  SetBreakpointCommandCallback(bp_options, oneliner.c_str(), extra_args_sp, 
+                               uses_extra_args);
+  return error;
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
     BreakpointOptions *bp_options,
     std::unique_ptr<BreakpointOptions::CommandData> &cmd_data_up) {
   Status error;
   error = GenerateBreakpointCommandCallbackData(cmd_data_up->user_source,
-                                                cmd_data_up->script_source);
+                                                cmd_data_up->script_source,
+                                                false);
   if (error.Fail()) {
     return error;
   }
   auto baton_sp =
       std::make_shared<BreakpointOptions::CommandBaton>(std::move(cmd_data_up));
   bp_options->SetCallback(
       ScriptInterpreterPythonImpl::BreakpointCallbackFunction, baton_sp);
   return error;
 }
 
-// Set a Python one-liner as the callback for the breakpoint.
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
     BreakpointOptions *bp_options, const char *command_body_text) {
-  auto data_up = std::make_unique<CommandDataPython>();
+  return SetBreakpointCommandCallback(bp_options, command_body_text, {},false);
+}
 
+// Set a Python one-liner as the callback for the breakpoint.
+Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
+    BreakpointOptions *bp_options, const char *command_body_text,
+    StructuredData::ObjectSP extra_args_sp,
+    bool uses_extra_args) {
+  auto data_up = std::make_unique<CommandDataPython>(extra_args_sp);
   // Split the command_body_text into lines, and pass that to
   // GenerateBreakpointCommandCallbackData.  That will wrap the body in an
   // auto-generated function, and return the function name in script_source.
   // That is what the callback will actually invoke.
 
   data_up->user_source.SplitIntoLines(command_body_text);
   Status error = GenerateBreakpointCommandCallbackData(data_up->user_source,
-                                                       data_up->script_source);
+                                                       data_up->script_source,
+                                                       uses_extra_args);
   if (error.Success()) {
     auto baton_sp =
         std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_up));
     bp_options->SetCallback(
         ScriptInterpreterPythonImpl::BreakpointCallbackFunction, baton_sp);
     return error;
   } else
     return error;

 bool ScriptInterpreterPythonImpl::GenerateTypeSynthClass(
     const char *oneliner, std::string &output, const void *name_token) {
   StringList input;
   input.SplitIntoLines(oneliner, strlen(oneliner));
   return GenerateTypeSynthClass(input, output, name_token);
 }
 
 Status ScriptInterpreterPythonImpl::GenerateBreakpointCommandCallbackData(
-    StringList &user_input, std::string &output) {
+    StringList &user_input, std::string &output,
+    bool has_extra_args) {
   static uint32_t num_created_functions = 0;
   user_input.RemoveBlankLines();
   StreamString sstr;
   Status error;
   if (user_input.GetSize() == 0) {
     error.SetErrorString("No input data.");
     return error;
   }
 
   std::string auto_generated_function_name(GenerateUniqueName(
       "lldb_autogen_python_bp_callback_func_", num_created_functions));
+  if (has_extra_args) 
+    sstr.Printf("def %s (frame, bp_loc, extra_args, internal_dict):",
+                auto_generated_function_name.c_str());
+  else
     sstr.Printf("def %s (frame, bp_loc, internal_dict):",
                 auto_generated_function_name.c_str());
 
   error = GenerateFunction(sstr.GetData(), user_input);
   if (!error.Success())
     return error;
 
   // Store the name of the auto-generated function to be called.
   output.assign(auto_generated_function_name);
   return error;
if (breakpoint_sp) {
         bool ret_val = true;
         {
           Locker py_lock(python_interpreter, Locker::AcquireLock |
                                                  Locker::InitSession |
                                                  Locker::NoSTDIN);
           ret_val = LLDBSwigPythonBreakpointCallbackFunction(
               python_function_name,
               python_interpreter->m_dictionary_name.c_str(), stop_frame_sp,
-              bp_loc_sp);
+              bp_loc_sp,
+              bp_option_data->m_extra_args_up.get());
         }
         return ret_val;
       }
     }
   }
   // We currently always true so we stop in case anything goes wrong when
   // trying to call the script function
   return true;

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

bool RunScriptBasedCommand(
       StructuredData::GenericSP impl_obj_sp, llvm::StringRef args,
       ScriptedCommandSynchronicity synchronicity,
       lldb_private::CommandReturnObject &cmd_retobj, Status &error,
       const lldb_private::ExecutionContext &exe_ctx) override;
 
   Status GenerateFunction(const char *signature,
                           const StringList &input) override;
 
-  Status GenerateBreakpointCommandCallbackData(StringList &input,
-                                               std::string &output) override;
+  Status GenerateBreakpointCommandCallbackData(
+      StringList &input,
+      std::string &output,
+      bool has_extra_args) override;
 
   bool GenerateWatchpointCommandCallbackData(StringList &input,
                                              std::string &output) override;
 
   bool GetScriptedSummary(const char *function_name, lldb::ValueObjectSP valobj,
                           StructuredData::ObjectSP &callee_wrapper_sp,
                           const TypeSummaryOptions &options,
                           std::string &retval) override;
public:
   void
   CollectDataForWatchpointCommandCallback(WatchpointOptions *wp_options,
                                           CommandReturnObject &result) override;
 
   /// Set the callback body text into the callback for the breakpoint.
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
                                       const char *callback_body) override;
 
-  void SetBreakpointCommandCallbackFunction(BreakpointOptions *bp_options,
-                                            const char *function_name) override;
+  Status SetBreakpointCommandCallbackFunction(
+      BreakpointOptions *bp_options,
+      const char *function_name,
+      StructuredData::ObjectSP extra_args_sp) override;
 
   /// This one is for deserialization:
   Status SetBreakpointCommandCallback(
       BreakpointOptions *bp_options,
       std::unique_ptr<BreakpointOptions::CommandData> &data_up) override;
 
+  Status SetBreakpointCommandCallback(
+      BreakpointOptions *bp_options, 
+       const char *command_body_text,
+       StructuredData::ObjectSP extra_args_sp,
+       bool uses_extra_args);
+
   /// Set a one-liner as the callback for the watchpoint.
   void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
                                     const char *oneliner) override;
 
   const char *GetDictionaryName() { return m_dictionary_name.c_str(); }
 
   PyThreadState *GetThreadState() { return m_command_thread_state; }
 
enum ActiveIOHandler {
     eIOHandlerWatchpoint
   };
 
   PythonObject &GetMainModule();
 
   PythonDictionary &GetSessionDictionary();
 
   PythonDictionary &GetSysModuleDictionary();
   
+  llvm::Expected<size_t> 
+  GetNumFixedArgumentsForCallable(const llvm::StringRef &callable_name) 
+      override;
+
   bool GetEmbeddedInterpreterModuleObjects();
 
   bool SetStdHandle(File &file, const char *py_name, PythonFile &save_file,
                     const char *mode);
 
   PythonFile m_saved_stdin;
   PythonFile m_saved_stdout;
   PythonFile m_saved_stderr;

lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

 #else
 extern "C" void init_lldb(void) {}
 #define LLDBSwigPyInit init_lldb
 #endif
 
 extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
     const lldb::StackFrameSP &sb_frame,
-    const lldb::BreakpointLocationSP &sb_bp_loc) {
+    const lldb::BreakpointLocationSP &sb_bp_loc,
+    StructuredDataImpl *args_impl) {
   return false;
 }
 
 extern "C" bool LLDBSwigPythonWatchpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
     const lldb::StackFrameSP &sb_frame, const lldb::WatchpointSP &sb_wp) {
   return false;
 }