[Lldb-commits] [lldb] [lldb] Small cleanup of ProcessEventData::ShouldStop (PR #98154)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -4194,8 +4188,7 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   // is, and it's better to let the user decide than continue behind their
   // backs.
 
-  for (idx = 0; idx < not_suspended_thread_list.GetSize(); ++idx) {
-curr_thread_list = process_sp->GetThreadList();

labath wrote:

curr_thread_list is a reference to the process's thread list, which made this a 
self-assignment.

https://github.com/llvm/llvm-project/pull/98154
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Small cleanup of ProcessEventData::ShouldStop (PR #98154)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/98154

>From 27d419b047d0c2e95978b8c88dd84641aae423f2 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Tue, 9 Jul 2024 15:00:05 +0200
Subject: [PATCH] [lldb] Small cleanup of ProcessEventData::ShouldStop

While looking at a TSAN report (patch coming soon) in the ThreadList
class, I noticed that this code would be simpler if it did not use the
ThreadList class.
---
 lldb/source/Target/Process.cpp | 32 +++-
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6fac0df1d7a66..dc7f6c9e86a47 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4152,7 +4152,6 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
 
   ThreadList _thread_list = process_sp->GetThreadList();
   uint32_t num_threads = curr_thread_list.GetSize();
-  uint32_t idx;
 
   // The actions might change one of the thread's stop_info's opinions about
   // whether we should stop the process, so we need to query that as we go.
@@ -4162,23 +4161,18 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   // get that wrong (which is possible) then the thread list might have
   // changed, and that would cause our iteration here to crash.  We could
   // make a copy of the thread list, but we'd really like to also know if it
-  // has changed at all, so we make up a vector of the thread ID's and check
-  // what we get back against this list & bag out if anything differs.
-  ThreadList not_suspended_thread_list(process_sp.get());
-  std::vector thread_index_array(num_threads);
-  uint32_t not_suspended_idx = 0;
-  for (idx = 0; idx < num_threads; ++idx) {
+  // has changed at all, so we store the original thread ID's of all threads 
and
+  // check what we get back against this list & bag out if anything differs.
+  std::vector> not_suspended_threads;
+  for (uint32_t idx = 0; idx < num_threads; ++idx) {
 lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
 
 /*
  Filter out all suspended threads, they could not be the reason
  of stop and no need to perform any actions on them.
  */
-if (thread_sp->GetResumeState() != eStateSuspended) {
-  not_suspended_thread_list.AddThread(thread_sp);
-  thread_index_array[not_suspended_idx] = thread_sp->GetIndexID();
-  not_suspended_idx++;
-}
+if (thread_sp->GetResumeState() != eStateSuspended)
+  not_suspended_threads.emplace_back(thread_sp, thread_sp->GetIndexID());
   }
 
   // Use this to track whether we should continue from here.  We will only
@@ -4194,8 +4188,7 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   // is, and it's better to let the user decide than continue behind their
   // backs.
 
-  for (idx = 0; idx < not_suspended_thread_list.GetSize(); ++idx) {
-curr_thread_list = process_sp->GetThreadList();
+  for (auto [thread_sp, thread_index] : not_suspended_threads) {
 if (curr_thread_list.GetSize() != num_threads) {
   Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
   LLDB_LOGF(
@@ -4205,14 +4198,11 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   break;
 }
 
-lldb::ThreadSP thread_sp = not_suspended_thread_list.GetThreadAtIndex(idx);
-
-if (thread_sp->GetIndexID() != thread_index_array[idx]) {
+if (thread_sp->GetIndexID() != thread_index) {
   Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
-  LLDB_LOGF(log,
-"The thread at position %u changed from %u to %u while "
-"processing event.",
-idx, thread_index_array[idx], thread_sp->GetIndexID());
+  LLDB_LOG(log,
+   "The thread {0} changed from {1} to {2} while processing 
event.",
+   thread_sp.get(), thread_index, thread_sp->GetIndexID());
   break;
 }
 

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


[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.

Thank you.

https://github.com/llvm/llvm-project/pull/96654
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Small cleanup of ProcessEventData::ShouldStop (PR #98154)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/98154

While looking at a TSAN report (patch coming soon) in the ThreadList class, I 
noticed that this code would be simpler if it did not use the ThreadList class.

>From 2e74468a99015645f1abbfdf42f6e5c9893e7cf3 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Tue, 9 Jul 2024 15:00:05 +0200
Subject: [PATCH] [lldb] Small cleanup of ProcessEventData::ShouldStop

While looking at a TSAN report (patch coming soon) in the ThreadList
class, I noticed that this code would be simpler if it did not use the
ThreadList class.
---
 lldb/source/Target/Process.cpp | 31 +++
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6fac0df1d7a66..085e3f502e093 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4152,7 +4152,6 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
 
   ThreadList _thread_list = process_sp->GetThreadList();
   uint32_t num_threads = curr_thread_list.GetSize();
-  uint32_t idx;
 
   // The actions might change one of the thread's stop_info's opinions about
   // whether we should stop the process, so we need to query that as we go.
@@ -4162,23 +4161,18 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   // get that wrong (which is possible) then the thread list might have
   // changed, and that would cause our iteration here to crash.  We could
   // make a copy of the thread list, but we'd really like to also know if it
-  // has changed at all, so we make up a vector of the thread ID's and check
-  // what we get back against this list & bag out if anything differs.
-  ThreadList not_suspended_thread_list(process_sp.get());
-  std::vector thread_index_array(num_threads);
-  uint32_t not_suspended_idx = 0;
-  for (idx = 0; idx < num_threads; ++idx) {
+  // has changed at all, so we store the original thread ID's of all threads 
and
+  // check what we get back against this list & bag out if anything differs.
+  std::vector> not_suspended_threads;
+  for (uint32_t idx = 0; idx < num_threads; ++idx) {
 lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
 
 /*
  Filter out all suspended threads, they could not be the reason
  of stop and no need to perform any actions on them.
  */
-if (thread_sp->GetResumeState() != eStateSuspended) {
-  not_suspended_thread_list.AddThread(thread_sp);
-  thread_index_array[not_suspended_idx] = thread_sp->GetIndexID();
-  not_suspended_idx++;
-}
+if (thread_sp->GetResumeState() != eStateSuspended)
+  not_suspended_threads.emplace_back(thread_sp, thread_sp->GetIndexID());
   }
 
   // Use this to track whether we should continue from here.  We will only
@@ -4194,7 +4188,7 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   // is, and it's better to let the user decide than continue behind their
   // backs.
 
-  for (idx = 0; idx < not_suspended_thread_list.GetSize(); ++idx) {
+  for(auto [thread_sp, thread_index] : not_suspended_threads) {
 curr_thread_list = process_sp->GetThreadList();
 if (curr_thread_list.GetSize() != num_threads) {
   Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
@@ -4205,14 +4199,11 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   break;
 }
 
-lldb::ThreadSP thread_sp = not_suspended_thread_list.GetThreadAtIndex(idx);
-
-if (thread_sp->GetIndexID() != thread_index_array[idx]) {
+if (thread_sp->GetIndexID() != thread_index) {
   Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
-  LLDB_LOGF(log,
-"The thread at position %u changed from %u to %u while "
-"processing event.",
-idx, thread_index_array[idx], thread_sp->GetIndexID());
+  LLDB_LOG(log,
+   "The thread {0} changed from {1} to {2} while processing 
event.",
+   thread_sp.get(), thread_index, thread_sp->GetIndexID());
   break;
 }
 

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


[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -113,6 +130,10 @@ def _get_platform_os(p):
 platform = "openbsd"
 return platform
 
+# Triple is not available if we're not connected yet
+if p.GetName() == "remote-linux":
+return "linux"
+

labath wrote:

Yes, I would very much prefer that, as the code you added only handles linux. 
To support other oses, we would need to create a whole new mapping from lldb 
platform names to os strings. That list could then get out of sync, etc., so it 
would be best to just avoid doing that.

I'm not worried about the debug target going down during testing, as that means 
the test results will be meaningless anyway. If we find out we regularly need 
the os name before connecting, then I think we ought to revisit the mechanism 
for determining the OS.

https://github.com/llvm/llvm-project/pull/96654
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Use correct path separator for C++ library files lookup (PR #98144)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/98144
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -113,6 +130,10 @@ def _get_platform_os(p):
 platform = "openbsd"
 return platform
 
+# Triple is not available if we're not connected yet
+if p.GetName() == "remote-linux":
+return "linux"
+

labath wrote:

Can you explain why was this necessary? I get the connected part, but who was 
asking for the platform before connecting?

https://github.com/llvm/llvm-project/pull/96654
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }
+};
+
+/// This determines if the type is a shared, unique or weak pointer, either
+/// from stdlibc++ or libc+++.
+bool IsSmartPtrType(CompilerType type);
+
+/// Finds the member field with the given name and type, stores the child index
+/// corresponding to the field in the idx vector and returns a DILMemberInfo
+/// struct with appropriate information about the field.
+DILMemberInfo GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp,
+CompilerType type,
+const std::string ,
+std::vector *idx,
+CompilerType empty_type,
+bool use_synthetic, bool is_dynamic);
+
+std::tuple>
+GetMemberInfo(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+  const std::string , bool use_synthetic);
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed, acquiring the process & target locks if
+/// appropriate.
+lldb::ValueObjectSP
+DILGetSPWithLock(lldb::ValueObjectSP valobj_sp,
+ lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+ bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class DILNodeKind {
+  kDILErrorNode,
+  kLiteralNode,
+  kIdentifierNode,
+  kBuiltinFunctionCallNode,
+  kCStyleCastNode,
+  kMemberOfNode,
+  kArraySubscriptNode,
+  kUnaryOpNode,
+  kSmartPtrToPtrDecay
+};
+
+/// The C-Style casts allowed by DIL.
+enum class CStyleCastKind {
+  kArithmetic,
+  kEnumeration,
+  kPointer,
+  kNullptr,
+  kReference,
+};
+
+/// The Unary operators recognized by DIL.
+enum class UnaryOpKind {
+  AddrOf, // "&"
+  Deref,  // "*"
+  Minus,  // "-"
+};
+
+/// Given a string representing a type, returns the CompilerType corresponding
+/// to the named type, if it exists.
+CompilerType
+ResolveTypeByName(const std::string ,
+  std::shared_ptr ctx_scope);
+
+/// Quick lookup to check if a type name already exists in a
+/// name-to-CompilerType map the DIL parser keeps of previously found
+/// name/type pairs.
+bool IsContextVar(const std::string );
+
+/// Checks to see if the CompilerType is a Smart Pointer (shared, unique, weak)
+/// or not. Only applicable for C++, which is why this is here and not part of
+/// the CompilerType class.
+bool IsSmartPtrType(CompilerType type);
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+private:
+  using MemberPath = std::vector;
+  using IdentifierInfoPtr = std::unique_ptr;
+
+public:
+  enum class Kind {
+kValue,
+kContextArg,
+kMemberPath,
+kThisKeyword,
+  };
+
+  static IdentifierInfoPtr FromValue(lldb::ValueObjectSP value_sp) {
+CompilerType type;
+lldb::ValueObjectSP value = DILGetSPWithLock(value_sp);
+if (value)
+  type = value->GetCompilerType();
+return IdentifierInfoPtr(new IdentifierInfo(Kind::kValue, type, value, 
{}));
+  }
+
+  static IdentifierInfoPtr FromContextArg(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(
+new IdentifierInfo(Kind::kContextArg, type, empty_value, {}));
+  }
+
+  static IdentifierInfoPtr FromMemberPath(CompilerType type, MemberPath path) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(new IdentifierInfo(Kind::kMemberPath, type,
+empty_value, std::move(path)));
+  }
+
+  static IdentifierInfoPtr FromThisKeyword(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(
+new 

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }
+};
+
+/// This determines if the type is a shared, unique or weak pointer, either
+/// from stdlibc++ or libc+++.
+bool IsSmartPtrType(CompilerType type);
+
+/// Finds the member field with the given name and type, stores the child index
+/// corresponding to the field in the idx vector and returns a DILMemberInfo
+/// struct with appropriate information about the field.
+DILMemberInfo GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp,
+CompilerType type,
+const std::string ,
+std::vector *idx,
+CompilerType empty_type,
+bool use_synthetic, bool is_dynamic);
+
+std::tuple>
+GetMemberInfo(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+  const std::string , bool use_synthetic);
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed, acquiring the process & target locks if
+/// appropriate.
+lldb::ValueObjectSP
+DILGetSPWithLock(lldb::ValueObjectSP valobj_sp,
+ lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+ bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class DILNodeKind {
+  kDILErrorNode,
+  kLiteralNode,
+  kIdentifierNode,
+  kBuiltinFunctionCallNode,
+  kCStyleCastNode,
+  kMemberOfNode,
+  kArraySubscriptNode,
+  kUnaryOpNode,
+  kSmartPtrToPtrDecay

labath wrote:

lldb doesn't use the `k` prefix for constants. For unscoped enums, we use `e`. 
I think some of the scoped ones have it as well, but an empty prefix is also 
pretty common (so you can then refer to them as `DILNodeKind::Literal` (or 
`dil::NodeKind::Literal` in the namespace version).

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,468 @@
+//===-- DILAST.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/DILAST.h"
+#include "lldb/API/SBType.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/RegisterContext.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
+namespace lldb_private {
+
+lldb::ValueObjectSP DILGetSPWithLock(lldb::ValueObjectSP in_valobj_sp,
+ lldb::DynamicValueType use_dynamic,
+ bool use_synthetic) {
+  Process::StopLocker stop_locker;
+  std::unique_lock lock;
+  Status error;
+
+  if (!in_valobj_sp) {
+error.SetErrorString("invalid value object");
+return in_valobj_sp;
+  }
+
+  lldb::ValueObjectSP value_sp = in_valobj_sp;
+
+  Target *target = value_sp->GetTargetSP().get();
+  // If this ValueObject holds an error, then it is valuable for that.
+  if (value_sp->GetError().Fail())
+return value_sp;
+
+  if (!target)
+return lldb::ValueObjectSP();
+
+  lock = std::unique_lock(target->GetAPIMutex());
+
+  lldb::ProcessSP process_sp(value_sp->GetProcessSP());
+  if (process_sp && !stop_locker.TryLock(_sp->GetRunLock())) {
+// We don't allow people to play around with ValueObject if the process
+// is running. If you want to look at values, pause the process, then
+// look.
+error.SetErrorString("process must be stopped.");
+return lldb::ValueObjectSP();
+  }
+
+  if (use_dynamic != lldb::eNoDynamicValues) {
+lldb::ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(use_dynamic);
+if (dynamic_sp)
+  value_sp = dynamic_sp;
+  }
+
+  if (use_synthetic) {
+lldb::ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue();
+if (synthetic_sp)
+  value_sp = synthetic_sp;
+  }
+
+  if (!value_sp)
+error.SetErrorString("invalid value object");
+
+  return value_sp;
+}
+
+CompilerType DILASTNode::result_type_deref() const {
+  auto type = result_type();
+  return type.IsReferenceType() ? type.GetNonReferenceType() : type;
+}
+
+static std::unordered_map context_args;
+
+bool IsContextVar(const std::string ) {
+  return context_args.find(name) != context_args.end();
+}
+
+static lldb::ValueObjectSP
+LookupStaticIdentifier(lldb::TargetSP target_sp,
+   const llvm::StringRef _ref,
+   ConstString unqualified_name) {
+  // List global variable with the same "basename". There can be many matches
+  // from other scopes (namespaces, classes), so we do additional filtering
+  // later.
+  std::vector values;
+  VariableList variable_list;
+  ConstString name(name_ref);
+  target_sp->GetImages().FindGlobalVariables(
+  name, (size_t)std::numeric_limits::max, variable_list);
+  if (!variable_list.Empty()) {
+ExecutionContextScope *exe_scope = target_sp->GetProcessSP().get();
+if (exe_scope == nullptr)
+  exe_scope = target_sp.get();
+for (const lldb::VariableSP _sp : variable_list) {
+  lldb::ValueObjectSP valobj_sp(
+  ValueObjectVariable::Create(exe_scope, var_sp));
+  if (valobj_sp)
+values.push_back(valobj_sp);
+}
+  }
+
+  // Find the corrent variable by matching the name. lldb::SBValue::GetName()
+  // can return strings like "::globarVar", "ns::i" or "int const ns::foo"
+  // depending on the version and the platform.
+  for (uint32_t i = 0; i < values.size(); ++i) {
+lldb::ValueObjectSP val = values[i];
+llvm::StringRef val_name_sstr = val->GetName().GetStringRef();
+llvm::StringRef name_sstr = name.GetStringRef();
+
+if (val->GetVariable() && 
val->GetVariable()->NameMatches(unqualified_name))
+  return val;
+
+if (val_name_sstr == name_sstr ||
+val_name_sstr == llvm::formatv("::{0}", name_sstr).str() ||
+val_name_sstr.ends_with(llvm::formatv(" {0}", name_sstr).str()) ||
+val_name_sstr.ends_with(llvm::formatv("*{0}", name_sstr).str()) ||
+val_name_sstr.ends_with(llvm::formatv("&{0}", name_sstr).str()))
+  return val;
+  }
+  lldb::ValueObjectSP empty_obj_sp;
+  return empty_obj_sp;
+}
+
+struct EnumMember {
+  CompilerType type;
+  ConstString name;
+  llvm::APSInt value;
+};
+
+static std::vector GetEnumMembers(CompilerType type) {
+  std::vector enum_member_list;
+  if (type.IsValid()) {
+type.ForEachEnumerator(
+[_member_list](const CompilerType _type, ConstString name,
+const llvm::APSInt ) -> bool {
+  EnumMember enum_member = {integer_type, name, value};
+  

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }
+};
+
+/// This determines if the type is a shared, unique or weak pointer, either
+/// from stdlibc++ or libc+++.
+bool IsSmartPtrType(CompilerType type);
+
+/// Finds the member field with the given name and type, stores the child index
+/// corresponding to the field in the idx vector and returns a DILMemberInfo
+/// struct with appropriate information about the field.
+DILMemberInfo GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp,
+CompilerType type,
+const std::string ,
+std::vector *idx,
+CompilerType empty_type,
+bool use_synthetic, bool is_dynamic);
+
+std::tuple>
+GetMemberInfo(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+  const std::string , bool use_synthetic);
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed, acquiring the process & target locks if
+/// appropriate.
+lldb::ValueObjectSP
+DILGetSPWithLock(lldb::ValueObjectSP valobj_sp,
+ lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+ bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class DILNodeKind {
+  kDILErrorNode,
+  kLiteralNode,
+  kIdentifierNode,
+  kBuiltinFunctionCallNode,
+  kCStyleCastNode,
+  kMemberOfNode,
+  kArraySubscriptNode,
+  kUnaryOpNode,
+  kSmartPtrToPtrDecay
+};
+
+/// The C-Style casts allowed by DIL.
+enum class CStyleCastKind {
+  kArithmetic,
+  kEnumeration,
+  kPointer,
+  kNullptr,
+  kReference,
+};
+
+/// The Unary operators recognized by DIL.
+enum class UnaryOpKind {
+  AddrOf, // "&"
+  Deref,  // "*"
+  Minus,  // "-"
+};
+
+/// Given a string representing a type, returns the CompilerType corresponding
+/// to the named type, if it exists.
+CompilerType
+ResolveTypeByName(const std::string ,
+  std::shared_ptr ctx_scope);
+
+/// Quick lookup to check if a type name already exists in a
+/// name-to-CompilerType map the DIL parser keeps of previously found
+/// name/type pairs.
+bool IsContextVar(const std::string );
+
+/// Checks to see if the CompilerType is a Smart Pointer (shared, unique, weak)
+/// or not. Only applicable for C++, which is why this is here and not part of
+/// the CompilerType class.
+bool IsSmartPtrType(CompilerType type);
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+private:
+  using MemberPath = std::vector;
+  using IdentifierInfoPtr = std::unique_ptr;
+
+public:
+  enum class Kind {
+kValue,
+kContextArg,
+kMemberPath,
+kThisKeyword,
+  };
+
+  static IdentifierInfoPtr FromValue(lldb::ValueObjectSP value_sp) {
+CompilerType type;
+lldb::ValueObjectSP value = DILGetSPWithLock(value_sp);
+if (value)
+  type = value->GetCompilerType();
+return IdentifierInfoPtr(new IdentifierInfo(Kind::kValue, type, value, 
{}));
+  }
+
+  static IdentifierInfoPtr FromContextArg(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(
+new IdentifierInfo(Kind::kContextArg, type, empty_value, {}));
+  }
+
+  static IdentifierInfoPtr FromMemberPath(CompilerType type, MemberPath path) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(new IdentifierInfo(Kind::kMemberPath, type,
+empty_value, std::move(path)));
+  }
+
+  static IdentifierInfoPtr FromThisKeyword(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(
+new 

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }
+};
+
+/// This determines if the type is a shared, unique or weak pointer, either
+/// from stdlibc++ or libc+++.
+bool IsSmartPtrType(CompilerType type);
+
+/// Finds the member field with the given name and type, stores the child index
+/// corresponding to the field in the idx vector and returns a DILMemberInfo
+/// struct with appropriate information about the field.
+DILMemberInfo GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp,
+CompilerType type,
+const std::string ,
+std::vector *idx,
+CompilerType empty_type,
+bool use_synthetic, bool is_dynamic);
+
+std::tuple>
+GetMemberInfo(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+  const std::string , bool use_synthetic);

labath wrote:

these aren't implemented anywhere.

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;

labath wrote:

Does every bitfield have to have a size? If yes, what would you say to 
`optional bitfield_bit_size` (with `nullopt` meaning "it's not a 
bitfield")?

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,43 @@
+(* LLDB Debug Expressions, a subset of C++ *)
+(* Insired by https://www.nongnu.org/hcb *)
+
+expression = cast_expression;
+
+cast_expression = unary_expression;
+
+unary_expression = postfix_expression
+ | unary_operator cast_expression;
+
+unary_operator = "*" | "&" | "-" ;
+
+postfix_expression = primary_expression
+   | postfix_expression "[" expression "]"
+   | postfix_expression "." id_expression
+   | postfix_expression "->" id_expression
+
+primary_expression = numeric_literal
+   | boolean_literal
+   | pointer_literal
+   | id_expression
+   | "this" ;
+
+nested_name_specifier = namespace_name '::'
+  | nested_name_specifier identifier "::";
+
+namespace_name = identifier ;
+
+id_expression = unqualified_id
+  | qualified_id ;
+
+unqualified_id = identifier ;
+
+qualified_id = [nested_name_specifier] unqualified_id
+ | identifier ;
+
+identifier = ? clang::tok::identifier ? ;
+
+numeric_literal = ? clang::tok::numeric_constant ? ;
+
+boolean_literal = "true" | "false" ;
+
+pointer_literal = "nullptr" ;

labath wrote:

true/false/nullptr are also spelled differently in different languages, and I 
don't believe they're necessary for the basic "frame variable" implementation. 
Can we skip those for now?

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }

labath wrote:

What would you say to deleting this and using `optional` instead?

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }
+};
+
+/// This determines if the type is a shared, unique or weak pointer, either
+/// from stdlibc++ or libc+++.
+bool IsSmartPtrType(CompilerType type);
+
+/// Finds the member field with the given name and type, stores the child index
+/// corresponding to the field in the idx vector and returns a DILMemberInfo
+/// struct with appropriate information about the field.
+DILMemberInfo GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp,
+CompilerType type,
+const std::string ,
+std::vector *idx,
+CompilerType empty_type,
+bool use_synthetic, bool is_dynamic);
+
+std::tuple>
+GetMemberInfo(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+  const std::string , bool use_synthetic);
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed, acquiring the process & target locks if
+/// appropriate.
+lldb::ValueObjectSP
+DILGetSPWithLock(lldb::ValueObjectSP valobj_sp,
+ lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+ bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class DILNodeKind {
+  kDILErrorNode,
+  kLiteralNode,
+  kIdentifierNode,
+  kBuiltinFunctionCallNode,
+  kCStyleCastNode,
+  kMemberOfNode,
+  kArraySubscriptNode,
+  kUnaryOpNode,
+  kSmartPtrToPtrDecay
+};
+
+/// The C-Style casts allowed by DIL.
+enum class CStyleCastKind {
+  kArithmetic,
+  kEnumeration,
+  kPointer,
+  kNullptr,
+  kReference,
+};
+
+/// The Unary operators recognized by DIL.
+enum class UnaryOpKind {
+  AddrOf, // "&"
+  Deref,  // "*"
+  Minus,  // "-"
+};
+
+/// Given a string representing a type, returns the CompilerType corresponding
+/// to the named type, if it exists.
+CompilerType
+ResolveTypeByName(const std::string ,
+  std::shared_ptr ctx_scope);
+
+/// Quick lookup to check if a type name already exists in a
+/// name-to-CompilerType map the DIL parser keeps of previously found
+/// name/type pairs.
+bool IsContextVar(const std::string );
+
+/// Checks to see if the CompilerType is a Smart Pointer (shared, unique, weak)
+/// or not. Only applicable for C++, which is why this is here and not part of
+/// the CompilerType class.
+bool IsSmartPtrType(CompilerType type);
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+private:
+  using MemberPath = std::vector;
+  using IdentifierInfoPtr = std::unique_ptr;
+
+public:
+  enum class Kind {
+kValue,
+kContextArg,
+kMemberPath,
+kThisKeyword,
+  };
+
+  static IdentifierInfoPtr FromValue(lldb::ValueObjectSP value_sp) {
+CompilerType type;
+lldb::ValueObjectSP value = DILGetSPWithLock(value_sp);
+if (value)
+  type = value->GetCompilerType();
+return IdentifierInfoPtr(new IdentifierInfo(Kind::kValue, type, value, 
{}));
+  }
+
+  static IdentifierInfoPtr FromContextArg(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(
+new IdentifierInfo(Kind::kContextArg, type, empty_value, {}));
+  }
+
+  static IdentifierInfoPtr FromMemberPath(CompilerType type, MemberPath path) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(new IdentifierInfo(Kind::kMemberPath, type,
+empty_value, std::move(path)));
+  }
+
+  static IdentifierInfoPtr FromThisKeyword(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return IdentifierInfoPtr(
+new 

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,43 @@
+(* LLDB Debug Expressions, a subset of C++ *)
+(* Insired by https://www.nongnu.org/hcb *)
+
+expression = cast_expression;
+
+cast_expression = unary_expression;
+
+unary_expression = postfix_expression
+ | unary_operator cast_expression;
+
+unary_operator = "*" | "&" | "-" ;
+
+postfix_expression = primary_expression
+   | postfix_expression "[" expression "]"
+   | postfix_expression "." id_expression
+   | postfix_expression "->" id_expression
+
+primary_expression = numeric_literal
+   | boolean_literal
+   | pointer_literal
+   | id_expression
+   | "this" ;

labath wrote:

Other languages may use a different keyword (`self` typically) to refer to 
"this" object (in which case `this` can refer to a regular variable). 
Special-casing `this` in the parser could make that difficult, so it may be 
better to treat `this` as a regular identifier, and only resolve it later, once 
we know which language we're in.

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct DILMemberInfo {
+  std::optional name;
+  CompilerType type;
+  bool is_bitfield;
+  uint32_t bitfield_size_in_bits;
+  bool is_synthetic;
+  bool is_dynamic;
+  lldb::ValueObjectSP val_obj_sp;
+
+  explicit operator bool() const { return type.IsValid(); }
+};
+
+/// This determines if the type is a shared, unique or weak pointer, either
+/// from stdlibc++ or libc+++.
+bool IsSmartPtrType(CompilerType type);
+
+/// Finds the member field with the given name and type, stores the child index
+/// corresponding to the field in the idx vector and returns a DILMemberInfo
+/// struct with appropriate information about the field.
+DILMemberInfo GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp,
+CompilerType type,
+const std::string ,
+std::vector *idx,
+CompilerType empty_type,
+bool use_synthetic, bool is_dynamic);
+
+std::tuple>
+GetMemberInfo(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+  const std::string , bool use_synthetic);
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed, acquiring the process & target locks if
+/// appropriate.
+lldb::ValueObjectSP
+DILGetSPWithLock(lldb::ValueObjectSP valobj_sp,
+ lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+ bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class DILNodeKind {
+  kDILErrorNode,
+  kLiteralNode,
+  kIdentifierNode,
+  kBuiltinFunctionCallNode,
+  kCStyleCastNode,
+  kMemberOfNode,
+  kArraySubscriptNode,
+  kUnaryOpNode,
+  kSmartPtrToPtrDecay
+};
+
+/// The C-Style casts allowed by DIL.
+enum class CStyleCastKind {
+  kArithmetic,
+  kEnumeration,
+  kPointer,
+  kNullptr,
+  kReference,
+};
+
+/// The Unary operators recognized by DIL.
+enum class UnaryOpKind {
+  AddrOf, // "&"
+  Deref,  // "*"
+  Minus,  // "-"
+};
+
+/// Given a string representing a type, returns the CompilerType corresponding
+/// to the named type, if it exists.
+CompilerType
+ResolveTypeByName(const std::string ,
+  std::shared_ptr ctx_scope);
+
+/// Quick lookup to check if a type name already exists in a
+/// name-to-CompilerType map the DIL parser keeps of previously found
+/// name/type pairs.
+bool IsContextVar(const std::string );
+
+/// Checks to see if the CompilerType is a Smart Pointer (shared, unique, weak)
+/// or not. Only applicable for C++, which is why this is here and not part of
+/// the CompilerType class.
+bool IsSmartPtrType(CompilerType type);
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+private:
+  using MemberPath = std::vector;
+  using IdentifierInfoPtr = std::unique_ptr;

labath wrote:

elsewhere, we'd use IdentifierInfoUP

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,468 @@
+//===-- DILAST.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/DILAST.h"
+#include "lldb/API/SBType.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/RegisterContext.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
+namespace lldb_private {
+
+lldb::ValueObjectSP DILGetSPWithLock(lldb::ValueObjectSP in_valobj_sp,
+ lldb::DynamicValueType use_dynamic,
+ bool use_synthetic) {
+  Process::StopLocker stop_locker;
+  std::unique_lock lock;
+  Status error;
+
+  if (!in_valobj_sp) {
+error.SetErrorString("invalid value object");
+return in_valobj_sp;
+  }
+
+  lldb::ValueObjectSP value_sp = in_valobj_sp;
+
+  Target *target = value_sp->GetTargetSP().get();
+  // If this ValueObject holds an error, then it is valuable for that.
+  if (value_sp->GetError().Fail())
+return value_sp;
+
+  if (!target)
+return lldb::ValueObjectSP();
+
+  lock = std::unique_lock(target->GetAPIMutex());
+
+  lldb::ProcessSP process_sp(value_sp->GetProcessSP());
+  if (process_sp && !stop_locker.TryLock(_sp->GetRunLock())) {
+// We don't allow people to play around with ValueObject if the process
+// is running. If you want to look at values, pause the process, then
+// look.
+error.SetErrorString("process must be stopped.");
+return lldb::ValueObjectSP();
+  }
+
+  if (use_dynamic != lldb::eNoDynamicValues) {
+lldb::ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(use_dynamic);
+if (dynamic_sp)
+  value_sp = dynamic_sp;
+  }
+
+  if (use_synthetic) {
+lldb::ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue();
+if (synthetic_sp)
+  value_sp = synthetic_sp;
+  }
+
+  if (!value_sp)
+error.SetErrorString("invalid value object");
+
+  return value_sp;
+}
+
+CompilerType DILASTNode::result_type_deref() const {
+  auto type = result_type();
+  return type.IsReferenceType() ? type.GetNonReferenceType() : type;
+}
+
+static std::unordered_map context_args;
+
+bool IsContextVar(const std::string ) {
+  return context_args.find(name) != context_args.end();
+}

labath wrote:

This definitely needs to be scoped to something (and not a global), but since 
its not used right now, let's remove it.

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,446 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DIL_AST_H_
+#define LLDB_DIL_AST_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+
+namespace lldb_private {

labath wrote:

Some of the names defined here have embedded namespace names (DILMemberInfo), 
while others (IdentifierInfo) have deceptively generic name even though they're 
specific to the DIL. What do you (all) think about putting all of this in a 
namespace (I suggest `dil`)?

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

> BTW, I have verified that this stripped down version passes all the frame 
> variable tests in LLDB.

That's cool. Just to confirm, have you looked at replacing `target variable` as 
well? It uses the same language as "frame var" under the hood, which  but it 
has a somewhat different starting point (it basically ignores the local scope 
and looks only at globals), which means it may need some special handling in 
the new parser.

> I agree with Jim re the DIL language: We should only have a single language 
> definition, and it can be a superset of the languages it supports. So there 
> may be parts of it that belong to particular languages, but that does not 
> mean it supports those languages exclusively.

This direction makes sense to me, but I think that's all the more reason to be 
conservative/cautious about adding new features to the language. We can't just 
put every possible feature of every language into it, as we'd end up with a 
unmaintainable mess.

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Put the new debugger in synchronous mode in TestGlobalModuleCache (PR #98041)

2024-07-09 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/98041
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Put the new debugger in synchronous mode in TestGlobalModuleCache (PR #98041)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98041
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Put the new debugger in synchronous mode in TestGlobalModuleCache (PR #98041)

2024-07-08 Thread Pavel Labath via lldb-commits

labath wrote:

This explains why the `test_OneTargetTwoDebuggers` flavour of the test was 
crashing. It doesn't explain why would the other two versions crash. However 
I'm not sure if the other two flavours are indeed crashing (all of the crashes 
I've seen were in this flavour).

https://github.com/llvm/llvm-project/pull/98041
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Put the new debugger in synchronous mode in TestGlobalModuleCache (PR #98041)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98041
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Put the new debugger in synchronous mode in TestGlobalModuleCache (PR #98041)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/98041

In async mode, the test terminates sooner that it should 
(`run_to_source_breakpoint` does not work in this mode), and then the test 
crashes due to #98038. Most of the time, the test does not fail because its 
already XFAILed, but the crash still registers as a failure.

>From afe79a9ccc3f08eb145ad41f5f690d3f4e1e719e Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 8 Jul 2024 18:08:45 +0200
Subject: [PATCH] [lldb] Put the new debugger in synchronous mode in
 TestGlobalModuleCache

In async mode, the test terminates sooner that it should
(`run_to_source_breakpoint` does not work in this mode), and then the
test crashes due to #98038. Most of the time, the test does not fail
because its already XFAILed, but the crash still registers as a failure.
---
 .../API/python_api/global_module_cache/TestGlobalModuleCache.py  | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py 
b/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
index 0942dcd655b750..ccefc28946e062 100644
--- a/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
+++ b/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
@@ -111,6 +111,7 @@ def do_test(self, one_target, one_debugger):
 else:
 if one_target:
 new_debugger = lldb.SBDebugger().Create()
+new_debugger.SetAsync(False)
 self.old_debugger = self.dbg
 self.dbg = new_debugger
 

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


[Lldb-commits] [lldb] [llvm] [lldb] [lldb-server] Introduce Host/qnx and NativeProcessQNX (PR #97630)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -36,10 +38,17 @@ using namespace lldb;
 using namespace lldb_private;
 
 static sig_atomic_t g_signal_flags[NSIG];
+#if defined(__QNX__)
+static llvm::DenseMap g_signal_info;

labath wrote:

This code is definitely not async-signal-safe. However, before you try to fix 
that, I'd like clear up a couple of things:
1. Is this absolutely necessary? I see you're using this to get the PID of the 
process that stopped, but is that the only way to determine that? Could you 
e.g. iterate through the list of processes (you will only likely have one or 
two, so it's not like it will be slow) and query each one? (that's how it's 
done on linux)
2. Is this guaranteed to work all the time? For example, what happens if you're 
debugging two processes and they both stop simultaneously. Does the OS 
guarantee to deliver two signals or can it coalesce them? (technically, I 
believe linux is allowed to do that, which is why this implementation does not 
rely on the contents/number of signals, just their presence. (plus it's also 
convenient to not have to worry about how to capture multiple instances of one 
signal safely)

https://github.com/llvm/llvm-project/pull/97630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] [lldb] [lldb-server] Introduce Host/qnx and NativeProcessQNX (PR #97630)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -222,6 +232,148 @@ struct ForkLaunchInfo {
 
 // End of code running in the child process.
 
+#if defined(__QNX__)
+static ::pid_t PosixSpawn(const ForkLaunchInfo ,

labath wrote:

This is basically reimplementing `LaunchProcessPosixSpawn` inside 
`ProcessLauncherPosixFork.cpp`. Instead of shoehorning the `posix_spawn` 
implementation into ProcessLauncherPosix**Fork** (I assume you're doing this 
because your os doesn't support `fork`), it would be better to create a 
separate ProcessLauncher subclass for that. If there are things that can be 
meaningfully shared between the two implementations, they can go into a 
separate base class (but I expect that there's more sharing potential with the 
aforementioned (currently, mac-specific) `LaunchProcessPosixSpawn` 
implementation.

https://github.com/llvm/llvm-project/pull/97630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] [lldb] [lldb-server] Introduce Host/qnx and NativeProcessQNX (PR #97630)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] [lldb] [lldb-server] Introduce Host/qnx and NativeProcessQNX (PR #97630)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

This is only a very light review. I agree with others that an RFC thread is a 
better place to discuss this. After we sort that out, this patch should be 
further subdivided into smaller pieces.

https://github.com/llvm/llvm-project/pull/97630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] [lldb] [lldb-server] Introduce Host/qnx and NativeProcessQNX (PR #97630)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -35,6 +39,10 @@
 #include 
 #endif
 
+#if !defined(EOK)

labath wrote:

Everything else just uses zero (`0`) as "no error". I don't see a reason for 
this code to be different.

https://github.com/llvm/llvm-project/pull/97630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -56,6 +56,10 @@ def target_is_android():
 return configuration.lldb_platform_name == "remote-android"
 
 
+def target_is_remote_linux():
+return configuration.lldb_platform_name == "remote-linux"
+
+

labath wrote:

I guess this isn't used anymore.

https://github.com/llvm/llvm-project/pull/96654
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -92,11 +96,22 @@ def match_android_device(device_arch, valid_archs=None, 
valid_api_levels=None):
 
 
 def finalize_build_dictionary(dictionary):
+if dictionary is None:
+dictionary = {}
 if target_is_android():
-if dictionary is None:
-dictionary = {}
 dictionary["OS"] = "Android"
 dictionary["PIE"] = 1
+elif platformIsDarwin():
+dictionary["OS"] = "Darwin"
+else:
+# Provide uname-like platform name
+platform_name_to_uname = { "linux": "Linux",
+"netbsd": "NetBSD",
+"freebsd": "FreeBSD",
+"windows": "Windows_NT",
+}
+dictionary["OS"] = platform_name_to_uname[getPlatform()]
+dictionary["HOST_OS"] = platform_name_to_uname[getHostPlatform()]

labath wrote:

Why is this under the `else:` branch? I would expect we need to set `HOST_OS` 
even if we're targetting android or darwin...

https://github.com/llvm/llvm-project/pull/96654
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/97579
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Fix type error when calling random.randrange with 'float' arg (PR #97328)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Fix type error when calling random.randrange with 'float' arg (PR #97328)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/97328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/96685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/96685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97754
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/97754
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -246,3 +270,119 @@ 
lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEndCreator(
   return (valobj_sp ? new LibcxxStdUnorderedMapSyntheticFrontEnd(valobj_sp)
 : nullptr);
 }
+
+lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+: SyntheticChildrenFrontEnd(*valobj_sp) {
+  if (valobj_sp)
+Update();
+}
+
+lldb::ChildCacheState lldb_private::formatters::
+LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
+  m_pair_sp.reset();
+
+  ValueObjectSP valobj_sp = m_backend.GetSP();
+  if (!valobj_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  TargetSP target_sp(valobj_sp->GetTargetSP());
+
+  if (!target_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  // Get the unordered_map::iterator
+  // m_backend is an 'unordered_map::iterator', aka a
+  // '__hash_map_iterator<__hash_table::iterator>'
+  //
+  // __hash_map_iterator::__i_ is a __hash_table::iterator (aka
+  // __hash_iterator<__node_pointer>)
+  auto hash_iter_sp = valobj_sp->GetChildMemberWithName("__i_");
+  if (!hash_iter_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  // Type is '__hash_iterator<__node_pointer>'
+  auto hash_iter_type = hash_iter_sp->GetCompilerType();
+  if (!hash_iter_type.IsValid())
+return lldb::ChildCacheState::eRefetch;
+
+  // Type is '__node_pointer'
+  auto node_pointer_type = hash_iter_type.GetTypeTemplateArgument(0);
+  if (!node_pointer_type.IsValid())
+return lldb::ChildCacheState::eRefetch;
+
+  // Cast the __hash_iterator to a __node_pointer (which stores our key/value
+  // pair)
+  auto hash_node_sp = hash_iter_sp->Cast(node_pointer_type);
+  if (!hash_node_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  auto key_value_sp = hash_node_sp->GetChildMemberWithName("__value_");
+  if (!key_value_sp) {
+// clang-format off
+// Since D101206 (ba79fb2e1f), libc++ wraps the `__value_` in an
+// anonymous union.
+// Child 0: __hash_node_base base class
+// Child 1: __hash_
+// Child 2: anonymous union
+// clang-format on
+auto anon_union_sp = hash_node_sp->GetChildAtIndex(2);
+if (!anon_union_sp)
+  return lldb::ChildCacheState::eRefetch;
+
+key_value_sp = anon_union_sp->GetChildMemberWithName("__value_");
+if (!key_value_sp)
+  return lldb::ChildCacheState::eRefetch;
+  }
+
+  // Create the synthetic child, which is a pair where the key and value can be
+  // retrieved // by querying the synthetic frontend for

labath wrote:

remove extra `//`

https://github.com/llvm/llvm-project/pull/97754
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Move std::unordered_map::iterator formatter into LibCxxUnorderedMap.cpp (PR #97752)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/97752
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -456,3 +477,97 @@ 
lldb_private::formatters::LibcxxStdMapSyntheticFrontEndCreator(
 CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
   return (valobj_sp ? new LibcxxStdMapSyntheticFrontEnd(valobj_sp) : nullptr);
 }
+
+lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
+LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+: SyntheticChildrenFrontEnd(*valobj_sp) {
+  if (valobj_sp)
+Update();
+}
+
+lldb::ChildCacheState
+lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
+  m_pair_sp.reset();
+
+  ValueObjectSP valobj_sp = m_backend.GetSP();
+  if (!valobj_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  TargetSP target_sp(valobj_sp->GetTargetSP());
+  if (!target_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  auto tree_iter_sp = valobj_sp->GetChildMemberWithName("__i_");
+  if (!tree_iter_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  auto node_pointer_type =
+  tree_iter_sp->GetCompilerType().GetDirectNestedTypeWithName(
+  "__node_pointer");
+  if (!node_pointer_type.IsValid())
+return lldb::ChildCacheState::eRefetch;
+
+  auto iter_pointer_sp = tree_iter_sp->GetChildMemberWithName("__ptr_");
+  if (!iter_pointer_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  auto node_pointer_sp = iter_pointer_sp->Cast(node_pointer_type);
+  if (!node_pointer_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  Status err;
+  node_pointer_sp = node_pointer_sp->Dereference(err);
+  if (!node_pointer_sp || err.Fail())
+return lldb::ChildCacheState::eRefetch;
+
+  auto key_value_sp = node_pointer_sp->GetChildMemberWithName("__value_");
+  if (!key_value_sp)
+return lldb::ChildCacheState::eRefetch;
+
+  key_value_sp = key_value_sp->Clone(ConstString("name"));
+  if (key_value_sp->GetNumChildrenIgnoringErrors() == 1) {
+auto child0_sp = key_value_sp->GetChildAtIndex(0);
+if (child0_sp &&
+(child0_sp->GetName() == "__cc_" || child0_sp->GetName() == "__cc"))
+  key_value_sp = child0_sp->Clone(ConstString("pair"));
+  }
+
+  m_pair_sp = key_value_sp;
+
+  return lldb::ChildCacheState::eRefetch;
+}
+
+llvm::Expected lldb_private::formatters::
+LibCxxMapIteratorSyntheticFrontEnd::CalculateNumChildren() {
+  return 2;
+}
+
+lldb::ValueObjectSP
+lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::GetChildAtIndex(
+uint32_t idx) {
+  if (!m_pair_sp)
+return nullptr;
+
+  return m_pair_sp->GetChildAtIndex(idx);
+}
+
+bool lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
+MightHaveChildren() {
+  return true;
+}
+
+size_t lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
+GetIndexOfChildWithName(ConstString name) {
+  if (name == "first")

labath wrote:

Since `GetChildAtIndex` is forwarding to m_pair_sp, could this do the same as 
well?

https://github.com/llvm/llvm-project/pull/97713
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/97713
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97713
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove Listener::SetShadow (PR #97555)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97555
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make variant formatter work with libstdc++-14 (PR #97568)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97568
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -55,14 +55,23 @@ def test_basic(self):
 self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets")
 
 # Verify the target names are correct.
-self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()")
-self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect 
bar2()")
-self.assertEqual(
-step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, 
int)"
-)
+# The order of funcA and funcB may change depending on the compiler 
ABI.
+funcA_target = None
+funcB_target = None
+for target in step_in_targets[0:2]:
+if "funcB" in target["label"]:
+funcB_target = target
+elif "funcA" in target["label"]:
+funcA_target = target
+else:
+self.fail(f"Unexpected step in target: {target}")
+
+self.assertIsNotNone(funcA_target, "expect funcA")
+self.assertIsNotNone(funcB_target, "expect funcB")
+self.assertIn("foo", step_in_targets[2]["label"], "expect foo")
 
-# Choose to step into second target and verify that we are in bar2()
-self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], 
waitForStop=True)
+# Choose to step into second target and verify that we are in funcB()

labath wrote:

funcB is not necessarily going to be the second target, so this comment is 
incorrect. However, instead of changing the comment, I think it'd be actually 
better to change the code so that it steps into the second function, regardless 
of what that function might be. This is because we would end up in the first 
function even if we did a "normal" step in operation rather than the fancy 
targetted one. Stepping into the second function verifies that we can correctly 
ignore the first function that gets called. (I wasn't the person who wrote the 
test, but I suspect this is roughly the motivation for it being written the way 
it is).

So basically, you could keep the `targetId=step_in_targets[1]["id"]` thing in 
the line below, but then change the expectation on line 77 to expect the 
function that we tried stepping into.

https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/95959
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Clean up LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair (PR #97551)

2024-07-08 Thread Pavel Labath via lldb-commits

labath wrote:

Why do we even have the `GetValueOffset` function? Wouldn't it be possible to 
dredge the actual type describing the node layout from somewhere (a template 
argument of something, or a return value of some method?)

https://github.com/llvm/llvm-project/pull/97551
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.

I'd say eight years is a sufficient compatibility window. :)

https://github.com/llvm/llvm-project/pull/97549
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make variant formatter work with libstdc++-14 (PR #97568)

2024-07-08 Thread Pavel Labath via lldb-commits

labath wrote:

> LGTM
> 
> Been wondering if we should follow the same principle as we did with 
> `std::string` and simulate the layouts for all the STL types, keeping a 
> record of them as they change. So we know we don't break the old layouts. Of 
> course it has a separate maintenance cost

Yeah, I think the biggest hurdle is the initial work in getting a stripped down 
version of the class in place. The problem is that the container types use a 
lot of template metaprogramming to implement various optimizations like empty 
base class and whatnot. Copying that verbatim would make the test huge (and in 
the case of libstdc++ its probably not even possible), and recreating it from 
scratch is not trivial. I believe that after I wrote the std::string test this 
way I tried to also do the same for some of the container types (it may have 
actually been variant), and just gave up after I saw hold long it would take.

Nonetheless, I think there's definitely value in tests like this (for one, 
going through the implementation in this way makes it obvious how many corner 
cases need testing), and it's nice to see there's interest for these tests. 
I'll keep that in mind when working on future changes.

https://github.com/llvm/llvm-project/pull/97568
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make variant formatter work with libstdc++-14 (PR #97568)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/97568

>From 53b9fda6f7bf0ec4df32869c9d4ba2203aa1870a Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 3 Jul 2024 12:20:12 +
Subject: [PATCH 1/2] [lldb] Make variant formatter work with libstdc++-14

In this version the internal data member has grown an additional
template parameter (bool), which was throwing the summary provider off.

This patch uses the type of the entire variant object. This is part of
the API/ABI, so it should be more stable, but it means we have to
explicitly strip typedefs and references to get to the interesting bits,
which is why I've extended the test case with examples of those.
---
 lldb/examples/synthetic/gnu_libstdcpp.py  |  7 +--
 .../TestDataFormatterLibStdcxxVariant.py  | 20 ++-
 .../libstdcpp/variant/main.cpp|  5 +
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index f778065aaca377..59970574a36040 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -914,12 +914,15 @@ def get_variant_npos_value(index_byte_size):
 if index == npos_value:
 return " No Value"
 
+# Strip references and typedefs.
+variant_type = raw_obj.GetType().GetCanonicalType().GetDereferencedType();
+template_arg_count = variant_type.GetNumberOfTemplateArguments()
+
 # Invalid index can happen when the variant is not initialized yet.
-template_arg_count = data_obj.GetType().GetNumberOfTemplateArguments()
 if index >= template_arg_count:
 return " "
 
-active_type = data_obj.GetType().GetTemplateArgumentType(index)
+active_type = variant_type.GetTemplateArgumentType(index)
 return f" Active Type = {active_type.GetDisplayTypeName()} "
 
 
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
index ba1641888b6f30..05f31087a566e8 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
@@ -21,15 +21,17 @@ def test_with_run_command(self):
 
 lldbutil.continue_to_breakpoint(self.process, bkpt)
 
-self.expect(
-"frame variable v1",
-substrs=["v1 =  Active Type = int  {", "Value = 12", "}"],
-)
-
-self.expect(
-"frame variable v1_ref",
-substrs=["v1_ref =  Active Type = int : {", "Value = 12", "}"],
-)
+for name in ["v1", "v1_typedef"]:
+  self.expect(
+  "frame variable " + name,
+  substrs=[name + " =  Active Type = int  {", "Value = 12", "}"],
+  )
+
+for name in ["v1_ref", "v1_typedef_ref"]:
+  self.expect(
+  "frame variable " + name,
+  substrs=[name + " =  Active Type = int : {", "Value = 12", "}"],
+  )
 
 self.expect(
 "frame variable v_v1",
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
index 545318f9358b67..36e0f74f831f8a 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
@@ -14,6 +14,10 @@ int main() {
 
   std::variant v1;
   std::variant _ref = v1;
+  using V1_typedef = std::variant;
+  V1_typedef v1_typedef;
+  V1_typedef _typedef_ref = v1_typedef;
+
   std::variant v2;
   std::variant v3;
   std::variant> v_v1;
@@ -43,6 +47,7 @@ int main() {
   v_many_types_no_value;
 
   v1 = 12; // v contains int
+  v1_typedef = v1;
   v_v1 = v1;
   int i = std::get(v1);
   printf("%d\n", i); // break here

>From 3c40961b34f641ea6c509feebad0719f9f87e970 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 8 Jul 2024 10:41:43 +0200
Subject: [PATCH 2/2] fix formatting

---
 lldb/examples/synthetic/gnu_libstdcpp.py |  2 +-
 .../variant/TestDataFormatterLibStdcxxVariant.py | 16 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index 59970574a36040..d98495b8a9df38 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -915,7 +915,7 @@ def get_variant_npos_value(index_byte_size):
 return " No Value"
 
 # Strip references and typedefs.
-variant_type = 

[Lldb-commits] [lldb] [lldb] Remove Listener::SetShadow (PR #97555)

2024-07-08 Thread Pavel Labath via lldb-commits


@@ -18,13 +18,10 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Listener::Listener(const char *name)
-: m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex(), m_is_shadow() {
+Listener::Listener(const char *name) : m_name(name) {
   Log *log = GetLog(LLDBLog::Object);
-  if (log != nullptr)
-LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
-  m_name.c_str());
+  LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
+m_name.c_str());

labath wrote:

Done. I though about it myself, but I had to stop with the drive-by changes 
somewhere.

https://github.com/llvm/llvm-project/pull/97555
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove Listener::SetShadow (PR #97555)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/97555

>From 9f81f804c4c451a5ff2266405740d208dffae60f Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 3 Jul 2024 10:37:52 +
Subject: [PATCH 1/2] [lldb] Remove Listener::SetShadow

It's not used since https://reviews.llvm.org/D157556.
---
 lldb/include/lldb/Utility/Listener.h | 3 ---
 lldb/source/API/SBAttachInfo.cpp | 8 +---
 lldb/source/API/SBLaunchInfo.cpp | 8 +---
 lldb/source/Utility/Listener.cpp | 9 +++--
 4 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/lldb/include/lldb/Utility/Listener.h 
b/lldb/include/lldb/Utility/Listener.h
index daa7deb345f301..f687852d558bdb 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -94,8 +94,6 @@ class Listener : public 
std::enable_shared_from_this {
 
   size_t HandleBroadcastEvent(lldb::EventSP _sp);
 
-  void SetShadow(bool is_shadow) { m_is_shadow = is_shadow; }
-
 private:
   // Classes that inherit from Listener can see and modify these
   struct BroadcasterInfo {
@@ -132,7 +130,6 @@ class Listener : public 
std::enable_shared_from_this {
   std::mutex m_events_mutex; // Protects m_broadcasters and m_events
   std::condition_variable m_events_condition;
   broadcaster_manager_collection m_broadcaster_managers;
-  bool m_is_shadow = false;
 
   void BroadcasterWillDestruct(Broadcaster *);
 
diff --git a/lldb/source/API/SBAttachInfo.cpp b/lldb/source/API/SBAttachInfo.cpp
index 8ce1f1d65c4964..a9f712c79c7fe0 100644
--- a/lldb/source/API/SBAttachInfo.cpp
+++ b/lldb/source/API/SBAttachInfo.cpp
@@ -266,13 +266,7 @@ SBListener SBAttachInfo::GetShadowListener() {
 void SBAttachInfo::SetShadowListener(SBListener ) {
   LLDB_INSTRUMENT_VA(this, listener);
 
-  ListenerSP listener_sp = listener.GetSP();
-  if (listener_sp && listener.IsValid())
-listener_sp->SetShadow(true);
-  else
-listener_sp = nullptr;
-
-  m_opaque_sp->SetShadowListener(listener_sp);
+  m_opaque_sp->SetShadowListener(listener.GetSP());
 }
 
 const char *SBAttachInfo::GetScriptedProcessClassName() const {
diff --git a/lldb/source/API/SBLaunchInfo.cpp b/lldb/source/API/SBLaunchInfo.cpp
index d5f935083e6c1e..d6b52e8a67a49e 100644
--- a/lldb/source/API/SBLaunchInfo.cpp
+++ b/lldb/source/API/SBLaunchInfo.cpp
@@ -402,11 +402,5 @@ SBListener SBLaunchInfo::GetShadowListener() {
 void SBLaunchInfo::SetShadowListener(SBListener ) {
   LLDB_INSTRUMENT_VA(this, listener);
 
-  ListenerSP listener_sp = listener.GetSP();
-  if (listener_sp && listener.IsValid())
-listener_sp->SetShadow(true);
-  else
-listener_sp = nullptr;
-
-  m_opaque_sp->SetShadowListener(listener_sp);
+  m_opaque_sp->SetShadowListener(listener.GetSP());
 }
diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index 0b28cb5cdc6424..317525335f0f6f 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -18,13 +18,10 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Listener::Listener(const char *name)
-: m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex(), m_is_shadow() {
+Listener::Listener(const char *name) : m_name(name) {
   Log *log = GetLog(LLDBLog::Object);
-  if (log != nullptr)
-LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
-  m_name.c_str());
+  LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
+m_name.c_str());
 }
 
 Listener::~Listener() {

>From 7259eab8b5660df7c57d18392e9d14cb5c877f7a Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 8 Jul 2024 10:32:48 +0200
Subject: [PATCH 2/2] rid of log

---
 lldb/source/Utility/Listener.cpp | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index 317525335f0f6f..5bbe6908afd2a6 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -19,18 +19,15 @@ using namespace lldb;
 using namespace lldb_private;
 
 Listener::Listener(const char *name) : m_name(name) {
-  Log *log = GetLog(LLDBLog::Object);
-  LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
-m_name.c_str());
+  LLDB_LOGF(GetLog(LLDBLog::Object), "%p Listener::Listener('%s')",
+static_cast(this), m_name.c_str());
 }
 
 Listener::~Listener() {
-  Log *log = GetLog(LLDBLog::Object);
-
   // Don't call Clear() from here as that can cause races. See #96750.
 
-  LLDB_LOGF(log, "%p Listener::%s('%s')", static_cast(this),
-__FUNCTION__, m_name.c_str());
+  LLDB_LOGF(GetLog(LLDBLog::Object), "%p Listener::%s('%s')",
+static_cast(this), __FUNCTION__, m_name.c_str());
 }
 
 void Listener::Clear() {

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


[Lldb-commits] [lldb] [lldb] Improve error message for unrecognized executables (PR #97490)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/97490
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] fix issue with debug-types-dwo-cross-reference.cpp.tmp test (PR #97381)

2024-07-08 Thread Pavel Labath via lldb-commits

labath wrote:

The test doesn't fail at HEAD per 
https://github.com/llvm/llvm-project/issues/97380#issuecomment-2205201579

https://github.com/llvm/llvm-project/pull/97381
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] fix issue with debug-types-dwo-cross-reference.cpp.tmp test (PR #97381)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97381
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Remove duplicate testcase names in API test-suite (PR #97043)

2024-07-08 Thread Pavel Labath via lldb-commits

labath wrote:

> > > > LGTM. This has definitely come up in the past. If you feel motivated, 
> > > > I'm sure there must be a way to detect this issue in Python and we 
> > > > could have assert/warning/error that captures this at the dotest level.
> > > 
> > > 
> > > Agreed, making it part of `dotest` would be amazing. Maybe someone with 
> > > better python knowledge has some ideas (@medismailben @kastiglione ?). In 
> > > the meantime I'll have a think of how one might do that
> > 
> > 
> > I think instead of having this be part of our test-suite, I'd run it as 
> > part of PR testing in a GitHub action
> 
> The test suite would/will run as part of PR testing. Are you saying you would 
> **only** run it at PR time? Why wait and not find out at your test? We 
> already collect all the test names to run them as separate lit tests (similar 
> but different problem) and I bet we also already iterate over the tests 
> inside a file too to build the variants.

Yes, but we do that on a fully parsed representation of the class, where the 
name conflicts would not be visible. Michael's script constructs the ast of the 
python file, and walks that manually. In that sense this check does seem more 
of like a thing that a linter would do, but I don't know if there are existing 
linters that would flag this sort of thing..

https://github.com/llvm/llvm-project/pull/97043
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make Listener::m_broadcasters_mutex non-recursive (PR #97552)

2024-07-08 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97552
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make variant formatter work with libstdc++-14 (PR #97568)

2024-07-03 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/97568

In this version the internal data member has grown an additional template 
parameter (bool), which was throwing the summary provider off.

This patch uses the type of the entire variant object. This is part of the 
API/ABI, so it should be more stable, but it means we have to explicitly strip 
typedefs and references to get to the interesting bits, which is why I've 
extended the test case with examples of those.

>From 53b9fda6f7bf0ec4df32869c9d4ba2203aa1870a Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 3 Jul 2024 12:20:12 +
Subject: [PATCH] [lldb] Make variant formatter work with libstdc++-14

In this version the internal data member has grown an additional
template parameter (bool), which was throwing the summary provider off.

This patch uses the type of the entire variant object. This is part of
the API/ABI, so it should be more stable, but it means we have to
explicitly strip typedefs and references to get to the interesting bits,
which is why I've extended the test case with examples of those.
---
 lldb/examples/synthetic/gnu_libstdcpp.py  |  7 +--
 .../TestDataFormatterLibStdcxxVariant.py  | 20 ++-
 .../libstdcpp/variant/main.cpp|  5 +
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index f778065aaca37..59970574a3604 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -914,12 +914,15 @@ def get_variant_npos_value(index_byte_size):
 if index == npos_value:
 return " No Value"
 
+# Strip references and typedefs.
+variant_type = raw_obj.GetType().GetCanonicalType().GetDereferencedType();
+template_arg_count = variant_type.GetNumberOfTemplateArguments()
+
 # Invalid index can happen when the variant is not initialized yet.
-template_arg_count = data_obj.GetType().GetNumberOfTemplateArguments()
 if index >= template_arg_count:
 return " "
 
-active_type = data_obj.GetType().GetTemplateArgumentType(index)
+active_type = variant_type.GetTemplateArgumentType(index)
 return f" Active Type = {active_type.GetDisplayTypeName()} "
 
 
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
index ba1641888b6f3..05f31087a566e 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
@@ -21,15 +21,17 @@ def test_with_run_command(self):
 
 lldbutil.continue_to_breakpoint(self.process, bkpt)
 
-self.expect(
-"frame variable v1",
-substrs=["v1 =  Active Type = int  {", "Value = 12", "}"],
-)
-
-self.expect(
-"frame variable v1_ref",
-substrs=["v1_ref =  Active Type = int : {", "Value = 12", "}"],
-)
+for name in ["v1", "v1_typedef"]:
+  self.expect(
+  "frame variable " + name,
+  substrs=[name + " =  Active Type = int  {", "Value = 12", "}"],
+  )
+
+for name in ["v1_ref", "v1_typedef_ref"]:
+  self.expect(
+  "frame variable " + name,
+  substrs=[name + " =  Active Type = int : {", "Value = 12", "}"],
+  )
 
 self.expect(
 "frame variable v_v1",
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
index 545318f9358b6..36e0f74f831f8 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
@@ -14,6 +14,10 @@ int main() {
 
   std::variant v1;
   std::variant _ref = v1;
+  using V1_typedef = std::variant;
+  V1_typedef v1_typedef;
+  V1_typedef _typedef_ref = v1_typedef;
+
   std::variant v2;
   std::variant v3;
   std::variant> v_v1;
@@ -43,6 +47,7 @@ int main() {
   v_many_types_no_value;
 
   v1 = 12; // v contains int
+  v1_typedef = v1;
   v_v1 = v1;
   int i = std::get(v1);
   printf("%d\n", i); // break here

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


[Lldb-commits] [lldb] [lldb] Remove Listener::SetShadow (PR #97555)

2024-07-03 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/97555

It's not used since https://reviews.llvm.org/D157556.

>From 9f81f804c4c451a5ff2266405740d208dffae60f Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 3 Jul 2024 10:37:52 +
Subject: [PATCH] [lldb] Remove Listener::SetShadow

It's not used since https://reviews.llvm.org/D157556.
---
 lldb/include/lldb/Utility/Listener.h | 3 ---
 lldb/source/API/SBAttachInfo.cpp | 8 +---
 lldb/source/API/SBLaunchInfo.cpp | 8 +---
 lldb/source/Utility/Listener.cpp | 9 +++--
 4 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/lldb/include/lldb/Utility/Listener.h 
b/lldb/include/lldb/Utility/Listener.h
index daa7deb345f30..f687852d558bd 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -94,8 +94,6 @@ class Listener : public 
std::enable_shared_from_this {
 
   size_t HandleBroadcastEvent(lldb::EventSP _sp);
 
-  void SetShadow(bool is_shadow) { m_is_shadow = is_shadow; }
-
 private:
   // Classes that inherit from Listener can see and modify these
   struct BroadcasterInfo {
@@ -132,7 +130,6 @@ class Listener : public 
std::enable_shared_from_this {
   std::mutex m_events_mutex; // Protects m_broadcasters and m_events
   std::condition_variable m_events_condition;
   broadcaster_manager_collection m_broadcaster_managers;
-  bool m_is_shadow = false;
 
   void BroadcasterWillDestruct(Broadcaster *);
 
diff --git a/lldb/source/API/SBAttachInfo.cpp b/lldb/source/API/SBAttachInfo.cpp
index 8ce1f1d65c496..a9f712c79c7fe 100644
--- a/lldb/source/API/SBAttachInfo.cpp
+++ b/lldb/source/API/SBAttachInfo.cpp
@@ -266,13 +266,7 @@ SBListener SBAttachInfo::GetShadowListener() {
 void SBAttachInfo::SetShadowListener(SBListener ) {
   LLDB_INSTRUMENT_VA(this, listener);
 
-  ListenerSP listener_sp = listener.GetSP();
-  if (listener_sp && listener.IsValid())
-listener_sp->SetShadow(true);
-  else
-listener_sp = nullptr;
-
-  m_opaque_sp->SetShadowListener(listener_sp);
+  m_opaque_sp->SetShadowListener(listener.GetSP());
 }
 
 const char *SBAttachInfo::GetScriptedProcessClassName() const {
diff --git a/lldb/source/API/SBLaunchInfo.cpp b/lldb/source/API/SBLaunchInfo.cpp
index d5f935083e6c1..d6b52e8a67a49 100644
--- a/lldb/source/API/SBLaunchInfo.cpp
+++ b/lldb/source/API/SBLaunchInfo.cpp
@@ -402,11 +402,5 @@ SBListener SBLaunchInfo::GetShadowListener() {
 void SBLaunchInfo::SetShadowListener(SBListener ) {
   LLDB_INSTRUMENT_VA(this, listener);
 
-  ListenerSP listener_sp = listener.GetSP();
-  if (listener_sp && listener.IsValid())
-listener_sp->SetShadow(true);
-  else
-listener_sp = nullptr;
-
-  m_opaque_sp->SetShadowListener(listener_sp);
+  m_opaque_sp->SetShadowListener(listener.GetSP());
 }
diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index 0b28cb5cdc642..317525335f0f6 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -18,13 +18,10 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Listener::Listener(const char *name)
-: m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex(), m_is_shadow() {
+Listener::Listener(const char *name) : m_name(name) {
   Log *log = GetLog(LLDBLog::Object);
-  if (log != nullptr)
-LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
-  m_name.c_str());
+  LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
+m_name.c_str());
 }
 
 Listener::~Listener() {

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


[Lldb-commits] [lldb] [lldb] Make Listener::m_broadcasters_mutex non-recursive (PR #97552)

2024-07-03 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/97552

Follow-up to #97400.  No changes apart from changing the type were necessary. 
The mutex was already not used recursively.

>From d183b37fdc4cda4f76b33b768a42e43368c68464 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 3 Jul 2024 10:23:26 +
Subject: [PATCH] [lldb] Make Listener::m_broadcasters_mutex non-recursive

Follow-up to #97400.  No changes apart from changing the type were
necessary. The mutex was already not used recursively.
---
 lldb/include/lldb/Utility/Listener.h |  2 +-
 lldb/source/Utility/Listener.cpp | 21 -
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Utility/Listener.h 
b/lldb/include/lldb/Utility/Listener.h
index daa7deb345f30..eec8af023f263 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -127,7 +127,7 @@ class Listener : public 
std::enable_shared_from_this {
 
   std::string m_name;
   broadcaster_collection m_broadcasters;
-  std::recursive_mutex m_broadcasters_mutex; // Protects m_broadcasters
+  std::mutex m_broadcasters_mutex; // Protects m_broadcasters
   event_collection m_events;
   std::mutex m_events_mutex; // Protects m_broadcasters and m_events
   std::condition_variable m_events_condition;
diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index 0b28cb5cdc642..6265c30af0d18 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -38,8 +38,7 @@ Listener::~Listener() {
 
 void Listener::Clear() {
   Log *log = GetLog(LLDBLog::Object);
-  std::lock_guard broadcasters_guard(
-  m_broadcasters_mutex);
+  std::lock_guard broadcasters_guard(m_broadcasters_mutex);
   broadcaster_collection::iterator pos, end = m_broadcasters.end();
   for (pos = m_broadcasters.begin(); pos != end; ++pos) {
 Broadcaster::BroadcasterImplSP broadcaster_sp(pos->first.lock());
@@ -68,8 +67,7 @@ uint32_t Listener::StartListeningForEvents(Broadcaster 
*broadcaster,
 // Scope for "locker"
 // Tell the broadcaster to add this object as a listener
 {
-  std::lock_guard broadcasters_guard(
-  m_broadcasters_mutex);
+  std::lock_guard broadcasters_guard(m_broadcasters_mutex);
   Broadcaster::BroadcasterImplWP 
impl_wp(broadcaster->GetBroadcasterImpl());
   m_broadcasters.insert(
   std::make_pair(impl_wp, BroadcasterInfo(event_mask)));
@@ -99,8 +97,7 @@ uint32_t Listener::StartListeningForEvents(Broadcaster 
*broadcaster,
 // Scope for "locker"
 // Tell the broadcaster to add this object as a listener
 {
-  std::lock_guard broadcasters_guard(
-  m_broadcasters_mutex);
+  std::lock_guard broadcasters_guard(m_broadcasters_mutex);
   Broadcaster::BroadcasterImplWP 
impl_wp(broadcaster->GetBroadcasterImpl());
   m_broadcasters.insert(std::make_pair(
   impl_wp, BroadcasterInfo(event_mask, callback, callback_user_data)));
@@ -131,8 +128,7 @@ bool Listener::StopListeningForEvents(Broadcaster 
*broadcaster,
   if (broadcaster) {
 // Scope for "locker"
 {
-  std::lock_guard broadcasters_guard(
-  m_broadcasters_mutex);
+  std::lock_guard broadcasters_guard(m_broadcasters_mutex);
   m_broadcasters.erase(broadcaster->GetBroadcasterImpl());
 }
 // Remove the broadcaster from our set of broadcasters
@@ -147,8 +143,7 @@ bool Listener::StopListeningForEvents(Broadcaster 
*broadcaster,
 void Listener::BroadcasterWillDestruct(Broadcaster *broadcaster) {
   // Scope for "broadcasters_locker"
   {
-std::lock_guard broadcasters_guard(
-m_broadcasters_mutex);
+std::lock_guard broadcasters_guard(m_broadcasters_mutex);
 m_broadcasters.erase(broadcaster->GetBroadcasterImpl());
   }
 
@@ -322,7 +317,7 @@ bool Listener::GetEvent(EventSP _sp, const 
Timeout ) {
 
 size_t Listener::HandleBroadcastEvent(EventSP _sp) {
   size_t num_handled = 0;
-  std::lock_guard guard(m_broadcasters_mutex);
+  std::lock_guard guard(m_broadcasters_mutex);
   Broadcaster *broadcaster = event_sp->GetBroadcaster();
   if (!broadcaster)
 return 0;
@@ -357,7 +352,7 @@ Listener::StartListeningForEventSpec(const 
BroadcasterManagerSP _sp,
   // The BroadcasterManager mutex must be locked before m_broadcasters_mutex to
   // avoid violating the lock hierarchy (manager before broadcasters).
   std::lock_guard manager_guard(manager_sp->m_manager_mutex);
-  std::lock_guard guard(m_broadcasters_mutex);
+  std::lock_guard guard(m_broadcasters_mutex);
 
   uint32_t bits_acquired = manager_sp->RegisterListenerForEventsNoLock(
   this->shared_from_this(), event_spec);
@@ -379,7 +374,7 @@ bool Listener::StopListeningForEventSpec(const 
BroadcasterManagerSP _sp,
   // The BroadcasterManager mutex must be locked before m_broadcasters_mutex to
   // avoid violating the lock hierarchy (manager before broadcasters).
   std::lock_guard 

[Lldb-commits] [lldb] [lldb] Make Broadcaster mutexes non-recursive (PR #97400)

2024-07-03 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97400
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

2024-07-03 Thread Pavel Labath via lldb-commits

labath wrote:

> AssertionError: 'funcB' not found in 'funcA()' : expect funcB

The step targets are coming in different order. It's probably an ABI thing, as 
the compiler produces the calls in different order as well: 
https://godbolt.org/z/cPqhsWba6

I guess we need to adjust the test to accept both orders.

https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Fix type error when calling random.randrange with 'float' arg (PR #97328)

2024-07-03 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

2024-07-02 Thread Pavel Labath via lldb-commits

labath wrote:

This looks fine to me. @jeffreytan81  ?

https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Fix type error when calling random.randrange with 'float' arg (PR #97328)

2024-07-02 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Fix type error when calling random.randrange with 'float' arg (PR #97328)

2024-07-02 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

Looks good, but note that this file was moved (to 
`packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py`) 
recently, so you'll need to rebase this PR to HEAD.

https://github.com/llvm/llvm-project/pull/97328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Fix type error when calling random.randrange with 'float' arg (PR #97328)

2024-07-02 Thread Pavel Labath via lldb-commits


@@ -75,7 +75,7 @@ def __init__(self):
 class Pipe(object):
 def __init__(self, prefix):
 while True:
-self.name = "lldb-" + str(random.randrange(1e10))
+self.name = "lldb-" + str(random.randrange(int(1e10)))

labath wrote:

How about `10**10` ?

(As the "original author" ;), I'm not really sure what happened here. I'm 
pretty sure I did not write a huge blob of windows-specific code without 
testing it. It could be that newer python versions got more string about typing 
or sth...)

https://github.com/llvm/llvm-project/pull/97328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make Broadcaster mutexes non-recursive (PR #97400)

2024-07-02 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/97400

Non-recursive mutexes encourage better locking discipline and avoid bugs like 
#96750, where one can unexpectedly re-enter the critical section on the same 
thread, and interrupt a presumed-indivisible operation.

In this case, the only needed fix was to remove locking from some 
BroadcastManager functions, which were only called from the Listener class (and 
the listener already locked those mutexes to preserve lock ordering).

While doing that, I noticed we don't have unit tests for these functions, so I 
added one.

>From e9de8bde8e2e75e2558f1f14df31f796aa97069d Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Tue, 2 Jul 2024 08:11:38 +
Subject: [PATCH] [lldb] Make Broadcaster mutexes non-recursive

Non-recursive mutexes encourage better locking discipline and avoid bugs
like #96750, where one can unexpectedly re-enter the critical section on
the same thread, and interrupt a presumed-indivisible operation.

In this case, the only needed fix was to remove locking from some
BroadcastManager functions, which were only called from the Listener
class (and the listener already locked those mutexes to preserve lock
ordering).
---
 lldb/include/lldb/Utility/Broadcaster.h | 17 ++-
 lldb/source/Utility/Broadcaster.cpp | 33 ++---
 lldb/source/Utility/Listener.cpp| 12 
 lldb/unittests/Utility/ListenerTest.cpp | 39 +
 4 files changed, 70 insertions(+), 31 deletions(-)

diff --git a/lldb/include/lldb/Utility/Broadcaster.h 
b/lldb/include/lldb/Utility/Broadcaster.h
index 58436ddb9f26d..c6f63f1916573 100644
--- a/lldb/include/lldb/Utility/Broadcaster.h
+++ b/lldb/include/lldb/Utility/Broadcaster.h
@@ -87,12 +87,6 @@ class BroadcasterManager
 
   ~BroadcasterManager() = default;
 
-  uint32_t RegisterListenerForEvents(const lldb::ListenerSP _sp,
- const BroadcastEventSpec _spec);
-
-  bool UnregisterListenerForEvents(const lldb::ListenerSP _sp,
-   const BroadcastEventSpec _spec);
-
   lldb::ListenerSP
   GetListenerForEventSpec(const BroadcastEventSpec _spec) const;
 
@@ -105,13 +99,20 @@ class BroadcasterManager
   void Clear();
 
 private:
+  uint32_t
+  RegisterListenerForEventsNoLock(const lldb::ListenerSP _sp,
+  const BroadcastEventSpec _spec);
+
+  bool UnregisterListenerForEventsNoLock(const lldb::ListenerSP _sp,
+ const BroadcastEventSpec _spec);
+
   typedef std::pair event_listener_key;
   typedef std::map collection;
   typedef std::set listener_collection;
   collection m_event_map;
   listener_collection m_listeners;
 
-  mutable std::recursive_mutex m_manager_mutex;
+  mutable std::mutex m_manager_mutex;
 };
 
 /// \class Broadcaster Broadcaster.h "lldb/Utility/Broadcaster.h" An event
@@ -441,7 +442,7 @@ class Broadcaster {
 collection m_listeners;
 
 /// A mutex that protects \a m_listeners.
-std::recursive_mutex m_listeners_mutex;
+std::mutex m_listeners_mutex;
 
 /// See the discussion of Broadcasters and Listeners above.
 lldb::ListenerSP m_primary_listener_sp;
diff --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index b6d8ae39325d3..c6b2606afe0c8 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -87,7 +87,7 @@ bool Broadcaster::BroadcasterImpl::HasListeners(uint32_t 
event_mask) {
 }
 
 void Broadcaster::BroadcasterImpl::Clear() {
-  std::lock_guard guard(m_listeners_mutex);
+  std::lock_guard guard(m_listeners_mutex);
 
   // Make sure the listener forgets about this broadcaster. We do this in the
   // broadcaster in case the broadcaster object initiates the removal.
@@ -137,7 +137,7 @@ Broadcaster::BroadcasterImpl::AddListener(const 
lldb::ListenerSP _sp,
   if (!listener_sp)
 return 0;
 
-  std::lock_guard guard(m_listeners_mutex);
+  std::lock_guard guard(m_listeners_mutex);
 
   // See if we already have this listener, and if so, update its mask
 
@@ -171,7 +171,7 @@ Broadcaster::BroadcasterImpl::AddListener(const 
lldb::ListenerSP _sp,
 }
 
 bool Broadcaster::BroadcasterImpl::EventTypeHasListeners(uint32_t event_type) {
-  std::lock_guard guard(m_listeners_mutex);
+  std::lock_guard guard(m_listeners_mutex);
 
   if (!m_hijacking_listeners.empty() && event_type & m_hijacking_masks.back())
 return true;
@@ -195,7 +195,7 @@ bool Broadcaster::BroadcasterImpl::RemoveListener(
 return true;
   }
 
-  std::lock_guard guard(m_listeners_mutex);
+  std::lock_guard guard(m_listeners_mutex);
   for (auto it = m_listeners.begin(); it != m_listeners.end();) {
 lldb::ListenerSP curr_listener_sp(it->first.lock());
 
@@ -243,7 +243,7 @@ void 
Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP _sp,
 
   const uint32_t event_type = event_sp->GetType();
 
-  std::lock_guard 

[Lldb-commits] [lldb] [lldb] Don't unregister a listener that's being destroyed (PR #97300)

2024-07-02 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/97300
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Don't unregister a listener that's being destroyed (PR #97300)

2024-07-02 Thread Pavel Labath via lldb-commits

labath wrote:

Thanks for the explanation. I believe I understand the purpose of the primary 
listener. While I'm not sure that a "shared pointer" is the best way to express 
the "someone must exist on the other end to pull the events" notion, I'm not 
interested in revisiting that decision, so I'll remove that part of the commit 
message.

https://github.com/llvm/llvm-project/pull/97300
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Don't unregister a listener that's being destroyed (PR #97300)

2024-07-02 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97300
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)

2024-07-02 Thread Pavel Labath via lldb-commits

labath wrote:

> > Looks good. Are you able to merge this on your own?
> 
> Nope, I'll need someone with write access to do it.

Sounds good. Can you just patch the formatter changes in, and then I'll submit 
it.

@bulbazord, if you don't respond, I'm going to assume that new version 
addresses your comments as well.

https://github.com/llvm/llvm-project/pull/96685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)

2024-07-02 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/96685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Don't unregister a listener that's being destroyed (PR #97300)

2024-07-01 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97300
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Don't unregister a listener that's being destroyed (PR #97300)

2024-07-01 Thread Pavel Labath via lldb-commits

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/97300

It's not necessary because the broadcasters (and broadcast managers) hold a 
weak_ptr (*) to it, and will delete the weak_ptr next time they try to lock it. 
Doing this prevents recursion in RemoveListener, where the function can end up 
holding the only shared_ptr to a listener, and its destruction can trigger 
another call to RemoveListener -- which will mess up the state of the first 
instance.

This is the same bug that we've have fixed in
https://reviews.llvm.org/D23406, but it was effectively undone in 
https://reviews.llvm.org/D157556. With the addition of a primary listener, a 
fix like D23406 becomes unwieldy (and it has already shown itself to be 
fragile), which is why this patch attempts a different approach.

Like in 2016, I don't know a good way to unit test this bug, since it depends 
on precise timing, but the thing I like about this approach is that it enables 
us to change the broadcaster mutex into a non-recursive one. While that doesn't 
prevent the bug from happening again, it will make it much easier to spot in 
the future, as the code will hang with a smoking gun (instead of crashing a 
little while later). I'm going to attempt that in a separate patch to minimize 
disruption.

(*) Technically a broadcaster holds the *primary* listener as a shared_ptr. 
That's probably not a good idea, because it means the listener will never get 
destroyed (unless it is explicitly unregistered), but it also means that this 
change has no impact on primary listeners.

>From 542d54217a060594e3bd51782194d3bbfbbff7e2 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 1 Jul 2024 10:48:51 +
Subject: [PATCH] [lldb] Don't unregister a listener that's being destroyed

It's not necessary because the broadcasters (and broadcast managers)
hold a weak_ptr (*) to it, and will delete the weak_ptr next time they try
to lock it. Doing this prevents recursion in RemoveListener, where the
function can end up holding the only shared_ptr to a listener, and its
destruction can trigger another call to RemoveListener -- which will
mess up the state of the first instance.

This is the same bug that we've have fixed in
https://reviews.llvm.org/D23406, but it was effectively undone in
https://reviews.llvm.org/D157556. With the addition of a primary
listener, a fix like D23406 becomes unwieldy (and it has already shown
itself to be fragile), which is why this patch attempts a different
approach.

Like in 2016, I don't know a good way to unit test this bug, since it
depends on precise timing, but the thing I like about this approach is
that it enables us to change the broadcaster mutex into a non-recursive
one. While that doesn't prevent the bug from happening again, it will
make it much easier to spot in the future, as the code will hang with a
smoking gun (instead of crashing a little while later). I'm going to
attempt that in a separate patch to minimize disruption.

(*) Technically a broadcaster holds the *primary* listener as a
shared_ptr. That's probably not a good idea, because it means the
listener will never get destroyed (unless it is explicitly
unregistered), but it also means that this change has no impact on
primary listeners.
---
 lldb/source/Utility/Listener.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index 6a74c530ad257..5aacb4104e1cf 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -30,7 +30,7 @@ Listener::Listener(const char *name)
 Listener::~Listener() {
   Log *log = GetLog(LLDBLog::Object);
 
-  Clear();
+  // Don't call Clear() from here as that can cause races. See #96750.
 
   LLDB_LOGF(log, "%p Listener::%s('%s')", static_cast(this),
 __FUNCTION__, m_name.c_str());

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


[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)

2024-07-01 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.

Looks good. Are you able to merge this on your own?

https://github.com/llvm/llvm-project/pull/96685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Make semantics of SupportFile equivalence explicit (PR #97126)

2024-07-01 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.

I like this. Thanks.

It would be nice to have a quick unit test to go along with it.

https://github.com/llvm/llvm-project/pull/97126
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] b22ea2d - [lldb] Use SmallVector::erase correctly

2024-06-28 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2024-06-28T10:40:13+02:00
New Revision: b22ea2dc1e065f14a88df44e22a62427965a9b05

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

LOG: [lldb] Use SmallVector::erase correctly

erase() invalidates the iterator, we must use the returned iterator to
continue iterating.

This does not actually make a difference with the current implementation
of SmallVector (erase will effectively return the same pointer), but it
avoids future confusion.

Added: 


Modified: 
lldb/source/Utility/Broadcaster.cpp

Removed: 




diff  --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index bd65ffd86a1d0..b6d8ae39325d3 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -202,7 +202,7 @@ bool Broadcaster::BroadcasterImpl::RemoveListener(
 if (!curr_listener_sp) {
   // The weak pointer for this listener didn't resolve, lets' prune it
   // as we go.
-  m_listeners.erase(it);
+  it = m_listeners.erase(it);
   continue;
 }
 
@@ -213,8 +213,8 @@ bool Broadcaster::BroadcasterImpl::RemoveListener(
   if (!it->second)
 m_listeners.erase(it);
   return true;
-} else
-  it++;
+}
+it++;
   }
   return false;
 }



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


[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)

2024-06-28 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)

2024-06-28 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,25 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN:   -o "b main" \

labath wrote:

It looks like you're not actually running the binary. Do you need the 
breakpoint?

https://github.com/llvm/llvm-project/pull/96932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)

2024-06-28 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,36 @@
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN:   -o "b main" \

labath wrote:

same here

https://github.com/llvm/llvm-project/pull/96932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)

2024-06-28 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/96932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)

2024-06-28 Thread Pavel Labath via lldb-commits

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/96932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)

2024-06-28 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.

This should be fine, although looking at the test case again, I think that even 
simply replacing `process.GetThreadAtIndex(0)` with `self.thread()` should be 
enough. `self.thread()` returns the "selected" thread and lldb should always be 
selecting the thread which has stopped "for a reason" (e.g. a breakpoint) and 
not a random (or first) thread in the process.

https://github.com/llvm/llvm-project/pull/96685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 2fefc04 - [lldb/test] Fix enum-declaration-uniqueness.cpp

2024-06-27 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2024-06-27T17:46:27+02:00
New Revision: 2fefc042ce8faf8516ae66e1529d87c7130094a1

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

LOG: [lldb/test] Fix enum-declaration-uniqueness.cpp

Dereferencing a pointer variable without a running process does not work
on every arch/os. Fix the test to x86-linux, where it is known to work.

Added: 
lldb/test/Shell/SymbolFile/DWARF/x86/enum-declaration-uniqueness.cpp

Modified: 


Removed: 
lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp



diff  --git a/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/x86/enum-declaration-uniqueness.cpp
similarity index 75%
rename from lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
rename to lldb/test/Shell/SymbolFile/DWARF/x86/enum-declaration-uniqueness.cpp
index 6fc47d7e4b53a..20322f1ccc37b 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/enum-declaration-uniqueness.cpp
@@ -1,6 +1,8 @@
-// RUN: %clangxx_host -gdwarf -c -o %t_a.o %s -DFILE_A
-// RUN: %clangxx_host -gdwarf -c -o %t_b.o %s -DFILE_B
-// RUN: %clangxx_host -o %t %t_a.o %t_b.o
+// REQUIRES: lld
+//
+// RUN: %clangxx --target=x86_64-pc-linux -g -c -o %t_a.o %s -DFILE_A
+// RUN: %clangxx --target=x86_64-pc-linux -g -c -o %t_b.o %s -DFILE_B
+// RUN: ld.lld -o %t %t_a.o %t_b.o
 // RUN: %lldb %t \
 // RUN:   -o "target variable my_enum my_enum_ref" -o "image dump ast" \
 // RUN:   -o exit | FileCheck %s
@@ -24,8 +26,6 @@ extern MyEnum my_enum;
 enum MyEnum : int { MyEnum_A };
 
 MyEnum my_enum = MyEnum_A;
-
-int main() {}
 #endif
 #ifdef FILE_B
 MyEnum _enum_ref = my_enum;



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


[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)

2024-06-27 Thread Pavel Labath via lldb-commits

labath wrote:

> This is a really nice cleanup! It actually aligns almost exactly with [our 
> in-progress version of 
> this](https://github.com/llvm/llvm-project/blob/caacb57a46f34bf663fa5ab2190b361ce29b255b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp).
> 
> LGTM
> 
> Agree with your point about `PrepareContextToReceiveMembers`. We can add 
> those as needed. In our version of this I had to only add it in 
> `ParseSubroutine`, as you've also effectively done.

Thanks. It's very nice when things fit together.

https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)

2024-06-27 Thread Pavel Labath via lldb-commits


@@ -1893,72 +1849,21 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
*unique_ast_entry_up);
 
-  if (!attrs.is_forward_declaration) {
-// Always start the definition for a class type so that if the class
-// has child classes or types that require the class to be created
-// for use as their decl contexts the class will be ready to accept
-// these child definitions.
-if (!def_die.HasChildren()) {
-  // No children for this struct/union/class, lets finish it
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
-dwarf->GetObjectFile()->GetModule()->ReportError(
-
-"DWARF DIE {0:x16} named \"{1}\" was not able to start its "
-"definition.\nPlease file a bug and attach the file at the "
-"start of this error message",
-def_die.GetID(), attrs.name.GetCString());
-  }
-
-  // Setting authority byte size and alignment for empty structures.
-  //
-  // If the byte size or alignmenet of the record is specified then
-  // overwrite the ones that would be computed by Clang.
-  // This is only needed as LLDB's TypeSystemClang is always in C++ mode,
-  // but some compilers such as GCC and Clang give empty structs a size of 0
-  // in C mode (in contrast to the size of 1 for empty structs that would 
be
-  // computed in C++ mode).
-  if (attrs.byte_size || attrs.alignment) {
-clang::RecordDecl *record_decl =
-TypeSystemClang::GetAsRecordDecl(clang_type);
-if (record_decl) {
-  ClangASTImporter::LayoutInfo layout;
-  layout.bit_size = attrs.byte_size.value_or(0) * 8;
-  layout.alignment = attrs.alignment.value_or(0) * 8;
-  GetClangASTImporter().SetRecordLayout(record_decl, layout);
-}
-  }
-} else if (clang_type_was_created) {
-  // Start the definition if the class is not objective C since the
-  // underlying decls respond to isCompleteDefinition(). Objective
-  // C decls don't respond to isCompleteDefinition() so we can't
-  // start the declaration definition right away. For C++
-  // class/union/structs we want to start the definition in case the
-  // class is needed as the declaration context for a contained class
-  // or type without the need to complete that type..
-
-  if (attrs.class_language != eLanguageTypeObjC &&
-  attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
-  // Leave this as a forward declaration until we need to know the
-  // details of the type. lldb_private::Type will automatically call
-  // the SymbolFile virtual function
-  // "SymbolFileDWARF::CompleteType(Type *)" When the definition
-  // needs to be defined.
-  assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
-  // Can't assume m_ast.GetSymbolFile() is actually a
-  // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
-  // binaries.
-  dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
-  ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
-  *def_die.GetDIERef());
-  m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
-}
+  if (clang_type_was_created) {

labath wrote:

I don't think so, or rather like, if it does, I would say that's a bug 
somewhere else, as an empty class should be just a special case of "a class". 
You could think of this as an optimization, but I'd be surprised if it made a 
difference -- our performance problems are coming from classes that in fact 
have members. (Also, die.HasChildren() is not a very reliable indicator of an 
empty class, as e.g. an empty template class will still have children)

https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)

2024-06-27 Thread Pavel Labath via lldb-commits

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

2024-06-27 Thread Pavel Labath via lldb-commits

labath wrote:

> > It looks like the test is flaky: 
> > https://lab.llvm.org/buildbot/#/builders/59/builds/535
> > It looks like the order of the types is nondeterministic. I think you 
> > should be able to use CHECK-DAG along with [this trick to match 
> > newlines](https://llvm.org/docs/CommandGuide/FileCheck.html#matching-newline-characters)
> >  to make the test accept both orderings.
> 
> What are lldb's guarantees in this regard? Clang/LLVM/etc require 
> nondeterministic output - presumably the same would be valuable to lldb, but 
> I don't know what lldb's precedents are in this regard.

It would be useful, but we've never made a serious effort to try to achieve 
that.

https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)

2024-06-27 Thread Pavel Labath via lldb-commits

labath wrote:

> Would it make more sense to make the ValueImpl that's going to represent the 
> synthetic value then check whether that really is synthetic, and return an 
> empty SBValue if it is not?

Sounds like a good idea to me.


https://github.com/llvm/llvm-project/pull/95959
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 5da6f64 - [lldb] Un-XFAIL TestStepScripted.test_misspelled_plan_name

2024-06-27 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2024-06-27T16:00:34+02:00
New Revision: 5da6f64db3184be89ee8b7cca4e5e055baaef964

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

LOG: [lldb] Un-XFAIL TestStepScripted.test_misspelled_plan_name

XFAIL in #96894 was too wide. This one actually passes.

Added: 


Modified: 
lldb/test/API/functionalities/step_scripted/TestStepScripted.py

Removed: 




diff  --git a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py 
b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
index 7a4992abd8fcb..bb7479414dbbb 100644
--- a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
+++ b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
@@ -45,7 +45,6 @@ def step_out_with_scripted_plan(self, name):
 stop_desc = thread.GetStopDescription(1000)
 self.assertIn("Stepping out from", stop_desc, "Got right description")
 
-@expectedFailureAll()
 def test_misspelled_plan_name(self):
 """Test that we get a useful error if we misspell the plan class 
name"""
 self.build()



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


[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)

2024-06-26 Thread Pavel Labath via lldb-commits


@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   }
 
   if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(

labath wrote:

For uniqueness, yes the unique map should already catch that (though, like 
we've seen, it can easily have bugs). However, it's still useful for preventing 
(infinite) recursion.

https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)

2024-06-26 Thread Pavel Labath via lldb-commits


@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   }
 
   if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(

labath wrote:

It's possible, but I am not sure if it's worth it, as this code will go away 
(or at least change substantially) when we switch to lazy definition lookups.

https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)

2024-06-26 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/96800
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)

2024-06-26 Thread Pavel Labath via lldb-commits


@@ -2192,87 +2097,82 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE ,
   ClangASTImporter::LayoutInfo layout_info;
   std::vector contained_type_dies;
 
-  if (die.HasChildren()) {
-const bool type_is_objc_object_or_interface =
-TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type);
-if (type_is_objc_object_or_interface) {
-  // For objective C we don't start the definition when the class is
-  // created.
-  TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-}
-
-AccessType default_accessibility = eAccessNone;
-if (tag == DW_TAG_structure_type) {
-  default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_union_type) {
-  default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_class_type) {
-  default_accessibility = eAccessPrivate;
-}
-
-std::vector> bases;
-// Parse members and base classes first
-std::vector member_function_dies;
-
-DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, bases, member_function_dies,
-  contained_type_dies, delayed_properties,
-  default_accessibility, layout_info);
-
-// Now parse any methods if there were any...
-for (const DWARFDIE  : member_function_dies)
-  dwarf->ResolveType(die);
-
-if (type_is_objc_object_or_interface) {
-  ConstString class_name(clang_type.GetTypeName());
-  if (class_name) {
-dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
-  method_die.ResolveType();
-  return true;
-});
+  if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
+return false; // No definition, cannot complete.
 
-for (DelayedAddObjCClassProperty  : delayed_properties)
-  property.Finalize();
-  }
-}
+  // Start the definition if the type is not being defined already. This can
+  // happen (e.g.) when adding nested types to a class type -- see
+  // PrepareContextToReceiveMembers.
+  if (!clang_type.IsBeingDefined())
+TypeSystemClang::StartTagDeclarationDefinition(clang_type);
 
-if (!bases.empty()) {
-  // Make sure all base classes refer to complete types and not forward
-  // declarations. If we don't do this, clang will crash with an
-  // assertion in the call to clang_type.TransferBaseClasses()
-  for (const auto _class : bases) {
-clang::TypeSourceInfo *type_source_info =
-base_class->getTypeSourceInfo();
-if (type_source_info)
-  TypeSystemClang::RequireCompleteType(
-  m_ast.GetType(type_source_info->getType()));
-  }
+  AccessType default_accessibility = eAccessNone;
+  if (tag == DW_TAG_structure_type) {
+default_accessibility = eAccessPublic;
+  } else if (tag == DW_TAG_union_type) {
+default_accessibility = eAccessPublic;
+  } else if (tag == DW_TAG_class_type) {
+default_accessibility = eAccessPrivate;
+  }
+
+  std::vector> bases;
+  // Parse members and base classes first
+  std::vector member_function_dies;
 
-  m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
-std::move(bases));
+  DelayedPropertyList delayed_properties;
+  ParseChildMembers(die, clang_type, bases, member_function_dies,
+contained_type_dies, delayed_properties,
+default_accessibility, layout_info);
+
+  // Now parse any methods if there were any...
+  for (const DWARFDIE  : member_function_dies)
+dwarf->ResolveType(die);
+
+  if (TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type)) {
+ConstString class_name(clang_type.GetTypeName());
+if (class_name) {
+  dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
+method_die.ResolveType();
+return true;
+  });
+
+  for (DelayedAddObjCClassProperty  : delayed_properties)
+property.Finalize();
 }
   }
 
+  if (!bases.empty()) {
+// Make sure all base classes refer to complete types and not forward
+// declarations. If we don't do this, clang will crash with an
+// assertion in the call to clang_type.TransferBaseClasses()
+for (const auto _class : bases) {
+  clang::TypeSourceInfo *type_source_info = 
base_class->getTypeSourceInfo();
+  if (type_source_info)
+TypeSystemClang::RequireCompleteType(
+m_ast.GetType(type_source_info->getType()));
+}
+
+m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(), 
std::move(bases));
+  }
+
   m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType());
   TypeSystemClang::BuildIndirectFields(clang_type);
   TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
 
-  if (!layout_info.field_offsets.empty() || !layout_info.base_offsets.empty() 
||
-  !layout_info.vbase_offsets.empty()) {

labath wrote:

The removal of these checks compensates for the deletion of lines 1914--1930. 

[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)

2024-06-26 Thread Pavel Labath via lldb-commits


@@ -1893,72 +1849,21 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
*unique_ast_entry_up);
 
-  if (!attrs.is_forward_declaration) {
-// Always start the definition for a class type so that if the class
-// has child classes or types that require the class to be created
-// for use as their decl contexts the class will be ready to accept
-// these child definitions.
-if (!def_die.HasChildren()) {
-  // No children for this struct/union/class, lets finish it
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
-dwarf->GetObjectFile()->GetModule()->ReportError(
-
-"DWARF DIE {0:x16} named \"{1}\" was not able to start its "
-"definition.\nPlease file a bug and attach the file at the "
-"start of this error message",
-def_die.GetID(), attrs.name.GetCString());
-  }
-
-  // Setting authority byte size and alignment for empty structures.
-  //
-  // If the byte size or alignmenet of the record is specified then
-  // overwrite the ones that would be computed by Clang.
-  // This is only needed as LLDB's TypeSystemClang is always in C++ mode,
-  // but some compilers such as GCC and Clang give empty structs a size of 0
-  // in C mode (in contrast to the size of 1 for empty structs that would 
be
-  // computed in C++ mode).
-  if (attrs.byte_size || attrs.alignment) {
-clang::RecordDecl *record_decl =
-TypeSystemClang::GetAsRecordDecl(clang_type);
-if (record_decl) {

labath wrote:

These lines.

https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


  1   2   3   4   5   6   7   8   9   10   >