Skip to content

Commit

Permalink
[lldb/Target] Support more than 2 symbols in StackFrameRecognizer
Browse files Browse the repository at this point in the history
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

Differential Revision: https://reviews.llvm.org/D76188

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
  • Loading branch information
medismailben committed Mar 18, 2020
1 parent dc2531c commit 526554f
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 99 deletions.
14 changes: 7 additions & 7 deletions lldb/include/lldb/Target/StackFrameRecognizer.h
Expand Up @@ -101,20 +101,20 @@ class ScriptedStackFrameRecognizer : public StackFrameRecognizer {
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);

Expand Down
2 changes: 1 addition & 1 deletion lldb/packages/Python/lldbsuite/test/lldbutil.py
Expand Up @@ -797,7 +797,7 @@ def run_to_breakpoint_do_run(test, target, bkpt, launch_info = None,
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)

Expand Down
41 changes: 25 additions & 16 deletions lldb/source/Commands/CommandObjectFrame.cpp
Expand Up @@ -747,7 +747,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed {
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;
Expand All @@ -761,7 +761,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed {

void OptionParsingStarting(ExecutionContext *execution_context) override {
m_module = "";
m_function = "";
m_symbols.clear();
m_class_name = "";
m_regex = false;
}
Expand All @@ -773,7 +773,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed {
// 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;
};

Expand Down Expand Up @@ -855,9 +855,18 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command,
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",
m_cmd_name.c_str());
result.SetStatus(eReturnStatusFailed);
return false;
}
Expand All @@ -877,12 +886,13 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command,
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

Expand Down Expand Up @@ -959,9 +969,9 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed {
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())
Expand All @@ -970,10 +980,9 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed {
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)";

Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Commands/Options.td
Expand Up @@ -384,7 +384,8 @@ let Command = "frame recognizer add" in {
"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.">;
Expand Down
Expand Up @@ -2721,9 +2721,10 @@ static void RegisterObjCExceptionRecognizer() {
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);
});
}
24 changes: 10 additions & 14 deletions lldb/source/Target/AssertFrameRecognizer.cpp
Expand Up @@ -21,8 +21,7 @@ namespace lldb_private {
/// 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.
Expand All @@ -39,12 +38,13 @@ bool GetAbortLocation(llvm::Triple::OSType os, SymbolLocation &location) {
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));
Expand All @@ -69,12 +69,12 @@ bool GetAssertLocation(llvm::Triple::OSType os, SymbolLocation &location) {
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));
Expand All @@ -97,8 +97,7 @@ void RegisterAssertFrameRecognizer(Process *process) {

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);
});
}
Expand Down Expand Up @@ -139,10 +138,7 @@ AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {

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
Expand Down
51 changes: 20 additions & 31 deletions lldb/source/Target/StackFrameRecognizer.cpp
Expand Up @@ -51,27 +51,25 @@ ScriptedStackFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame) {
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;
Expand All @@ -83,16 +81,11 @@ class StackFrameRecognizerManagerImpl {
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);
}
}
}
Expand Down Expand Up @@ -128,10 +121,8 @@ class StackFrameRecognizerManagerImpl {
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)
Expand Down Expand Up @@ -160,8 +151,7 @@ class StackFrameRecognizerManagerImpl {
bool is_regexp;
ConstString module;
RegularExpressionSP module_regexp;
ConstString symbol;
ConstString alternate_symbol;
std::vector<ConstString> symbols;
RegularExpressionSP symbol_regexp;
bool first_instruction_only;
};
Expand All @@ -176,10 +166,10 @@ StackFrameRecognizerManagerImpl &GetStackFrameRecognizerManagerImpl() {
}

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(
Expand All @@ -191,9 +181,8 @@ void StackFrameRecognizerManager::AddRecognizer(

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);
}

Expand Down

0 comments on commit 526554f

Please sign in to comment.