[Lldb-commits] [lldb] [lldb] Small cleanup of ProcessEventData::ShouldStop (PR #98154)
@@ -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)
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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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
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)
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)
@@ -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)
@@ -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)
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)
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)
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
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)
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)
@@ -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)
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)
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)
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
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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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