[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

Tue Mar 17 00:52:14 PDT 2020

labath added a comment.

In D76188#1924840 <https://reviews.llvm.org/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).

================
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:880
     auto func =
-        RegularExpressionSP(new RegularExpression(m_options.m_function));
+        RegularExpressionSP(new RegularExpression(m_options.m_symbols.front()));
     StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func);
----------------
mib wrote:
> labath wrote:
> > 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)
> Few lines above, we make sure the symbols list is not empty.
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 ?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76188/new/

https://reviews.llvm.org/D76188