[Lldb-commits] [lldb] 1a39f1b - [lldb] Fix+re-enable Assert StackFrame Recognizer on Linux
Davide Italiano via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 10 13:29:43 PST 2020
This caused three failures on macOS.
I reverted the patch to unbreak the bots.
http://lab.llvm.org:8080/green/job/lldb-cmake/8565/console <http://lab.llvm.org:8080/green/job/lldb-cmake/8565/console>
Thanks,
—
Davide
> On Feb 10, 2020, at 01:33, Jan Kratochvil via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>
>
> Author: Jan Kratochvil
> Date: 2020-02-10T10:29:32+01:00
> New Revision: 1a39f1b966a8d8f15ed0d5a832d5097cccefe93b
>
> URL: https://github.com/llvm/llvm-project/commit/1a39f1b966a8d8f15ed0d5a832d5097cccefe93b
> DIFF: https://github.com/llvm/llvm-project/commit/1a39f1b966a8d8f15ed0d5a832d5097cccefe93b.diff
>
> LOG: [lldb] Fix+re-enable Assert StackFrame Recognizer on Linux
>
> D73303 was failing on Fedora Linux and so it was disabled by Skip the
> AssertFrameRecognizer test for Linux.
>
> I find no easy way how to find out if it gets recognized as
> `__assert_fail` or `__GI___assert_fail` as during `Process` ctor
> libc.so.6 is not yet loaded by the debuggee.
>
> DWARF symbol `__GI___assert_fail` overrides the ELF symbol `__assert_fail`.
> While external debug info (=DWARF) gets disabled for testsuite (D55859)
> that sure does not apply for real world usage.
>
> Differential Revision: https://reviews.llvm.org/D74252
>
> Added:
>
>
> Modified:
> 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
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/lldb/include/lldb/Target/StackFrameRecognizer.h b/lldb/include/lldb/Target/StackFrameRecognizer.h
> index b509e0760b31..92cfca4227cf 100644
> --- a/lldb/include/lldb/Target/StackFrameRecognizer.h
> +++ b/lldb/include/lldb/Target/StackFrameRecognizer.h
> @@ -101,8 +101,8 @@ class ScriptedStackFrameRecognizer : public StackFrameRecognizer {
> 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 @@ class StackFrameRecognizerManager {
> 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);
>
>
> diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
> index d86b50bd7aad..5af9e2e314be 100644
> --- a/lldb/source/Commands/CommandObjectFrame.cpp
> +++ b/lldb/source/Commands/CommandObjectFrame.cpp
> @@ -881,7 +881,7 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command,
> } 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
>
> @@ -960,12 +960,13 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed {
> StackFrameRecognizerManager::ForEach(
> [&result, &any_printed](uint32_t recognizer_id, std::string name,
> std::string function, std::string symbol,
> - bool regexp) {
> + std::string alternate_symbol, bool regexp) {
> if (name == "")
> 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)" : "");
> + "%d: %s, module %s, function %s{%s}%s\n", recognizer_id,
> + name.c_str(), function.c_str(), symbol.c_str(),
> + alternate_symbol.c_str(), regexp ? " (regexp)" : "");
> any_printed = true;
> });
>
>
> diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
> index 6acc23176248..62d16296bd66 100644
> --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
> +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
> @@ -2691,6 +2691,7 @@ static void RegisterObjCExceptionRecognizer() {
> 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);
> });
> }
>
> diff --git a/lldb/source/Target/AssertFrameRecognizer.cpp b/lldb/source/Target/AssertFrameRecognizer.cpp
> index 89ed3ce022d9..b024ee7ba97c 100644
> --- a/lldb/source/Target/AssertFrameRecognizer.cpp
> +++ b/lldb/source/Target/AssertFrameRecognizer.cpp
> @@ -16,26 +16,6 @@ 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));
> -}
> -
> /// Fetches the abort frame location depending on the current platform.
> ///
> /// \param[in] process_sp
> @@ -43,14 +23,15 @@ bool ModuleHasDebugInfo(Target &target, FileSpec &module_spec,
> /// the target and the platform.
> /// \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.
> +/// the abort module as a \a FileSpec and two symbol names as two \a
> +/// StringRef. The second \a StringRef may be empty.
> /// Otherwise, returns \a llvm::None.
> -llvm::Optional<std::tuple<FileSpec, StringRef>>
> +llvm::Optional<std::tuple<FileSpec, StringRef, StringRef>>
> GetAbortLocation(Process *process) {
> Target &target = process->GetTarget();
>
> FileSpec module_spec;
> - StringRef symbol_name;
> + StringRef symbol_name, alternate_symbol_name;
>
> switch (target.GetArchitecture().GetTriple().getOS()) {
> case llvm::Triple::Darwin:
> @@ -60,9 +41,8 @@ GetAbortLocation(Process *process) {
> 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";
> + symbol_name = "raise";
> + alternate_symbol_name = "__GI_raise";
> break;
> default:
> Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
> @@ -70,7 +50,7 @@ GetAbortLocation(Process *process) {
> return llvm::None;
> }
>
> - return std::make_tuple(module_spec, symbol_name);
> + return std::make_tuple(module_spec, symbol_name, alternate_symbol_name);
> }
>
> /// Fetches the assert frame location depending on the current platform.
> @@ -80,15 +60,15 @@ GetAbortLocation(Process *process) {
> /// the target and the platform.
> /// \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.
> +/// the asserting frame module as a \a FileSpec and two possible symbol
> +/// names as two \a StringRef. The second \a StringRef may be empty.
> /// Otherwise, returns \a llvm::None.
> -llvm::Optional<std::tuple<FileSpec, StringRef>>
> +llvm::Optional<std::tuple<FileSpec, StringRef, StringRef>>
> GetAssertLocation(Process *process) {
> Target &target = process->GetTarget();
>
> FileSpec module_spec;
> - StringRef symbol_name;
> + StringRef symbol_name, alternate_symbol_name;
>
> switch (target.GetArchitecture().GetTriple().getOS()) {
> case llvm::Triple::Darwin:
> @@ -98,9 +78,8 @@ GetAssertLocation(Process *process) {
> 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";
> + symbol_name = "__assert_fail";
> + alternate_symbol_name = "__GI___assert_fail";
> break;
> default:
> Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
> @@ -108,7 +87,7 @@ GetAssertLocation(Process *process) {
> return llvm::None;
> }
>
> - return std::make_tuple(module_spec, symbol_name);
> + return std::make_tuple(module_spec, symbol_name, alternate_symbol_name);
> }
>
> void RegisterAssertFrameRecognizer(Process *process) {
> @@ -120,12 +99,14 @@ void RegisterAssertFrameRecognizer(Process *process) {
> return;
>
> FileSpec module_spec;
> - StringRef function_name;
> - std::tie(module_spec, function_name) = *abort_location;
> + StringRef function_name, alternate_function_name;
> + std::tie(module_spec, function_name, alternate_function_name) =
> + *abort_location;
>
> StackFrameRecognizerManager::AddRecognizer(
> StackFrameRecognizerSP(new AssertFrameRecognizer()),
> - module_spec.GetFilename(), ConstString(function_name), false);
> + module_spec.GetFilename(), ConstString(function_name),
> + ConstString(alternate_function_name), /*first_instruction_only*/ false);
> });
> }
>
> @@ -142,8 +123,9 @@ AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {
> return RecognizedStackFrameSP();
>
> FileSpec module_spec;
> - StringRef function_name;
> - std::tie(module_spec, function_name) = *assert_location;
> + StringRef function_name, alternate_function_name;
> + std::tie(module_spec, function_name, alternate_function_name) =
> + *assert_location;
>
> const uint32_t frames_to_fetch = 5;
> const uint32_t last_frame_index = frames_to_fetch - 1;
> @@ -163,8 +145,13 @@ AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {
> 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(module_spec))
> + continue;
> +
> + ConstString func_name = sym_ctx.GetFunctionName();
> + if (func_name == ConstString(function_name) ||
> + alternate_function_name.empty() ||
> + func_name == ConstString(alternate_function_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.
>
> diff --git a/lldb/source/Target/StackFrameRecognizer.cpp b/lldb/source/Target/StackFrameRecognizer.cpp
> index 14cba23b4bfa..479e13fadc08 100644
> --- a/lldb/source/Target/StackFrameRecognizer.cpp
> +++ b/lldb/source/Target/StackFrameRecognizer.cpp
> @@ -50,24 +50,28 @@ ScriptedStackFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame) {
>
> 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,12 @@ class StackFrameRecognizerManagerImpl {
> 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);
> + callback(entry.recognizer_id, entry.recognizer->GetName(),
> + entry.module.GetCString(), entry.symbol.GetCString(),
> + entry.alternate_symbol.GetCString(), false);
> }
> }
> }
> @@ -120,7 +125,10 @@ class StackFrameRecognizerManagerImpl {
> 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 +157,7 @@ class StackFrameRecognizerManagerImpl {
> ConstString module;
> RegularExpressionSP module_regexp;
> ConstString symbol;
> + ConstString alternate_symbol;
> RegularExpressionSP symbol_regexp;
> bool first_instruction_only;
> };
> @@ -163,10 +172,10 @@ StackFrameRecognizerManagerImpl &GetStackFrameRecognizerManagerImpl() {
> }
>
> 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 +186,10 @@ void StackFrameRecognizerManager::AddRecognizer(
> }
>
> 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);
> }
>
>
> diff --git a/lldb/test/Shell/Recognizer/assert.test b/lldb/test/Shell/Recognizer/assert.test
> index 6bcc009f333e..9b4aa21611e0 100644
> --- a/lldb/test/Shell/Recognizer/assert.test
> +++ b/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
>
> diff --git a/lldb/unittests/Target/StackFrameRecognizerTest.cpp b/lldb/unittests/Target/StackFrameRecognizerTest.cpp
> index 5220796acb4b..f7b7829e2bb8 100644
> --- a/lldb/unittests/Target/StackFrameRecognizerTest.cpp
> +++ b/lldb/unittests/Target/StackFrameRecognizerTest.cpp
> @@ -77,6 +77,7 @@ TEST_F(StackFrameRecognizerTest, NullModuleRegex) {
> 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);
>
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200210/22ac1092/attachment-0001.html>
More information about the lldb-commits
mailing list