[lldb/Target] Support more than 2 symbols in StackFrameRecognizer

[mib]

Summary

This patch changes the way the StackFrame Recognizers match a certain
frame.

Until now, recognizers could be registered with a function
name but also an alternate symbol.
This change is motivated by a test failure for the Assert frame
recognizer on Linux. Depending the version of the libc, the abort
function (triggered by an assertion), could have more than two
signatures (i.e. raise, __GI_raise and gsignal).

Instead of only checking the default symbol name and the alternate one,
lldb will iterate over a list of symbols to match against.

rdar://60386577

Signed-off-by: Med Ismail Bennani <[e-mail]>

[friss]

lldb/include/lldb/Target/StackFrameRecognizer.h

113–117

can this be a const vector, or even better an ArrayRef? We certainly don't want the callbacks to modify the list?

lldb/source/Commands/CommandObjectFrame.cpp

749

Does the command actually accept multiple symbols names now? If yes, it should be tested and a warning should be added when you use the regex option and you pass multiple -n options. If it doesn't accept multiple names, it shouldn't store a vector.

859

*at least

lldb/source/Target/AssertFrameRecognizer.cpp

141

This reads a little weird to me. Should we call it MatchesSymbol instead?

lldb/source/Target/StackFrameRecognizer.cpp

64–67

Is this really the clang-format formatting?

83–84

It's kinda weird to do this here, and I also think it's wrong as it will leave the recognizer with an entry in its symbols list which changes its semantics. If the callback was taking an ArrayRef, it would be easy (and cheap) to create one for a single element as well as from a vector.

[mib]

lldb/source/Target/StackFrameRecognizer.cpp

64–67

It is but I'll change it back to be more compact.

[mib]

Addressed Fred's comments.
Added testing for multi-symbol frame recognizers.

[mib]

Add missing test annotation.

[friss]

lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py

152–157

Please rewrite using lldbutil.run_to_name_breakpoint, otherwise this LGTM

[mib]

Refactored test to use lldbutil.run_to_name_breakpoint.

[labath]

Being able to match multiple symbols sounds useful in its own right, but the motivating case is a bit shady. raise, __GI_raise and gsignal are all aliases to one another (they have the same address). The reason you're sometimes getting gsignal here is not because some glibcs really call this function from their assert macro. It's because we happen to pick that symbol (maybe because it comes first in the symtab, depending on how the library was linked) when doing address resolution.

I'm wondering if that doesn't signal a flaw in the recognizer infrastructure. If we changed the matching logic so that it resolves the name it is supposed to search for, and then does a match by address, then only one name would be sufficient here.

lldb/include/lldb/Target/StackFrameRecognizer.h

105

Using std::vector might be good here as the function "consumes" the argument (by assigning it to the internal recognizer list. But in that case you ought to use std::move, as appropriate to prevent needless copying. If you don't want to optimize things that much, maybe just use ArrayRef here too?

lldb/source/Commands/CommandObjectFrame.cpp

888

Is there something which ensure that m_symbols contains at least one element here? (i.e., that we do not silently drop the extra symbols arguments)

lldb/source/Target/AssertFrameRecognizer.cpp

27

llvm::is_contained(symbols, symbol)

lldb/source/Target/StackFrameRecognizer.cpp

64–67

instead of arguing with clang-format, you could change this to m_recognizers.emplace_front(arguments, without, braces), which will be more efficient, and probably result in better formatting.

125–126

is_contained

[mib]

I'll investigate matching the symbol by address instead of name, but I still think having the ability to register a recognizer with multiple symbols can be useful.

In D76188#1924083, @labath wrote:

Being able to match multiple symbols sounds useful in its own right, but the motivating case is a bit shady. raise, __GI_raise and gsignal are all aliases to one another (they have the same address). The reason you're sometimes getting gsignal here is not because some glibcs really call this function from their assert macro. It's because we happen to pick that symbol (maybe because it comes first in the symtab, depending on how the library was linked) when doing address resolution.

I'm wondering if that doesn't signal a flaw in the recognizer infrastructure. If we changed the matching logic so that it resolves the name it is supposed to search for, and then does a match by address, then only one name would be sufficient here.

lldb/source/Commands/CommandObjectFrame.cpp

888

Few lines above, we make sure the symbols list is not empty.

[mib]

Addressed Pavel's comment.

[jingham]

In D76188#1924083, @labath wrote:

Being able to match multiple symbols sounds useful in its own right, but the motivating case is a bit shady. raise, __GI_raise and gsignal are all aliases to one another (they have the same address). The reason you're sometimes getting gsignal here is not because some glibcs really call this function from their assert macro. It's because we happen to pick that symbol (maybe because it comes first in the symtab, depending on how the library was linked) when doing address resolution.

I'm wondering if that doesn't signal a flaw in the recognizer infrastructure. If we changed the matching logic so that it resolves the name it is supposed to search for, and then does a match by address, then only one name would be sufficient here.

Note that a frame recognizer should trigger any time the you stop in a recognized frame, not just at the beginning. So if you wanted to do address matching, you would have to get the symbol address and its extent, and then match the incoming address against that. This seems like a nice optimization, with the side effect that it handles aliased symbols transparently.

[mib]

@labath Do you object if I land this patch to revert our CIs to green and work on matching the symbol by address instead of name on a separate one ?

Also do you know what's the "default" symbol I should look for with the assert frame recognizer ?

[JDevlieghere]

In D76188#1924840, @mib wrote:

@labath Do you object if I land this patch to revert our CIs to green and work on matching the symbol by address instead of name on a separate one ?

Can we temporarily skip the test?

[labath]

In D76188#1924840, @mib wrote:

@labath Do you object if I land this patch to revert our CIs to green and work on matching the symbol by address instead of name on a separate one ?

I think this is fine -- a vector is definitely cleaner than two member variables, and I can imagine there being a situation where you'd need to match multiple symbols that aren't just aliases of one another.

I just have one question about the regex handling.

Also do you know what's the "default" symbol I should look for with the assert frame recognizer ?

raise is the canonical name. __GI_raise is an internal name you'll only get if you have debug info (.symtab) for libc installed. gsignal is an alias to raise only due to a "historical mistake". I am not sure if anyone is actually using it (in fact, I didn't know the thing exists until yesterday).

lldb/source/Commands/CommandObjectFrame.cpp

888

Not empty is one thing, but what about the case when it contains 2 or more elements (frame recognizer add --name foo --name bar --regex)? Will that produce some kind of an error, or will it just ignore the second regex ?

[mib]

lldb/source/Commands/CommandObjectFrame.cpp

888

It takes only the first symbol and ignores the other ones.

[labath]

lldb/source/Commands/CommandObjectFrame.cpp

888

Hmm.. that doesn't sound very useful. I think that should be an error.

[jingham]

lldb/source/Commands/CommandObjectFrame.cpp

888

Just put --name and --regex in different option groups when you define the command. Then the command machinery won't allow you to specify both at the same time (and the help will show they are mutually incompatible).

I agree with Pavel, an error is what you want in that case.

[mib]

lldb/source/Commands/CommandObjectFrame.cpp

888

@jingham I think this is wrong because --regex is just a boolean flag, not the actual regular expression.

When this flag is enabled, the overloading for AddRecognizerthat is called is the one that holds RegularExpressionSPs instead of the overloading with a ConstString and llvm::ArrayRef<ConstString> ...

I'll add the check above to make sure m_symbols has only 1 entry when the --regex flag is enable and return an error otherwise.

[mib]

Added error when passing multiple symbols with --regex flag.

[jingham]

lldb/source/Commands/CommandObjectFrame.cpp

888

Oh, that's unfortunate. It's weird that you can pass --name twice w/o the --regex but only once with. But it's probably too late to change that now.

You solution sounds fine, though you should also amend the help string for --name to say "can be specified more than once except if --regex is provided" or this will be really confusing. BTW, if there any reason why this can't take more than one regex? I allowed only one for breakpoints and things like the "no step into source regex" on the theory that you can always just use "|" to concatenate two regex's. That is in the end probably more efficient but for maintenance it's easier if your regex's don't get too long, so I have wished on occasion since then that I had allowed more than one regex for these options.

Note, the reason I don't like "--regex" as a flag is it makes the job of providing completions for --name harder. Note, it's still possible if the user typed:

(lldb) whatever --regex --name A<TAB>

because the option completion for --name can actually look at the whole command line. For instance,

--shlib libBar.dylib --name Foo<TAB>

will limit the completions to names in libBar. But that makes it trickier to do.

For folks designing commands, IMO would be better to have --name <NAME> and --regex <REGEX> and not have the --regex be just a flag.

[mib]

lldb/source/Commands/CommandObjectFrame.cpp

888

I didn't know that completion could be restricted by other flags, this sounds like a neat "optimization".

The issue with making the --regex as the actual regular expression is that it will prevent us to distinguish the shared library regex from the symbol regex.

I'm not against changing that behavior to have better completion and more homogeneous commands but this would need to be done on a separate patch.

[jingham]

lldb/source/Commands/CommandObjectFrame.cpp

888

It's fine to do that as a separate patch, or leave it for later as your time dictates. This feature doesn't demand it.

But you should document the current behavior in the option string for --name.

[mib]

Updated frame recognizer add help description.

[labath]

looks good to me

lldb/include/lldb/Target/StackFrameRecognizer.h

 /// \class StackFrameRecognizerManager
 ///
 /// Static class that provides a registry of known stack frame recognizers.
 /// Has static methods to add, enumerate, remove, query and invoke recognizers.
 
 class StackFrameRecognizerManager {
 public:
   static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
-                            ConstString module, ConstString symbol,
-                            ConstString alternate_symbol,
+                            ConstString module,
+                            llvm::ArrayRef<ConstString> symbols,
                             bool first_instruction_only = true);
 
   static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
                             lldb::RegularExpressionSP module,
                             lldb::RegularExpressionSP symbol,
                             bool first_instruction_only = true);
 
-  static void ForEach(
-      std::function<void(uint32_t recognizer_id, std::string recognizer_name,
-                         std::string module, std::string symbol,
-                         std::string alternate_symbol, bool regexp)> const
-          &callback);
+  static void
+  ForEach(std::function<void(uint32_t recognizer_id,
+                             std::string recognizer_name, std::string module,
+                             llvm::ArrayRef<ConstString> symbols,
+                             bool regexp)> const &callback);
 
   static bool RemoveRecognizerWithID(uint32_t recognizer_id);
 
   static void RemoveAllRecognizers();
 
   static lldb::StackFrameRecognizerSP GetRecognizerForFrame(
       lldb::StackFrameSP frame);
 

lldb/packages/Python/lldbsuite/test/lldbutil.py

def run_to_breakpoint_do_run(test, target, bkpt, launch_info = None,
     threads = get_threads_stopped_at_breakpoint(
                 process, bkpt)
 
     num_threads = len(threads)
     if only_one_thread:
         test.assertEqual(num_threads, 1, "Expected 1 thread to stop at breakpoint, %d did."%(num_threads))
     else:
         test.assertGreater(num_threads, 0, "No threads stopped at breakpoint")
-        
     thread = threads[0]
     return (target, process, thread, bkpt)
 
 def run_to_name_breakpoint (test, bkpt_name, launch_info = None,
                             exe_name = "a.out",
                             bkpt_module = None,
                             in_cwd = True,
                             only_one_thread = True,

lldb/source/Commands/CommandObjectFrame.cpp

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
       switch (short_option) {
       case 'l':
         m_class_name = std::string(option_arg);
         break;
       case 's':
         m_module = std::string(option_arg);
         break;
       case 'n':
-        m_function = std::string(option_arg);
+        m_symbols.push_back(std::string(option_arg));
         break;
       case 'x':
         m_regex = true;
         break;
       default:
         llvm_unreachable("Unimplemented option");
       }
 
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_module = "";
-      m_function = "";
+      m_symbols.clear();
       m_class_name = "";
       m_regex = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_frame_recognizer_add_options);
     }
 
     // Instance variables to hold the values for command options.
     std::string m_class_name;
     std::string m_module;
-    std::string m_function;
+    std::vector<std::string> m_symbols;
     bool m_regex;
   };
 
   CommandOptions m_options;
 
   Options *GetOptions() override { return &m_options; }
 
 protected:
#if LLDB_ENABLE_PYTHON
 
   if (m_options.m_module.empty()) {
     result.AppendErrorWithFormat("%s needs a module name (-s argument).\n",
                                  m_cmd_name.c_str());
     result.SetStatus(eReturnStatusFailed);
     return false;
   }
 
-  if (m_options.m_function.empty()) {
-    result.AppendErrorWithFormat("%s needs a function name (-n argument).\n",
-                                 m_cmd_name.c_str());
+  if (m_options.m_symbols.empty()) {
+    result.AppendErrorWithFormat(
+        "%s needs at least one symbol name (-n argument).\n",
+        m_cmd_name.c_str());
+    result.SetStatus(eReturnStatusFailed);
+    return false;
+  }
+
+  if (m_options.m_regex && m_options.m_symbols.size() > 1) {
+    result.AppendErrorWithFormat(
+        "%s needs only one symbol regular expression (-n argument).\n",
     result.SetStatus(eReturnStatusFailed);
     return false;
   }
 
   ScriptInterpreter *interpreter = GetDebugger().GetScriptInterpreter();
 
   if (interpreter &&
       !interpreter->CheckObjectExists(m_options.m_class_name.c_str())) {
     result.AppendWarning("The provided class does not exist - please define it "
                          "before attempting to use this frame recognizer");
   }
 
   StackFrameRecognizerSP recognizer_sp =
       StackFrameRecognizerSP(new ScriptedStackFrameRecognizer(
           interpreter, m_options.m_class_name.c_str()));
   if (m_options.m_regex) {
     auto module =
         RegularExpressionSP(new RegularExpression(m_options.m_module));
     auto func =
-        RegularExpressionSP(new RegularExpression(m_options.m_function));
+        RegularExpressionSP(new RegularExpression(m_options.m_symbols.front()));
     StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func);
   } else {
     auto module = ConstString(m_options.m_module);
-    auto func = ConstString(m_options.m_function);
-    StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func, {});
+    std::vector<ConstString> symbols(m_options.m_symbols.begin(),
+                                     m_options.m_symbols.end());
+    StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, symbols);
   }
 #endif
 
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
   return result.Succeeded();
 }
 
 class CommandObjectFrameRecognizerClear : public CommandObjectParsed {
CommandObjectFrameRecognizerList(CommandInterpreter &interpreter)
                             nullptr) {}
 
   ~CommandObjectFrameRecognizerList() override = default;
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     bool any_printed = false;
     StackFrameRecognizerManager::ForEach(
-        [&result, &any_printed](uint32_t recognizer_id, std::string name,
-                                std::string module, std::string symbol,
-                                std::string alternate_symbol, bool regexp) {
+        [&result, &any_printed](
+            uint32_t recognizer_id, std::string name, std::string module,
+            llvm::ArrayRef<ConstString> symbols, bool regexp) {
           Stream &stream = result.GetOutputStream();
 
           if (name.empty())
             name = "(internal)";
 
           stream << std::to_string(recognizer_id) << ": " << name;
           if (!module.empty())
             stream << ", module " << module;
-          if (!symbol.empty())
-            stream << ", function " << symbol;
-          if (!alternate_symbol.empty())
-            stream << ", symbol " << alternate_symbol;
+          if (!symbols.empty())
+            for (auto &symbol : symbols)
+              stream << ", symbol " << symbol;
           if (regexp)
             stream << " (regexp)";
 
           stream.EOL();
           stream.Flush();
 
           any_printed = true;
         });

lldb/source/Commands/Options.td

 
 let Command = "frame recognizer add" in {
   def frame_recognizer_shlib : Option<"shlib", "s">, Arg<"ShlibName">,
     Completion<"Module">,
     Desc<"Name of the module or shared library that this recognizer applies "
     "to.">;
   def frame_recognizer_function : Option<"function", "n">, Arg<"Name">,
     Completion<"Symbol">,
-    Desc<"Name of the function that this recognizer applies to.">;
+    Desc<"Name of the function that this recognizer applies to. "
+         "Can be specified more than once except if -x|--regex is provided.">;
   def frame_recognizer_python_class : Option<"python-class", "l">, Group<2>,
     Arg<"PythonClass">,
     Desc<"Give the name of a Python class to use for this frame recognizer.">;
   def frame_recognizer_regex : Option<"regex", "x">,
     Desc<"Function name and module name are actually regular expressions.">;
 }
 
 let Command = "history" in {

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

 };
 
 static void RegisterObjCExceptionRecognizer() {
   static llvm::once_flag g_once_flag;
   llvm::call_once(g_once_flag, []() {
     FileSpec module;
     ConstString function;
     std::tie(module, function) = AppleObjCRuntime::GetExceptionThrowLocation();
+    std::vector<ConstString> symbols = {function};
     StackFrameRecognizerManager::AddRecognizer(
         StackFrameRecognizerSP(new ObjCExceptionThrowFrameRecognizer()),
-        module.GetFilename(), function, /*alternate_symbol*/ {},
+        module.GetFilename(), symbols,
         /*first_instruction_only*/ true);
   });
 }

lldb/source/Target/AssertFrameRecognizer.cpp

 using namespace lldb_private;
 
 namespace lldb_private {
 
 /// Stores a function module spec, symbol name and possibly an alternate symbol
 /// name.
 struct SymbolLocation {
   FileSpec module_spec;
-  ConstString symbol_name;
-  ConstString alternate_symbol_name;
+  std::vector<ConstString> symbols;
 };
 
 /// Fetches the abort frame location depending on the current platform.
 ///
 /// \param[in] os
 ///    The target's os type.
 /// \param[in,out] location
 ///    The struct that will contain the abort module spec and symbol names.
 /// \return
 ///    \b true, if the platform is supported
 ///    \b false, otherwise.
 bool GetAbortLocation(llvm::Triple::OSType os, SymbolLocation &location) {
   switch (os) {
   case llvm::Triple::Darwin:
   case llvm::Triple::MacOSX:
     location.module_spec = FileSpec("libsystem_kernel.dylib");
-    location.symbol_name.SetString("__pthread_kill");
+    location.symbols.push_back(ConstString("__pthread_kill"));
     break;
   case llvm::Triple::Linux:
     location.module_spec = FileSpec("libc.so.6");
-    location.symbol_name.SetString("raise");
-    location.alternate_symbol_name.SetString("__GI_raise");
+    location.symbols.push_back(ConstString("raise"));
+    location.symbols.push_back(ConstString("__GI_raise"));
+    location.symbols.push_back(ConstString("gsignal"));
     break;
   default:
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
     LLDB_LOG(log, "AssertFrameRecognizer::GetAbortLocation Unsupported OS");
     return false;
   }
 
   return true;
 }
 
 /// Fetches the assert frame location depending on the current platform.
 ///
 /// \param[in] os
 ///    The target's os type.
 /// \param[in,out] location
 ///    The struct that will contain the assert module spec and symbol names.
 /// \return
 ///    \b true, if the platform is supported
 ///    \b false, otherwise.
 bool GetAssertLocation(llvm::Triple::OSType os, SymbolLocation &location) {
   switch (os) {
   case llvm::Triple::Darwin:
   case llvm::Triple::MacOSX:
     location.module_spec = FileSpec("libsystem_c.dylib");
-    location.symbol_name.SetString("__assert_rtn");
+    location.symbols.push_back(ConstString("__assert_rtn"));
     break;
   case llvm::Triple::Linux:
     location.module_spec = FileSpec("libc.so.6");
-    location.symbol_name.SetString("__assert_fail");
-    location.alternate_symbol_name.SetString("__GI___assert_fail");
+    location.symbols.push_back(ConstString("__assert_fail"));
+    location.symbols.push_back(ConstString("__GI___assert_fail"));
     break;
   default:
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
     LLDB_LOG(log, "AssertFrameRecognizer::GetAssertLocation Unsupported OS");
     return false;
   }
 
   return true;
 }
 
 void RegisterAssertFrameRecognizer(Process *process) {
   static llvm::once_flag g_once_flag;
   llvm::call_once(g_once_flag, [process]() {
     Target &target = process->GetTarget();
     llvm::Triple::OSType os = target.GetArchitecture().GetTriple().getOS();
     SymbolLocation location;
 
     if (!GetAbortLocation(os, location))
       return;
 
     StackFrameRecognizerManager::AddRecognizer(
         StackFrameRecognizerSP(new AssertFrameRecognizer()),
-        location.module_spec.GetFilename(), ConstString(location.symbol_name),
-        ConstString(location.alternate_symbol_name),
+        location.module_spec.GetFilename(), location.symbols,
         /*first_instruction_only*/ false);
   });
 }
 
 } // namespace lldb_private
 
 lldb::RecognizedStackFrameSP
 AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {
for (uint32_t frame_index = 0; frame_index < frames_to_fetch; frame_index++) {
     SymbolContext sym_ctx =
         prev_frame_sp->GetSymbolContext(eSymbolContextEverything);
 
     if (!sym_ctx.module_sp->GetFileSpec().FileEquals(location.module_spec))
       continue;
 
     ConstString func_name = sym_ctx.GetFunctionName();
 
-    if (func_name == location.symbol_name ||
-        (!location.alternate_symbol_name.IsEmpty() &&
-         func_name == location.alternate_symbol_name)) {
-
+    if (llvm::is_contained(location.symbols, func_name)) {
       // We go a frame beyond the assert location because the most relevant
       // frame for the user is the one in which the assert function was called.
       // If the assert location is the last frame fetched, then it is set as
       // the most relevant frame.
 
       StackFrameSP most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(
           std::min(frame_index + 1, last_frame_index));
 

lldb/source/Target/StackFrameRecognizer.cpp

ScriptedStackFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame) {
 
   return RecognizedStackFrameSP(
       new ScriptedRecognizedStackFrame(args_synthesized));
 }
 
 class StackFrameRecognizerManagerImpl {
 public:
   void AddRecognizer(StackFrameRecognizerSP recognizer, ConstString module,
-                     ConstString symbol, ConstString alternate_symbol,
+                     llvm::ArrayRef<ConstString> symbols,
                      bool first_instruction_only) {
     m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
-                              false, module, RegularExpressionSP(), symbol,
-                              alternate_symbol, RegularExpressionSP(),
-                              first_instruction_only});
+                              false, module, RegularExpressionSP(), symbols,
+                              RegularExpressionSP(), first_instruction_only});
   }
 
   void AddRecognizer(StackFrameRecognizerSP recognizer,
                      RegularExpressionSP module, RegularExpressionSP symbol,
                      bool first_instruction_only) {
-    m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
-                              true, ConstString(), module, ConstString(),
-                              ConstString(), symbol, first_instruction_only});
+    m_recognizers.push_front(
+        {(uint32_t)m_recognizers.size(), false, recognizer, true, ConstString(),
+         module, std::vector<ConstString>(), symbol, first_instruction_only});
   }
 
-  void ForEach(
-      std::function<void(uint32_t recognized_id, std::string recognizer_name,
-                         std::string module, std::string symbol,
-                         std::string alternate_symbol, bool regexp)> const
-          &callback) {
+  void ForEach(std::function<
+               void(uint32_t recognized_id, std::string recognizer_name,
+                    std::string module, llvm::ArrayRef<ConstString> symbols,
+                    bool regexp)> const &callback) {
     for (auto entry : m_recognizers) {
       if (entry.is_regexp) {
         std::string module_name;
         std::string symbol_name;
 
         if (entry.module_regexp)
           module_name = entry.module_regexp->GetText().str();
         if (entry.symbol_regexp)
           symbol_name = entry.symbol_regexp->GetText().str();
 
         callback(entry.recognizer_id, entry.recognizer->GetName(), module_name,
-                 symbol_name, {}, true);
+                 llvm::makeArrayRef(ConstString(symbol_name)), true);
 
       } else {
-        std::string alternate_symbol;
-        if (!entry.alternate_symbol.IsEmpty())
-          alternate_symbol.append(entry.alternate_symbol.GetCString());
-
         callback(entry.recognizer_id, entry.recognizer->GetName(),
-                 entry.module.GetCString(), entry.symbol.GetCString(),
-                 alternate_symbol, false);
+                 entry.module.GetCString(), entry.symbols, false);
       }
     }
   }
 
   bool RemoveRecognizerWithID(uint32_t recognizer_id) {
     if (recognizer_id >= m_recognizers.size()) return false;
     if (m_recognizers[recognizer_id].deleted) return false;
     m_recognizers[recognizer_id].deleted = true;
StackFrameRecognizerSP GetRecognizerForFrame(StackFrameSP frame) {
     for (auto entry : m_recognizers) {
       if (entry.deleted) continue;
       if (entry.module)
         if (entry.module != module_name) continue;
 
       if (entry.module_regexp)
         if (!entry.module_regexp->Execute(module_name.GetStringRef())) continue;
 
-      if (entry.symbol)
-        if (entry.symbol != function_name &&
-            (!entry.alternate_symbol ||
-             entry.alternate_symbol != function_name))
+      if (!entry.symbols.empty())
+        if (!llvm::is_contained(entry.symbols, function_name))
           continue;
 
       if (entry.symbol_regexp)
         if (!entry.symbol_regexp->Execute(function_name.GetStringRef()))
           continue;
 
       if (entry.first_instruction_only)
         if (start_addr != current_addr) continue;
 

  private:
   struct RegisteredEntry {
     uint32_t recognizer_id;
     bool deleted;
     StackFrameRecognizerSP recognizer;
     bool is_regexp;
     ConstString module;
     RegularExpressionSP module_regexp;
-    ConstString symbol;
-    ConstString alternate_symbol;
+    std::vector<ConstString> symbols;
     RegularExpressionSP symbol_regexp;
     bool first_instruction_only;
   };
 
   std::deque<RegisteredEntry> m_recognizers;
 };
 
 StackFrameRecognizerManagerImpl &GetStackFrameRecognizerManagerImpl() {
   static StackFrameRecognizerManagerImpl instance =
       StackFrameRecognizerManagerImpl();
   return instance;
 }
 
 void StackFrameRecognizerManager::AddRecognizer(
-    StackFrameRecognizerSP recognizer, ConstString module, ConstString symbol,
-    ConstString alternate_symbol, bool first_instruction_only) {
+    StackFrameRecognizerSP recognizer, ConstString module,
+    llvm::ArrayRef<ConstString> symbols, bool first_instruction_only) {
   GetStackFrameRecognizerManagerImpl().AddRecognizer(
-      recognizer, module, symbol, alternate_symbol, first_instruction_only);
+      recognizer, module, symbols, first_instruction_only);
 }
 
 void StackFrameRecognizerManager::AddRecognizer(
     StackFrameRecognizerSP recognizer, RegularExpressionSP module,
     RegularExpressionSP symbol, bool first_instruction_only) {
   GetStackFrameRecognizerManagerImpl().AddRecognizer(recognizer, module, symbol,
                                                      first_instruction_only);
 }
 
 void StackFrameRecognizerManager::ForEach(
     std::function<void(uint32_t recognized_id, std::string recognizer_name,
-                       std::string module, std::string symbol,
-                       std::string alternate_symbol, bool regexp)> const
-        &callback) {
+                       std::string module, llvm::ArrayRef<ConstString> symbols,
+                       bool regexp)> const &callback) {
   GetStackFrameRecognizerManagerImpl().ForEach(callback);
 }
 
 void StackFrameRecognizerManager::RemoveAllRecognizers() {
   GetStackFrameRecognizerManagerImpl().RemoveAllRecognizers();
 }
 
 bool StackFrameRecognizerManager::RemoveRecognizerWithID(uint32_t recognizer_id) {

lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py

 class FrameRecognizerTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
     @skipUnlessDarwin
     def test_frame_recognizer_1(self):
         self.build()
-
-        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
-        self.assertTrue(target, VALID_TARGET)
+        exe = self.getBuildArtifact("a.out")
 
         # Clear internal & plugins recognizers that get initialized at launch
         self.runCmd("frame recognizer clear")
 
         self.runCmd("command script import " + os.path.join(self.getSourceDir(), "recognizer.py"))
 
         self.expect("frame recognizer list",
                     substrs=['no matching results found.'])
 
         self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo")
 
         self.expect("frame recognizer list",
-                    substrs=['0: recognizer.MyFrameRecognizer, module a.out, function foo'])
+                    substrs=['0: recognizer.MyFrameRecognizer, module a.out, symbol foo'])
 
         self.runCmd("frame recognizer add -l recognizer.MyOtherFrameRecognizer -s a.out -n bar -x")
 
         self.expect(
             "frame recognizer list",
             substrs=[
-                '1: recognizer.MyOtherFrameRecognizer, module a.out, function bar (regexp)',
-                '0: recognizer.MyFrameRecognizer, module a.out, function foo'
+                '1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)',
+                '0: recognizer.MyFrameRecognizer, module a.out, symbol foo'
             ])
 
         self.runCmd("frame recognizer delete 0")
 
         self.expect("frame recognizer list",
-                    substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, function bar (regexp)'])
+                    substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)'])
 
         self.runCmd("frame recognizer clear")
 
         self.expect("frame recognizer list",
                     substrs=['no matching results found.'])
 
         self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo")
 
-        lldbutil.run_break_set_by_symbol(self, "foo")
-        self.runCmd("r")
-
-        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-                    substrs=['stopped', 'stop reason = breakpoint'])
-
-        process = target.GetProcess()
-        thread = process.GetSelectedThread()
+        target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo",
+                                                                 exe_name = exe)
         frame = thread.GetSelectedFrame()
 
-        self.assertEqual(frame.GetSymbol().GetName(), "foo")
-        self.assertFalse(frame.GetLineEntry().IsValid())
-
         self.expect("frame variable",
                     substrs=['(int) a = 42', '(int) b = 56'])
 
         # Recognized arguments don't show up by default...
         variables = frame.GetVariables(lldb.SBVariablesOptions())
         self.assertEqual(variables.GetSize(), 0)
 
         # ...unless you set target.display-recognized-arguments to 1...
def test_frame_recognizer_1(self):
         self.expect("frame recognizer info 999", error=True,
                     substrs=['no frame with index 999'])
 
         self.expect("frame recognizer info 1",
                     substrs=['frame 1 not recognized by any recognizer'])
 
         # FIXME: The following doesn't work yet, but should be fixed.
         """
-        lldbutil.run_break_set_by_symbol(self, "bar")
-        self.runCmd("c")
+        target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "bar",
+                                                                 exe_name = exe)
+        frame = thread.GetSelectedFrame()
 
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped', 'stop reason = breakpoint'])
 
         self.expect("frame variable -t",
                     substrs=['(int *) a = '])
 
         self.expect("frame variable -t *a",
                     substrs=['*a = 78'])
         """
+
+    @skipUnlessDarwin
+    def test_frame_recognizer_multi_symbol(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+
+        # Clear internal & plugins recognizers that get initialized at launch
+        self.runCmd("frame recognizer clear")
+
+        self.runCmd("command script import " + os.path.join(self.getSourceDir(), "recognizer.py"))
+
+        self.expect("frame recognizer list",
+                    substrs=['no matching results found.'])
+
+        self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar")
+
+        self.expect("frame recognizer list",
+                    substrs=['recognizer.MyFrameRecognizer, module a.out, symbol foo, symbol bar'])
+
+        target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo",
+                                                                 exe_name = exe)
+        frame = thread.GetSelectedFrame()
+
+        self.expect("frame recognizer info 0",
+                    substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
+
+        target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "bar",
+                                                                 exe_name = exe)
+        frame = thread.GetSelectedFrame()
+
+        self.expect("frame recognizer info 0",
+                    substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])

lldb/test/API/commands/frame/recognizer/main.m

 #import <Foundation/Foundation.h>
 
 void foo(int a, int b)
 {
     printf("%d %d\n", a, b);
 }
 
-void bar(int *ptr)
-{
-  printf("%d\n", *ptr);
-}
+void bar(int *ptr) { printf("%d\n", *ptr); }
 
 int main (int argc, const char * argv[])
 {
     foo(42, 56);
     int i = 78;
     bar(&i);
     return 0;
 }

lldb/unittests/Target/StackFrameRecognizerTest.cpp

TEST_F(StackFrameRecognizerTest, NullModuleRegex) {
   DebuggerSP debugger_sp = Debugger::CreateInstance();
   ASSERT_TRUE(debugger_sp);
 
   RegisterDummyStackFrameRecognizer();
 
   bool any_printed = false;
   StackFrameRecognizerManager::ForEach(
       [&any_printed](uint32_t recognizer_id, std::string name,
-                     std::string function, std::string symbol,
-                     std::string alternate_symbol,
+                     std::string function, llvm::ArrayRef<ConstString> symbols,
                      bool regexp) { any_printed = true; });
 
   EXPECT_TRUE(any_printed);
 }