[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Erich Keane via Phabricator via lldb-commits]

erichkeane added a comment.

In D151683#4382408 , @philnik wrote:

> In D151683#4380877 , @erichkeane 
> wrote:
>
>> What is the justification for this?
>
> What exactly are you asking for? Why I'd like to back port it? This would 
> make quite a bit of code in libc++ simpler and avoids pit-falls where an 
> attribute works in some place in some version but not in another.
>
>> Do other compilers do this?
>
> ICC and NVC++ support this: https://godbolt.org/z/TeMnGdGsY
>
>> Was there an RFC?
>
> No. I guess, since you are asking I should write one for this? Only for the 
> removal of `-fdouble-square-bracket-attributes`, or also for back porting 
> this?

The RFC I would expect for "allow C/C++ style attributes as an extension in 
previous modes".  This is a pretty significant feature to allow as an 
extension, particularly since our two main alternative compilers (G++/MSVC) 
don't have this as an extension. I'd be curious how the other C compilers do 
this as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151683/new/

https://reviews.llvm.org/D151683

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cb463c3 - [lldb] Take StringRef name in GetChildMemberWithName (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-05-31T08:08:40-07:00
New Revision: cb463c34dd4c3ad2ac6c13f98edcf684a3fcbe38

URL: 
https://github.com/llvm/llvm-project/commit/cb463c34dd4c3ad2ac6c13f98edcf684a3fcbe38
DIFF: 
https://github.com/llvm/llvm-project/commit/cb463c34dd4c3ad2ac6c13f98edcf684a3fcbe38.diff

LOG: [lldb] Take StringRef name in GetChildMemberWithName (NFC)

`GetChildMemberWithName` does not need a `ConstString`. This change makes the 
function
take a `StringRef` instead, which alleviates the need for callers to construct a
`ConstString`. I don't expect this change to improve performance, only 
ergonomics.

This is in support of Alex's effort to replace `ConstString` where appropriate.

There are related `ValueObject` functions that can also be changed, if this is 
accepted.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/source/API/SBValue.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index a666d0bab1730..00fdb87c79279 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -487,7 +487,7 @@ class ValueObject {
   GetChildAtNamePath(llvm::ArrayRef> names,
  ConstString *name_of_error = nullptr);
 
-  virtual lldb::ValueObjectSP GetChildMemberWithName(ConstString name,
+  virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create);
 
   virtual size_t GetIndexOfChildWithName(ConstString name);

diff  --git a/lldb/include/lldb/Core/ValueObjectRegister.h 
b/lldb/include/lldb/Core/ValueObjectRegister.h
index 60c299c5fb407..96e8b3067efb4 100644
--- a/lldb/include/lldb/Core/ValueObjectRegister.h
+++ b/lldb/include/lldb/Core/ValueObjectRegister.h
@@ -52,7 +52,7 @@ class ValueObjectRegisterSet : public ValueObject {
   ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
   int32_t synthetic_index) override;
 
-  lldb::ValueObjectSP GetChildMemberWithName(ConstString name,
+  lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create) override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;

diff  --git a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h 
b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
index bdd6c1be4212e..da54ef156daf5 100644
--- a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -53,7 +53,7 @@ class ValueObjectSynthetic : public ValueObject {
 
   lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create) override;
 
-  lldb::ValueObjectSP GetChildMemberWithName(ConstString name,
+  lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create) override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;

diff  --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index f52041b4144d2..573ee3a82fa03 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -711,12 +711,11 @@ SBValue::GetChildMemberWithName(const char *name,
   LLDB_INSTRUMENT_VA(this, name, use_dynamic_value);
 
   lldb::ValueObjectSP child_sp;
-  const ConstString str_name(name);
 
   ValueLocker locker

[Lldb-commits] [PATCH] D151615: [lldb] Take StringRef name in GetChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb463c34dd4c: [lldb] Take StringRef name in 
GetChildMemberWithName (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151615/new/

https://reviews.llvm.org/D151615

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -514,7 +514,7 @@
 ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
 lldb::ValueObjectSP exception_sp) {
   ValueObjectSP reserved_dict =
-  exception_sp->GetChildMemberWithName(ConstString("reserved"), true);
+  exception_sp->GetChildMemberWithName("reserved", true);
   if (!reserved_dict)
 return FailExceptionParsing("Failed to get 'reserved' member.");
 
@@ -567,18 +567,15 @@
 
   if (!return_addresses)
 return FailExceptionParsing("Failed to get return addresses.");
-  auto frames_value =
-  return_addresses->GetChildMemberWithName(ConstString("_frames"), true);
+  auto frames_value = return_addresses->GetChildMemberWithName("_frames", true);
   if (!frames_value)
 return FailExceptionParsing("Failed to get frames_value.");
   addr_t frames_addr = frames_value->GetValueAsUnsigned(0);
-  auto count_value =
-  return_addresses->GetChildMemberWithName(ConstString("_cnt"), true);
+  auto count_value = return_addresses->GetChildMemberWithName("_cnt", true);
   if (!count_value)
 return FailExceptionParsing("Failed to get count_value.");
   size_t count = count_value->GetValueAsUnsigned(0);
-  auto ignore_value =
-  return_addresses->GetChildMemberWithName(ConstString("_ignore"), true);
+  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore", true);
   if (!ignore_value)
 return FailExceptionParsing("Failed to get ignore_value.");
   size_t ignore = ignore_value->GetValueAsUnsigned(0);
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -138,12 +138,11 @@
   //we will obtain the name from this pointer.
   // 5) a free function. A pointer to the function will stored after the vtable
   //we will obtain the name from this pointer.
-  ValueObjectSP member_f_(
-  valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
+  ValueObjectSP member_f_(valobj_sp->GetChildMemberWithName("__f_", true));
 
   if (member_f_) {
 ValueObjectSP sub_member_f_(
-   member_f_->GetChildMemberWithName(ConstString("__f_"), true));
+member_f_->GetChildMemberWithName("__f_", true));
 
 if (sub_member_f_)
 member_f_ = sub_member_f_;
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -69,13 +69,12 @@
   if (!valobj_sp)
 return nullptr;
 
-  ValueObjectSP obj_child_sp =
-  valobj_sp->GetChildMemberWithName(ConstString("_M_t"), true);
+  

[Lldb-commits] [PATCH] D151384: [lldb] Override GetVariable in ValueObjectSynthetic (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Jim approved offline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151384/new/

https://reviews.llvm.org/D151384

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7578672 - [lldb] Override GetVariable in ValueObjectSynthetic (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-05-31T08:19:36-07:00
New Revision: 7578672c96e18feb5982192e595459b2a65867cf

URL: 
https://github.com/llvm/llvm-project/commit/7578672c96e18feb5982192e595459b2a65867cf
DIFF: 
https://github.com/llvm/llvm-project/commit/7578672c96e18feb5982192e595459b2a65867cf.diff

LOG: [lldb] Override GetVariable in ValueObjectSynthetic (NFC)

Make `GetVariable` a passthrough function the the underlying value object in 
`ValueObjectSynthetic`.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h 
b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
index da54ef156daf..a65e7eb1b808 100644
--- a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -82,6 +82,10 @@ class ValueObjectSynthetic : public ValueObject {
   : lldb::eNoDynamicValues);
   }
 
+  lldb::VariableSP GetVariable() override {
+return m_parent != nullptr ? m_parent->GetVariable() : nullptr;
+  }
+
   ValueObject *GetParent() override {
 return ((m_parent != nullptr) ? m_parent->GetParent() : nullptr);
   }



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151384: [lldb] Override GetVariable in ValueObjectSynthetic (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7578672c96e1: [lldb] Override GetVariable in 
ValueObjectSynthetic (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151384/new/

https://reviews.llvm.org/D151384

Files:
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h


Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
===
--- lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -82,6 +82,10 @@
   : lldb::eNoDynamicValues);
   }
 
+  lldb::VariableSP GetVariable() override {
+return m_parent != nullptr ? m_parent->GetVariable() : nullptr;
+  }
+
   ValueObject *GetParent() override {
 return ((m_parent != nullptr) ? m_parent->GetParent() : nullptr);
   }


Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
===
--- lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -82,6 +82,10 @@
   : lldb::eNoDynamicValues);
   }
 
+  lldb::VariableSP GetVariable() override {
+return m_parent != nullptr ? m_parent->GetVariable() : nullptr;
+  }
+
   ValueObject *GetParent() override {
 return ((m_parent != nullptr) ? m_parent->GetParent() : nullptr);
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151810: [lldb] Take StringRef name in GetIndexOfChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: bulbazord, jingham.
Herald added a subscriber: arphaman.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Change the type of the `name` parameter from `char *` to `StringRef`.

Follow up to D151615 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151810

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -741,9 +741,9 @@
 // index 1 is the child index for "m_b" within class A
 
 size_t CompilerType::GetIndexOfChildMemberWithName(
-const char *name, bool omit_empty_base_classes,
+llvm::StringRef name, bool omit_empty_base_classes,
 std::vector &child_indexes) const {
-  if (IsValid() && name && name[0]) {
+  if (IsValid() && !name.empty()) {
 if (auto type_system_sp = GetTypeSystem())
   return type_system_sp->GetIndexOfChildMemberWithName(
 m_type, name, omit_empty_base_classes, child_indexes);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -888,7 +888,8 @@
   // so we catch all names that match a given child name, not just the first.
   size_t
   GetIndexOfChildMemberWithName(lldb::opaque_compiler_type_t type,
-const char *name, bool omit_empty_base_classes,
+llvm::StringRef name,
+bool omit_empty_base_classes,
 std::vector &child_indexes) override;
 
   bool IsTemplateType(lldb::opaque_compiler_type_t type) override;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6728,9 +6728,9 @@
 // index 1 is the child index for "m_b" within class A
 
 size_t TypeSystemClang::GetIndexOfChildMemberWithName(
-lldb::opaque_compiler_type_t type, const char *name,
+lldb::opaque_compiler_type_t type, llvm::StringRef name,
 bool omit_empty_base_classes, std::vector &child_indexes) {
-  if (type && name && name[0]) {
+  if (type && !name.empty()) {
 clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
 const clang::Type::TypeClass type_class = qual_type->getTypeClass();
 switch (type_class) {
@@ -6748,7 +6748,6 @@
 
 // Try and find a field that matches NAME
 clang::RecordDecl::field_iterator field, field_end;
-llvm::StringRef name_sref(name);
 for (field = record_decl->field_begin(),
 field_end = record_decl->field_end();
  field != field_end; ++field, ++child_idx) {
@@ -6761,7 +6760,7 @@
   return child_indexes.size();
 child_indexes.pop_back();
 
-  } else if (field_name.equals(name_sref)) {
+  } else if (field_name.equals(name)) {
 // We have to add on the number of base classes to this index!
 child_indexes.push_back(
 child_idx + TypeSystemClang::GetNumBaseClasses(
@@ -6774,8 +6773,7 @@
   const clang::RecordDecl *parent_record_decl = cxx_record_decl;
 
   // Didn't find things easily, lets let clang do its thang...
-  clang::IdentifierInfo &ident_ref =
-  getASTContext().Idents.get(name_sref);
+  clang::IdentifierInfo &ident_ref = getASTContext().Idents.get(name);
   clang::DeclarationName decl_name(&ident_ref);
 
   clang::CXXBasePaths paths;
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -483,7 +483,7 @@
 
   const size_t num_child_indexes =
   GetCompilerType().GetIndexOfChildMemberWithName(
-  name.str().data(), omit_empty_base_classes, child_indexes);
+  name, omit_empty_base_classes, child_indexes);
   if (num_child_indexes == 0)
 return nullptr;
 
Index: lldb/include/lldb/Symbol/TypeSystem.h
===
--- lldb/include/lldb/Symbol/TypeSystem.h
+++ lldb/include/lldb/Symbol/TypeSystem.h
@@ -357,10 +357,9 @@
   // TODO: Return all matches for a given name by returning a
   /

[Lldb-commits] [PATCH] D151811: [lldb] Take StringRef name in GetIndexOfChildWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: bulbazord, jingham.
Herald added a subscriber: arphaman.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As with D151615 , which changed 
`GetIndexOfChildMemberWithName` to take a `StringRef`
instead of a `ConstString`, this change does the same for 
`GetIndexOfChildWithName`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151811

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -810,12 +810,12 @@
 // matches can include base class names.
 
 uint32_t
-CompilerType::GetIndexOfChildWithName(const char *name,
+CompilerType::GetIndexOfChildWithName(llvm::StringRef name,
   bool omit_empty_base_classes) const {
-  if (IsValid() && name && name[0]) {
+  if (IsValid() && !name.empty()) {
 if (auto type_system_sp = GetTypeSystem())
   return type_system_sp->GetIndexOfChildWithName(m_type, name,
-  omit_empty_base_classes);
+ omit_empty_base_classes);
   }
   return UINT32_MAX;
 }
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -877,7 +877,7 @@
   // Lookup a child given a name. This function will match base class names and
   // member member names in "clang_type" only, not descendants.
   uint32_t GetIndexOfChildWithName(lldb::opaque_compiler_type_t type,
-   const char *name,
+   llvm::StringRef name,
bool omit_empty_base_classes) override;
 
   // Lookup a child member given a name. This function will match member names
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6966,9 +6966,9 @@
 
 uint32_t
 TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type,
- const char *name,
+ llvm::StringRef name,
  bool omit_empty_base_classes) {
-  if (type && name && name[0]) {
+  if (type && !name.empty()) {
 clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
 
 const clang::Type::TypeClass type_class = qual_type->getTypeClass();
@@ -7013,11 +7013,10 @@
 
 // Try and find a field that matches NAME
 clang::RecordDecl::field_iterator field, field_end;
-llvm::StringRef name_sref(name);
 for (field = record_decl->field_begin(),
 field_end = record_decl->field_end();
  field != field_end; ++field, ++child_idx) {
-  if (field->getName().equals(name_sref))
+  if (field->getName().equals(name))
 return child_idx;
 }
   }
@@ -7026,7 +7025,6 @@
 case clang::Type::ObjCObject:
 case clang::Type::ObjCInterface:
   if (GetCompleteType(type)) {
-llvm::StringRef name_sref(name);
 const clang::ObjCObjectType *objc_class_type =
 llvm::dyn_cast(qual_type.getTypePtr());
 assert(objc_class_type);
@@ -7045,7 +7043,7 @@
  ivar_pos != ivar_end; ++ivar_pos, ++child_idx) {
   const clang::ObjCIvarDecl *ivar_decl = *ivar_pos;
 
-  if (ivar_decl->getName().equals(name_sref)) {
+  if (ivar_decl->getName().equals(name)) {
 if ((!omit_empty_base_classes && superclass_interface_decl) ||
 (omit_empty_base_classes &&
  ObjCDeclHasIVars(superclass_interface_decl, true)))
@@ -7056,7 +7054,7 @@
 }
 
 if (superclass_interface_decl) {
-  if (superclass_interface_decl->getName().equals(name_sref))
+  if (superclass_in

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: bulbazord, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Following D151810 , this changes 
`GetChildAtNamePath` to take a path of `StringRef`
values instead of `ConstString`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151813

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp

Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -437,9 +437,8 @@
   if (!ptr_sp)
 return false;
 
-  ValueObjectSP usecount_sp(valobj_sp->GetChildAtNamePath(
-  {ConstString("_M_refcount"), ConstString("_M_pi"),
-   ConstString("_M_use_count")}));
+  ValueObjectSP usecount_sp(
+  valobj_sp->GetChildAtNamePath({"_M_refcount", "_M_pi", "_M_use_count"}));
   if (!usecount_sp)
 return false;
 
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -112,8 +112,7 @@
 ValueObjectSP hash_sp = node_sp->GetChildMemberWithName("__hash_", true);
 if (!hash_sp || !value_sp) {
   if (!m_element_type) {
-auto p1_sp = m_backend.GetChildAtNamePath({ConstString("__table_"),
-   ConstString("__p1_")});
+auto p1_sp = m_backend.GetChildAtNamePath({"__table_", "__p1_"});
 if (!p1_sp)
   return nullptr;
 
@@ -199,21 +198,19 @@
 
   ValueObjectSP p2_sp = table_sp->GetChildMemberWithName("__p2_", true);
   ValueObjectSP num_elements_sp = nullptr;
-  llvm::SmallVector next_path;
+  llvm::SmallVector next_path;
   switch (p2_sp->GetCompilerType().GetNumDirectBaseClasses()) {
   case 1:
 // Assume a pre llvm r300140 __compressed_pair implementation:
 num_elements_sp = p2_sp->GetChildMemberWithName("__first_", true);
-next_path.append({ConstString("__p1_"), ConstString("__first_"),
-  ConstString("__next_")});
+next_path.append({"__p1_", "__first_", "__next_"});
 break;
   case 2: {
 // Assume a post llvm r300140 __compressed_pair implementation:
 ValueObjectSP first_elem_parent = p2_sp->GetChildAtIndex(0, true);
 num_elements_sp =
 first_elem_parent->GetChildMemberWithName("__value_", true);
-next_path.append({ConstString("__p1_"), ConstString("__value_"),
-  ConstString("__next_")});
+next_path.append({"__p1_", "__value_", "__next_"});
 break;
   }
   default:
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -241,9 +241,6 @@
 }
 
 bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
-  static ConstString g_tree_("__tree_");
-  static ConstString g_pair3("__pair3_");
-
   if (m_element_type.IsValid())
 return true;
   m_element_type.Clear();
@@ -257,7 +254,7 @@
 m_element_type = deref->GetCompilerType();
 return true;
   }
-  deref = m_backend.GetChildAtNamePath({g_tree_, g_pair3});
+  deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
   if (!deref)
 return false;
   m_element_type = deref->GetCompilerType()
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -153,10 +153,10 @@
   if (!valobj_sp)
 return false;
   ValueObjectSP ptr_sp(valobj_sp->GetChildMemberWithName("__ptr_", true));
-  ValueObjectSP count_sp(valobj_sp->GetChildAtNamePath(
-  {ConstString("__cntrl_"), ConstString("__shared_owners_")}));
-  ValueObjectSP weakcount_sp(valobj_sp->GetChildAtNamePath(
-  {ConstString("__cntrl_"), ConstString("__shared_weak_owners_")}));
+  ValueObjectSP count_sp(
+  valobj_sp->GetChildAtNamePath({"__cntrl_", "__shared_owners_"}));
+  ValueObjectSP weakcount_sp(
+  valobj_sp->GetChildAtNamePath({"__cntrl_", "__shared_weak_owners_"}));
 
   if (!ptr_sp)
 return false;
@@ -810,8 +810,7 @@
 return {};
 
   ValueObjectSP is_long = short_sp->GetChildMemberWithName("__is_long_", true);
-  ValueObjectSP size_sp =
-  short_sp->Get

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Michael Buch via Phabricator via lldb-commits]

Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: lldb/include/lldb/Core/ValueObject.h:483
   // this will always create the children if necessary
-  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef names,
- ConstString *name_of_error = nullptr);
+  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef 
names);
 

Guessing removing the parameter is fine because no callers were actually 
passing it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151813/new/

https://reviews.llvm.org/D151813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151810: [lldb] Take StringRef name in GetIndexOfChildMemberWithName (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151810/new/

https://reviews.llvm.org/D151810

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151811: [lldb] Take StringRef name in GetIndexOfChildWithName (NFC)

[Michael Buch via Phabricator via lldb-commits]

Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: lldb/source/Core/ValueObjectSyntheticFilter.cpp:338
   if (!did_find && m_synth_filter_up != nullptr) {
 uint32_t index = m_synth_filter_up->GetIndexOfChildWithName(name);
 if (index == UINT32_MAX)

Could pass name_ref directly now, instead of doing the implicit conversion


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151811/new/

https://reviews.llvm.org/D151811

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:434
   ValueObjectSP root(GetSP());
-  for (ConstString name : names) {
+  for (auto name : names) {
 root = root->GetChildMemberWithName(name, true);

I don’t think this qualifies for ‘auto’ according to the LLVM coding guidelines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151813/new/

https://reviews.llvm.org/D151813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/include/lldb/Core/ValueObject.h:483
   // this will always create the children if necessary
-  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef names,
- ConstString *name_of_error = nullptr);
+  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef 
names);
 

Michael137 wrote:
> Guessing removing the parameter is fine because no callers were actually 
> passing it?
I forgot I had made this change too. Correct, no callers were using this, so I 
removed it. I can imagine there could be a hypothetical case that uses it to 
determine a fallback action, but none do and it seems unlikely.



Comment at: lldb/source/Core/ValueObject.cpp:434
   ValueObjectSP root(GetSP());
-  for (ConstString name : names) {
+  for (auto name : names) {
 root = root->GetChildMemberWithName(name, true);

JDevlieghere wrote:
> I don’t think this qualifies for ‘auto’ according to the LLVM coding 
> guidelines.
I find the guidelines to be not specific enough. Case in point, this section 
shows `auto` being used in range for loops: 
https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151813/new/

https://reviews.llvm.org/D151813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151810: [lldb] Take StringRef name in GetIndexOfChildMemberWithName (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

Very straightforward. LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151810/new/

https://reviews.llvm.org/D151810

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:434
   ValueObjectSP root(GetSP());
-  for (ConstString name : names) {
+  for (auto name : names) {
 root = root->GetChildMemberWithName(name, true);

kastiglione wrote:
> JDevlieghere wrote:
> > I don’t think this qualifies for ‘auto’ according to the LLVM coding 
> > guidelines.
> I find the guidelines to be not specific enough. Case in point, this section 
> shows `auto` being used in range for loops: 
> https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto
nit: mark `name` as `const` -> `const auto name`.
I suppose it's ok if it's a copy since it's a StringRef and it's less expensive 
to make the copy?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151813/new/

https://reviews.llvm.org/D151813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151597: [lldb][NFCI] Remove use of ConstString from IOHandler

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

TBH, it feels like a lot of - risky - changes just so 
`IOHandlerGetControlSequence` can return a `llvm::StringRef` ? Is that really 
necessary ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151597/new/

https://reviews.llvm.org/D151597

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151591: HostInfoMacOS: Add a utility function for finding an SDK-specific tool

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/unittests/Host/HostInfoTest.cpp:90
+  EXPECT_FALSE(find_tool("MacOSX.sdk", "clang").empty());
+  EXPECT_TRUE(find_tool("MacOSX.sdk", "CeciNestPasUnOutil").empty());
+}

JDevlieghere wrote:
> LOL
^^


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151591/new/

https://reviews.llvm.org/D151591

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:434
   ValueObjectSP root(GetSP());
-  for (ConstString name : names) {
+  for (auto name : names) {
 root = root->GetChildMemberWithName(name, true);

bulbazord wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > I don’t think this qualifies for ‘auto’ according to the LLVM coding 
> > > guidelines.
> > I find the guidelines to be not specific enough. Case in point, this 
> > section shows `auto` being used in range for loops: 
> > https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto
> nit: mark `name` as `const` -> `const auto name`.
> I suppose it's ok if it's a copy since it's a StringRef and it's less 
> expensive to make the copy?
yes I figure a StringRef is cheap to copy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151813/new/

https://reviews.llvm.org/D151813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 57c122d - [lldb] Take StringRef name in GetIndexOfChildMemberWithName (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-05-31T11:02:44-07:00
New Revision: 57c122d0ea1db38116ea9128a2c273204248bc67

URL: 
https://github.com/llvm/llvm-project/commit/57c122d0ea1db38116ea9128a2c273204248bc67
DIFF: 
https://github.com/llvm/llvm-project/commit/57c122d0ea1db38116ea9128a2c273204248bc67.diff

LOG: [lldb] Take StringRef name in GetIndexOfChildMemberWithName (NFC)

Change the type of the `name` parameter from `char *` to `StringRef`.

Follow up to D151615.

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

Added: 


Modified: 
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Core/ValueObject.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 50587f4aab827..ba75eb9abd4b3 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -397,7 +397,8 @@ class CompilerType {
   /// vector>
   /// so we catch all names that match a given child name, not just the first.
   size_t
-  GetIndexOfChildMemberWithName(const char *name, bool omit_empty_base_classes,
+  GetIndexOfChildMemberWithName(llvm::StringRef name,
+bool omit_empty_base_classes,
 std::vector &child_indexes) const;
 
   /// Return the number of template arguments the type has.

diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index dfef87232628b..21e5915fab0ce 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -357,10 +357,9 @@ class TypeSystem : public PluginInterface,
   // TODO: Return all matches for a given name by returning a
   // vector>
   // so we catch all names that match a given child name, not just the first.
-  virtual size_t
-  GetIndexOfChildMemberWithName(lldb::opaque_compiler_type_t type,
-const char *name, bool omit_empty_base_classes,
-std::vector &child_indexes) = 0;
+  virtual size_t GetIndexOfChildMemberWithName(
+  lldb::opaque_compiler_type_t type, llvm::StringRef name,
+  bool omit_empty_base_classes, std::vector &child_indexes) = 0;
 
   virtual bool IsTemplateType(lldb::opaque_compiler_type_t type);
 

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 48cdcb913f129..2041a54feafa2 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -483,7 +483,7 @@ ValueObjectSP 
ValueObject::GetChildMemberWithName(llvm::StringRef name,
 
   const size_t num_child_indexes =
   GetCompilerType().GetIndexOfChildMemberWithName(
-  name.str().data(), omit_empty_base_classes, child_indexes);
+  name, omit_empty_base_classes, child_indexes);
   if (num_child_indexes == 0)
 return nullptr;
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index fa6ab9b2f86b5..d0222bce6c67a 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6728,9 +6728,9 @@ uint32_t TypeSystemClang::GetIndexForRecordChild(
 // index 1 is the child index for "m_b" within class A
 
 size_t TypeSystemClang::GetIndexOfChildMemberWithName(
-lldb::opaque_compiler_type_t type, const char *name,
+lldb::opaque_compiler_type_t type, llvm::StringRef name,
 bool omit_empty_base_classes, std::vector &child_indexes) {
-  if (type && name && name[0]) {
+  if (type && !name.empty()) {
 clang::QualType qual_type = 
RemoveWrappingTypes(GetCanonicalQualType(type));
 const clang::Type::TypeClass type_class = qual_type->getTypeClass();
 switch (type_class) {
@@ -6748,7 +6748,6 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
 
 // Try and find a field that matches NAME
 clang::RecordDecl::field_iterator field, field_end;
-llvm::StringRef name_sref(name);
 for (field = record_decl->field_begin(),
 field_end = record_decl->field_end();
  field != field_end; ++field, ++child_idx) {
@@ -6761,7 +6760,7 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
   return child_indexes.size();
 child_indexes.pop_back();
 
-  } else if (field_name.equals(name_sref)) {
+  } else if (field_name.equals(name)) {
 // We have to add on the number of base classes to this index!
 child_indexes.push_back(
 child_idx + TypeSystemClang::GetNumBaseClasses(
@@ -6774,8 +6773,7 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
   const clang::Reco

[Lldb-commits] [PATCH] D151810: [lldb] Take StringRef name in GetIndexOfChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG57c122d0ea1d: [lldb] Take StringRef name in 
GetIndexOfChildMemberWithName (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151810/new/

https://reviews.llvm.org/D151810

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -741,9 +741,9 @@
 // index 1 is the child index for "m_b" within class A
 
 size_t CompilerType::GetIndexOfChildMemberWithName(
-const char *name, bool omit_empty_base_classes,
+llvm::StringRef name, bool omit_empty_base_classes,
 std::vector &child_indexes) const {
-  if (IsValid() && name && name[0]) {
+  if (IsValid() && !name.empty()) {
 if (auto type_system_sp = GetTypeSystem())
   return type_system_sp->GetIndexOfChildMemberWithName(
 m_type, name, omit_empty_base_classes, child_indexes);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -888,7 +888,8 @@
   // so we catch all names that match a given child name, not just the first.
   size_t
   GetIndexOfChildMemberWithName(lldb::opaque_compiler_type_t type,
-const char *name, bool omit_empty_base_classes,
+llvm::StringRef name,
+bool omit_empty_base_classes,
 std::vector &child_indexes) override;
 
   bool IsTemplateType(lldb::opaque_compiler_type_t type) override;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6728,9 +6728,9 @@
 // index 1 is the child index for "m_b" within class A
 
 size_t TypeSystemClang::GetIndexOfChildMemberWithName(
-lldb::opaque_compiler_type_t type, const char *name,
+lldb::opaque_compiler_type_t type, llvm::StringRef name,
 bool omit_empty_base_classes, std::vector &child_indexes) {
-  if (type && name && name[0]) {
+  if (type && !name.empty()) {
 clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
 const clang::Type::TypeClass type_class = qual_type->getTypeClass();
 switch (type_class) {
@@ -6748,7 +6748,6 @@
 
 // Try and find a field that matches NAME
 clang::RecordDecl::field_iterator field, field_end;
-llvm::StringRef name_sref(name);
 for (field = record_decl->field_begin(),
 field_end = record_decl->field_end();
  field != field_end; ++field, ++child_idx) {
@@ -6761,7 +6760,7 @@
   return child_indexes.size();
 child_indexes.pop_back();
 
-  } else if (field_name.equals(name_sref)) {
+  } else if (field_name.equals(name)) {
 // We have to add on the number of base classes to this index!
 child_indexes.push_back(
 child_idx + TypeSystemClang::GetNumBaseClasses(
@@ -6774,8 +6773,7 @@
   const clang::RecordDecl *parent_record_decl = cxx_record_decl;
 
   // Didn't find things easily, lets let clang do its thang...
-  clang::IdentifierInfo &ident_ref =
-  getASTContext().Idents.get(name_sref);
+  clang::IdentifierInfo &ident_ref = getASTContext().Idents.get(name);
   clang::DeclarationName decl_name(&ident_ref);
 
   clang::CXXBasePaths paths;
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -483,7 +483,7 @@
 
   const size_t num_child_indexes =
   GetCompilerType().GetIndexOfChildMemberWithName(
-  name.str().data(), omit_empty_base_classes, child_indexes);
+  name, omit_empty_base_classes, child_indexes);
   if (num_child_indexes == 0)
 return nullptr;
 
Index: lldb/include/lldb/Symbol/TypeSystem.h
===
--- lldb/include/lldb/Symbol/TypeSystem.h
+++ lldb/include/lldb/Symbol/TypeSystem.h
@@ -357,10 +357,9 @@
   // TODO: Return all matches for a given name by returning a
   // vector>
   // so we catch all names that match a given child name, not just the first.
-  virtual size_t
-  GetIndexOfChildMembe

[Lldb-commits] [PATCH] D151597: [lldb][NFCI] Remove use of ConstString from IOHandler

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D151597#4384734 , @mib wrote:

> TBH, it feels like a lot of - risky - changes just so 
> `IOHandlerGetControlSequence` can return a `llvm::StringRef` ? Is that really 
> necessary ?

I assume you're referring to `IOHandlerDelegateMultiline` storing its 
`m_end_line` member with a newline at the end now? My thought process was this: 
Pretty much every method changed here returns a constant string except for 
`IOHandlerDelegateMultiline::IOHandlerGetControlSequence`. That one creates a 
brand new string with `\n` at the end of it, so if `m_end_line` doesn't have a 
`\n` at the end of it, the we must always return a `std::string` which feels a 
little wasteful since the majority of these methods hand back a constant 
string. That also percolates up: `IOHandlerGetControlSequence` is the "lowest 
layer" of these call stacks, so whatever it returns is what the others must 
return as well.

The other option is to change `IOHandlerDelegateMultiline` to always take a 
string with a `\n`, but that is a lot harder to enforce. What do you think? I'm 
ok making whatever change makes this less risky in a separate commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151597/new/

https://reviews.llvm.org/D151597

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a5e9f2c - Factor out xcrun into a function (NFC)

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2023-05-31T14:46:35-07:00
New Revision: a5e9f2c81ebced1ea41060fc0d89b9233bc1b7be

URL: 
https://github.com/llvm/llvm-project/commit/a5e9f2c81ebced1ea41060fc0d89b9233bc1b7be
DIFF: 
https://github.com/llvm/llvm-project/commit/a5e9f2c81ebced1ea41060fc0d89b9233bc1b7be.diff

LOG: Factor out xcrun into a function (NFC)

https://reviews.llvm.org/D151588

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 0b4fc1885cae9..96461f9869e4d 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -374,84 +374,86 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
   return g_developer_directory;
 }
 
-llvm::Expected GetXcodeSDK(XcodeSDK sdk) {
-  XcodeSDK::Info info = sdk.Parse();
-  std::string sdk_name = XcodeSDK::GetCanonicalName(info);
-  if (sdk_name.empty())
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Unrecognized SDK type: " + 
sdk.GetString());
+static llvm::Expected
+xcrun(const std::string &sdk, llvm::ArrayRef arguments,
+  llvm::StringRef developer_dir = "") {
+  Args args;
+  if (!developer_dir.empty()) {
+args.AppendArgument("/usr/bin/env");
+args.AppendArgument("DEVELOPER_DIR=" + developer_dir.str());
+  }
+  args.AppendArgument("/usr/bin/xcrun");
+  args.AppendArgument("--sdk");
+  args.AppendArgument(sdk);
+  for (auto arg: arguments)
+args.AppendArgument(arg);
 
   Log *log = GetLog(LLDBLog::Host);
+  if (log) {
+std::string cmdstr;
+args.GetCommandString(cmdstr);
+log->Printf("GetXcodeSDK() running shell cmd '%s'", cmdstr.c_str());
+  }
 
-  auto xcrun = [](const std::string &sdk,
-  llvm::StringRef developer_dir =
-  "") -> llvm::Expected {
-Args args;
-if (!developer_dir.empty()) {
-  args.AppendArgument("/usr/bin/env");
-  args.AppendArgument("DEVELOPER_DIR=" + developer_dir.str());
-}
-args.AppendArgument("/usr/bin/xcrun");
-args.AppendArgument("--show-sdk-path");
-args.AppendArgument("--sdk");
-args.AppendArgument(sdk);
-
-Log *log = GetLog(LLDBLog::Host);
-if (log) {
-  std::string cmdstr;
-  args.GetCommandString(cmdstr);
-  log->Printf("GetXcodeSDK() running shell cmd '%s'", cmdstr.c_str());
-}
+  int status = 0;
+  int signo = 0;
+  std::string output_str;
+  // The first time after Xcode was updated or freshly installed,
+  // xcrun can take surprisingly long to build up its database.
+  auto timeout = std::chrono::seconds(60);
+  bool run_in_shell = false;
+  lldb_private::Status error = Host::RunShellCommand(
+  args, FileSpec(), &status, &signo, &output_str, timeout, run_in_shell);
+
+  // Check that xcrun returned something useful.
+  if (error.Fail()) {
+// Catastrophic error.
+LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+return error.ToError();
+  }
+  if (status != 0) {
+// xcrun didn't find a matching SDK. Not an error, we'll try
+// 
diff erent spellings.
+LLDB_LOG(log, "xcrun returned exit code %d", status);
+return "";
+  }
+  if (output_str.empty()) {
+LLDB_LOG(log, "xcrun returned no results");
+return "";
+  }
 
-int status = 0;
-int signo = 0;
-std::string output_str;
-// The first time after Xcode was updated or freshly installed,
-// xcrun can take surprisingly long to build up its database.
-auto timeout = std::chrono::seconds(60);
-bool run_in_shell = false;
-lldb_private::Status error = Host::RunShellCommand(
-args, FileSpec(), &status, &signo, &output_str, timeout, run_in_shell);
-
-// Check that xcrun returned something useful.
-if (error.Fail()) {
-  // Catastrophic error.
-  LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
-  return error.ToError();
-}
-if (status != 0) {
-  // xcrun didn't find a matching SDK. Not an error, we'll try
-  // 
diff erent spellings.
-  LLDB_LOG(log, "xcrun returned exit code %d", status);
-  return "";
-}
-if (output_str.empty()) {
-  LLDB_LOG(log, "xcrun returned no results");
-  return "";
-}
+  // Convert to a StringRef so we can manipulate the string without modifying
+  // the underlying data.
+  llvm::StringRef output(output_str);
 
-// Convert to a StringRef so we can manipulate the string without modifying
-// the underlying data.
-llvm::StringRef output(output_str);
+  // Remove any trailing newline characters.
+  output = output.rtrim();
 
-// Remove any trailing newline characters.
-output = output.rtrim();
+  // Strip any leading newline characters and everything before them.
+  const size_t 

[Lldb-commits] [lldb] 7de4352 - HostInfoMacOS: Add a utility function for finding an SDK-specific tool

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2023-05-31T14:46:35-07:00
New Revision: 7de43526e3cc07a02d01a0c4bf0670900495b938

URL: 
https://github.com/llvm/llvm-project/commit/7de43526e3cc07a02d01a0c4bf0670900495b938
DIFF: 
https://github.com/llvm/llvm-project/commit/7de43526e3cc07a02d01a0c4bf0670900495b938.diff

LOG: HostInfoMacOS: Add a utility function for finding an SDK-specific tool

This is an API needed by swift-lldb.

https://reviews.llvm.org/D151591

Added: 


Modified: 
lldb/include/lldb/Host/HostInfoBase.h
lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/unittests/Host/HostInfoTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/HostInfoBase.h 
b/lldb/include/lldb/Host/HostInfoBase.h
index 4082cd7f62bc6..705aad559f3b7 100644
--- a/lldb/include/lldb/Host/HostInfoBase.h
+++ b/lldb/include/lldb/Host/HostInfoBase.h
@@ -16,6 +16,7 @@
 #include "lldb/Utility/XcodeSDK.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
 
 #include 
 
@@ -135,6 +136,12 @@ class HostInfoBase {
 return llvm::make_error("cannot determine SDK root");
   }
 
+  /// Return the path to a specific tool in the specified Xcode SDK.
+  static llvm::Expected FindSDKTool(XcodeSDK sdk,
+ llvm::StringRef tool) {
+return llvm::errorCodeToError(llvm::errc::no_such_file_or_directory);
+  }
+
   /// Return information about module \p image_name if it is loaded in
   /// the current process's address space.
   static SharedCacheImageInfo

diff  --git a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h 
b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
index 74d979d965a73..8eb2ede382c22 100644
--- a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -12,6 +12,7 @@
 #include "lldb/Host/posix/HostInfoPosix.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/XcodeSDK.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/VersionTuple.h"
 #include 
 
@@ -32,6 +33,8 @@ class HostInfoMacOSX : public HostInfoPosix {
 
   /// Query xcrun to find an Xcode SDK directory.
   static llvm::Expected GetSDKRoot(SDKOptions options);
+  static llvm::Expected FindSDKTool(XcodeSDK sdk,
+ llvm::StringRef tool);
 
   /// Shared cache utilities
   static SharedCacheImageInfo

diff  --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 96461f9869e4d..c80d2002b8f18 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -523,41 +523,69 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
   return path;
 }
 
-llvm::Expected HostInfoMacOSX::GetSDKRoot(SDKOptions options) 
{
-  struct ErrorOrPath {
-std::string str;
-bool is_error;
-  };
-  static llvm::StringMap g_sdk_path;
-  static std::mutex g_sdk_path_mutex;
+namespace {
+struct ErrorOrPath {
+  std::string str;
+  bool is_error;
+};
+} // namespace
 
-  std::lock_guard guard(g_sdk_path_mutex);
+static llvm::Expected
+find_cached_path(llvm::StringMap &cache, std::mutex &mutex,
+ llvm::StringRef key,
+ std::function(void)> compute) {
+  std::lock_guard guard(mutex);
   LLDB_SCOPED_TIMER();
 
-  if (!options.XcodeSDKSelection)
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "XCodeSDK not specified");
-  XcodeSDK sdk = *options.XcodeSDKSelection;
-
-  auto key = sdk.GetString();
-  auto it = g_sdk_path.find(key);
-  if (it != g_sdk_path.end()) {
+  auto it = cache.find(key);
+  if (it != cache.end()) {
 if (it->second.is_error)
   return llvm::createStringError(llvm::inconvertibleErrorCode(),
  it->second.str);
-else
-  return it->second.str;
+return it->second.str;
   }
-  auto path_or_err = GetXcodeSDK(sdk);
+  auto path_or_err = compute();
   if (!path_or_err) {
 std::string error = toString(path_or_err.takeError());
-g_sdk_path.insert({key, {error, true}});
+cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
-  auto it_new = g_sdk_path.insert({key, {*path_or_err, false}});
+  auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+llvm::Expected HostInfoMacOSX::GetSDKRoot(SDKOptions options) 
{
+  static llvm::StringMap g_sdk_path;
+  static std::mutex g_sdk_path_mutex;
+  if (!options.XcodeSDKSelection)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "XcodeSDK not specified");
+  XcodeSDK sdk = *options.XcodeSDKSelection;
+  auto key = sdk.GetString();
+  return find_c

[Lldb-commits] [PATCH] D151843: Add EXC_SYSCALL to the allowable ignored exceptions for Darwin

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, mib.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

EXC_SYSCALL has the same problem as EXC_BAD_INSTRUCTION and EXC_BAD_ACCESS, 
they all should get turned into BSD signals when forwarded to the host port, 
but lldb isn't allowed to do that from the outside.  So the only way to get 
that to happen is to have debugserver not listen for them.  I overlooked them 
when originally adding the ignored-exception setting.  I added some tests to 
make sure we accept the ones we accept and don't accept the ones we shouldn't - 
because lldb needs them to operate.

I didn't add an end-to-end test for EXC_SYSCALL because it's actually quite 
hard to get the kernel to generate this exception - most of the time is goes 
straight to a SIGSYS.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151843

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py


Index: lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
===
--- lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
+++ lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
@@ -30,10 +30,22 @@
 "EXC_BAD_AXESS",
 error=True,
 )
-# Now set ourselves to ignore some exceptions.  The test depends on 
ignoring EXC_BAD_ACCESS, but I passed a couple
-# to make sure they parse:
+# Make sure that we don't accept exceptions that lldb/debugserver need:
+self.match(
+"settings set platform.plugin.darwin.ignored-exceptions 
EXC_BREAKPOINT",
+"EXC_BREAKPOINT",
+error=True,
+)
+# Make sure that we don't accept exceptions that lldb/debugserver need:
+self.match(
+"settings set platform.plugin.darwin.ignored-exceptions 
EXC_SOFT_SIGNAL",
+"EXC_SOFT_SIGNAL",
+error=True,
+)
+# Now set ourselves to ignore some exceptions.  The test depends on 
ignoring EXC_BAD_ACCESS, but I passed all the
+# ones we currently accept to make sure they parse:
 self.runCmd(
-"settings set platform.plugin.darwin.ignored-exceptions 
EXC_BAD_ACCESS|EXC_ARITHMETIC"
+"settings set platform.plugin.darwin.ignored-exceptions 
EXC_BAD_ACCESS|EXC_BAD_INSTRUCTION|EXC_ARITHMETIC|EXC_RESOURCE|EXC_GUARD|EXC_SYSCALL"
 )
 (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
 self, "Stop here to get things going", self.main_source_file
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -63,7 +63,8 @@
   || candidate == "EXC_BAD_INSTRUCTION"
   || candidate == "EXC_ARITHMETIC"
   || candidate == "EXC_RESOURCE"
-  || candidate == "EXC_GUARD")) {
+  || candidate == "EXC_GUARD"
+  || candidate == "EXC_SYSCALL")) {
   error.SetErrorStringWithFormat("invalid exception type: '%s'",
   candidate.str().c_str());
   return error;


Index: lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
===
--- lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
+++ lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
@@ -30,10 +30,22 @@
 "EXC_BAD_AXESS",
 error=True,
 )
-# Now set ourselves to ignore some exceptions.  The test depends on ignoring EXC_BAD_ACCESS, but I passed a couple
-# to make sure they parse:
+# Make sure that we don't accept exceptions that lldb/debugserver need:
+self.match(
+"settings set platform.plugin.darwin.ignored-exceptions EXC_BREAKPOINT",
+"EXC_BREAKPOINT",
+error=True,
+)
+# Make sure that we don't accept exceptions that lldb/debugserver need:
+self.match(
+"settings set platform.plugin.darwin.ignored-exceptions EXC_SOFT_SIGNAL",
+"EXC_SOFT_SIGNAL",
+error=True,
+)
+# Now set ourselves to ignore some exceptions.  The test depends on ignoring EXC_BAD_ACCESS, but I passed all the
+# ones we currently accept to make sure they parse:
 self.runCmd(
-"settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_ACCESS|EXC_ARITHMETIC"
+"settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_ACCESS|EXC_BAD_INSTRUCTION|EXC_ARITHMETIC|EXC_RESOURCE|EXC_GUARD|EXC_SYSCALL"
 )
 (target, process, thread, bkpt) = lldbutil.run_to_sour

[Lldb-commits] [PATCH] D151844: [lldb/crashlog] Fix crash when loading non-symbolicated report

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should address the crashes when parsing a the crash report
frame dictionary.

If the crash report is not symbolicated, the `symbolLocation` key will
be missing. In that case, we should just use the `imageOffset`.

rdar://109836386

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151844

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -596,9 +596,11 @@
 image_addr = self.get_used_image(image_id)["base"]
 pc = image_addr + frame_offset
 
-if "symbol" in json_frame:
-symbol = json_frame["symbol"]
-location = int(json_frame["symbolLocation"])
+if 'symbol' in json_frame:
+symbol = json_frame['symbol']
+location = 0
+if "symbolLocation" in json_frame and 
json_frame["symbolLocation"]:
+location = int(json_frame["symbolLocation"])
 image = self.images[image_id]
 image.symbols[symbol] = {
 "name": symbol,


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -596,9 +596,11 @@
 image_addr = self.get_used_image(image_id)["base"]
 pc = image_addr + frame_offset
 
-if "symbol" in json_frame:
-symbol = json_frame["symbol"]
-location = int(json_frame["symbolLocation"])
+if 'symbol' in json_frame:
+symbol = json_frame['symbol']
+location = 0
+if "symbolLocation" in json_frame and json_frame["symbolLocation"]:
+location = int(json_frame["symbolLocation"])
 image = self.images[image_id]
 image.symbols[symbol] = {
 "name": symbol,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151844: [lldb/crashlog] Fix crash when loading non-symbolicated report

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/examples/python/crashlog.py:599-602
+if 'symbol' in json_frame:
+symbol = json_frame['symbol']
+location = 0
+if "symbolLocation" in json_frame and 
json_frame["symbolLocation"]:

nit: Why use single quotes for `'symbol'` but double quotes for 
`"symbolLocation"`? Did some python formatter gives you this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151844/new/

https://reviews.llvm.org/D151844

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch changes the way we load a crash report into a scripted
process by creating a empty target.

To do so, it parses the architecture information from the report (for
both the legacy and json format) and uses that to create a target that
doesn't have any executable, like what we do when attaching to a process.

For the legacy format, we mostly rely on the `Code Type` line, since the
architure is an optional field on the `Binary Images` sections.

However for the json format, we first try to get the architecture while
parsing the image dictionary if we couldn't find it, we try to infer it
using the "flavor" key when parsing the frame's registers.

If the architecture is still not set after parsing the report, we use
the host architecture, as a fallback.

rdar://107850263

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151849

Files:
  lldb/examples/python/crashlog.py
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2515,7 +2515,7 @@
 
   FileSpec exe_spec_to_use;
   if (!exe_module) {
-if (!launch_info.GetExecutableFile()) {
+if (!launch_info.GetExecutableFile() && !launch_info.IsScriptedProcess()) {
   error.SetErrorString("executable module does not exist");
   return error;
 }
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -395,6 +395,10 @@
 self.version = -1
 self.target = None
 self.verbose = verbose
+self.process_id = None
+self.process_identifier = None
+self.process_path = None
+self.process_arch = None
 
 def dump(self):
 print("Crash Log File: %s" % (self.path))
@@ -484,9 +488,9 @@
 def __init__(self, debugger, path, verbose):
 self.path = os.path.expanduser(path)
 self.verbose = verbose
-self.crashlog = CrashLog(debugger, self.path, self.verbose)
 # List of DarwinImages sorted by their index.
 self.images = list()
+self.crashlog = CrashLog(debugger, self.path, self.verbose)
 
 @abc.abstractmethod
 def parse(self):
@@ -547,6 +551,8 @@
 def parse_process_info(self, json_data):
 self.crashlog.process_id = json_data["pid"]
 self.crashlog.process_identifier = json_data["procName"]
+if "procPath" in json_data:
+self.crashlog.process_path = json_data["procPath"]
 
 def parse_crash_reason(self, json_exception):
 self.crashlog.exception = json_exception
@@ -574,6 +580,10 @@
 darwin_image = self.crashlog.DarwinImage(
 low, high, name, version, img_uuid, path, self.verbose
 )
+if "arch" in json_image:
+darwin_image.arch = json_image["arch"]
+if path == self.crashlog.process_path:
+self.crashlog.process_arch = darwin_image.arch
 self.images.append(darwin_image)
 self.crashlog.images.append(darwin_image)
 
@@ -740,6 +750,13 @@
 gpr_dict = {str(idx): reg for idx, reg in enumerate(state)}
 registers.update(self.parse_thread_registers(gpr_dict, key))
 continue
+if key == "flavor":
+if not self.crashlog.process_arch:
+if state == "ARM_THREAD_STATE64":
+self.crashlog.process_arch = "arm64"
+elif state == "X86_THREAD_STATE":
+self.crashlog.process_arch = "x86_64"
+continue
 try:
 value = int(state["value"])
 registers["{}{}".format(prefix or "", key)] = value
@@ -912,6 +929,8 @@
 line[8:].strip().split(" [")
 )
 self.crashlog.process_id = pid_with_brackets.strip("[]")
+elif line.startswith("Path:"):
+self.crashlog.process_path = line[5:].strip()
 elif line.startswith("Identifier:"):
 self.crashlog.process_identifier = line[11:].strip()
 elif line.startswith("Version:"):
@@ -923,6 +942,11 @@
 else:
 self.crashlog.process = version_string
 self.crashlog.process_compatability_version = version_string
+elif line.startswith("Code Type:"):
+if "ARM-64" in line:
+self.crashlog.process_arch = "arm64"
+elif "X86-64" in line:
+self.crashlog.process_arch = "x86_64"
 elif self.parent_process_regex.search(line):
   

[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Why did you choose to have a separate FileSpecBuilder class, rather than making 
FileSpec smarter about the structure of the path (e.g. have an array of 
StringRef's into the paths for each component.)   We could do the parse once 
the first time a path element was requested, and then operations like 
"RemovePathComponent" would be trivial.  We could maybe even get rid of the 
distinction between "path" and "filename" since "filename" is really just "last 
path component".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151765/new/

https://reviews.llvm.org/D151765

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151844: [lldb/crashlog] Fix crash when loading non-symbolicated report

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:599-602
+if 'symbol' in json_frame:
+symbol = json_frame['symbol']
+location = 0
+if "symbolLocation" in json_frame and 
json_frame["symbolLocation"]:

bulbazord wrote:
> nit: Why use single quotes for `'symbol'` but double quotes for 
> `"symbolLocation"`? Did some python formatter gives you this?
yep


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151844/new/

https://reviews.llvm.org/D151844

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/examples/python/crashlog.py:493
 self.images = list()
+self.crashlog = CrashLog(debugger, self.path, self.verbose)
 

Any reason you moved this?



Comment at: lldb/examples/python/crashlog.py:552-553
 def parse_process_info(self, json_data):
 self.crashlog.process_id = json_data["pid"]
 self.crashlog.process_identifier = json_data["procName"]
+if "procPath" in json_data:

I assume "pid" and "procName" are not optional right? Maybe not in this one but 
in a follow-up we can add some error handling here.



Comment at: lldb/examples/python/crashlog.py:1373-1374
+arch = crashlog.process_arch
+if not arch:
+arch = platform.machine()
+target = debugger.CreateTargetWithFileAndArch(None, arch)

The fallback of "use the host platform's architecture" seems like it could lead 
to some issues no? I'm not sure we can do better if the crashlog doesn't have 
any info about a process architecture though... 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151849/new/

https://reviews.llvm.org/D151849

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked an inline comment as done.
mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:493
 self.images = list()
+self.crashlog = CrashLog(debugger, self.path, self.verbose)
 

bulbazord wrote:
> Any reason you moved this?
It makes more sense to me to initialize the crashlog object last.



Comment at: lldb/examples/python/crashlog.py:1373-1374
+arch = crashlog.process_arch
+if not arch:
+arch = platform.machine()
+target = debugger.CreateTargetWithFileAndArch(None, arch)

bulbazord wrote:
> The fallback of "use the host platform's architecture" seems like it could 
> lead to some issues no? I'm not sure we can do better if the crashlog doesn't 
> have any info about a process architecture though... 
Do you think we shouldn't fallback to the host architecture in this case and 
raise an exception instead ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151849/new/

https://reviews.llvm.org/D151849

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/examples/python/crashlog.py:1373-1374
+arch = crashlog.process_arch
+if not arch:
+arch = platform.machine()
+target = debugger.CreateTargetWithFileAndArch(None, arch)

mib wrote:
> bulbazord wrote:
> > The fallback of "use the host platform's architecture" seems like it could 
> > lead to some issues no? I'm not sure we can do better if the crashlog 
> > doesn't have any info about a process architecture though... 
> Do you think we shouldn't fallback to the host architecture in this case and 
> raise an exception instead ?
That might be a better idea. For example, if you're debugging a crashlog for a 
different platform, lldb would make this assumption silently which can lead to 
strange problems that are difficult to reason about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151849/new/

https://reviews.llvm.org/D151849

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, bulbazord, jingham.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch add the ability for the user to set a name for a target.

This can be very useful when debugging targets with the same executables
in the same session.

Names can be set either at the target creation in the command
interpreter or at any time using the SBAPI.

Target names show up in the `target list` output, following the target
index, and they also allow the user to switch targets using them.

rdar://105016191

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151859

Files:
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Target/Target.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp

Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -82,8 +82,14 @@
   if (!exe_valid)
 ::strcpy(exe_path, "");
 
-  strm.Printf("%starget #%u: %s", prefix_cstr ? prefix_cstr : "", target_idx,
-  exe_path);
+  std::string formatted_name = "";
+  const std::string &name = target->GetName();
+  if (!name.empty()) {
+formatted_name = " (" + name + ")";
+  }
+
+  strm.Printf("%starget #%u%s: %s", prefix_cstr ? prefix_cstr : "", target_idx,
+  formatted_name.data(), exe_path);
 
   uint32_t properties = 0;
   if (target_arch.IsValid()) {
@@ -209,6 +215,8 @@
 m_platform_options(true), // Include the --platform option.
 m_core_file(LLDB_OPT_SET_1, false, "core", 'c', 0, eArgTypeFilename,
 "Fullpath to a core file to use for this target."),
+m_name(LLDB_OPT_SET_1, false, "name", 'n', 0, eArgTypeName,
+   "Optional name for this target.", nullptr),
 m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
   eArgTypeFilename,
   "Fullpath to a stand alone debug "
@@ -234,6 +242,7 @@
 m_option_group.Append(&m_arch_option, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
 m_option_group.Append(&m_platform_options, LLDB_OPT_SET_ALL, 1);
 m_option_group.Append(&m_core_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
+m_option_group.Append(&m_name, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
 m_option_group.Append(&m_symbol_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
 m_option_group.Append(&m_remote_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
 m_option_group.Append(&m_add_dependents, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
@@ -303,6 +312,10 @@
 return false;
   }
 
+  llvm::StringRef name = m_name.GetOptionValue().GetCurrentValueAsRef();
+  if (!name.empty())
+target_sp->SetName(name);
+
   auto on_error = llvm::make_scope_exit(
   [&target_list = debugger.GetTargetList(), &target_sp]() {
 target_list.DeleteTarget(target_sp);
@@ -455,6 +468,7 @@
   OptionGroupArchitecture m_arch_option;
   OptionGroupPlatform m_platform_options;
   OptionGroupFile m_core_file;
+  OptionGroupString m_name;
   OptionGroupFile m_symbol_file;
   OptionGroupFile m_remote_file;
   OptionGroupDependents m_add_dependents;
@@ -504,10 +518,10 @@
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 if (args.GetArgumentCount() == 1) {
   const char *target_idx_arg = args.GetArgumentAtIndex(0);
-  uint32_t target_idx;
+  uint32_t target_idx = LLDB_INVALID_INDEX32;
+  TargetList &target_list = GetDebugger().GetTargetList();
+  const uint32_t num_targets = target_list.GetNumTargets();
   if (llvm::to_integer(target_idx_arg, target_idx)) {
-TargetList &target_list = GetDebugger().GetTargetList();
-const uint32_t num_targets = target_list.GetNumTargets();
 if (target_idx < num_targets) {
   target_list.SetSelectedTarget(target_idx);
   Stream &strm = result.GetOutputStream();
@@ -526,8 +540,28 @@
   }
 }
   } else {
-result.AppendErrorWithFormat("invalid index string value '%s'\n",
- target_idx_arg);
+for (size_t i = 0; i < num_targets; i++) {
+  if (TargetSP target_sp = target_list.GetTargetAtIndex(i)) {
+const std::string &name = target_sp->GetName();
+if (!name.empty()) {
+  if (name == target_idx_arg) {
+target_idx = i;
+break;
+  }
+}
+  }
+}
+
+if (target_idx != LLDB_INVALID_INDEX32) {
+  target_list.SetSelectedTarget(target_idx);
+  Stream &strm = result.GetOutputStream();
+  bool show_stopped_process_status = false;
+  DumpTargetList(target_list, show_stopped_process_status, strm);
+  result.SetStat

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

I still need to add some tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:315
 
+  llvm::StringRef name = m_name.GetOptionValue().GetCurrentValueAsRef();
+  if (!name.empty())

nit:



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:520
 if (args.GetArgumentCount() == 1) {
   const char *target_idx_arg = args.GetArgumentAtIndex(0);
+  uint32_t target_idx = LLDB_INVALID_INDEX32;

Maybe we can rename `target_idx_arg` to something like `target_identifier`? 
Since it's not necessarily an index anymore.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:546-551
+if (!name.empty()) {
+  if (name == target_idx_arg) {
+target_idx = i;
+break;
+  }
+}

nit:

Also, do we need the empty check? Or is it possible that `target_idx_arg` could 
be an empty string?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ed7be0d - lldb: Fix cross-cu-reference test to explicitly request that feature

[David Blaikie via lldb-commits]

Author: David Blaikie
Date: 2023-06-01T00:35:39Z
New Revision: ed7be0d4d17b5d1470587643cd5c55157414bb9c

URL: 
https://github.com/llvm/llvm-project/commit/ed7be0d4d17b5d1470587643cd5c55157414bb9c
DIFF: 
https://github.com/llvm/llvm-project/commit/ed7be0d4d17b5d1470587643cd5c55157414bb9c.diff

LOG: lldb: Fix cross-cu-reference test to explicitly request that feature

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll 
b/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll
index 9c2348750cfb3..0cc8b1f1b8e85 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll
@@ -1,7 +1,7 @@
 ; Check handling of dwo files with multiple compile units. Right now this is 
not
 ; supported, but it should not cause us to crash or misbehave either...
 
-; RUN: llc %s -filetype=obj -o %t.o --split-dwarf-file=%t.o
+; RUN: llc %s -filetype=obj -o %t.o --split-dwarf-file=%t.o 
-split-dwarf-cross-cu-references
 ; RUN: %lldb %t.o -o "image lookup -s x1 -v" -o "image lookup -s x2 -v" -b | 
FileCheck %s
 
 ; CHECK: image lookup -s x1



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151861: Don't emit a bunch of spurious "Unknown platform 0" warnings from debugserver

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: jasonmolenda.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The code that was trying to get the platform string was passing the 
MachProcess::DeploymentInfo.platform to GetPlatformString w/o checking first 
whether that load command actually provided a platform number.  DeploymentInfo 
already had a bool operator that checked if the platform had been set, so just 
check that first before calling GetPlatformString.  NFC except a little less 
spam in the debugserver log output.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151861

Files:
  lldb/tools/debugserver/source/DNB.cpp


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -1456,9 +1456,13 @@
 major_version = info.major_version;
 minor_version = info.minor_version;
 patch_version = info.patch_version;
+// MachProcess::DeploymentInfo has a bool operator to tell whether we have
+// set the platform.  If that's not true, don't report out the platform:
+if (!info)
+  return {};
 return procSP->GetPlatformString(info.platform);
   }
-  return nullptr;
+  return {};
 }
 
 // Get the current shared library information for a process. Only return


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -1456,9 +1456,13 @@
 major_version = info.major_version;
 minor_version = info.minor_version;
 patch_version = info.patch_version;
+// MachProcess::DeploymentInfo has a bool operator to tell whether we have
+// set the platform.  If that's not true, don't report out the platform:
+if (!info)
+  return {};
 return procSP->GetPlatformString(info.platform);
   }
-  return nullptr;
+  return {};
 }
 
 // Get the current shared library information for a process. Only return
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151861: Don't emit a bunch of spurious "Unknown platform 0" warnings from debugserver

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

Ah good catch, thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151861/new/

https://reviews.llvm.org/D151861

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Make sure we do something sensible with "target select " if the user has 
given the same name to two targets (or disallow doing that, one or the other).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D151859#4385986 , @jingham wrote:

> Make sure we do something sensible with "target select " if the user 
> has given the same name to two targets (or disallow doing that, one or the 
> other).

In addition to this, what happens if the user sets the target's name to a 
number? Something like `10`. I don't know what format `llvm::to_integer` 
expects, but I wonder if it can extract `0xF` as `15`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I'd also maybe call this the Target "Label" not the Name.  We have a fairly 
strong use of Name for breakpoint names, and this doesn't have that character 
at all.  Also, if they are Labels, I think it's legit for us to keep them 
unique, which I think is more sane than trying to handle two targets with the 
same label...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

[Alex Langford via Phabricator via lldb-commits]

bulbazord planned changes to this revision.
bulbazord added a comment.

In D151765#4385711 , @jingham wrote:

> Why did you choose to have a separate FileSpecBuilder class, rather than 
> making FileSpec smarter about the structure of the path (e.g. have an array 
> of StringRef's into the paths for each component.)   We could do the parse 
> once the first time a path element was requested, and then operations like 
> "RemovePathComponent" would be trivial.  We could maybe even get rid of the 
> distinction between "path" and "filename" since "filename" is really just 
> "last path component".

You mean have FileSpec replace its `ConstString m_directory` and `ConstString 
m_filename` fields with `llvm::SmallString m_path`? That would indeed avoid 
something like `FileSpecBuilder` (and probably be simpler in the end). As long 
as the API of FileSpec stays the same, the changes to call sites should be 
minimal. The thing I'm mostly concerned about is lifetimes... How often do we 
grab a ConstString from a FileSpec and not think about the lifetime? Hopefully 
not many places.

I'll try this out and see what the fallout is. If it's the better way to go, 
then I'll just refactor all the places where those assumptions are problematic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151765/new/

https://reviews.llvm.org/D151765

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 527251.
jasonmolenda added a comment.

Updated patch with Alex and Jonas' feedback incorporated.  Most significantly, 
instead of making target.process.virtual-addressable-bits an array of uint 
values (between zero to two of them), I am leaving virtual-addressable-bits 
as-is, and adding a new target.process.highmem-virtual-addressable-bits 
setting.  When this is set, its value will be used for setting high-memory 
signed addresses on AArch64 using the Apple ABI plugin.  And the value in 
virtual-addressable-bits will be used for clearing low-memory signed addresses 
on AArch64 Apple.  If this new highmem-virtual-addressable-bits is not set (the 
99.9% most common case), virtual-addressable-bits applies to both address 
ranges.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151292/new/

https://reviews.llvm.org/D151292

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -255,6 +255,9 @@
   def VirtualAddressableBits: Property<"virtual-addressable-bits", "UInt64">,
 DefaultUnsignedValue<0>,
 Desc<"The number of bits used for addressing. If the value is 39, then bits 0..38 are used for addressing. The default value of 0 means unspecified.">;
+  def HighmemVirtualAddressableBits: Property<"highmem-virtual-addressable-bits", "UInt64">,
+DefaultUnsignedValue<0>,
+Desc<"The number of bits used for addressing high memory, when it differs from low memory in the same Process. When this is non-zero, target.process.virtual-addressable-bits will be the value for low memory (0x000... addresses) and this setting will be the value for high memory (0xfff... addresses). When this is zero, target.process.virtual-addressable-bits applies to all addresses. It is very uncommon to use this setting.">;
   def FollowForkMode: Property<"follow-fork-mode", "Enum">,
 DefaultEnumValue<"eFollowParent">,
 EnumValues<"OptionEnumValues(g_follow_fork_mode_values)">,
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -227,6 +227,18 @@
   const uint32_t idx = ePropertyVirtualAddressableBits;
   SetPropertyAtIndex(idx, static_cast(bits));
 }
+
+uint32_t ProcessProperties::GetHighmemVirtualAddressableBits() const {
+  const uint32_t idx = ePropertyHighmemVirtualAddressableBits;
+  return GetPropertyAtIndexAs(
+  idx, g_process_properties[idx].default_uint_value);
+}
+
+void ProcessProperties::SetHighmemVirtualAddressableBits(uint32_t bits) {
+  const uint32_t idx = ePropertyHighmemVirtualAddressableBits;
+  SetPropertyAtIndex(idx, static_cast(bits));
+}
+
 void ProcessProperties::SetPythonOSPluginPath(const FileSpec &file) {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   SetPropertyAtIndex(idx, file);
@@ -5651,25 +5663,31 @@
 }
 
 lldb::addr_t Process::GetCodeAddressMask() {
-  if (m_code_address_mask == 0) {
-if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
-  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
-  SetCodeAddressMask(address_mask);
-}
-  }
+  if (uint32_t num_bits_setting = GetVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+
   return m_code_address_mask;
 }
 
 lldb::addr_t Process::GetDataAddressMask() {
-  if (m_data_address_mask == 0) {
-if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
-  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
-  SetDataAddressMask(address_mask);
-}
-  }
+  if (uint32_t num_bits_setting = GetVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+
   return m_data_address_mask;
 }
 
+lldb::addr_t Process::GetHighmemCodeAddressMask() {
+  if (uint32_t num_bits_setting = GetHighmemVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+  return GetCodeAddressMask();
+}
+
+lldb::addr_t Process::GetHighmemDataAddressMask() {
+  if (uint32_t num_bits_setting = GetHighmemVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+  return GetDataAddressMask();
+}
+
 void Process::DidExec() {
   Log *log = GetLog(LLDBLog::Process);
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKern

[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG21dfaf60a763: Setting to control addressable bits in high 
memory (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151292/new/

https://reviews.llvm.org/D151292

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -255,6 +255,9 @@
   def VirtualAddressableBits: Property<"virtual-addressable-bits", "UInt64">,
 DefaultUnsignedValue<0>,
 Desc<"The number of bits used for addressing. If the value is 39, then bits 0..38 are used for addressing. The default value of 0 means unspecified.">;
+  def HighmemVirtualAddressableBits: Property<"highmem-virtual-addressable-bits", "UInt64">,
+DefaultUnsignedValue<0>,
+Desc<"The number of bits used for addressing high memory, when it differs from low memory in the same Process. When this is non-zero, target.process.virtual-addressable-bits will be the value for low memory (0x000... addresses) and this setting will be the value for high memory (0xfff... addresses). When this is zero, target.process.virtual-addressable-bits applies to all addresses. It is very uncommon to use this setting.">;
   def FollowForkMode: Property<"follow-fork-mode", "Enum">,
 DefaultEnumValue<"eFollowParent">,
 EnumValues<"OptionEnumValues(g_follow_fork_mode_values)">,
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -227,6 +227,18 @@
   const uint32_t idx = ePropertyVirtualAddressableBits;
   SetPropertyAtIndex(idx, static_cast(bits));
 }
+
+uint32_t ProcessProperties::GetHighmemVirtualAddressableBits() const {
+  const uint32_t idx = ePropertyHighmemVirtualAddressableBits;
+  return GetPropertyAtIndexAs(
+  idx, g_process_properties[idx].default_uint_value);
+}
+
+void ProcessProperties::SetHighmemVirtualAddressableBits(uint32_t bits) {
+  const uint32_t idx = ePropertyHighmemVirtualAddressableBits;
+  SetPropertyAtIndex(idx, static_cast(bits));
+}
+
 void ProcessProperties::SetPythonOSPluginPath(const FileSpec &file) {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   SetPropertyAtIndex(idx, file);
@@ -5651,25 +5663,31 @@
 }
 
 lldb::addr_t Process::GetCodeAddressMask() {
-  if (m_code_address_mask == 0) {
-if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
-  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
-  SetCodeAddressMask(address_mask);
-}
-  }
+  if (uint32_t num_bits_setting = GetVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+
   return m_code_address_mask;
 }
 
 lldb::addr_t Process::GetDataAddressMask() {
-  if (m_data_address_mask == 0) {
-if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
-  lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
-  SetDataAddressMask(address_mask);
-}
-  }
+  if (uint32_t num_bits_setting = GetVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+
   return m_data_address_mask;
 }
 
+lldb::addr_t Process::GetHighmemCodeAddressMask() {
+  if (uint32_t num_bits_setting = GetHighmemVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+  return GetCodeAddressMask();
+}
+
+lldb::addr_t Process::GetHighmemDataAddressMask() {
+  if (uint32_t num_bits_setting = GetHighmemVirtualAddressableBits())
+return ~((1ULL << num_bits_setting) - 1);
+  return GetDataAddressMask();
+}
+
 void Process::DidExec() {
   Log *log = GetLog(LLDBLog::Process);
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -1080,15 +1080,18 @@
   }
   // If the kernel global with the T1Sz setting is available,
   // update the target.process.virtual-addressable-bits to be correct.
+  // NB the xnu kernel always has T0Sz and T1Sz the same value.  If
+  // it wasn't the same, we would need to set
+  // target.process.virtual-addressable-bits = T0Sz
+  // target.process.highmem-virtual-addressable-bits = T1Sz
   symbol = m_kernel.GetModule()->FindFirstSymbolWithNameAndType(
   arm64

[Lldb-commits] [lldb] 21dfaf6 - Setting to control addressable bits in high memory

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2023-05-31T18:38:34-07:00
New Revision: 21dfaf60a763795e3834d67def48fc2ba5214e59

URL: 
https://github.com/llvm/llvm-project/commit/21dfaf60a763795e3834d67def48fc2ba5214e59
DIFF: 
https://github.com/llvm/llvm-project/commit/21dfaf60a763795e3834d67def48fc2ba5214e59.diff

LOG: Setting to control addressable bits in high memory

On AArch64, it is possible to have a program that accesses both low
(0x000...) and high (0xfff...) memory, and with pointer authentication,
you can have different numbers of bits used for pointer authentication
depending on whether the address is in high or low memory.

This adds a new target.process.highmem-virtual-addressable-bits
setting which the AArch64 Mac ABI plugin will use, when set, to
always set those unaddressable high bits for high memory addresses,
and will use the existing target.process.virtual-addressable-bits
setting for low memory addresses.

This patch does not change the existing behavior when only
target.process.virtual-addressable-bits is set.  In that case, the
value will apply to all addresses.

Not yet done is recognizing metadata in a live process connection
(gdb-remote qHostInfo) or a Mach-O corefile LC_NOTE to set the
correct number of addressing bits for both memory ranges.  That
will be a future change.

Differential Revision: https://reviews.llvm.org/D151292
rdar://109746900

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 207b4939d0232..d565931af7087 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -81,6 +81,8 @@ class ProcessProperties : public Properties {
   FileSpec GetPythonOSPluginPath() const;
   uint32_t GetVirtualAddressableBits() const;
   void SetVirtualAddressableBits(uint32_t bits);
+  uint32_t GetHighmemVirtualAddressableBits() const;
+  void SetHighmemVirtualAddressableBits(uint32_t bits);
   void SetPythonOSPluginPath(const FileSpec &file);
   bool GetIgnoreBreakpointsInExpressions() const;
   void SetIgnoreBreakpointsInExpressions(bool ignore);
@@ -1371,6 +1373,9 @@ class Process : public 
std::enable_shared_from_this,
   lldb::addr_t GetCodeAddressMask();
   lldb::addr_t GetDataAddressMask();
 
+  lldb::addr_t GetHighmemCodeAddressMask();
+  lldb::addr_t GetHighmemDataAddressMask();
+
   void SetCodeAddressMask(lldb::addr_t code_address_mask) {
 m_code_address_mask = code_address_mask;
   }
@@ -1379,6 +1384,14 @@ class Process : public 
std::enable_shared_from_this,
 m_data_address_mask = data_address_mask;
   }
 
+  void SetHighmemCodeAddressMask(lldb::addr_t code_address_mask) {
+m_highmem_code_address_mask = code_address_mask;
+  }
+
+  void SetHighmemDataAddressMask(lldb::addr_t data_address_mask) {
+m_highmem_data_address_mask = data_address_mask;
+  }
+
   /// Get the Modification ID of the process.
   ///
   /// \return
@@ -3000,9 +3013,13 @@ void PruneThreadPlans();
   /// Mask for code an data addresses. The default value (0) means no mask is
   /// set.  The bits set to 1 indicate bits that are NOT significant for
   /// addressing.
+  /// The highmem versions are for targets where we may have 
diff erent masks
+  /// for low memory versus high memory addresses.
   /// @{
   lldb::addr_t m_code_address_mask = 0;
   lldb::addr_t m_data_address_mask = 0;
+  lldb::addr_t m_highmem_code_address_mask = 0;
+  lldb::addr_t m_highmem_data_address_mask = 0;
   /// @}
 
   bool m_clear_thread_plans_on_stop;

diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index 4d2505ece50de..b54f6846db3b6 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -814,15 +814,41 @@ ValueObjectSP ABIMacOSX_arm64::GetReturnValueObjectImpl(
   return return_valobj_sp;
 }
 
-lldb::addr_t ABIMacOSX_arm64::FixAddress(addr_t pc, addr_t mask) {
-  lldb::addr_t pac_sign_extension = 0x0080ULL;
-  // When no mask is specified, clear/set the top byte; preserve
-  // the low 55 bits (00..54) for addressing and bit 55 to indicate
-  // sign.
-  if (mask == 0) {
-// ~((1ULL<<55)-1)
-mask = 0xff80;
+addr_t ABIMacOSX_arm64::FixCodeAddress(addr_t pc) {
+  addr_t pac_sign_extension = 0x0080ULL;
+  addr_t tbi_mask = 0xff80ULL;
+  addr_t mask = 0;
+
+  if (ProcessSP process_sp = GetProcessSP()) {
+mask = process_sp->GetCodeAddressMask();
+if (pc & pac_sign_extension) {
+  addr_t highmem_mask = process_sp->GetHi