[lldb/test] Add alternate symbol to StackFrame Recognizer

[mib]

Summary

This patch reverts commit 6b2979c12300b90a1e69791d43ee9cff14f4265e and updates it to reflect the addition of the alternate symbol attribute to StackFrame Recognizer.

This should fix the test failures it caused on macOS, and continue working on Linux.

It also addresses D74252 feedbacks.

[mib]

Hi @jankratochvil,

Could you let me know if this works on your end ?

Thanks!

[jankratochvil]

This patch reverts commit 6b2979c12300b90a1e69791d43ee9cff14f4265e

The patch is already reverted, this patch reimplements it.

In D74388#1868935, @mib wrote:

Could you let me know if this works on your end ?

Yes, it works on Fedora 31 x86_64.

lldb/source/Target/AssertFrameRecognizer.cpp

144

I see I made a mistake here, thanks for fixing it. That was probably the OSX regression.

[mib]

This patch landed in commit cb0c4ee3ebfe55809c9d0be72726b05668028fc4

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 module, ConstString symbol,
+                            ConstString alternate_symbol,
                             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,
-                         bool regexp)> const &callback);
+                         std::string alternate_symbol, bool regexp)> const
+          &callback);
 
   static bool RemoveRecognizerWithID(uint32_t recognizer_id);
 
   static void RemoveAllRecognizers();
 
   static lldb::StackFrameRecognizerSP GetRecognizerForFrame(
       lldb::StackFrameSP frame);
 

lldb/source/Commands/CommandObjectFrame.cpp

if (m_options.m_regex) {
     auto module =
         RegularExpressionSP(new RegularExpression(m_options.m_module));
     auto func =
         RegularExpressionSP(new RegularExpression(m_options.m_function));
     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);
+    StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func, {});
   }
 #endif
 
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
   return result.Succeeded();
 }
 
 class CommandObjectFrameRecognizerClear : public CommandObjectParsed {
public:
 
   ~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 function, std::string symbol,
-                                bool regexp) {
-          if (name == "")
+                                std::string module, std::string symbol,
+                                std::string alternate_symbol, bool regexp) {
+          Stream &stream = result.GetOutputStream();
+
+          if (name.empty())
             name = "(internal)";
-          result.GetOutputStream().Printf(
-              "%d: %s, module %s, function %s%s\n", recognizer_id, name.c_str(),
-              function.c_str(), symbol.c_str(), regexp ? " (regexp)" : "");
+
+          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 (regexp)
+            stream << " (regexp)";
+
+          stream.EOL();
+          stream.Flush();
+
           any_printed = true;
         });
 
     if (any_printed)
       result.SetStatus(eReturnStatusSuccessFinishResult);
     else {
       result.GetOutputStream().PutCString("no matching results found.\n");
       result.SetStatus(eReturnStatusSuccessFinishNoResult);

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();
     StackFrameRecognizerManager::AddRecognizer(
         StackFrameRecognizerSP(new ObjCExceptionThrowFrameRecognizer()),
-        module.GetFilename(), function, /*first_instruction_only*/ true);
+        module.GetFilename(), function, /*alternate_symbol*/ {},
+        /*first_instruction_only*/ true);
   });
 }

lldb/source/Target/AssertFrameRecognizer.cpp

 
 #include "lldb/Target/AssertFrameRecognizer.h"
 
 using namespace llvm;
 using namespace lldb;
 using namespace lldb_private;
 
 namespace lldb_private {
-/// Checkes if the module containing a symbol has debug info.
-///
-/// \param[in] target
-///    The target containing the module.
-/// \param[in] module_spec
-///    The module spec that should contain the symbol.
-/// \param[in] symbol_name
-///    The symbol's name that should be contained in the debug info.
-/// \return
-///    If  \b true the symbol was found, \b false otherwise.
-bool ModuleHasDebugInfo(Target &target, FileSpec &module_spec,
-                        StringRef symbol_name) {
-  ModuleSP module_sp = target.GetImages().FindFirstModule(module_spec);
-
-  if (!module_sp)
-    return false;
 
-  return module_sp->FindFirstSymbolWithNameAndType(ConstString(symbol_name));
-}
+/// 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;
+};
 
 /// Fetches the abort frame location depending on the current platform.
 ///
-/// \param[in] process_sp
-///    The process that is currently aborting. This will give us information on
-///    the target and the 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
-///    If the platform is supported, returns an optional tuple containing
-///    the abort module as a \a FileSpec and the symbol name as a \a StringRef.
-///    Otherwise, returns \a llvm::None.
-llvm::Optional<std::tuple<FileSpec, StringRef>>
-GetAbortLocation(Process *process) {
-  Target &target = process->GetTarget();
-
-  FileSpec module_spec;
-  StringRef symbol_name;
-
-  switch (target.GetArchitecture().GetTriple().getOS()) {
+///    \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:
-    module_spec = FileSpec("libsystem_kernel.dylib");
-    symbol_name = "__pthread_kill";
+    location.module_spec = FileSpec("libsystem_kernel.dylib");
+    location.symbol_name.SetString("__pthread_kill");
     break;
   case llvm::Triple::Linux:
-    module_spec = FileSpec("libc.so.6");
-    symbol_name = "__GI_raise";
-    if (!ModuleHasDebugInfo(target, module_spec, symbol_name))
-      symbol_name = "raise";
+    location.module_spec = FileSpec("libc.so.6");
+    location.symbol_name.SetString("raise");
+    location.alternate_symbol_name.SetString("__GI_raise");
     break;
   default:
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
     LLDB_LOG(log, "AssertFrameRecognizer::GetAbortLocation Unsupported OS");
-    return llvm::None;
+    return false;
   }
 
-  return std::make_tuple(module_spec, symbol_name);
+  return true;
 }
 
 /// Fetches the assert frame location depending on the current platform.
 ///
-/// \param[in] process_sp
-///    The process that is currently asserting. This will give us information on
-///    the target and the 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
-///    If the platform is supported, returns an optional tuple containing
-///    the asserting frame module as  a \a FileSpec and the symbol name as a \a
-///    StringRef.
-///    Otherwise, returns \a llvm::None.
-llvm::Optional<std::tuple<FileSpec, StringRef>>
-GetAssertLocation(Process *process) {
-  Target &target = process->GetTarget();
-
-  FileSpec module_spec;
-  StringRef symbol_name;
-
-  switch (target.GetArchitecture().GetTriple().getOS()) {
+///    \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:
-    module_spec = FileSpec("libsystem_c.dylib");
-    symbol_name = "__assert_rtn";
+    location.module_spec = FileSpec("libsystem_c.dylib");
+    location.symbol_name.SetString("__assert_rtn");
     break;
   case llvm::Triple::Linux:
-    module_spec = FileSpec("libc.so.6");
-    symbol_name = "__GI___assert_fail";
-    if (!ModuleHasDebugInfo(target, module_spec, symbol_name))
-      symbol_name = "__assert_fail";
+    location.module_spec = FileSpec("libc.so.6");
+    location.symbol_name.SetString("__assert_fail");
+    location.alternate_symbol_name.SetString("__GI___assert_fail");
     break;
   default:
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
     LLDB_LOG(log, "AssertFrameRecognizer::GetAssertLocation Unsupported OS");
-    return llvm::None;
+    return false;
   }
 
-  return std::make_tuple(module_spec, symbol_name);
+  return true;
 }
 
 void RegisterAssertFrameRecognizer(Process *process) {
   static llvm::once_flag g_once_flag;
   llvm::call_once(g_once_flag, [process]() {
-    auto abort_location = GetAbortLocation(process);
+    Target &target = process->GetTarget();
+    llvm::Triple::OSType os = target.GetArchitecture().GetTriple().getOS();
+    SymbolLocation location;
 
-    if (!abort_location.hasValue())
+    if (!GetAbortLocation(os, location))
       return;
 
-    FileSpec module_spec;
-    StringRef function_name;
-    std::tie(module_spec, function_name) = *abort_location;
-
     StackFrameRecognizerManager::AddRecognizer(
         StackFrameRecognizerSP(new AssertFrameRecognizer()),
-        module_spec.GetFilename(), ConstString(function_name), false);
+        location.module_spec.GetFilename(), ConstString(location.symbol_name),
+        ConstString(location.alternate_symbol_name),
+        /*first_instruction_only*/ false);
   });
 }
 
 } // namespace lldb_private
 
 lldb::RecognizedStackFrameSP
 AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {
   ThreadSP thread_sp = frame_sp->GetThread();
   ProcessSP process_sp = thread_sp->GetProcess();
+  Target &target = process_sp->GetTarget();
+  llvm::Triple::OSType os = target.GetArchitecture().GetTriple().getOS();
+  SymbolLocation location;
 
-  auto assert_location = GetAssertLocation(process_sp.get());
-
-  if (!assert_location.hasValue())
+  if (!GetAssertLocation(os, location))
     return RecognizedStackFrameSP();
 
-  FileSpec module_spec;
-  StringRef function_name;
-  std::tie(module_spec, function_name) = *assert_location;
-
   const uint32_t frames_to_fetch = 5;
   const uint32_t last_frame_index = frames_to_fetch - 1;
   StackFrameSP prev_frame_sp = nullptr;
 
   // Fetch most relevant frame
   for (uint32_t frame_index = 0; frame_index < frames_to_fetch; frame_index++) {
     prev_frame_sp = thread_sp->GetStackFrameAtIndex(frame_index);
 
     if (!prev_frame_sp) {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
       LLDB_LOG(log, "Abort Recognizer: Hit unwinding bound ({1} frames)!",
                frames_to_fetch);
       break;
     }
 
     SymbolContext sym_ctx =
         prev_frame_sp->GetSymbolContext(eSymbolContextEverything);
 
-    if (sym_ctx.module_sp->GetFileSpec().FileEquals(module_spec) &&
-        sym_ctx.GetFunctionName() == ConstString(function_name)) {
+    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)) {
 
       // 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,
+  void AddRecognizer(StackFrameRecognizerSP recognizer, ConstString module,
+                     ConstString symbol, ConstString alternate_symbol,
                      bool first_instruction_only) {
-    m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer, false, module, RegularExpressionSP(),
-                              symbol, RegularExpressionSP(),
+    m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
+                              false, module, RegularExpressionSP(), symbol,
+                              alternate_symbol, 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,
+    m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
+                              true, ConstString(), module, ConstString(),
                               ConstString(), symbol, first_instruction_only});
   }
 
   void ForEach(
-      std::function<void(uint32_t recognized_id, std::string recognizer_name, std::string module,
-                         std::string symbol, bool regexp)> const &callback) {
+      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) {
     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);
+                 symbol_name, {}, true);
 
       } else {
-        callback(entry.recognizer_id, entry.recognizer->GetName(), entry.module.GetCString(),
-                 entry.symbol.GetCString(), false);
+        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);
       }
     }
   }
 
   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;
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) continue;
+        if (entry.symbol != function_name &&
+            (!entry.alternate_symbol ||
+             entry.alternate_symbol != 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;
     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, bool first_instruction_only) {
-  GetStackFrameRecognizerManagerImpl().AddRecognizer(recognizer, module, symbol,
-                                                     first_instruction_only);
+    StackFrameRecognizerSP recognizer, ConstString module, ConstString symbol,
+    ConstString alternate_symbol, bool first_instruction_only) {
+  GetStackFrameRecognizerManagerImpl().AddRecognizer(
+      recognizer, module, symbol, alternate_symbol, 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, bool regexp)> const &callback) {
+    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) {
   GetStackFrameRecognizerManagerImpl().ForEach(callback);
 }
 
 void StackFrameRecognizerManager::RemoveAllRecognizers() {
   GetStackFrameRecognizerManagerImpl().RemoveAllRecognizers();
 }
 
 bool StackFrameRecognizerManager::RemoveRecognizerWithID(uint32_t recognizer_id) {

lldb/test/Shell/Recognizer/assert.test

-# UNSUPPORTED: system-windows, system-linux
+# UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
 # CHECK: thread #{{.*}}stop reason = hit program assert
 frame info
 # CHECK: frame #{{.*}}`main at assert.c
 frame recognizer info 0
 # CHECK: frame 0 is recognized by Assert StackFrame Recognizer
 set set thread-format "{${thread.stop-reason-raw}}\n"
 thread info
 # CHECK: signal SIGABRT
 q

lldb/unittests/Target/StackFrameRecognizerTest.cpp

TEST_F(StackFrameRecognizerTest, NullModuleRegex) {
   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,
                      bool regexp) { any_printed = true; });
 
   EXPECT_TRUE(any_printed);
 }