[Lldb-commits] [lldb] [LLDB][libc++] Adds valarray proxy data formatters. (PR #88613)

2024-04-15 Thread Michael Buch via lldb-commits


@@ -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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Miro Bucko via lldb-commits

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)

2024-04-15 Thread Louis Dionne via lldb-commits

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)

2024-04-15 Thread Jonas Devlieghere via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Mark de Wever via lldb-commits

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)

2024-04-15 Thread Daniil Kovalev via lldb-commits

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)

2024-04-15 Thread via lldb-commits


@@ -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)

2024-04-15 Thread Mark de Wever via lldb-commits

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)

2024-04-15 Thread Mark de Wever via lldb-commits


@@ -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)

2024-04-15 Thread Mark de Wever via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Mark de Wever via lldb-commits

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)

2024-04-15 Thread via lldb-commits


@@ -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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Jason Molenda via lldb-commits

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)

2024-04-15 Thread Alex Langford via lldb-commits

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)

2024-04-15 Thread Alex Langford via lldb-commits

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)

2024-04-15 Thread Greg Clayton via lldb-commits

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)

2024-04-15 Thread Greg Clayton via lldb-commits

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)

2024-04-15 Thread Greg Clayton via lldb-commits


@@ -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)

2024-04-15 Thread Greg Clayton via lldb-commits


@@ -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)

2024-04-15 Thread Miro Bucko via lldb-commits

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)

2024-04-15 Thread Miro Bucko via lldb-commits

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

2024-04-15 Thread Jonas Devlieghere via lldb-commits

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)

2024-04-15 Thread Zequan Wu via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Michael Buch via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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

2024-04-15 Thread Jonas Devlieghere via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Jonas Devlieghere via lldb-commits

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)

2024-04-15 Thread Jason Molenda via lldb-commits

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)

2024-04-15 Thread Adrian Prantl via lldb-commits

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

2024-04-15 Thread Adrian Prantl via lldb-commits

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)

2024-04-15 Thread Adrian Prantl via lldb-commits

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)

2024-04-15 Thread Anthony Ha via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Alex Langford via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Chelsea Cassanova via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Usama Hameed via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Usama Hameed via lldb-commits

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)

2024-04-15 Thread Jonas Devlieghere via lldb-commits

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)

2024-04-15 Thread Chelsea Cassanova via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Chelsea Cassanova via lldb-commits

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)

2024-04-15 Thread Anthony Ha via lldb-commits

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)

2024-04-15 Thread Anthony Ha via lldb-commits


@@ -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)

2024-04-15 Thread Anthony Ha via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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)

2024-04-15 Thread Daniil Kovalev via lldb-commits

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)

2024-04-15 Thread via lldb-commits

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(