[lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

[jankratochvil]

Summary

While on regular Linux system (Fedora 34 GA, not updated):

* thread #1, name = '1', stop reason = hit program assert
    frame #0: 0x00007ffff7e242a2 libc.so.6`raise + 322
    frame #1: 0x00007ffff7e0d8a4 libc.so.6`abort + 278
    frame #2: 0x00007ffff7e0d789 libc.so.6`__assert_fail_base.cold + 15
    frame #3: 0x00007ffff7e1ca16 libc.so.6`__assert_fail + 70
  * frame #4: 0x00000000004011bd 1`main at assert.c:7:3

On Fedora 35 pre-release one gets:

* thread #1, name = '1', stop reason = signal SIGABRT
  * frame #0: 0x00007ffff7e48ee3 libc.so.6`pthread_kill@GLIBC_2.2.5 + 67
    frame #1: 0x00007ffff7dfb986 libc.so.6`raise + 22
    frame #2: 0x00007ffff7de5806 libc.so.6`abort + 230
    frame #3: 0x00007ffff7de571b libc.so.6`__assert_fail_base.cold + 15
    frame #4: 0x00007ffff7df4646 libc.so.6`__assert_fail + 70
    frame #5: 0x00000000004011bd 1`main at assert.c:7:3

I did not write a testcase as one needs the specific glibc. An artificial test would just copy the changed source.

[fweimer]

lldb/source/Target/AssertFrameRecognizer.cpp

52

The symbol version is architecture-dependent (not every architecture uses GLIBC_2.2.5), and there is also another GLIBC_2.34 symbol version even on x86-64.

[mib]

Another possibility could be to add a bool is_regex to the SymbolLocation struct and set it to true for Linux, then in RegisterAssertFrameRecognizer if that flag is set, build a RegularExpression out of the every symbol string in the struct and call the FrameRecognizerManager::AddRecognizer overload that uses RegularExpressionSP arguments.

That'd allow you to have some flexibility with library versions while only registering 1 recognizer. I think using regexes could also make it future-proof.

[jankratochvil]

lldb/source/Target/AssertFrameRecognizer.cpp

115

There would be nice llvm::join. But it wants Elem.size() while ConstString here provides Elem.GetLength(). I did not try to extend StringExtras.h.

[mib]

LGTM!

lldb/source/Target/AssertFrameRecognizer.cpp

115

That's unfortunate indeed.

[jankratochvil]

Unfortunately AddRecognizer requires both or none module and symbols as regexes. And we cannot make module a regex as GetFileSpec().FileEquals would not work then. So I have just escaped the regex.
It looks a bit fishy as StackFrameRecognizer uses entry.module_regexp->Execute ignoring case insensitivity on OSX but then I do not target that platform myself.

[jankratochvil]

Thanks for the review.

[mib]

In D105133#2852246, @jankratochvil wrote:

Thanks for the review.

No problem! Thank you for the patch!

lldb/source/Target/AssertFrameRecognizer.cpp

 
 namespace lldb_private {
 
 /// Stores a function module spec, symbol name and possibly an alternate symbol
 /// name.
 struct SymbolLocation {
   FileSpec module_spec;
   std::vector<ConstString> symbols;
+
+  // The symbols are regular expressions. In such case all symbols are matched
+  // with their trailing @VER symbol version stripped.
+  bool symbols_are_regex = false;
 };
 
 /// 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.symbols.push_back(ConstString("__pthread_kill"));
     break;
   case llvm::Triple::Linux:
     location.module_spec = FileSpec("libc.so.6");
     location.symbols.push_back(ConstString("raise"));
     location.symbols.push_back(ConstString("__GI_raise"));
     location.symbols.push_back(ConstString("gsignal"));
+    location.symbols.push_back(ConstString("pthread_kill"));
+    location.symbols_are_regex = true;
     break;
   default:
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
     LLDB_LOG(log, "AssertFrameRecognizer::GetAbortLocation Unsupported OS");
     return false;
   }
 
   return true;

 void RegisterAssertFrameRecognizer(Process *process) {
   Target &target = process->GetTarget();
   llvm::Triple::OSType os = target.GetArchitecture().GetTriple().getOS();
   SymbolLocation location;
 
   if (!GetAbortLocation(os, location))
     return;
 
+  if (!location.symbols_are_regex) {
     target.GetFrameRecognizerManager().AddRecognizer(
-      StackFrameRecognizerSP(new AssertFrameRecognizer()),
+        std::make_shared<AssertFrameRecognizer>(),
         location.module_spec.GetFilename(), location.symbols,
         /*first_instruction_only*/ false);
+    return;
+  }
+  std::string module_re = "^";
+  for (char c : location.module_spec.GetFilename().GetStringRef()) {
+    if (c == '.')
+      module_re += '\\';
+    module_re += c;
+  }
+  module_re += '$';
+  std::string symbol_re = "^(";
+  for (auto it = location.symbols.cbegin(); it != location.symbols.cend();
+       ++it) {
+    if (it != location.symbols.cbegin())
+      symbol_re += '|';
+    symbol_re += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.
+  symbol_re += ")(@.*)?$";
+  target.GetFrameRecognizerManager().AddRecognizer(
+      std::make_shared<AssertFrameRecognizer>(),
+      std::make_shared<RegularExpression>(std::move(module_re)),
+      std::make_shared<RegularExpression>(std::move(symbol_re)),
+      /*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;
 
   if (!GetAssertLocation(os, location))
     return RecognizedStackFrameSP();
 
-  const uint32_t frames_to_fetch = 5;
+  const uint32_t frames_to_fetch = 6;
   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) {