[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)
@@ -0,0 +1,194 @@ +//===-- LibCxxProxyArray.cpp---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "LibCxx.h" + +#include "lldb/Core/ValueObject.h" +#include "lldb/DataFormatters/FormattersHelpers.h" +#include + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::formatters; + +namespace lldb_private { +namespace formatters { + +/// Data formatter for libc++'s std::"proxy_array". +/// +/// A proxy_array's are created by using: +/// std::gslice_array operator[](const std::gslice& gslicearr); +/// std::mask_array operator[](const std::valarray& boolarr); +/// std::indirect_array operator[](const std::valarray& indarr); +/// +/// These arrays have the following members: +/// - __vp_ points to std::valarray::__begin_ +/// - __1d_ an array of offsets of the elements from @a __vp_ +class LibcxxStdProxyArraySyntheticFrontEnd : public SyntheticChildrenFrontEnd { +public: + LibcxxStdProxyArraySyntheticFrontEnd(lldb::ValueObjectSP valobj_sp); + + ~LibcxxStdProxyArraySyntheticFrontEnd() override; + + llvm::Expected CalculateNumChildren() override; + + lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override; + + lldb::ChildCacheState Update() override; + + bool MightHaveChildren() override; + + size_t GetIndexOfChildWithName(ConstString name) override; + +private: + /// A non-owning pointer to the array's __vp_. + ValueObject *m_base = nullptr; + /// The type of the array's template argument T. + CompilerType m_element_type; + /// The sizeof the array's template argument T. + uint32_t m_element_size = 0; + + /// A non-owning pointer to the array's __1d_.__begin_. + ValueObject *m_start = nullptr; + /// A non-owning pointer to the array's __1d_.__end_. + ValueObject *m_finish = nullptr; + /// The type of the __1d_ array's template argument T (size_t). + CompilerType m_element_type_size_t; + /// The sizeof the __1d_ array's template argument T (size_t) + uint32_t m_element_size_size_t = 0; +}; + +} // namespace formatters +} // namespace lldb_private + +lldb_private::formatters::LibcxxStdProxyArraySyntheticFrontEnd:: +LibcxxStdProxyArraySyntheticFrontEnd(lldb::ValueObjectSP valobj_sp) +: SyntheticChildrenFrontEnd(*valobj_sp), m_element_type() { + if (valobj_sp) +Update(); +} + +lldb_private::formatters::LibcxxStdProxyArraySyntheticFrontEnd:: +~LibcxxStdProxyArraySyntheticFrontEnd() { + // these need to stay around because they are child objects who will follow + // their parent's life cycle + // delete m_base; +} + +llvm::Expected lldb_private::formatters:: +LibcxxStdProxyArraySyntheticFrontEnd::CalculateNumChildren() { Michael137 wrote: This is outside the scope of this PR but eventually we might want to pull this out into a helper or something since we have this implementation across multiple formatters now. Same with `GetChildAtIndex`. https://github.com/llvm/llvm-project/pull/88613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)
https://github.com/Michael137 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/88613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Turn ChildCacheState into an unscoped enum (PR #88703)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/88703 Done for consistency with the rest of the enums in `lldb-enumerations.h`. See https://github.com/llvm/llvm-project/pull/80167#issuecomment-2043721298 >From ba9d7105b334e3969e7f9f172cae37ea4f2f553e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 10:10:17 +0100 Subject: [PATCH] [lldb] Turn ChildCacheState into an unscoped enum Done for consistency with the rest of the enums in `lldb-enumerations.h`. See https://github.com/llvm/llvm-project/pull/80167#issuecomment-2043721298 --- lldb/include/lldb/lldb-enumerations.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index f3b07ea6d20395..15e45857186091 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1310,7 +1310,7 @@ enum CompletionType { /// Specifies if children need to be re-computed /// after a call to \ref SyntheticChildrenFrontEnd::Update. -enum class ChildCacheState { +enum ChildCacheState { eRefetch = 0, ///< Children need to be recomputed dynamically. eReuse = 1, ///< Children did not change and don't need to be recomputed; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Turn ChildCacheState into an unscoped enum (PR #88703)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes Done for consistency with the rest of the enums in `lldb-enumerations.h`. See https://github.com/llvm/llvm-project/pull/80167#issuecomment-2043721298 --- Full diff: https://github.com/llvm/llvm-project/pull/88703.diff 1 Files Affected: - (modified) lldb/include/lldb/lldb-enumerations.h (+1-1) ``diff diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index f3b07ea6d20395..15e45857186091 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1310,7 +1310,7 @@ enum CompletionType { /// Specifies if children need to be re-computed /// after a call to \ref SyntheticChildrenFrontEnd::Update. -enum class ChildCacheState { +enum ChildCacheState { eRefetch = 0, ///< Children need to be recomputed dynamically. eReuse = 1, ///< Children did not change and don't need to be recomputed; `` https://github.com/llvm/llvm-project/pull/88703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Turn ChildCacheState into an unscoped enum (PR #88703)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/88703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (PR #88721)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/88721 This logic was originally copied from `CompilerInstance::parseLangArgs`. Since then, the `CompilerInstance` uses `LangOptions::setLangDefaults` to set up new `LangOptions` instances. In our case, we only ever passed `Language::ObjCXX` into LLDB's `ParseLangArgs`, so most of this function was dead code. This patch replaces the duplicated logic with a call to `LangOptions::setLangDefaults`. >From f0b309c52a7f497aa021f38f3ce272a1bb3e66ea Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 13:16:58 +0100 Subject: [PATCH] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions This logic was originally copied from `CompilerInstance::parseLangArgs`. Since then, the `CompilerInstance` uses `LangOptions::setLangDefaults` to set up new `LangOptions` instances. In our case, we only ever passed `Language::ObjCXX` into LLDB's `ParseLangArgs`, so most of this function was dead code. This patch replaces the duplicated logic with a call to `LangOptions::setLangDefaults`. --- .../TypeSystem/Clang/TypeSystemClang.cpp | 79 ++- 1 file changed, 6 insertions(+), 73 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 44bd02bd4b367d..be0ddb06f82c18 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -459,85 +459,19 @@ TypeSystemClang::ConvertAccessTypeToAccessSpecifier(AccessType access) { return AS_none; } -static void ParseLangArgs(LangOptions &Opts, InputKind IK, const char *triple) { +static void ParseLangArgs(LangOptions &Opts, ArchSpec arch) { // FIXME: Cleanup per-file based stuff. - // Set some properties which depend solely on the input kind; it would be - // nice to move these to the language standard, and have the driver resolve - // the input kind + language standard. - if (IK.getLanguage() == clang::Language::Asm) { -Opts.AsmPreprocessor = 1; - } else if (IK.isObjectiveC()) { -Opts.ObjC = 1; - } - - LangStandard::Kind LangStd = LangStandard::lang_unspecified; - - if (LangStd == LangStandard::lang_unspecified) { -// Based on the base language, pick one. -switch (IK.getLanguage()) { -case clang::Language::Unknown: -case clang::Language::CIR: -case clang::Language::LLVM_IR: -case clang::Language::RenderScript: - llvm_unreachable("Invalid input kind!"); -case clang::Language::OpenCL: - LangStd = LangStandard::lang_opencl10; - break; -case clang::Language::OpenCLCXX: - LangStd = LangStandard::lang_openclcpp10; - break; -case clang::Language::Asm: -case clang::Language::C: -case clang::Language::ObjC: - LangStd = LangStandard::lang_gnu99; - break; -case clang::Language::CXX: -case clang::Language::ObjCXX: - LangStd = LangStandard::lang_gnucxx98; - break; -case clang::Language::CUDA: -case clang::Language::HIP: - LangStd = LangStandard::lang_gnucxx17; - break; -case clang::Language::HLSL: - LangStd = LangStandard::lang_hlsl; - break; -} - } - - const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd); - Opts.LineComment = Std.hasLineComments(); - Opts.C99 = Std.isC99(); - Opts.CPlusPlus = Std.isCPlusPlus(); - Opts.CPlusPlus11 = Std.isCPlusPlus11(); - Opts.CPlusPlus14 = Std.isCPlusPlus14(); - Opts.CPlusPlus17 = Std.isCPlusPlus17(); - Opts.CPlusPlus20 = Std.isCPlusPlus20(); - Opts.Digraphs = Std.hasDigraphs(); - Opts.GNUMode = Std.isGNUMode(); - Opts.GNUInline = !Std.isC99(); - Opts.HexFloats = Std.hasHexFloats(); - - Opts.WChar = true; - - // OpenCL has some additional defaults. - if (LangStd == LangStandard::lang_opencl10) { -Opts.OpenCL = 1; -Opts.AltiVec = 1; -Opts.CXXOperatorNames = 1; -Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::All); - } - - // OpenCL and C++ both have bool, true, false keywords. - Opts.Bool = Opts.OpenCL || Opts.CPlusPlus; + std::vector Includes; + LangOptions::setLangDefaults(Opts, clang::Language::ObjCXX, arch.GetTriple(), + Includes, clang::LangStandard::lang_gnucxx98); Opts.setValueVisibilityMode(DefaultVisibility); // Mimicing gcc's behavior, trigraphs are only enabled if -trigraphs is // specified, or -std is set to a conforming mode. Opts.Trigraphs = !Opts.GNUMode; - Opts.CharIsSigned = ArchSpec(triple).CharIsSignedByDefault(); + Opts.CharIsSigned = arch.CharIsSignedByDefault(); Opts.OptimizeSize = 0; // FIXME: Eliminate this dependency. @@ -727,8 +661,7 @@ void TypeSystemClang::CreateASTContext() { m_ast_owned = true; m_language_options_up = std::make_unique(); - ParseLangArgs(*m_language_options_up, clang::Language::ObjCXX, -GetTa
[Lldb-commits] [lldb] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (PR #88721)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes This logic was originally copied from `CompilerInstance::parseLangArgs`. Since then, the `CompilerInstance` uses `LangOptions::setLangDefaults` to set up new `LangOptions` instances. In our case, we only ever passed `Language::ObjCXX` into LLDB's `ParseLangArgs`, so most of this function was dead code. This patch replaces the duplicated logic with a call to `LangOptions::setLangDefaults`. --- Full diff: https://github.com/llvm/llvm-project/pull/88721.diff 1 Files Affected: - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+6-73) ``diff diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 44bd02bd4b367d..be0ddb06f82c18 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -459,85 +459,19 @@ TypeSystemClang::ConvertAccessTypeToAccessSpecifier(AccessType access) { return AS_none; } -static void ParseLangArgs(LangOptions &Opts, InputKind IK, const char *triple) { +static void ParseLangArgs(LangOptions &Opts, ArchSpec arch) { // FIXME: Cleanup per-file based stuff. - // Set some properties which depend solely on the input kind; it would be - // nice to move these to the language standard, and have the driver resolve - // the input kind + language standard. - if (IK.getLanguage() == clang::Language::Asm) { -Opts.AsmPreprocessor = 1; - } else if (IK.isObjectiveC()) { -Opts.ObjC = 1; - } - - LangStandard::Kind LangStd = LangStandard::lang_unspecified; - - if (LangStd == LangStandard::lang_unspecified) { -// Based on the base language, pick one. -switch (IK.getLanguage()) { -case clang::Language::Unknown: -case clang::Language::CIR: -case clang::Language::LLVM_IR: -case clang::Language::RenderScript: - llvm_unreachable("Invalid input kind!"); -case clang::Language::OpenCL: - LangStd = LangStandard::lang_opencl10; - break; -case clang::Language::OpenCLCXX: - LangStd = LangStandard::lang_openclcpp10; - break; -case clang::Language::Asm: -case clang::Language::C: -case clang::Language::ObjC: - LangStd = LangStandard::lang_gnu99; - break; -case clang::Language::CXX: -case clang::Language::ObjCXX: - LangStd = LangStandard::lang_gnucxx98; - break; -case clang::Language::CUDA: -case clang::Language::HIP: - LangStd = LangStandard::lang_gnucxx17; - break; -case clang::Language::HLSL: - LangStd = LangStandard::lang_hlsl; - break; -} - } - - const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd); - Opts.LineComment = Std.hasLineComments(); - Opts.C99 = Std.isC99(); - Opts.CPlusPlus = Std.isCPlusPlus(); - Opts.CPlusPlus11 = Std.isCPlusPlus11(); - Opts.CPlusPlus14 = Std.isCPlusPlus14(); - Opts.CPlusPlus17 = Std.isCPlusPlus17(); - Opts.CPlusPlus20 = Std.isCPlusPlus20(); - Opts.Digraphs = Std.hasDigraphs(); - Opts.GNUMode = Std.isGNUMode(); - Opts.GNUInline = !Std.isC99(); - Opts.HexFloats = Std.hasHexFloats(); - - Opts.WChar = true; - - // OpenCL has some additional defaults. - if (LangStd == LangStandard::lang_opencl10) { -Opts.OpenCL = 1; -Opts.AltiVec = 1; -Opts.CXXOperatorNames = 1; -Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::All); - } - - // OpenCL and C++ both have bool, true, false keywords. - Opts.Bool = Opts.OpenCL || Opts.CPlusPlus; + std::vector Includes; + LangOptions::setLangDefaults(Opts, clang::Language::ObjCXX, arch.GetTriple(), + Includes, clang::LangStandard::lang_gnucxx98); Opts.setValueVisibilityMode(DefaultVisibility); // Mimicing gcc's behavior, trigraphs are only enabled if -trigraphs is // specified, or -std is set to a conforming mode. Opts.Trigraphs = !Opts.GNUMode; - Opts.CharIsSigned = ArchSpec(triple).CharIsSignedByDefault(); + Opts.CharIsSigned = arch.CharIsSignedByDefault(); Opts.OptimizeSize = 0; // FIXME: Eliminate this dependency. @@ -727,8 +661,7 @@ void TypeSystemClang::CreateASTContext() { m_ast_owned = true; m_language_options_up = std::make_unique(); - ParseLangArgs(*m_language_options_up, clang::Language::ObjCXX, -GetTargetTriple()); + ParseLangArgs(*m_language_options_up, ArchSpec(GetTargetTriple())); m_identifier_table_up = std::make_unique(*m_language_options_up, nullptr); `` https://github.com/llvm/llvm-project/pull/88721 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (PR #88724)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/88724 This member was never actually used, ever since its introduction in `ca4e0fd7e63b90e6f68044af47248c64f250ee8f`. >From e85bf75077dec2d6aa7d6983bbde222d1c2b3f29 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 10:37:05 +0100 Subject: [PATCH 1/2] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function This member was never actually used, ever since its introduction in `ca4e0fd7e63b90e6f68044af47248c64f250ee8f`. --- .../ExpressionParser/Clang/ClangExpressionDeclMap.cpp| 9 ++--- .../Plugins/ExpressionParser/Clang/NameSearchContext.h | 1 - 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 2d306b42760b18..bf310cfcd4c8af 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1049,7 +1049,6 @@ void ClangExpressionDeclMap::LookupInModulesDeclVendor( context.AddNamedDecl(copied_function); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (auto copied_var = dyn_cast(copied_decl)) { context.AddNamedDecl(copied_var); context.m_found_variable = true; @@ -1299,7 +1298,6 @@ void ClangExpressionDeclMap::LookupFunction( AddOneFunction(context, sym_ctx.function, nullptr); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (sym_ctx.symbol) { Symbol *symbol = sym_ctx.symbol; if (target && symbol->GetType() == eSymbolTypeReExported) { @@ -1329,13 +1327,10 @@ void ClangExpressionDeclMap::LookupFunction( } if (!context.m_found_function_with_type_info) { - if (extern_symbol) { + if (extern_symbol) AddOneFunction(context, nullptr, extern_symbol); -context.m_found_function = true; - } else if (non_extern_symbol) { + else if (non_extern_symbol) AddOneFunction(context, nullptr, non_extern_symbol); -context.m_found_function = true; - } } } } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h index dc8621dd6aba52..9a3320636081be 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h @@ -41,7 +41,6 @@ struct NameSearchContext { bool m_found_variable = false; bool m_found_function_with_type_info = false; - bool m_found_function = false; bool m_found_local_vars_nsp = false; bool m_found_type = false; >From 86314daa00da7be807a14f81ae98816dc7172b29 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 13:40:55 +0100 Subject: [PATCH 2/2] fixup! revert noisy formatting change --- .../ExpressionParser/Clang/ClangExpressionDeclMap.cpp| 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index bf310cfcd4c8af..31f6447d66f642 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1327,10 +1327,11 @@ void ClangExpressionDeclMap::LookupFunction( } if (!context.m_found_function_with_type_info) { - if (extern_symbol) + if (extern_symbol) { AddOneFunction(context, nullptr, extern_symbol); - else if (non_extern_symbol) + } else if (non_extern_symbol) { AddOneFunction(context, nullptr, non_extern_symbol); + } } } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (PR #88724)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes This member was never actually used, ever since its introduction in `ca4e0fd7e63b90e6f68044af47248c64f250ee8f`. --- Full diff: https://github.com/llvm/llvm-project/pull/88724.diff 2 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp (-4) - (modified) lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h (-1) ``diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 2d306b42760b18..31f6447d66f642 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1049,7 +1049,6 @@ void ClangExpressionDeclMap::LookupInModulesDeclVendor( context.AddNamedDecl(copied_function); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (auto copied_var = dyn_cast(copied_decl)) { context.AddNamedDecl(copied_var); context.m_found_variable = true; @@ -1299,7 +1298,6 @@ void ClangExpressionDeclMap::LookupFunction( AddOneFunction(context, sym_ctx.function, nullptr); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (sym_ctx.symbol) { Symbol *symbol = sym_ctx.symbol; if (target && symbol->GetType() == eSymbolTypeReExported) { @@ -1331,10 +1329,8 @@ void ClangExpressionDeclMap::LookupFunction( if (!context.m_found_function_with_type_info) { if (extern_symbol) { AddOneFunction(context, nullptr, extern_symbol); -context.m_found_function = true; } else if (non_extern_symbol) { AddOneFunction(context, nullptr, non_extern_symbol); -context.m_found_function = true; } } } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h index dc8621dd6aba52..9a3320636081be 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h @@ -41,7 +41,6 @@ struct NameSearchContext { bool m_found_variable = false; bool m_found_function_with_type_info = false; - bool m_found_function = false; bool m_found_local_vars_nsp = false; bool m_found_type = false; `` https://github.com/llvm/llvm-project/pull/88724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
https://github.com/mbucko updated https://github.com/llvm/llvm-project/pull/88564 >From c588870cc8ff14806165f454d242f862ef19e89c Mon Sep 17 00:00:00 2001 From: Miro Bucko Date: Fri, 12 Apr 2024 09:55:46 -0700 Subject: [PATCH] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam Summary: AddMemoryList() was returning the last error status returned by ReadMemory(). So if an invalid memory region was read last, the function would return an error. Test Plan: ./bin/llvm-lit -sv ~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py Reviewers: Subscribers: Tasks: Tags: --- .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 50d1b563f469cf..5bf0cf1ad79d16 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -657,8 +657,11 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail() || bytes_read == 0) { + error.Clear(); continue; +} + // We have a good memory region with valid bytes to store. LocationDescriptor memory_dump; memory_dump.DataSize = static_cast(bytes_read); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)
https://github.com/ldionne approved this pull request. LGTM, thanks for picking this up! https://github.com/llvm/llvm-project/pull/88312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Turn ChildCacheState into an unscoped enum (PR #88703)
https://github.com/JDevlieghere approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/88703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b4776b8 - [libc++][CI] Tests LLDB libc++ data formatters. (#88312)
Author: Mark de Wever Date: 2024-04-15T17:58:37+02:00 New Revision: b4776b8d8ea742a46039002fac4c280e619ac48d URL: https://github.com/llvm/llvm-project/commit/b4776b8d8ea742a46039002fac4c280e619ac48d DIFF: https://github.com/llvm/llvm-project/commit/b4776b8d8ea742a46039002fac4c280e619ac48d.diff LOG: [libc++][CI] Tests LLDB libc++ data formatters. (#88312) This enables testing of the LLDB libc++ specific data formatters. This is enabled in the bootstrap build since building LLDB requires Clang and this is quite expensive. Adding this test changes the build time from 31 to 34 minutes. Added: Modified: libcxx/utils/ci/run-buildbot lldb/packages/Python/lldbsuite/test/lldbtest.py Removed: diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot index cc72f4639b1a9b..23a2a1bbbc63fa 100755 --- a/libcxx/utils/ci/run-buildbot +++ b/libcxx/utils/ci/run-buildbot @@ -368,18 +368,22 @@ bootstrapping-build) -DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \ - -DLLVM_ENABLE_PROJECTS="clang" \ + -DLLVM_ENABLE_PROJECTS="clang;lldb" \ -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \ -DLLVM_RUNTIME_TARGETS="$(${CXX} --print-target-triple)" \ + -DLLVM_HOST_TRIPLE="$(${CXX} --print-target-triple)" \ -DLLVM_TARGETS_TO_BUILD="host" \ -DRUNTIMES_BUILD_ALLOW_DARWIN=ON \ -DLLVM_ENABLE_ASSERTIONS=ON \ -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests" -echo "+++ Running the libc++ and libc++abi tests" +echo "+++ Running the LLDB libc++ data formatter tests" +${NINJA} -vC "${BUILD_DIR}" check-lldb-api-functionalities-data-formatter-data-formatter-stl-libcxx + +echo "--- Running the libc++ and libc++abi tests" ${NINJA} -vC "${BUILD_DIR}" check-runtimes -echo "--- Installing libc++ and libc++abi to a fake location" +echo "+++ Installing libc++ and libc++abi to a fake location" ${NINJA} -vC "${BUILD_DIR}" install-runtimes ccache -s diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index c28a78a2c4a27a..7a7afec7345707 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -751,6 +751,8 @@ def setUpCommands(cls): "settings set symbols.enable-external-lookup false", # Inherit the TCC permissions from the inferior's parent. "settings set target.inherit-tcc true", +# Based on https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4 +"settings set target.disable-aslr false", # Kill rather than detach from the inferior if something goes wrong. "settings set target.detach-on-error false", # Disable fix-its by default so that incorrect expressions in tests don't ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)
https://github.com/mordante closed https://github.com/llvm/llvm-project/pull/88312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
kovdan01 wrote: Thanks @jasonmolenda for your feedback and suggestion! See f96989dd1f832284b74d07d1e457a15a0b16c199 - I've deleted the test with corefile and added the test you've mentioned. Basically, I've just left the most simple test from "normal" `Testx86AssemblyInspectionEngine` and checked the `GetNonCallSiteUnwindPlanFromAssembly` call result against false. I've also ensured that w/o the patch applied, we just fail with nullptr dereference - so the test actually covers the case it's designed for. https://github.com/llvm/llvm-project/pull/82603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
@@ -657,8 +657,11 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail() || bytes_read == 0) { jeffreytan81 wrote: Instead of silently swallow the failure, we should emit the error message to terminal/console so that users are aware of region skipping. https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)
https://github.com/mordante edited https://github.com/llvm/llvm-project/pull/88613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)
@@ -0,0 +1,194 @@ +//===-- LibCxxProxyArray.cpp---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "LibCxx.h" + +#include "lldb/Core/ValueObject.h" +#include "lldb/DataFormatters/FormattersHelpers.h" +#include + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::formatters; + +namespace lldb_private { +namespace formatters { + +/// Data formatter for libc++'s std::"proxy_array". +/// +/// A proxy_array's are created by using: +/// std::gslice_array operator[](const std::gslice& gslicearr); +/// std::mask_array operator[](const std::valarray& boolarr); +/// std::indirect_array operator[](const std::valarray& indarr); +/// +/// These arrays have the following members: +/// - __vp_ points to std::valarray::__begin_ +/// - __1d_ an array of offsets of the elements from @a __vp_ +class LibcxxStdProxyArraySyntheticFrontEnd : public SyntheticChildrenFrontEnd { +public: + LibcxxStdProxyArraySyntheticFrontEnd(lldb::ValueObjectSP valobj_sp); + + ~LibcxxStdProxyArraySyntheticFrontEnd() override; + + llvm::Expected CalculateNumChildren() override; + + lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override; + + lldb::ChildCacheState Update() override; + + bool MightHaveChildren() override; + + size_t GetIndexOfChildWithName(ConstString name) override; + +private: + /// A non-owning pointer to the array's __vp_. + ValueObject *m_base = nullptr; + /// The type of the array's template argument T. + CompilerType m_element_type; + /// The sizeof the array's template argument T. + uint32_t m_element_size = 0; + + /// A non-owning pointer to the array's __1d_.__begin_. + ValueObject *m_start = nullptr; + /// A non-owning pointer to the array's __1d_.__end_. + ValueObject *m_finish = nullptr; + /// The type of the __1d_ array's template argument T (size_t). + CompilerType m_element_type_size_t; + /// The sizeof the __1d_ array's template argument T (size_t) + uint32_t m_element_size_size_t = 0; +}; + +} // namespace formatters +} // namespace lldb_private + +lldb_private::formatters::LibcxxStdProxyArraySyntheticFrontEnd:: +LibcxxStdProxyArraySyntheticFrontEnd(lldb::ValueObjectSP valobj_sp) +: SyntheticChildrenFrontEnd(*valobj_sp), m_element_type() { + if (valobj_sp) +Update(); +} + +lldb_private::formatters::LibcxxStdProxyArraySyntheticFrontEnd:: +~LibcxxStdProxyArraySyntheticFrontEnd() { + // these need to stay around because they are child objects who will follow + // their parent's life cycle + // delete m_base; +} + +llvm::Expected lldb_private::formatters:: +LibcxxStdProxyArraySyntheticFrontEnd::CalculateNumChildren() { mordante wrote: Agreed. I noticed some duplication when looking at these too. This one is slightly different due to the indirect access. https://github.com/llvm/llvm-project/pull/88613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)
https://github.com/mordante commented: Thanks for the review! https://github.com/llvm/llvm-project/pull/88613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a348875 - [LLDB][libc++] Adds valarray proxy data formatters. (#88613)
Author: Mark de Wever Date: 2024-04-15T18:46:58+02:00 New Revision: a34887550b7f2926ea335d4aedf8b72811f9c945 URL: https://github.com/llvm/llvm-project/commit/a34887550b7f2926ea335d4aedf8b72811f9c945 DIFF: https://github.com/llvm/llvm-project/commit/a34887550b7f2926ea335d4aedf8b72811f9c945.diff LOG: [LLDB][libc++] Adds valarray proxy data formatters. (#88613) These proxies are returned by operator[](...). These proxies all "behave" the same. They store a pointer to the data of the valarray they are a proxy for and they have an internal array of indices. This internal array is considered its contents. Added: lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp Modified: lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxx.h lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/valarray/TestDataFormatterLibcxxValarray.py lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/valarray/main.cpp Removed: diff --git a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt index 0c6fdb2b957315..f59032c423880f 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt +++ b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt @@ -14,6 +14,7 @@ add_lldb_library(lldbPluginCPlusPlusLanguage PLUGIN LibCxxQueue.cpp LibCxxRangesRefView.cpp LibCxxSliceArray.cpp + LibCxxProxyArray.cpp LibCxxSpan.cpp LibCxxTuple.cpp LibCxxUnorderedMap.cpp diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp index afb683f7d846a6..5f0684163328f5 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -760,6 +760,12 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) { lldb_private::formatters::LibcxxStdSliceArraySyntheticFrontEndCreator, "libc++ std::slice_array synthetic children", "^std::__[[:alnum:]]+::slice_array<.+>$", stl_deref_flags, true); + AddCXXSynthetic( + cpp_category_sp, + lldb_private::formatters::LibcxxStdProxyArraySyntheticFrontEndCreator, + "libc++ synthetic children for the valarray proxy arrays", + "^std::__[[:alnum:]]+::(gslice|mask|indirect)_array<.+>$", + stl_deref_flags, true); AddCXXSynthetic( cpp_category_sp, lldb_private::formatters::LibcxxStdForwardListSyntheticFrontEndCreator, @@ -890,6 +896,11 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) { "libc++ std::slice_array summary provider", "^std::__[[:alnum:]]+::slice_array<.+>$", stl_summary_flags, true); + AddCXXSummary(cpp_category_sp, +lldb_private::formatters::LibcxxContainerSummaryProvider, +"libc++ summary provider for the valarray proxy arrays", +"^std::__[[:alnum:]]+::(gslice|mask|indirect)_array<.+>$", +stl_summary_flags, true); AddCXXSummary( cpp_category_sp, lldb_private::formatters::LibcxxContainerSummaryProvider, "libc++ std::list summary provider", diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h index 8e97174dc30757..7fe15d1bf3f7af 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h @@ -231,6 +231,10 @@ SyntheticChildrenFrontEnd * LibcxxStdSliceArraySyntheticFrontEndCreator(CXXSyntheticChildren *, lldb::ValueObjectSP); +SyntheticChildrenFrontEnd * +LibcxxStdProxyArraySyntheticFrontEndCreator(CXXSyntheticChildren *, +lldb::ValueObjectSP); + SyntheticChildrenFrontEnd * LibcxxStdListSyntheticFrontEndCreator(CXXSyntheticChildren *, lldb::ValueObjectSP); diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp new file mode 100644 index 00..726f06523b97b4 --- /dev/null +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp @@ -0,0 +1,194 @@ +//===-- LibCxxProxyArray.cpp---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "LibCxx.h" + +#include "lldb/Core/ValueObject.h" +#include "lldb/DataFormatters/FormattersHelpers.h" +
[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)
https://github.com/mordante closed https://github.com/llvm/llvm-project/pull/88613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
@@ -657,8 +657,11 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail() || bytes_read == 0) { kusmour wrote: Would that be too chatty? Maybe a warning msg if there's any and use log channel for more details? https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
jeffreytan81 wrote: Thanks for fixing this. There are several related issues in the failing tests: 1. At least some of the memory regions ReadMemory() failures are caused by we incorrectly try to read from non-readable regions. We should explicitly check we have at least read permission. 2. The current ObjectFile plugin model swallow any error from the corresponding object file plugin and report a generic "Failed to save core file for process: no ObjectFile plugins were able to save a core for this process". We should distinguish the error reporting from "I do not support" from "I tried but got an error" and let users know. 3. As a stretch goal, we should implement "MemoryInfoList" stream type in minidump, then users can know what memory regions are available in original memory process space, but when he failed to read memory from that memory region/address during post-mortem debugging, it would be hint that that region exists but we just failed to save it. https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
https://github.com/jasonmolenda approved this pull request. Looks good, please land it. Thanks for the pings and rewriting the test case! https://github.com/llvm/llvm-project/pull/82603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (PR #88724)
https://github.com/bulbazord approved this pull request. Seems straightforward. https://github.com/llvm/llvm-project/pull/88724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (PR #88721)
https://github.com/bulbazord approved this pull request. Makes sense to me. https://github.com/llvm/llvm-project/pull/88721 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
@@ -657,8 +657,11 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail() || bytes_read == 0) { clayborg wrote: So a few things here: 1 - We should fix `Process::CalculateCoreFileSaveRanges(...)` to not include memory ranges that have no read access. The fix will need to go into this function in Process.cpp: ``` static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, Process::CoreFileMemoryRanges &ranges) { // Don't add empty ranges or ranges with no permissions. if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0) return; if (try_dirty_pages && AddDirtyPages(region, ranges)) return; ranges.push_back(CreateCoreFileMemoryRange(region)); } ``` Above is the current version of this function, we just need to fix the first if statement to not just check if there are any permissions, but to check if the section is readable: ``` if (region.GetRange().GetByteSize() == 0 || ((region.GetLLDBPermissions() & lldb::ePermissionsReadable) == 0)) ``` https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
@@ -657,8 +657,11 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail() || bytes_read == 0) { clayborg wrote: This function should report any errors that are found when reading memory, but it should also save _any_ bytes that are read if `bytes_read` is not zero. So we might ask to read an entire region, but only get part of it for some reason, in that case we might get an error saying "error: not all bytes read from section", but if `bytes_read` is greater than zero, we need to save those bytes and not skip them. https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
https://github.com/mbucko updated https://github.com/llvm/llvm-project/pull/88564 >From cfb233c0fb13c269e6431ceef4910d8c3cabb014 Mon Sep 17 00:00:00 2001 From: Miro Bucko Date: Fri, 12 Apr 2024 09:55:46 -0700 Subject: [PATCH] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam Summary: AddMemoryList() was returning the last error status returned by ReadMemory(). So if an invalid memory region was read last, the function would return an error. Also, the permission check for the memory reagion was incorrect. Test Plan: ./bin/llvm-lit -sv ~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py Reviewers: Subscribers: Tasks: Tags: --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp| 14 +++--- lldb/source/Target/Process.cpp | 11 +++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 50d1b563f469cf..bc42c629decd48 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -20,6 +20,8 @@ #include "lldb/Target/StopInfo.h" #include "lldb/Target/ThreadList.h" #include "lldb/Utility/DataExtractor.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" #include "lldb/Utility/RegisterValue.h" #include "llvm/ADT/StringRef.h" @@ -649,16 +651,22 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, DataBufferHeap helper_data; std::vector mem_descriptors; for (const auto &core_range : core_ranges) { -// Skip empty memory regions or any regions with no permissions. -if (core_range.range.empty() || core_range.lldb_permissions == 0) +// Skip empty memory regions. +if (core_range.range.empty()) continue; const addr_t addr = core_range.range.start(); const addr_t size = core_range.range.size(); auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail() || bytes_read == 0) { + Log *log = GetLog(LLDBLog::SystemRuntime); + LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s", +bytes_read, error.AsCString()); + error.Clear(); continue; +} + // We have a good memory region with valid bytes to store. LocationDescriptor memory_dump; memory_dump.DataSize = static_cast(bytes_read); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index f02ec37cb0f08f..84a0a65b0e4ebf 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { // case we should tell it to stop doing that. Normally, we don't NEED // to do that because we will next close the communication to the stub // and that will get it to shut down. But there are remote debugging -// cases where relying on that side-effect causes the shutdown to be -// flakey, so we should send a positive signal to interrupt the wait. +// cases where relying on that side-effect causes the shutdown to be +// flakey, so we should send a positive signal to interrupt the wait. Status error = HaltPrivate(); BroadcastEvent(eBroadcastBitInterrupt, nullptr); } else if (StateIsRunningState(m_last_broadcast_state)) { @@ -6325,8 +6325,11 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion, // ranges. static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, Process::CoreFileMemoryRanges &ranges) { - // Don't add empty ranges or ranges with no permissions. - if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0) + // Don't add empty ranges. + if (region.GetRange().GetByteSize() == 0) +return; + // Don't add ranges with no read permissions. + if ((region.GetLLDBPermissions() & lldb::ePermissionsReadable) == 0) return; if (try_dirty_pages && AddDirtyPages(region, ranges)) return; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
https://github.com/mbucko updated https://github.com/llvm/llvm-project/pull/88564 >From 27f7d03cfda9c6eea67973b9d8c3089abde8b732 Mon Sep 17 00:00:00 2001 From: Miro Bucko Date: Fri, 12 Apr 2024 09:55:46 -0700 Subject: [PATCH] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam Summary: AddMemoryList() was returning the last error status returned by ReadMemory(). So if an invalid memory region was read last, the function would return an error. Also, one of the reasons why the invalid memory region was read was because the check for reading permission in AddRegion() was incorrect. Test Plan: ./bin/llvm-lit -sv ~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py Reviewers: Subscribers: Tasks: Tags: --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 17 ++--- lldb/source/Target/Process.cpp | 11 +++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 50d1b563f469cf..9499d441ebe7ac 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -20,6 +20,8 @@ #include "lldb/Target/StopInfo.h" #include "lldb/Target/ThreadList.h" #include "lldb/Utility/DataExtractor.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" #include "lldb/Utility/RegisterValue.h" #include "llvm/ADT/StringRef.h" @@ -649,16 +651,25 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp, DataBufferHeap helper_data; std::vector mem_descriptors; for (const auto &core_range : core_ranges) { -// Skip empty memory regions or any regions with no permissions. -if (core_range.range.empty() || core_range.lldb_permissions == 0) +// Skip empty memory regions. +if (core_range.range.empty()) continue; const addr_t addr = core_range.range.start(); const addr_t size = core_range.range.size(); auto data_up = std::make_unique(size, 0); const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(), size, error); -if (bytes_read == 0) +if (error.Fail()) { + Log *log = GetLog(LLDBLog::SystemRuntime); + LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s", +bytes_read, error.AsCString()); + error.Clear(); +} + +if (bytes_read == 0) { continue; +} + // We have a good memory region with valid bytes to store. LocationDescriptor memory_dump; memory_dump.DataSize = static_cast(bytes_read); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index f02ec37cb0f08f..84a0a65b0e4ebf 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { // case we should tell it to stop doing that. Normally, we don't NEED // to do that because we will next close the communication to the stub // and that will get it to shut down. But there are remote debugging -// cases where relying on that side-effect causes the shutdown to be -// flakey, so we should send a positive signal to interrupt the wait. +// cases where relying on that side-effect causes the shutdown to be +// flakey, so we should send a positive signal to interrupt the wait. Status error = HaltPrivate(); BroadcastEvent(eBroadcastBitInterrupt, nullptr); } else if (StateIsRunningState(m_last_broadcast_state)) { @@ -6325,8 +6325,11 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion, // ranges. static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, Process::CoreFileMemoryRanges &ranges) { - // Don't add empty ranges or ranges with no permissions. - if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0) + // Don't add empty ranges. + if (region.GetRange().GetByteSize() == 0) +return; + // Don't add ranges with no read permissions. + if ((region.GetLLDBPermissions() & lldb::ePermissionsReadable) == 0) return; if (try_dirty_pages && AddDirtyPages(region, ranges)) return; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 071ac0a - [lldb] Skip lldb-server unit tests when building with ASan
Author: Jonas Devlieghere Date: 2024-04-15T13:20:54-07:00 New Revision: 071ac0ae3029594167d2bb4d28859bdc36367034 URL: https://github.com/llvm/llvm-project/commit/071ac0ae3029594167d2bb4d28859bdc36367034 DIFF: https://github.com/llvm/llvm-project/commit/071ac0ae3029594167d2bb4d28859bdc36367034.diff LOG: [lldb] Skip lldb-server unit tests when building with ASan The lldb-server unit tests are timing out on GreenDragon and swift-ci. We haven't been able to reproduce this locally or figure out why. We've made several attempts to address potential issues, but we've reached a point where we need to skip them to get signal out of this bot again. Added: Modified: lldb/unittests/tools/CMakeLists.txt Removed: diff --git a/lldb/unittests/tools/CMakeLists.txt b/lldb/unittests/tools/CMakeLists.txt index 055fc6e6f5df47..42b0c25dd1fc92 100644 --- a/lldb/unittests/tools/CMakeLists.txt +++ b/lldb/unittests/tools/CMakeLists.txt @@ -1,3 +1,5 @@ if(LLDB_TOOL_LLDB_SERVER_BUILD) - add_subdirectory(lldb-server) + if (NOT LLVM_USE_SANITIZER MATCHES ".*Address.*") +add_subdirectory(lldb-server) + endif() endif() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/88792 If user sets a breakpoint at `_dl_debug_state` before the process launched, the breakpoint is not resolved yet. When lldb loads dynamic loader module, it's created with `Target::GetOrCreateModule` which notifies any pending breakpoint to resolve. However, the module's sections are not loaded at this time. They are loaded after returned from [Target::GetOrCreateModule](https://github.com/llvm/llvm-project/blob/0287a5cc4e2a5ded1ae2e4079f91052e6a6b8d9b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L574-L577). This change fixes it by manually resolving breakpoints after creating dynamic loader module. >From 26528cd679478448edf446e0e82e5f207ffd6113 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Mon, 15 Apr 2024 16:30:38 -0400 Subject: [PATCH] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. --- lldb/include/lldb/Target/Target.h | 3 +++ .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 5 + lldb/source/Target/Target.cpp | 17 ++--- .../breakpoint_command/TestBreakpointCommand.py | 17 + 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 2c2e6b2831ccee..3554ef0ea5a140 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -878,6 +878,9 @@ class Target : public std::enable_shared_from_this, // the address of its previous instruction and return that address. lldb::addr_t GetBreakableLoadAddress(lldb::addr_t addr); + void UpdateBreakpoints(ModuleList &module_list, bool added, + bool delete_locations); + void ModulesDidLoad(ModuleList &module_list); void ModulesDidUnload(ModuleList &module_list, bool delete_locations); diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 9baf86da4dc799..901fa53682da8e 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -576,6 +576,11 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() { UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base, false); m_interpreter_module = module_sp; +// Manually update breakpoints after updating loaded sections, because they +// cannot be resolve at the time when creating this module. +ModuleList module_list; +module_list.Append(module_sp); +m_process->GetTarget().UpdateBreakpoints(module_list, true, false); return module_sp; } return nullptr; diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 09b0ac42631d1a..ec2dea5cc238d3 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -1687,6 +1687,13 @@ void Target::NotifyModulesRemoved(lldb_private::ModuleList &module_list) { ModulesDidUnload(module_list, false); } +void Target::UpdateBreakpoints(ModuleList &module_list, bool added, + bool delete_locations) { + m_breakpoint_list.UpdateBreakpoints(module_list, added, delete_locations); + m_internal_breakpoint_list.UpdateBreakpoints(module_list, added, + delete_locations); +} + void Target::ModulesDidLoad(ModuleList &module_list) { const size_t num_images = module_list.GetSize(); if (m_valid && num_images) { @@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_list) { ModuleSP module_sp(module_list.GetModuleAtIndex(idx)); LoadScriptingResourceForModule(module_sp, this); } -m_breakpoint_list.UpdateBreakpoints(module_list, true, false); -m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false); +UpdateBreakpoints(module_list, true, false); if (m_process_sp) { m_process_sp->ModulesDidLoad(module_list); } @@ -1713,8 +1719,7 @@ void Target::SymbolsDidLoad(ModuleList &module_list) { } } -m_breakpoint_list.UpdateBreakpoints(module_list, true, false); -m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false); +UpdateBreakpoints(module_list, true, false); auto data_sp = std::make_shared(shared_from_this(), module_list); BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp); @@ -1727,9 +1732,7 @@ void Target::ModulesDidUnload(ModuleList &module_list, bool delete_locations) { auto data_sp = std::make_shared(shared_from_this(), module_list); BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp); -m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations); -m_internal_breakpoint_list.UpdateBreakpoints(module_list, false, -
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) Changes If user sets a breakpoint at `_dl_debug_state` before the process launched, the breakpoint is not resolved yet. When lldb loads dynamic loader module, it's created with `Target::GetOrCreateModule` which notifies any pending breakpoint to resolve. However, the module's sections are not loaded at this time. They are loaded after returned from [Target::GetOrCreateModule](https://github.com/llvm/llvm-project/blob/0287a5cc4e2a5ded1ae2e4079f91052e6a6b8d9b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L574-L577). This change fixes it by manually resolving breakpoints after creating dynamic loader module. --- Full diff: https://github.com/llvm/llvm-project/pull/88792.diff 4 Files Affected: - (modified) lldb/include/lldb/Target/Target.h (+3) - (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+5) - (modified) lldb/source/Target/Target.cpp (+10-7) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py (+17) ``diff diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 2c2e6b2831ccee..3554ef0ea5a140 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -878,6 +878,9 @@ class Target : public std::enable_shared_from_this, // the address of its previous instruction and return that address. lldb::addr_t GetBreakableLoadAddress(lldb::addr_t addr); + void UpdateBreakpoints(ModuleList &module_list, bool added, + bool delete_locations); + void ModulesDidLoad(ModuleList &module_list); void ModulesDidUnload(ModuleList &module_list, bool delete_locations); diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 9baf86da4dc799..901fa53682da8e 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -576,6 +576,11 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() { UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base, false); m_interpreter_module = module_sp; +// Manually update breakpoints after updating loaded sections, because they +// cannot be resolve at the time when creating this module. +ModuleList module_list; +module_list.Append(module_sp); +m_process->GetTarget().UpdateBreakpoints(module_list, true, false); return module_sp; } return nullptr; diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 09b0ac42631d1a..ec2dea5cc238d3 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -1687,6 +1687,13 @@ void Target::NotifyModulesRemoved(lldb_private::ModuleList &module_list) { ModulesDidUnload(module_list, false); } +void Target::UpdateBreakpoints(ModuleList &module_list, bool added, + bool delete_locations) { + m_breakpoint_list.UpdateBreakpoints(module_list, added, delete_locations); + m_internal_breakpoint_list.UpdateBreakpoints(module_list, added, + delete_locations); +} + void Target::ModulesDidLoad(ModuleList &module_list) { const size_t num_images = module_list.GetSize(); if (m_valid && num_images) { @@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_list) { ModuleSP module_sp(module_list.GetModuleAtIndex(idx)); LoadScriptingResourceForModule(module_sp, this); } -m_breakpoint_list.UpdateBreakpoints(module_list, true, false); -m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false); +UpdateBreakpoints(module_list, true, false); if (m_process_sp) { m_process_sp->ModulesDidLoad(module_list); } @@ -1713,8 +1719,7 @@ void Target::SymbolsDidLoad(ModuleList &module_list) { } } -m_breakpoint_list.UpdateBreakpoints(module_list, true, false); -m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false); +UpdateBreakpoints(module_list, true, false); auto data_sp = std::make_shared(shared_from_this(), module_list); BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp); @@ -1727,9 +1732,7 @@ void Target::ModulesDidUnload(ModuleList &module_list, bool delete_locations) { auto data_sp = std::make_shared(shared_from_this(), module_list); BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp); -m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations); -m_internal_breakpoint_list.UpdateBreakpoints(module_list, false, - delete_locations); +UpdateBreakpoints(module_list, false, delete_locations); // If a module was torn dow
[Lldb-commits] [lldb] 7eb5022 - [lldb][NFC] Turn ChildCacheState into an unscoped enum (#88703)
Author: Michael Buch Date: 2024-04-15T21:45:00+01:00 New Revision: 7eb5022cbb11fdf5e74d707b4c93d3669a46eccf URL: https://github.com/llvm/llvm-project/commit/7eb5022cbb11fdf5e74d707b4c93d3669a46eccf DIFF: https://github.com/llvm/llvm-project/commit/7eb5022cbb11fdf5e74d707b4c93d3669a46eccf.diff LOG: [lldb][NFC] Turn ChildCacheState into an unscoped enum (#88703) Done for consistency with the rest of the enums in `lldb-enumerations.h`. See https://github.com/llvm/llvm-project/pull/80167#issuecomment-2043721298 Added: Modified: lldb/include/lldb/lldb-enumerations.h Removed: diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index f3b07ea6d20395..15e45857186091 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1310,7 +1310,7 @@ enum CompletionType { /// Specifies if children need to be re-computed /// after a call to \ref SyntheticChildrenFrontEnd::Update. -enum class ChildCacheState { +enum ChildCacheState { eRefetch = 0, ///< Children need to be recomputed dynamically. eReuse = 1, ///< Children did not change and don't need to be recomputed; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Turn ChildCacheState into an unscoped enum (PR #88703)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/88703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (PR #88721)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/88721 >From f0b309c52a7f497aa021f38f3ce272a1bb3e66ea Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 13:16:58 +0100 Subject: [PATCH] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions This logic was originally copied from `CompilerInstance::parseLangArgs`. Since then, the `CompilerInstance` uses `LangOptions::setLangDefaults` to set up new `LangOptions` instances. In our case, we only ever passed `Language::ObjCXX` into LLDB's `ParseLangArgs`, so most of this function was dead code. This patch replaces the duplicated logic with a call to `LangOptions::setLangDefaults`. --- .../TypeSystem/Clang/TypeSystemClang.cpp | 79 ++- 1 file changed, 6 insertions(+), 73 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 44bd02bd4b367d..be0ddb06f82c18 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -459,85 +459,19 @@ TypeSystemClang::ConvertAccessTypeToAccessSpecifier(AccessType access) { return AS_none; } -static void ParseLangArgs(LangOptions &Opts, InputKind IK, const char *triple) { +static void ParseLangArgs(LangOptions &Opts, ArchSpec arch) { // FIXME: Cleanup per-file based stuff. - // Set some properties which depend solely on the input kind; it would be - // nice to move these to the language standard, and have the driver resolve - // the input kind + language standard. - if (IK.getLanguage() == clang::Language::Asm) { -Opts.AsmPreprocessor = 1; - } else if (IK.isObjectiveC()) { -Opts.ObjC = 1; - } - - LangStandard::Kind LangStd = LangStandard::lang_unspecified; - - if (LangStd == LangStandard::lang_unspecified) { -// Based on the base language, pick one. -switch (IK.getLanguage()) { -case clang::Language::Unknown: -case clang::Language::CIR: -case clang::Language::LLVM_IR: -case clang::Language::RenderScript: - llvm_unreachable("Invalid input kind!"); -case clang::Language::OpenCL: - LangStd = LangStandard::lang_opencl10; - break; -case clang::Language::OpenCLCXX: - LangStd = LangStandard::lang_openclcpp10; - break; -case clang::Language::Asm: -case clang::Language::C: -case clang::Language::ObjC: - LangStd = LangStandard::lang_gnu99; - break; -case clang::Language::CXX: -case clang::Language::ObjCXX: - LangStd = LangStandard::lang_gnucxx98; - break; -case clang::Language::CUDA: -case clang::Language::HIP: - LangStd = LangStandard::lang_gnucxx17; - break; -case clang::Language::HLSL: - LangStd = LangStandard::lang_hlsl; - break; -} - } - - const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd); - Opts.LineComment = Std.hasLineComments(); - Opts.C99 = Std.isC99(); - Opts.CPlusPlus = Std.isCPlusPlus(); - Opts.CPlusPlus11 = Std.isCPlusPlus11(); - Opts.CPlusPlus14 = Std.isCPlusPlus14(); - Opts.CPlusPlus17 = Std.isCPlusPlus17(); - Opts.CPlusPlus20 = Std.isCPlusPlus20(); - Opts.Digraphs = Std.hasDigraphs(); - Opts.GNUMode = Std.isGNUMode(); - Opts.GNUInline = !Std.isC99(); - Opts.HexFloats = Std.hasHexFloats(); - - Opts.WChar = true; - - // OpenCL has some additional defaults. - if (LangStd == LangStandard::lang_opencl10) { -Opts.OpenCL = 1; -Opts.AltiVec = 1; -Opts.CXXOperatorNames = 1; -Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::All); - } - - // OpenCL and C++ both have bool, true, false keywords. - Opts.Bool = Opts.OpenCL || Opts.CPlusPlus; + std::vector Includes; + LangOptions::setLangDefaults(Opts, clang::Language::ObjCXX, arch.GetTriple(), + Includes, clang::LangStandard::lang_gnucxx98); Opts.setValueVisibilityMode(DefaultVisibility); // Mimicing gcc's behavior, trigraphs are only enabled if -trigraphs is // specified, or -std is set to a conforming mode. Opts.Trigraphs = !Opts.GNUMode; - Opts.CharIsSigned = ArchSpec(triple).CharIsSignedByDefault(); + Opts.CharIsSigned = arch.CharIsSignedByDefault(); Opts.OptimizeSize = 0; // FIXME: Eliminate this dependency. @@ -727,8 +661,7 @@ void TypeSystemClang::CreateASTContext() { m_ast_owned = true; m_language_options_up = std::make_unique(); - ParseLangArgs(*m_language_options_up, clang::Language::ObjCXX, -GetTargetTriple()); + ParseLangArgs(*m_language_options_up, ArchSpec(GetTargetTriple())); m_identifier_table_up = std::make_unique(*m_language_options_up, nullptr); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b8c16db - [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (#88721)
Author: Michael Buch Date: 2024-04-15T21:45:25+01:00 New Revision: b8c16db938327efdabc63f21fdc770069ac4406b URL: https://github.com/llvm/llvm-project/commit/b8c16db938327efdabc63f21fdc770069ac4406b DIFF: https://github.com/llvm/llvm-project/commit/b8c16db938327efdabc63f21fdc770069ac4406b.diff LOG: [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (#88721) This logic was originally copied from `CompilerInstance::parseLangArgs`. Since then, the `CompilerInstance` uses `LangOptions::setLangDefaults` to set up new `LangOptions` instances. In our case, we only ever passed `Language::ObjCXX` into LLDB's `ParseLangArgs`, so most of this function was dead code. This patch replaces the duplicated logic with a call to `LangOptions::setLangDefaults`. Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 44bd02bd4b367d..be0ddb06f82c18 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -459,85 +459,19 @@ TypeSystemClang::ConvertAccessTypeToAccessSpecifier(AccessType access) { return AS_none; } -static void ParseLangArgs(LangOptions &Opts, InputKind IK, const char *triple) { +static void ParseLangArgs(LangOptions &Opts, ArchSpec arch) { // FIXME: Cleanup per-file based stuff. - // Set some properties which depend solely on the input kind; it would be - // nice to move these to the language standard, and have the driver resolve - // the input kind + language standard. - if (IK.getLanguage() == clang::Language::Asm) { -Opts.AsmPreprocessor = 1; - } else if (IK.isObjectiveC()) { -Opts.ObjC = 1; - } - - LangStandard::Kind LangStd = LangStandard::lang_unspecified; - - if (LangStd == LangStandard::lang_unspecified) { -// Based on the base language, pick one. -switch (IK.getLanguage()) { -case clang::Language::Unknown: -case clang::Language::CIR: -case clang::Language::LLVM_IR: -case clang::Language::RenderScript: - llvm_unreachable("Invalid input kind!"); -case clang::Language::OpenCL: - LangStd = LangStandard::lang_opencl10; - break; -case clang::Language::OpenCLCXX: - LangStd = LangStandard::lang_openclcpp10; - break; -case clang::Language::Asm: -case clang::Language::C: -case clang::Language::ObjC: - LangStd = LangStandard::lang_gnu99; - break; -case clang::Language::CXX: -case clang::Language::ObjCXX: - LangStd = LangStandard::lang_gnucxx98; - break; -case clang::Language::CUDA: -case clang::Language::HIP: - LangStd = LangStandard::lang_gnucxx17; - break; -case clang::Language::HLSL: - LangStd = LangStandard::lang_hlsl; - break; -} - } - - const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd); - Opts.LineComment = Std.hasLineComments(); - Opts.C99 = Std.isC99(); - Opts.CPlusPlus = Std.isCPlusPlus(); - Opts.CPlusPlus11 = Std.isCPlusPlus11(); - Opts.CPlusPlus14 = Std.isCPlusPlus14(); - Opts.CPlusPlus17 = Std.isCPlusPlus17(); - Opts.CPlusPlus20 = Std.isCPlusPlus20(); - Opts.Digraphs = Std.hasDigraphs(); - Opts.GNUMode = Std.isGNUMode(); - Opts.GNUInline = !Std.isC99(); - Opts.HexFloats = Std.hasHexFloats(); - - Opts.WChar = true; - - // OpenCL has some additional defaults. - if (LangStd == LangStandard::lang_opencl10) { -Opts.OpenCL = 1; -Opts.AltiVec = 1; -Opts.CXXOperatorNames = 1; -Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::All); - } - - // OpenCL and C++ both have bool, true, false keywords. - Opts.Bool = Opts.OpenCL || Opts.CPlusPlus; + std::vector Includes; + LangOptions::setLangDefaults(Opts, clang::Language::ObjCXX, arch.GetTriple(), + Includes, clang::LangStandard::lang_gnucxx98); Opts.setValueVisibilityMode(DefaultVisibility); // Mimicing gcc's behavior, trigraphs are only enabled if -trigraphs is // specified, or -std is set to a conforming mode. Opts.Trigraphs = !Opts.GNUMode; - Opts.CharIsSigned = ArchSpec(triple).CharIsSignedByDefault(); + Opts.CharIsSigned = arch.CharIsSignedByDefault(); Opts.OptimizeSize = 0; // FIXME: Eliminate this dependency. @@ -727,8 +661,7 @@ void TypeSystemClang::CreateASTContext() { m_ast_owned = true; m_language_options_up = std::make_unique(); - ParseLangArgs(*m_language_options_up, clang::Language::ObjCXX, -GetTargetTriple()); + ParseLangArgs(*m_language_options_up, ArchSpec(GetTargetTriple())); m_identifier_table_up = std::make_unique(*m_language_options_up, nullptr); ___ lldb-commits mailing list lld
[Lldb-commits] [lldb] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (PR #88724)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/88724 >From e85bf75077dec2d6aa7d6983bbde222d1c2b3f29 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 10:37:05 +0100 Subject: [PATCH 1/2] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function This member was never actually used, ever since its introduction in `ca4e0fd7e63b90e6f68044af47248c64f250ee8f`. --- .../ExpressionParser/Clang/ClangExpressionDeclMap.cpp| 9 ++--- .../Plugins/ExpressionParser/Clang/NameSearchContext.h | 1 - 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 2d306b42760b18..bf310cfcd4c8af 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1049,7 +1049,6 @@ void ClangExpressionDeclMap::LookupInModulesDeclVendor( context.AddNamedDecl(copied_function); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (auto copied_var = dyn_cast(copied_decl)) { context.AddNamedDecl(copied_var); context.m_found_variable = true; @@ -1299,7 +1298,6 @@ void ClangExpressionDeclMap::LookupFunction( AddOneFunction(context, sym_ctx.function, nullptr); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (sym_ctx.symbol) { Symbol *symbol = sym_ctx.symbol; if (target && symbol->GetType() == eSymbolTypeReExported) { @@ -1329,13 +1327,10 @@ void ClangExpressionDeclMap::LookupFunction( } if (!context.m_found_function_with_type_info) { - if (extern_symbol) { + if (extern_symbol) AddOneFunction(context, nullptr, extern_symbol); -context.m_found_function = true; - } else if (non_extern_symbol) { + else if (non_extern_symbol) AddOneFunction(context, nullptr, non_extern_symbol); -context.m_found_function = true; - } } } } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h index dc8621dd6aba52..9a3320636081be 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h @@ -41,7 +41,6 @@ struct NameSearchContext { bool m_found_variable = false; bool m_found_function_with_type_info = false; - bool m_found_function = false; bool m_found_local_vars_nsp = false; bool m_found_type = false; >From 86314daa00da7be807a14f81ae98816dc7172b29 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 15 Apr 2024 13:40:55 +0100 Subject: [PATCH 2/2] fixup! revert noisy formatting change --- .../ExpressionParser/Clang/ClangExpressionDeclMap.cpp| 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index bf310cfcd4c8af..31f6447d66f642 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1327,10 +1327,11 @@ void ClangExpressionDeclMap::LookupFunction( } if (!context.m_found_function_with_type_info) { - if (extern_symbol) + if (extern_symbol) { AddOneFunction(context, nullptr, extern_symbol); - else if (non_extern_symbol) + } else if (non_extern_symbol) { AddOneFunction(context, nullptr, non_extern_symbol); + } } } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang][NFCI] Use LangOptions::setLangDefaults when creating new LangOptions (PR #88721)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/88721 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 905d2ec - [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (#88724)
Author: Michael Buch Date: 2024-04-15T21:45:41+01:00 New Revision: 905d2ecbb63f4087b50d633b012c9915ab3a6829 URL: https://github.com/llvm/llvm-project/commit/905d2ecbb63f4087b50d633b012c9915ab3a6829 DIFF: https://github.com/llvm/llvm-project/commit/905d2ecbb63f4087b50d633b012c9915ab3a6829.diff LOG: [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (#88724) This member was never actually used, ever since its introduction in `ca4e0fd7e63b90e6f68044af47248c64f250ee8f`. Added: Modified: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h Removed: diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 2d306b42760b18..31f6447d66f642 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1049,7 +1049,6 @@ void ClangExpressionDeclMap::LookupInModulesDeclVendor( context.AddNamedDecl(copied_function); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (auto copied_var = dyn_cast(copied_decl)) { context.AddNamedDecl(copied_var); context.m_found_variable = true; @@ -1299,7 +1298,6 @@ void ClangExpressionDeclMap::LookupFunction( AddOneFunction(context, sym_ctx.function, nullptr); context.m_found_function_with_type_info = true; -context.m_found_function = true; } else if (sym_ctx.symbol) { Symbol *symbol = sym_ctx.symbol; if (target && symbol->GetType() == eSymbolTypeReExported) { @@ -1331,10 +1329,8 @@ void ClangExpressionDeclMap::LookupFunction( if (!context.m_found_function_with_type_info) { if (extern_symbol) { AddOneFunction(context, nullptr, extern_symbol); -context.m_found_function = true; } else if (non_extern_symbol) { AddOneFunction(context, nullptr, non_extern_symbol); -context.m_found_function = true; } } } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h index dc8621dd6aba52..9a3320636081be 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h @@ -41,7 +41,6 @@ struct NameSearchContext { bool m_found_variable = false; bool m_found_function_with_type_info = false; - bool m_found_function = false; bool m_found_local_vars_nsp = false; bool m_found_type = false; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionDeclMap][NFC] Remove unused NameSearchContext::m_found_function (PR #88724)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/88724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 0287a5cc4e2a5ded1ae2e4079f91052e6a6b8d9b...26528cd679478448edf446e0e82e5f207ffd6113 lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py `` View the diff from darker here. ``diff --- TestBreakpointCommand.py2024-04-15 20:36:32.00 + +++ TestBreakpointCommand.py2024-04-15 20:45:33.746929 + @@ -679,12 +679,12 @@ process is launched. """ self.build() exe = self.getBuildArtifact("a.out") self.runCmd("target create %s" % exe) -bpid = lldbutil.run_break_set_by_symbol(self, "_dl_debug_state", -num_expected_locations=0) +bpid = lldbutil.run_break_set_by_symbol( +self, "_dl_debug_state", num_expected_locations=0 +) self.runCmd("run") self.assertIsNotNone( -lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), - bpid) -) +lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), bpid) +) `` https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a855eea - [lldb] Fix the standalone Xcode build after #88317
Author: Jonas Devlieghere Date: 2024-04-15T14:08:56-07:00 New Revision: a855eea7fe86ef09a87f6251b3b711b821ae32bf URL: https://github.com/llvm/llvm-project/commit/a855eea7fe86ef09a87f6251b3b711b821ae32bf DIFF: https://github.com/llvm/llvm-project/commit/a855eea7fe86ef09a87f6251b3b711b821ae32bf.diff LOG: [lldb] Fix the standalone Xcode build after #88317 In #88317, the clang resource headers was converted to an interface library. Update LLDB and fix the Xcode standalone build. Thanks Evan for the help! Added: Modified: lldb/cmake/modules/LLDBFramework.cmake Removed: diff --git a/lldb/cmake/modules/LLDBFramework.cmake b/lldb/cmake/modules/LLDBFramework.cmake index 81fc596ef4244e..f915839f6b45a5 100644 --- a/lldb/cmake/modules/LLDBFramework.cmake +++ b/lldb/cmake/modules/LLDBFramework.cmake @@ -119,7 +119,7 @@ add_custom_command(TARGET liblldb POST_BUILD if(NOT APPLE_EMBEDDED) if (TARGET clang-resource-headers) add_dependencies(liblldb clang-resource-headers) -set(clang_resource_headers_dir $) +set(clang_resource_headers_dir $) else() set(clang_resource_headers_dir ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}/include) if(NOT EXISTS ${clang_resource_headers_dir}) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
jimingham wrote: When a shared library gets loaded, its sections have to have been loaded before asking the breakpoint system to try to set new breakpoints. Presumably this is how it happens for all the other shared library loads, or breakpoints in shared libraries wouldn't work. I'm missing why the _dl_debug_state breakpoint is special here, such that it requires a force load of the section info? How does that happen? https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add asan tests for libsanitizers. (PR #88349)
https://github.com/JDevlieghere approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/88349 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
jasonmolenda wrote: Skimming DynamicLoaderPOSIXDYLD (as it is written today), it looks like attach/launch call `SetRendezvousBreakpoint()` which (1) loads ld.so into lldb if it isn't already there, and (2) sets the breakpoint to be notified about future binaries loading. It loads ld.so via `LoadInterpreterModule` which Finds It somehow (I stopped reading), adds it to the Target, and sets the section load addresses, returns the module_sp. Then it sets breakpoints on a half dozen notification function names in the ld.so Module. This is all special-cased for ld.so, and we do not evaluate user breakpoints because we didn't call `Target::ModulesDidLoad` for ld.so. The code path for other binaries goes through `DynamicLoaderPOSIXDYLD::RefreshModules` which does the normal approach of adding binaries to the Target, setting the load addresses, then calling ModulesDidLoad which will evaluate breakpoints that may need to be set in the new binaries. It seems like a simpler fix would be to have `DynamicLoaderPOSIXDYLD::LoadInterpreterModule` create a ModuleList with the ModuleSP of ld.so that it has just added to Target and set its section load addresses, then call `ModulesDidLoad` on it so that user breakpoints are evaluated and inserted. The patch as it is written right now is taking one part of ModulesDidLoad and putting it in a separate method that `DynamicLoaderPOSIXDYLD::LoadInterpreterModule` is calling -- why not just call ModulesDidLoad and delegate this (and possibly other binary-just-loaded) behaviors to it? https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)
adrian-prantl wrote: @mordante This broke the green dragon bot https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1069/ This change seems to break the `frame diagnose` test: +# Based on https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4 +"settings set target.disable-aslr false", https://github.com/llvm/llvm-project/pull/88312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 466017c - Work around test failure due to new aslr default
Author: Adrian Prantl Date: 2024-04-15T15:50:17-07:00 New Revision: 466017c8dab74f66ce513c8752f0c1dcd16a8a63 URL: https://github.com/llvm/llvm-project/commit/466017c8dab74f66ce513c8752f0c1dcd16a8a63 DIFF: https://github.com/llvm/llvm-project/commit/466017c8dab74f66ce513c8752f0c1dcd16a8a63.diff LOG: Work around test failure due to new aslr default Added: Modified: lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py Removed: diff --git a/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py b/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py index d8f45161378b0f..4d9b036f5102cb 100644 --- a/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py +++ b/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py @@ -19,6 +19,9 @@ def test_diagnose_dereference_function_return(self): TestBase.setUp(self) self.build() exe = self.getBuildArtifact("a.out") +# FIXME: This default changed in lldbtest.py and this test +# seems to rely on having it turned off. +self.runCmd("settings set target.disable-aslr true") self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) self.runCmd("run", RUN_SUCCEEDED) self.expect("thread list", "Thread should be stopped", substrs=["stopped"]) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)
adrian-prantl wrote: I worked around the problem in 466017c8dab74f66ce513c8752f0c1dcd16a8a63. https://github.com/llvm/llvm-project/pull/88312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
https://github.com/Awfa created https://github.com/llvm/llvm-project/pull/88812 This PR adds a check within `PutFile` to exit early when both local and destination files have matching MD5 hashes. If they differ, or there is trouble getting the hashes, the regular code path to put the file is run. As I needed this to talk to an `lldb-server` which runs the gdb-remote protocol, I enabled `CalculateMD5` within `Platform/gdb-server` and also found and fixed a parsing bug within it as well. Before this PR, the client is incorrectly parsing the response packet containing the checksum; after this PR, hopefully this is fixed. There is a test for the parsing behavior included in this PR. >From 095696411172034f80233f1722e293c1f458d67f Mon Sep 17 00:00:00 2001 From: Anthony Ha Date: Mon, 15 Apr 2024 19:34:19 + Subject: [PATCH] [lldb] Skip remote PutFile when MD5 hashes equal --- .../gdb-server/PlatformRemoteGDBServer.cpp| 9 + .../gdb-server/PlatformRemoteGDBServer.h | 3 ++ .../GDBRemoteCommunicationClient.cpp | 36 +-- lldb/source/Target/Platform.cpp | 30 +++- .../GDBRemoteCommunicationClientTest.cpp | 23 5 files changed, 98 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp index 88f1ad15b6b485..d9f3e40019cf29 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { +// Note that high/low are switched in the gdb remote communication client +return m_gdb_client_up->CalculateMD5(file_spec, high, low); + } + return false; +} + void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() { m_trap_handlers.push_back(ConstString("_sigtramp")); } diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h index 638f7db5ef800c..d83fc386f59427 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -146,6 +146,9 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver { void CalculateTrapHandlerSymbolNames() override; + bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, +uint64_t &high) override; + const lldb::UnixSignalsSP &GetRemoteUnixSignals() override; size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 1f6116967a4f64..8c79d5fecf12fe 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get +// the byte string for each low and high hex separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// Get low part +auto part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +bool conversionErrored = part.getAsInteger(16, low); +if (conversionErrored) + return false; + +// Get high part +part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +conversionError
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Anthony Ha (Awfa) Changes This PR adds a check within `PutFile` to exit early when both local and destination files have matching MD5 hashes. If they differ, or there is trouble getting the hashes, the regular code path to put the file is run. As I needed this to talk to an `lldb-server` which runs the gdb-remote protocol, I enabled `CalculateMD5` within `Platform/gdb-server` and also found and fixed a parsing bug within it as well. Before this PR, the client is incorrectly parsing the response packet containing the checksum; after this PR, hopefully this is fixed. There is a test for the parsing behavior included in this PR. --- Full diff: https://github.com/llvm/llvm-project/pull/88812.diff 5 Files Affected: - (modified) lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp (+9) - (modified) lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h (+3) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+34-2) - (modified) lldb/source/Target/Platform.cpp (+29-1) - (modified) lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp (+23) ``diff diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp index 88f1ad15b6b485..d9f3e40019cf29 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { +// Note that high/low are switched in the gdb remote communication client +return m_gdb_client_up->CalculateMD5(file_spec, high, low); + } + return false; +} + void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() { m_trap_handlers.push_back(ConstString("_sigtramp")); } diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h index 638f7db5ef800c..d83fc386f59427 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -146,6 +146,9 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver { void CalculateTrapHandlerSymbolNames() override; + bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, +uint64_t &high) override; + const lldb::UnixSignalsSP &GetRemoteUnixSignals() override; size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 1f6116967a4f64..8c79d5fecf12fe 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get +// the byte string for each low and high hex separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// Get low part +auto part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +bool conversionErrored = part.getAsInteger(16, low); +if (conversionErrored) + return false; + +// Get high part +part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +conversionErrored = part.getAsInteger(16, high); +if (conv
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { bulbazord wrote: Suggestion: Flip the condition and do an early return with false. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (!source_file) return Status(source_file.takeError()); Status error; + + bool requires_upload = true; + { bulbazord wrote: Why is this all in its own block? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get bulbazord wrote: Why not fix the bug instead of working around it here? What causes the bug? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { +// Note that high/low are switched in the gdb remote communication client bulbazord wrote: It's good that you have a comment for this because it "feels" wrong. But why are `high/low` swapped in the gdb remote communication client in the first place? Is that intentional or is this working around a bug? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (!source_file) return Status(source_file.takeError()); Status error; + + bool requires_upload = true; + { +uint64_t dest_md5_low, dest_md5_high; +bool success = CalculateMD5(destination, dest_md5_low, dest_md5_high); +if (!success) { + LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of destination"); +} else { + auto local_md5 = llvm::sys::fs::md5_contents(source.GetPath()); + if (!local_md5) { +LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of source"); + } else { +uint64_t local_md5_high, local_md5_low; +std::tie(local_md5_high, local_md5_low) = local_md5->words(); bulbazord wrote: LLDB uses c++17 so you can use structured binding for this. ``` const auto [local_md5_high, local_md5_low] = local_md5->words(); ``` https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec &arch, Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, uint32_t uid, uint32_t gid) { Log *log = GetLog(LLDBLog::Platform); - LLDB_LOGF(log, "[PutFile] Using block by block transfer\n"); + LLDB_LOGF(log, "[PutFile] Using block by block transfer"); bulbazord wrote: Why did this get changed? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get +// the byte string for each low and high hex separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// Get low part +auto part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); bulbazord wrote: Can you add some meaning to `sizeof(uint64_t) * 2`? It's used a lot, so giving it a name would help with the readability. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get +// the byte string for each low and high hex separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// Get low part +auto part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +bool conversionErrored = part.getAsInteger(16, low); +if (conversionErrored) bulbazord wrote: suggestion: `if (part.getAsInteger(/*radix=*/16, low))` https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get Awfa wrote: `response.GetHexMaxU64` consumes the response stream until a non-valid hex character is encountered. This is just the wrong function to use because the server returns both the low/high parts concatenated. When it was used, what would happen is `low = response.GetHexMaxU64...` would consume the whole packet, when it should just be consuming the first half of it. This also leads to `high` being set to 0 because `low` consumed the entire packet. This part of the patch isn't a workaround, it's the fix. This fixes the client so it can correctly parse the server's response. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { Awfa wrote: Should I change the other functions in `PlatformRemoteGDBServer` to do the same? For example `PlatformRemoteGDBServer::GetFileExists` does the same currently. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { +// Note that high/low are switched in the gdb remote communication client Awfa wrote: Good point - I don't see a reason why it is swapped there. I'll change it so it's just consistent. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec &arch, Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, uint32_t uid, uint32_t gid) { Log *log = GetLog(LLDBLog::Platform); - LLDB_LOGF(log, "[PutFile] Using block by block transfer\n"); + LLDB_LOGF(log, "[PutFile] Using block by block transfer"); Awfa wrote: When testing this change, and using `log enable lldb platform` to read the logs, I just noticed this had a `\n` and removed it since `LLDB_LOGF` already adds a new line, and it didn't seem like most other logs had `\n`. Is this bad to have in the patch? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (!source_file) return Status(source_file.takeError()); Status error; + + bool requires_upload = true; + { Awfa wrote: This is how I write code to scope the variables `dest_md5_low`, `dest_md5_high`, `success` away from the main scope of the function. Is this not good to do in the llvm-project? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get +// the byte string for each low and high hex separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// Get low part +auto part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); Awfa wrote: Will do https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (!source_file) return Status(source_file.takeError()); Status error; + + bool requires_upload = true; + { +uint64_t dest_md5_low, dest_md5_high; +bool success = CalculateMD5(destination, dest_md5_low, dest_md5_high); +if (!success) { + LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of destination"); +} else { + auto local_md5 = llvm::sys::fs::md5_contents(source.GetPath()); + if (!local_md5) { +LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of source"); + } else { +uint64_t local_md5_high, local_md5_low; +std::tie(local_md5_high, local_md5_low) = local_md5->words(); Awfa wrote: Will do https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lit] Guard MallocNanoZone envvar in shell tests (PR #88824)
https://github.com/chelcassanova created https://github.com/llvm/llvm-project/pull/88824 Previously the MallocNanoZone envvar would be set to 0 on Darwin for the LLDB shell tests, but this should guarded behind ASan being enabled as opposed to simply running the test suite behind Darwin. This required that the LLVM_USE_SANITIZER option be added as an attribute to the lit config for shell tests. >From 08a98fef47798998703df2b1deda0be69e2849cd Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Mon, 15 Apr 2024 15:29:06 -0700 Subject: [PATCH] [lldb][lit] Guard MallocNanoZone envvar in shell tests Previously the MallocNanoZone envvar would be set to 0 on Darwin for the LLDB shell tests, but this should guarded behind ASan being enabled as opposed to simply running the test suite behind Darwin. This required that the LLVM_USE_SANITIZER option be added as an attribute to the lit config for shell tests. --- lldb/test/Shell/lit.cfg.py | 13 + lldb/test/Shell/lit.site.cfg.py.in | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py index 290569576ac80d..8379c3183cc084 100644 --- a/lldb/test/Shell/lit.cfg.py +++ b/lldb/test/Shell/lit.cfg.py @@ -50,10 +50,15 @@ ) # Enable sanitizer runtime flags. -config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" -config.environment["TSAN_OPTIONS"] = "halt_on_error=1" -if platform.system() == "Darwin": -config.environment["MallocNanoZone"] = "0" +if "Address" in config.llvm_use_sanitizer: +config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" +if platform.system() == "Darwin": +config.environment["MallocNanoZone"] = "0" + +if "Thread" in config.llvm_use_sanitizer: +config.environment["TSAN_OPTIONS"] = "halt_on_error=1" + + # Support running the test suite under the lldb-repro wrapper. This makes it # possible to capture a test suite run and then rerun all the test from the diff --git a/lldb/test/Shell/lit.site.cfg.py.in b/lldb/test/Shell/lit.site.cfg.py.in index 736dfc335732b5..b69e7bce1bc0be 100644 --- a/lldb/test/Shell/lit.site.cfg.py.in +++ b/lldb/test/Shell/lit.site.cfg.py.in @@ -26,6 +26,7 @@ config.lldb_enable_lua = @LLDB_ENABLE_LUA@ config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@" config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@ config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@ +config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@" # The shell tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lit] Guard MallocNanoZone envvar in shell tests (PR #88824)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) Changes Previously the MallocNanoZone envvar would be set to 0 on Darwin for the LLDB shell tests, but this should guarded behind ASan being enabled as opposed to simply running the test suite behind Darwin. This required that the LLVM_USE_SANITIZER option be added as an attribute to the lit config for shell tests. --- Full diff: https://github.com/llvm/llvm-project/pull/88824.diff 2 Files Affected: - (modified) lldb/test/Shell/lit.cfg.py (+9-4) - (modified) lldb/test/Shell/lit.site.cfg.py.in (+1) ``diff diff --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py index 290569576ac80d..8379c3183cc084 100644 --- a/lldb/test/Shell/lit.cfg.py +++ b/lldb/test/Shell/lit.cfg.py @@ -50,10 +50,15 @@ ) # Enable sanitizer runtime flags. -config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" -config.environment["TSAN_OPTIONS"] = "halt_on_error=1" -if platform.system() == "Darwin": -config.environment["MallocNanoZone"] = "0" +if "Address" in config.llvm_use_sanitizer: +config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" +if platform.system() == "Darwin": +config.environment["MallocNanoZone"] = "0" + +if "Thread" in config.llvm_use_sanitizer: +config.environment["TSAN_OPTIONS"] = "halt_on_error=1" + + # Support running the test suite under the lldb-repro wrapper. This makes it # possible to capture a test suite run and then rerun all the test from the diff --git a/lldb/test/Shell/lit.site.cfg.py.in b/lldb/test/Shell/lit.site.cfg.py.in index 736dfc335732b5..b69e7bce1bc0be 100644 --- a/lldb/test/Shell/lit.site.cfg.py.in +++ b/lldb/test/Shell/lit.site.cfg.py.in @@ -26,6 +26,7 @@ config.lldb_enable_lua = @LLDB_ENABLE_LUA@ config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@" config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@ config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@ +config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@" # The shell tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell") `` https://github.com/llvm/llvm-project/pull/88824 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lit] Guard MallocNanoZone envvar in shell tests (PR #88824)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 9c970d5ecd6a85188cd2b0a941fcd4d60063ef81...08a98fef47798998703df2b1deda0be69e2849cd lldb/test/Shell/lit.cfg.py `` View the diff from darker here. ``diff --- lit.cfg.py 2024-04-15 22:29:06.00 + +++ lit.cfg.py 2024-04-16 00:38:55.259382 + @@ -55,11 +55,10 @@ if platform.system() == "Darwin": config.environment["MallocNanoZone"] = "0" if "Thread" in config.llvm_use_sanitizer: config.environment["TSAN_OPTIONS"] = "halt_on_error=1" - # Support running the test suite under the lldb-repro wrapper. This makes it # possible to capture a test suite run and then rerun all the test from the # just captured reproducer. `` https://github.com/llvm/llvm-project/pull/88824 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add asan tests for libsanitizers. (PR #88349)
https://github.com/usama54321 updated https://github.com/llvm/llvm-project/pull/88349 >From 18757485878c4455032883ebfeb13483dc3ab469 Mon Sep 17 00:00:00 2001 From: usama Date: Wed, 10 Apr 2024 21:07:11 -0700 Subject: [PATCH] [LLDB] Add asan tests for libsanitizers. This patch tests LLDB integration with libsanitizers for ASan. This integration works through the ASanLibsanitizers plugin in the InstrumentationRuntime. rdar://111856681 --- lldb/test/API/functionalities/asan/Makefile | 6 +- .../functionalities/asan/TestMemoryHistory.py | 73 ++- .../functionalities/asan/TestReportData.py| 20 - .../API/functionalities/libsanitizers/util.py | 3 + 4 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 lldb/test/API/functionalities/libsanitizers/util.py diff --git a/lldb/test/API/functionalities/asan/Makefile b/lldb/test/API/functionalities/asan/Makefile index 4913a18d8cc6f9..d66696fed7078f 100644 --- a/lldb/test/API/functionalities/asan/Makefile +++ b/lldb/test/API/functionalities/asan/Makefile @@ -1,4 +1,8 @@ C_SOURCES := main.c -CFLAGS_EXTRAS := -fsanitize=address -g -gcolumn-info +asan: CFLAGS_EXTRAS := -fsanitize=address -g -gcolumn-info +asan: all + +libsanitizers: CFLAGS_EXTRAS := -fsanitize=address -fsanitize-stable-abi -g -gcolumn-info +libsanitizers: all include Makefile.rules diff --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py b/lldb/test/API/functionalities/asan/TestMemoryHistory.py index 00162ae8822c74..ee7939203ead18 100644 --- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py +++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py @@ -9,15 +9,21 @@ from lldbsuite.test import lldbplatform from lldbsuite.test import lldbutil +from functionalities.libsanitizers.util import no_libsanitizers class AsanTestCase(TestBase): @skipIfFreeBSD # llvm.org/pr21136 runtimes not yet available by default @expectedFailureNetBSD @skipUnlessAddressSanitizer def test(self): -self.build() +self.build(make_targets=["asan"]) self.asan_tests() +@skipIf(oslist=no_match(["macosx"])) +def test_libsanitizers_asan(self): +self.build(make_targets=["libsanitizers"]) +self.libsanitizer_tests() + def setUp(self): # Call super's setUp(). TestBase.setUp(self) @@ -26,6 +32,71 @@ def setUp(self): self.line_free = line_number("main.c", "// free line") self.line_breakpoint = line_number("main.c", "// break line") +# Test line numbers: rdar://126237493 +def libsanitizer_tests(self): +target = self.createTestTarget() + +if no_libsanitizers(self): +self.skipTest("libsanitizers not found") + +self.runCmd( +"env SanitizersAddress=1 MallocSanitizerZone=1 MallocSecureAllocator=0" +) + +self.runCmd("run") + +# In libsanitizers, memory history is not supported until a report has been generated +self.expect( +"thread list", +"Process should be stopped due to ASan report", +substrs=["stopped", "stop reason = Use of deallocated memory"], +) + +# test the 'memory history' command +self.expect( +"memory history 'pointer'", +substrs=[ +"Memory deallocated by Thread", +"a.out`f2", +"main.c", +"Memory allocated by Thread", +"a.out`f1", +"main.c", +], +) + +# do the same using SB API +process = self.dbg.GetSelectedTarget().process +val = ( + process.GetSelectedThread().GetSelectedFrame().EvaluateExpression("pointer") +) +addr = val.GetValueAsUnsigned() +threads = process.GetHistoryThreads(addr) +self.assertEqual(threads.GetSize(), 2) + +history_thread = threads.GetThreadAtIndex(0) +self.assertTrue(history_thread.num_frames >= 2) +self.assertEqual( + history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), +"main.c", +) + +history_thread = threads.GetThreadAtIndex(1) +self.assertTrue(history_thread.num_frames >= 2) +self.assertEqual( + history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), +"main.c", +) + +# let's free the container (SBThreadCollection) and see if the +# SBThreads still live +threads = None +self.assertTrue(history_thread.num_frames >= 2) +self.assertEqual( + history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), +"main.c", +) + def asan_tests(self): target = self.createTestTarget() diff --git a/lldb/test/API/functionalities/asan/TestReportData.py b/lldb/test/API/functionalities/asan/TestReportData.py index 543c5fe66a208d..de0c1206a57ad6 100644
[Lldb-commits] [lldb] 82f479b - Add asan tests for libsanitizers. (#88349)
Author: Usama Hameed Date: 2024-04-15T19:42:45-07:00 New Revision: 82f479ba315a417b6cd01a8c2efdc15c26689f2e URL: https://github.com/llvm/llvm-project/commit/82f479ba315a417b6cd01a8c2efdc15c26689f2e DIFF: https://github.com/llvm/llvm-project/commit/82f479ba315a417b6cd01a8c2efdc15c26689f2e.diff LOG: Add asan tests for libsanitizers. (#88349) This patch tests LLDB integration with libsanitizers for ASan. rdar://111856681 Added: lldb/test/API/functionalities/libsanitizers/util.py Modified: lldb/test/API/functionalities/asan/Makefile lldb/test/API/functionalities/asan/TestMemoryHistory.py lldb/test/API/functionalities/asan/TestReportData.py Removed: diff --git a/lldb/test/API/functionalities/asan/Makefile b/lldb/test/API/functionalities/asan/Makefile index 4913a18d8cc6f9..d66696fed7078f 100644 --- a/lldb/test/API/functionalities/asan/Makefile +++ b/lldb/test/API/functionalities/asan/Makefile @@ -1,4 +1,8 @@ C_SOURCES := main.c -CFLAGS_EXTRAS := -fsanitize=address -g -gcolumn-info +asan: CFLAGS_EXTRAS := -fsanitize=address -g -gcolumn-info +asan: all + +libsanitizers: CFLAGS_EXTRAS := -fsanitize=address -fsanitize-stable-abi -g -gcolumn-info +libsanitizers: all include Makefile.rules diff --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py b/lldb/test/API/functionalities/asan/TestMemoryHistory.py index 00162ae8822c74..ee7939203ead18 100644 --- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py +++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py @@ -9,15 +9,21 @@ from lldbsuite.test import lldbplatform from lldbsuite.test import lldbutil +from functionalities.libsanitizers.util import no_libsanitizers class AsanTestCase(TestBase): @skipIfFreeBSD # llvm.org/pr21136 runtimes not yet available by default @expectedFailureNetBSD @skipUnlessAddressSanitizer def test(self): -self.build() +self.build(make_targets=["asan"]) self.asan_tests() +@skipIf(oslist=no_match(["macosx"])) +def test_libsanitizers_asan(self): +self.build(make_targets=["libsanitizers"]) +self.libsanitizer_tests() + def setUp(self): # Call super's setUp(). TestBase.setUp(self) @@ -26,6 +32,71 @@ def setUp(self): self.line_free = line_number("main.c", "// free line") self.line_breakpoint = line_number("main.c", "// break line") +# Test line numbers: rdar://126237493 +def libsanitizer_tests(self): +target = self.createTestTarget() + +if no_libsanitizers(self): +self.skipTest("libsanitizers not found") + +self.runCmd( +"env SanitizersAddress=1 MallocSanitizerZone=1 MallocSecureAllocator=0" +) + +self.runCmd("run") + +# In libsanitizers, memory history is not supported until a report has been generated +self.expect( +"thread list", +"Process should be stopped due to ASan report", +substrs=["stopped", "stop reason = Use of deallocated memory"], +) + +# test the 'memory history' command +self.expect( +"memory history 'pointer'", +substrs=[ +"Memory deallocated by Thread", +"a.out`f2", +"main.c", +"Memory allocated by Thread", +"a.out`f1", +"main.c", +], +) + +# do the same using SB API +process = self.dbg.GetSelectedTarget().process +val = ( + process.GetSelectedThread().GetSelectedFrame().EvaluateExpression("pointer") +) +addr = val.GetValueAsUnsigned() +threads = process.GetHistoryThreads(addr) +self.assertEqual(threads.GetSize(), 2) + +history_thread = threads.GetThreadAtIndex(0) +self.assertTrue(history_thread.num_frames >= 2) +self.assertEqual( + history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), +"main.c", +) + +history_thread = threads.GetThreadAtIndex(1) +self.assertTrue(history_thread.num_frames >= 2) +self.assertEqual( + history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), +"main.c", +) + +# let's free the container (SBThreadCollection) and see if the +# SBThreads still live +threads = None +self.assertTrue(history_thread.num_frames >= 2) +self.assertEqual( + history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), +"main.c", +) + def asan_tests(self): target = self.createTestTarget() diff --git a/lldb/test/API/functionalities/asan/TestReportData.py b/lldb/test/API/functionalities/asan/TestReportData.py index 543c5fe66a208d..de0c1206a57ad6 100644 --- a/lldb/test/API/functionalitie
[Lldb-commits] [lldb] Add asan tests for libsanitizers. (PR #88349)
https://github.com/usama54321 closed https://github.com/llvm/llvm-project/pull/88349 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lit] Guard MallocNanoZone envvar in shell tests (PR #88824)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/88824 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lit] Guard MallocNanoZone envvar in shell tests (PR #88824)
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/88824 >From 7b95abd2247ca3e34985ce2c7e93c3dbe3a609c5 Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Mon, 15 Apr 2024 15:29:06 -0700 Subject: [PATCH] [lldb][lit] Guard MallocNanoZone envvar in shell tests Previously the MallocNanoZone envvar would be set to 0 on Darwin for the LLDB shell tests, but this should guarded behind ASan being enabled as opposed to simply running the test suite behind Darwin. This required that the LLVM_USE_SANITIZER option be added as an attribute to the lit config for shell tests. --- lldb/test/Shell/lit.cfg.py | 12 lldb/test/Shell/lit.site.cfg.py.in | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py index 290569576ac80d..e24f3fbb4d9318 100644 --- a/lldb/test/Shell/lit.cfg.py +++ b/lldb/test/Shell/lit.cfg.py @@ -50,10 +50,14 @@ ) # Enable sanitizer runtime flags. -config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" -config.environment["TSAN_OPTIONS"] = "halt_on_error=1" -if platform.system() == "Darwin": -config.environment["MallocNanoZone"] = "0" +if "Address" in config.llvm_use_sanitizer: +config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" +if platform.system() == "Darwin": +config.environment["MallocNanoZone"] = "0" + +if "Thread" in config.llvm_use_sanitizer: +config.environment["TSAN_OPTIONS"] = "halt_on_error=1" + # Support running the test suite under the lldb-repro wrapper. This makes it # possible to capture a test suite run and then rerun all the test from the diff --git a/lldb/test/Shell/lit.site.cfg.py.in b/lldb/test/Shell/lit.site.cfg.py.in index 736dfc335732b5..b69e7bce1bc0be 100644 --- a/lldb/test/Shell/lit.site.cfg.py.in +++ b/lldb/test/Shell/lit.site.cfg.py.in @@ -26,6 +26,7 @@ config.lldb_enable_lua = @LLDB_ENABLE_LUA@ config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@" config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@ config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@ +config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@" # The shell tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] fe48bf6 - [lldb][lit] Guard MallocNanoZone envvar in shell tests (#88824)
Author: Chelsea Cassanova Date: 2024-04-15T21:26:18-07:00 New Revision: fe48bf672e1ab293368a3212203db94a4e21c533 URL: https://github.com/llvm/llvm-project/commit/fe48bf672e1ab293368a3212203db94a4e21c533 DIFF: https://github.com/llvm/llvm-project/commit/fe48bf672e1ab293368a3212203db94a4e21c533.diff LOG: [lldb][lit] Guard MallocNanoZone envvar in shell tests (#88824) Previously the MallocNanoZone envvar would be set to 0 on Darwin for the LLDB shell tests, but this should guarded behind ASan being enabled as opposed to simply running the test suite behind Darwin. This required that the LLVM_USE_SANITIZER option be added as an attribute to the lit config for shell tests. Added: Modified: lldb/test/Shell/lit.cfg.py lldb/test/Shell/lit.site.cfg.py.in Removed: diff --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py index 290569576ac80d..e24f3fbb4d9318 100644 --- a/lldb/test/Shell/lit.cfg.py +++ b/lldb/test/Shell/lit.cfg.py @@ -50,10 +50,14 @@ ) # Enable sanitizer runtime flags. -config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" -config.environment["TSAN_OPTIONS"] = "halt_on_error=1" -if platform.system() == "Darwin": -config.environment["MallocNanoZone"] = "0" +if "Address" in config.llvm_use_sanitizer: +config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1" +if platform.system() == "Darwin": +config.environment["MallocNanoZone"] = "0" + +if "Thread" in config.llvm_use_sanitizer: +config.environment["TSAN_OPTIONS"] = "halt_on_error=1" + # Support running the test suite under the lldb-repro wrapper. This makes it # possible to capture a test suite run and then rerun all the test from the diff --git a/lldb/test/Shell/lit.site.cfg.py.in b/lldb/test/Shell/lit.site.cfg.py.in index 736dfc335732b5..b69e7bce1bc0be 100644 --- a/lldb/test/Shell/lit.site.cfg.py.in +++ b/lldb/test/Shell/lit.site.cfg.py.in @@ -26,6 +26,7 @@ config.lldb_enable_lua = @LLDB_ENABLE_LUA@ config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@" config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@ config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@ +config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@" # The shell tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lit] Guard MallocNanoZone envvar in shell tests (PR #88824)
https://github.com/chelcassanova closed https://github.com/llvm/llvm-project/pull/88824 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
https://github.com/Awfa updated https://github.com/llvm/llvm-project/pull/88812 >From 095696411172034f80233f1722e293c1f458d67f Mon Sep 17 00:00:00 2001 From: Anthony Ha Date: Mon, 15 Apr 2024 19:34:19 + Subject: [PATCH 1/2] [lldb] Skip remote PutFile when MD5 hashes equal --- .../gdb-server/PlatformRemoteGDBServer.cpp| 9 + .../gdb-server/PlatformRemoteGDBServer.h | 3 ++ .../GDBRemoteCommunicationClient.cpp | 36 +-- lldb/source/Target/Platform.cpp | 30 +++- .../GDBRemoteCommunicationClientTest.cpp | 23 5 files changed, 98 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp index 88f1ad15b6b485..d9f3e40019cf29 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { +// Note that high/low are switched in the gdb remote communication client +return m_gdb_client_up->CalculateMD5(file_spec, high, low); + } + return false; +} + void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() { m_trap_handlers.push_back(ConstString("_sigtramp")); } diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h index 638f7db5ef800c..d83fc386f59427 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -146,6 +146,9 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver { void CalculateTrapHandlerSymbolNames() override; + bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, +uint64_t &high) override; + const lldb::UnixSignalsSP &GetRemoteUnixSignals() override; size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 1f6116967a4f64..8c79d5fecf12fe 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet - which is a bug. Instead, we get +// the byte string for each low and high hex separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// Get low part +auto part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +bool conversionErrored = part.getAsInteger(16, low); +if (conversionErrored) + return false; + +// Get high part +part = response.GetStringRef().substr(response.GetFilePos(), + sizeof(uint64_t) * 2); +if (part.size() != sizeof(uint64_t) * 2) + return false; +response.SetFilePos(response.GetFilePos() + part.size()); + +conversionErrored = part.getAsInteger(16, high); +if (conversionErrored) + return false; + return true; } return false; diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index 4ce290dfbe035f..cdbafb17a5df4d 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec &arch, Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, uint32_t uid, uint32_t gid) { Log *log = GetLog(LLDBLog::Platform); - LLDB_LOGF(log, "[PutFile]
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (IsConnected()) { Awfa wrote: I'll change just this instance to meet your suggestion https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
https://github.com/Awfa created https://github.com/llvm/llvm-project/pull/88845 `lldb-server`'s platform mode seems to have an issue with its `--min-gdbserver-port` `--max-gdbserver-port` flags (and probably the `--gdbserver-port` flag, but I didn't test it). How the platform code seems to work is that it listens on a port, and whenever there's an incoming connection, it forks the process to handle the connection. To handle the port flags, the main process uses an instance of the helper class `GDBRemoteCommunicationServerPlatform::PortMap`, that can be configured and track usages of ports. The child process handling the platform connection, can then use the port map to allocate a port for the gdb-server connection it will make (this is another process it spawns). However, in the current code, this works only once. After the first connection is handled by forking a child process, the main platform listener code loops around, and then 'forgets' about the port map. This is because this code: ```cpp GDBRemoteCommunicationServerPlatform platform( acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); if (!gdbserver_portmap.empty()) { platform.SetPortMap(std::move(gdbserver_portmap)); } ``` is within the connection listening loop. This results in the `gdbserver_portmap` being moved into the platform object at the beginning of the first iteration of the loop, but on the second iteration, after the first fork, the next instance of the platform object will not have its platform port mapped. The result of this bug is that subsequent connections to the platform, when spawning the gdb-remote connection, will be supplied a random port - which isn't bounded by the `--min-gdbserver-port` and `--max-gdbserver--port` parameters passed in by the user. This PR fixes this issue by having the port map be maintained by the parent platform listener process. On connection, the listener allocates a single available port from the port map, associates the child process pid with the port, and lets the connection handling child use that single port number. Additionally, when cleaning up child processes, the main listener process tracks the child that exited to deallocate the previously associated port, so it can be reused for a new connection. For review: - I haven't used C++ optionals before this, and was going off of surrounding related code. Let me know if I got something wrong. - I'm not sure where this code could be tested. This was only manually tested by me so far. >From 3d75f42b5f61ea126001919491aa09ebd15ba9f8 Mon Sep 17 00:00:00 2001 From: Anthony Ha Date: Mon, 15 Apr 2024 19:36:34 + Subject: [PATCH] [lldb] Have lldb-server assign ports to children in platform mode --- lldb/tools/lldb-server/lldb-platform.cpp | 41 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 3e126584eb25b4..384709ba79b656 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -282,17 +282,10 @@ int main_platform(int argc, char *argv[]) { } } - do { -GDBRemoteCommunicationServerPlatform platform( -acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); - -if (port_offset > 0) - platform.SetPortOffset(port_offset); - -if (!gdbserver_portmap.empty()) { - platform.SetPortMap(std::move(gdbserver_portmap)); -} + GDBRemoteCommunicationServerPlatform platform( + acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); + do { const bool children_inherit_accept_socket = true; Connection *conn = nullptr; error = acceptor_up->Accept(children_inherit_accept_socket, conn); @@ -301,13 +294,31 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } printf("Connection established.\n"); + +std::optional port = 0; if (g_server) { // Collect child zombie processes. #if !defined(_WIN32) - while (waitpid(-1, nullptr, WNOHANG) > 0) -; + auto waitResult = waitpid(-1, nullptr, WNOHANG); + while (waitResult > 0) { +// waitResult is the child pid +gdbserver_portmap.FreePortForProcess(waitResult); +waitResult = waitpid(-1, nullptr, WNOHANG); + } #endif - if (fork()) { + llvm::Expected available_port = + gdbserver_portmap.GetNextAvailablePort(); + if (available_port) +port = *available_port; + + else { +fprintf(stderr, "no available port for connection - dropping...\n"); +delete conn; +continue; + } + auto childPid = fork(); + if (childPid) { +gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid); // Parent doesn't need a connection to the lldb client delete conn; @@ -324,6 +335,10 @@ int main_platform(int argc, char *argv[]) { // connect
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Anthony Ha (Awfa) Changes `lldb-server`'s platform mode seems to have an issue with its `--min-gdbserver-port` `--max-gdbserver-port` flags (and probably the `--gdbserver-port` flag, but I didn't test it). How the platform code seems to work is that it listens on a port, and whenever there's an incoming connection, it forks the process to handle the connection. To handle the port flags, the main process uses an instance of the helper class `GDBRemoteCommunicationServerPlatform::PortMap`, that can be configured and track usages of ports. The child process handling the platform connection, can then use the port map to allocate a port for the gdb-server connection it will make (this is another process it spawns). However, in the current code, this works only once. After the first connection is handled by forking a child process, the main platform listener code loops around, and then 'forgets' about the port map. This is because this code: ```cpp GDBRemoteCommunicationServerPlatform platform( acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); if (!gdbserver_portmap.empty()) { platform.SetPortMap(std::move(gdbserver_portmap)); } ``` is within the connection listening loop. This results in the `gdbserver_portmap` being moved into the platform object at the beginning of the first iteration of the loop, but on the second iteration, after the first fork, the next instance of the platform object will not have its platform port mapped. The result of this bug is that subsequent connections to the platform, when spawning the gdb-remote connection, will be supplied a random port - which isn't bounded by the `--min-gdbserver-port` and `--max-gdbserver--port` parameters passed in by the user. This PR fixes this issue by having the port map be maintained by the parent platform listener process. On connection, the listener allocates a single available port from the port map, associates the child process pid with the port, and lets the connection handling child use that single port number. Additionally, when cleaning up child processes, the main listener process tracks the child that exited to deallocate the previously associated port, so it can be reused for a new connection. For review: - I haven't used C++ optionals before this, and was going off of surrounding related code. Let me know if I got something wrong. - I'm not sure where this code could be tested. This was only manually tested by me so far. --- Full diff: https://github.com/llvm/llvm-project/pull/88845.diff 1 Files Affected: - (modified) lldb/tools/lldb-server/lldb-platform.cpp (+28-13) ``diff diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 3e126584eb25b4..384709ba79b656 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -282,17 +282,10 @@ int main_platform(int argc, char *argv[]) { } } - do { -GDBRemoteCommunicationServerPlatform platform( -acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); - -if (port_offset > 0) - platform.SetPortOffset(port_offset); - -if (!gdbserver_portmap.empty()) { - platform.SetPortMap(std::move(gdbserver_portmap)); -} + GDBRemoteCommunicationServerPlatform platform( + acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); + do { const bool children_inherit_accept_socket = true; Connection *conn = nullptr; error = acceptor_up->Accept(children_inherit_accept_socket, conn); @@ -301,13 +294,31 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } printf("Connection established.\n"); + +std::optional port = 0; if (g_server) { // Collect child zombie processes. #if !defined(_WIN32) - while (waitpid(-1, nullptr, WNOHANG) > 0) -; + auto waitResult = waitpid(-1, nullptr, WNOHANG); + while (waitResult > 0) { +// waitResult is the child pid +gdbserver_portmap.FreePortForProcess(waitResult); +waitResult = waitpid(-1, nullptr, WNOHANG); + } #endif - if (fork()) { + llvm::Expected available_port = + gdbserver_portmap.GetNextAvailablePort(); + if (available_port) +port = *available_port; + + else { +fprintf(stderr, "no available port for connection - dropping...\n"); +delete conn; +continue; + } + auto childPid = fork(); + if (childPid) { +gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid); // Parent doesn't need a connection to the lldb client delete conn; @@ -324,6 +335,10 @@ int main_platform(int argc, char *argv[]) { // connections while a connection is active. acceptor_up.reset(); } + +GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; +portmap_for_child.Allo
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
https://github.com/kovdan01 closed https://github.com/llvm/llvm-project/pull/82603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7e49b0d - [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (#82603)
Author: Daniil Kovalev Date: 2024-04-16T09:53:33+03:00 New Revision: 7e49b0d5a67f212e84f8ec0ec2e39a6a8673bfaf URL: https://github.com/llvm/llvm-project/commit/7e49b0d5a67f212e84f8ec0ec2e39a6a8673bfaf DIFF: https://github.com/llvm/llvm-project/commit/7e49b0d5a67f212e84f8ec0ec2e39a6a8673bfaf.diff LOG: [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (#82603) If `LLVM_TARGETS_TO_BUILD` does not contain `X86` and we try to run an x86 binary in lldb, we get a `nullptr` dereference in `LLVMDisasmInstruction(...)`. We try to call `getDisAsm()` method on a `LLVMDisasmContext *DC` which is null. The pointer is passed from `x86AssemblyInspectionEngine::instruction_length(...)` and is originally `m_disasm_context` member of `x86AssemblyInspectionEngine`. This should be filled by `LLVMCreateDisasm(...)` in the class constructor, but not having X86 target enabled in llvm makes `TargetRegistry::lookupTarget(...)` call return `nullptr`, which results in `m_disasm_context` initialized with `nullptr` as well. This patch adds if statements against `m_disasm_context` in `x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(...)` and `x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction(...)` so subsequent calls to `x86AssemblyInspectionEngine::instruction_length(...)` do not cause a null pointer dereference. Added: lldb/unittests/UnwindAssembly/x86-but-no-x86-target/CMakeLists.txt lldb/unittests/UnwindAssembly/x86-but-no-x86-target/Testx86AssemblyInspectionEngine.cpp Modified: lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp lldb/unittests/UnwindAssembly/CMakeLists.txt Removed: diff --git a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp index 2032c5a68d054c..6bfaa54135a959 100644 --- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp +++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp @@ -909,6 +909,9 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly( if (!m_register_map_initialized) return false; + if (m_disasm_context == nullptr) +return false; + addr_t current_func_text_offset = 0; int current_sp_bytes_offset_from_fa = 0; bool is_aligned = false; @@ -1570,6 +1573,9 @@ bool x86AssemblyInspectionEngine::FindFirstNonPrologueInstruction( if (!m_register_map_initialized) return false; + if (m_disasm_context == nullptr) +return false; + while (offset < size) { int regno; int insn_len; diff --git a/lldb/unittests/UnwindAssembly/CMakeLists.txt b/lldb/unittests/UnwindAssembly/CMakeLists.txt index 136fcd9ae97981..d6e4471af4ecb3 100644 --- a/lldb/unittests/UnwindAssembly/CMakeLists.txt +++ b/lldb/unittests/UnwindAssembly/CMakeLists.txt @@ -9,3 +9,7 @@ endif() if ("X86" IN_LIST LLVM_TARGETS_TO_BUILD) add_subdirectory(x86) endif() + +if (NOT "X86" IN_LIST LLVM_TARGETS_TO_BUILD) + add_subdirectory(x86-but-no-x86-target) +endif() diff --git a/lldb/unittests/UnwindAssembly/x86-but-no-x86-target/CMakeLists.txt b/lldb/unittests/UnwindAssembly/x86-but-no-x86-target/CMakeLists.txt new file mode 100644 index 00..d28e9629a64cfc --- /dev/null +++ b/lldb/unittests/UnwindAssembly/x86-but-no-x86-target/CMakeLists.txt @@ -0,0 +1,10 @@ +add_lldb_unittest(UnwindAssemblyX86ButNoX86TargetTests +Testx86AssemblyInspectionEngine.cpp +LINK_LIBS + lldbCore + lldbSymbol + lldbPluginUnwindAssemblyX86 +LINK_COMPONENTS + Support + ${LLVM_TARGETS_TO_BUILD} + ) diff --git a/lldb/unittests/UnwindAssembly/x86-but-no-x86-target/Testx86AssemblyInspectionEngine.cpp b/lldb/unittests/UnwindAssembly/x86-but-no-x86-target/Testx86AssemblyInspectionEngine.cpp new file mode 100644 index 00..ed093d146440e3 --- /dev/null +++ b/lldb/unittests/UnwindAssembly/x86-but-no-x86-target/Testx86AssemblyInspectionEngine.cpp @@ -0,0 +1,103 @@ +//===-- Testx86AssemblyInspectionEngine.cpp ---===// + +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "gtest/gtest.h" + +#include "Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h" +#include "lldb/Core/AddressRange.h" +#include "lldb/Symbol/UnwindPlan.h" +#include "lldb/Utility/ArchSpec.h" + +#include "llvm/Support/TargetSelect.h" + +#include +#include + +using namespace lldb; +using namespace lldb_private; + +class Testx86AssemblyInspectionEngine : public testing::Test { +public: + static void SetUpTestCase(); +}; + +void Testx86AssemblyInspectionEngine::SetUpTestCase(