mib created this revision.
mib added reviewers: jankratochvil, teemperor.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.
mib added a comment.
Hi @jankratochvil,
Could you let me know if this works on your end ?
Thanks!
This patch reverts commit 6b2979c12300b90a1e69791d43ee9cff14f4265e
<https://reviews.llvm.org/rG6b2979c12300b90a1e69791d43ee9cff14f4265e> 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 <https://reviews.llvm.org/D74252> feedbacks.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74388
Files:
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Target/AssertFrameRecognizer.cpp
lldb/source/Target/StackFrameRecognizer.cpp
lldb/test/Shell/Recognizer/assert.test
lldb/unittests/Target/StackFrameRecognizerTest.cpp
Index: lldb/unittests/Target/StackFrameRecognizerTest.cpp
===================================================================
--- lldb/unittests/Target/StackFrameRecognizerTest.cpp
+++ lldb/unittests/Target/StackFrameRecognizerTest.cpp
@@ -77,6 +77,7 @@
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);
Index: lldb/test/Shell/Recognizer/assert.test
===================================================================
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -1,4 +1,4 @@
-# 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
Index: lldb/source/Target/StackFrameRecognizer.cpp
===================================================================
--- lldb/source/Target/StackFrameRecognizer.cpp
+++ lldb/source/Target/StackFrameRecognizer.cpp
@@ -50,24 +50,28 @@
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;
@@ -79,11 +83,16 @@
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);
}
}
}
@@ -120,7 +129,10 @@
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()))
@@ -149,6 +161,7 @@
ConstString module;
RegularExpressionSP module_regexp;
ConstString symbol;
+ ConstString alternate_symbol;
RegularExpressionSP symbol_regexp;
bool first_instruction_only;
};
@@ -163,10 +176,10 @@
}
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(
@@ -177,8 +190,10 @@
}
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);
}
Index: lldb/source/Target/AssertFrameRecognizer.cpp
===================================================================
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -16,116 +16,90 @@
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);
});
}
@@ -135,16 +109,13 @@
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;
@@ -163,8 +134,14 @@
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.
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2691,6 +2691,7 @@
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);
});
}
Index: lldb/source/Commands/CommandObjectFrame.cpp
===================================================================
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -881,7 +881,7 @@
} 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
@@ -959,13 +959,26 @@
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;
});
Index: lldb/include/lldb/Target/StackFrameRecognizer.h
===================================================================
--- lldb/include/lldb/Target/StackFrameRecognizer.h
+++ lldb/include/lldb/Target/StackFrameRecognizer.h
@@ -101,8 +101,8 @@
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,
@@ -113,7 +113,8 @@
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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
