[Lldb-commits] [lldb] [WIP] [lldb][Progress] Report progress when completing types from DWARF (PR #91452)
Michael137 wrote: > I am somewhat worried about this slowing down the actual operations it is > reporting progress on. I didn't really measure this, but intuitively, I'd > expect that a one of these operations (parsing/importing one type) would be > pretty fast, and that the whole process takes so long simply because > performing a very large number of these ops. > > Can you get some numbers on this? E.g., the number of events (per second?) > that this generates, the timings of expression evaluation with/without the > patch, or something like that? I've been living on this patch for the past two weeks, debugging Clang/LLDB and haven't noticed a slowdown in the expression evaluator, but I'll try to follow-up with some concrete numbers. > Is the code that emits the progress event recursive too? The reason I ask is > because on the command line, nested progress events will get shadowed. The > same is true for coalesced progress events. I'm not sure how VSCode/DAP > clients deal with this, so maybe they're shown there? > Anyway, if the code is recursive, we might need to do something like we did > for Swift, with one top-level event and callback that updates the details. Yes it is recursive, and some of the progress events do shadow each other. I'll take a look at what Swift does, thanks for the pointer! https://github.com/llvm/llvm-project/pull/91452 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ELF] Return address class map changes from symbol table parsing methods (PR #91585)
DavidSpickett wrote: > The Symtab class is generic, while the approach of marking address types via > fake symtab symbols is (I think) an elf invention. I didn't see any use of `AddressClass::eCodeAlternateISA` outside of ObjectFileELF so I think you're right. I'll go with this PR then. https://github.com/llvm/llvm-project/pull/91585 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a76518c - [lldb][ELF] Return address class map changes from symbol table parsing methods (#91585)
Author: David Spickett Date: 2024-05-10T09:20:48+01:00 New Revision: a76518cadc5eaa6b6d07334e2b5bc08382aabe49 URL: https://github.com/llvm/llvm-project/commit/a76518cadc5eaa6b6d07334e2b5bc08382aabe49 DIFF: https://github.com/llvm/llvm-project/commit/a76518cadc5eaa6b6d07334e2b5bc08382aabe49.diff LOG: [lldb][ELF] Return address class map changes from symbol table parsing methods (#91585) Instead of updating the member of the ObjectFileELF instance. This means that if one object file asks another to parse the symbol table, that first object's can update its address class map with the same changes that the other object did. (I'm not returning a reference to the other object's m_address_class_map member because there may be other things in there not related to the symbol table being parsed) This will fix the code added in https://github.com/llvm/llvm-project/pull/90622 which broke the test `Expr/TestStringLiteralExpr.test` on 32 bit Arm Linux. This happened because we had the program file, then asked for a better object file, which returned the same program file again. This creates a second ObjectFileELF for the same file, so when we tell the second instance to parse the symbol table it actually calls into the first instance, leaving the address class map of the second instance empty. Which caused us to put an Arm breakpoint instuction at a Thumb return address and broke the ability to call mmap. Added: Modified: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h Removed: diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp index 1646ee9aa34a6..d88f2d0830192 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -2060,13 +2060,17 @@ static char FindArmAarch64MappingSymbol(const char *symbol_name) { #define IS_MICROMIPS(ST_OTHER) (((ST_OTHER)&STO_MIPS_ISA) == STO_MICROMIPS) // private -unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id, - SectionList *section_list, - const size_t num_symbols, - const DataExtractor &symtab_data, - const DataExtractor &strtab_data) { +std::pair +ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id, +SectionList *section_list, const size_t num_symbols, +const DataExtractor &symtab_data, +const DataExtractor &strtab_data) { ELFSymbol symbol; lldb::offset_t offset = 0; + // The changes these symbols would make to the class map. We will also update + // m_address_class_map but need to tell the caller what changed because the + // caller may be another object file. + FileAddressToAddressClassMap address_class_map; static ConstString text_section_name(".text"); static ConstString init_section_name(".init"); @@ -2213,18 +2217,18 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id, switch (mapping_symbol) { case 'a': // $a[.]* - marks an ARM instruction sequence - m_address_class_map[symbol.st_value] = AddressClass::eCode; + address_class_map[symbol.st_value] = AddressClass::eCode; break; case 'b': case 't': // $b[.]* - marks a THUMB BL instruction sequence // $t[.]* - marks a THUMB instruction sequence - m_address_class_map[symbol.st_value] = + address_class_map[symbol.st_value] = AddressClass::eCodeAlternateISA; break; case 'd': // $d[.]* - marks a data item sequence (e.g. lit pool) - m_address_class_map[symbol.st_value] = AddressClass::eData; + address_class_map[symbol.st_value] = AddressClass::eData; break; } } @@ -2238,11 +2242,11 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id, switch (mapping_symbol) { case 'x': // $x[.]* - marks an A64 instruction sequence - m_address_class_map[symbol.st_value] = AddressClass::eCode; + address_class_map[symbol.st_value] = AddressClass::eCode; break; case 'd': // $d[.]* - marks a data item sequence (e.g. lit pool) - m_address_class_map[symbol.st_value] = AddressClass::eData; + address_class_map[symbol.st_value] = AddressClass::eData; break; } } @@ -2260,11 +2264,11 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id, /
[Lldb-commits] [lldb] [lldb][ELF] Return address class map changes from symbol table parsing methods (PR #91585)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/91585 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ELF] Move address class map into the symbol table (PR #91603)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/91603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ELF] Move address class map into the symbol table (PR #91603)
DavidSpickett wrote: Went with https://github.com/llvm/llvm-project/pull/91585 instead. https://github.com/llvm/llvm-project/pull/91603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ELF] Move address class map into the symbol table (PR #91603)
@@ -23,6 +23,8 @@ class Symtab { public: typedef std::vector IndexCollection; typedef UniqueCStringMap NameToIndexMap; + typedef std::map + FileAddressToAddressClassMap; DavidSpickett wrote: Yes, we binary search it later. I will at least add a comment to explain the type choice. https://github.com/llvm/llvm-project/pull/91603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ELF] Return address class map changes from symbol table parsing methods (PR #91585)
DavidSpickett wrote: There are a couple in MachO actually but you're right, different mechanism being used. https://github.com/llvm/llvm-project/pull/91585 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp, FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); - if (!dsym_fspec) -return nullptr; + if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) { +// If we have a stripped binary or if we got a DWP file, we should prefer +// symbols in the executable acquired through a plugin. +ModuleSpec unstripped_spec = +PluginManager::LocateExecutableObjectFile(module_spec); +if (!unstripped_spec) + return nullptr; +dsym_fspec = unstripped_spec.GetFileSpec(); + } DavidSpickett wrote: That PR is in, so at least Arm Linux is happy for this to go back in. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
DavidSpickett wrote: I've fixed the underlying issue on Arm, you do not need to make any changes to this code to fix it. It should "just work" now. I see some unresolved comments about other test failures, and if I reland this I'll get all the buildbot emails and have no clue what to do to fix them. So instead I think it's best if you go over those, make any changes you need to and post a new PR to reland this. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1aca8ed - [lldb][ELF] Add a comment to explain address class map type
Author: David Spickett Date: 2024-05-10T09:25:03Z New Revision: 1aca8ed5a7eeed264fdc2694deca8a4a4dba3689 URL: https://github.com/llvm/llvm-project/commit/1aca8ed5a7eeed264fdc2694deca8a4a4dba3689 DIFF: https://github.com/llvm/llvm-project/commit/1aca8ed5a7eeed264fdc2694deca8a4a4dba3689.diff LOG: [lldb][ELF] Add a comment to explain address class map type It was pointed out that ordering is crucial here, so note that. I also looked into using a vector instead, as described in https://llvm.org/docs/ProgrammersManual.html#dss-sortedvectorset. Which this is in theory perfect for, but we have at least 2 places that update the map and both would need to sort/unique each time. Plus this code is pretty bug prone. If there is future refactoring it's one thing to consider. Added: Modified: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h Removed: diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h index 716bbe01638f3..844e981b1d890 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h @@ -187,6 +187,9 @@ class ObjectFileELF : public lldb_private::ObjectFile { typedef DynamicSymbolColl::iterator DynamicSymbolCollIter; typedef DynamicSymbolColl::const_iterator DynamicSymbolCollConstIter; + /// An ordered map of file address to address class. Used on architectures + /// like Arm where there is an alternative ISA mode like Thumb. The container + /// is ordered so that it can be binary searched. typedef std::map FileAddressToAddressClassMap; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
kevinfrei wrote: Thanks for the diagnostics & fix. I really appreciate it. I'm on vacation this week, so I'll get this ready & relanded next Monday. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -12,6 +12,9 @@ #include "lldb/Utility/ProcessInfo.h" #include "gtest/gtest.h" +#include +#include emaste wrote: Much of this file would work just fine on FreeBSD as well so that might make sense, although I'm not sure what the best structure would be -- maybe much of this file should move to a new lldb/unittests/Host/posix/HostTest.cpp, leaving anything Linux-specific here? https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Allow env override for LLDB_ARGDUMPER_PATH (PR #91688)
https://github.com/walter-erquinigo approved this pull request. lgtm https://github.com/llvm/llvm-project/pull/91688 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Fix a race during shutdown (PR #91591)
https://github.com/clayborg approved this pull request. Sounds good, thanks for the clarification. https://github.com/llvm/llvm-project/pull/91591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
jimingham wrote: I'm not sure checking the debugger here resolves the difficulty I pointed out earlier. If I make an SBStream and call SBBreakpoint::GetDescription, I will have passed in an SBStream with `use_color` explicitly off. It still seems to me that should take precedence over the debugger's global setting. I wanted this for programmatic uses presumably, and shouldn't have to deal with color codes... Having to temporarily unset the global use color in the debugger to get this effect is not a good design. https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -91,14 +87,16 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, if (Rest.empty()) return false; StatFields stat_fields; - if (sscanf(Rest.data(), - "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld", - &stat_fields.pid, stat_fields.comm, &stat_fields.state, - &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session, - &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags, - &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt, - &stat_fields.cmajflt, &stat_fields.utime, &stat_fields.stime, - &stat_fields.cutime, &stat_fields.cstime) < 0) { + if (sscanf( + Rest.data(), + "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld", + &stat_fields.pid, stat_fields.comm, &stat_fields.state, + &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session, + &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags, + &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt, + &stat_fields.cmajflt, &stat_fields.utime, &stat_fields.stime, + &stat_fields.cutime, &stat_fields.cstime, + &stat_fields.realtime_priority, &stat_fields.priority) < 0) { clayborg wrote: are the priority fields always in this data? Just want to make sure that this `sscanf` call won't fail on some certain versions of linux/unix? https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -254,6 +280,8 @@ class ProcessInstanceInfo : public ProcessInfo { struct timespec m_system_time {}; struct timespec m_cumulative_user_time {}; struct timespec m_cumulative_system_time {}; + std::optional m_priority_value = std::nullopt; + bool m_zombie = false; clayborg wrote: making this optional as well would be good. If it isn't set, it means we don't know, but if it is set to true/false we do know. Again, almost all members of this structure could benefit from being std::optional https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -12,6 +12,9 @@ #include "lldb/Utility/ProcessInfo.h" #include "gtest/gtest.h" +#include +#include clayborg wrote: This is fine if this is a host test, so nothing needs to be done. We are expecting linux + posix here which is fine. https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -144,6 +144,19 @@ class ProcessInstanceInfo : public ProcessInfo { long int tv_usec = 0; }; + enum class ProcessState { +Unknown, +Dead, +DiskSleep, +Idle, +Paging, +Parked, +Running, +Sleeping, +TracedOrStopped, +Zombie, + }; + clayborg wrote: There doesn't seem to be a reason for this to be here, so if we can move it back, that would be great. Ideally this file would be OS agnostic, but it started out a bit unix specific in the first place. https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9a7262c - [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (#90663)
Author: Zequan Wu Date: 2024-05-10T12:26:52-04:00 New Revision: 9a7262c2601874e5aa64c5db19746770212d4b44 URL: https://github.com/llvm/llvm-project/commit/9a7262c2601874e5aa64c5db19746770212d4b44 DIFF: https://github.com/llvm/llvm-project/commit/9a7262c2601874e5aa64c5db19746770212d4b44.diff LOG: [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (#90663) This is the implementation for https://discourse.llvm.org/t/rfc-delay-definition-die-searching-when-parse-a-declaration-die-for-record-type/78526. Motivation Currently, lldb eagerly searches for definition DIE when parsing a declaration DIE for struct/class/union definition DIE. It will search for all definition DIEs with the same unqualified name (just `DW_AT_name` ) and then find out those DIEs with same fully qualified name. Then lldb will try to resolve those DIEs to create the Types from definition DIEs. It works fine most time. However, when built with `-gsimple-template-names`, the search graph expands very quickly, because for the specialized-template classes, they don’t have template parameter names encoded inside `DW_AT_name`. They have `DW_TAG_template_type_parameter` to reference the types used as template parameters. In order to identify if a definition DIE matches a declaration DIE, lldb needs to resolve all template parameter types first and those template parameter types might be template classes as well, and so on… So, the search graph explodes, causing a lot unnecessary searching/type-resolving to just get the fully qualified names for a specialized-template class. This causes lldb stack overflow for us internally on template-heavy libraries. Implementation Instead of searching for definition DIEs when parsing declaration DIEs, we always construct the record type from the DIE regardless if it's definition or declaration. The process of searching for definition DIE is refactored to `DWARFASTParserClang::FindDefinitionTypeForDIE` which is invoked when 1) completing the type on `SymbolFileDWARF::CompleteType`. 2) the record type needs to start its definition as a containing type so that nested classes can be added into it in `PrepareContextToReceiveMembers`. The key difference is `SymbolFileDWARF::ResolveType` return a `Type*` that might be created from declaration DIE, which means it hasn't starts its definition yet. We also need to change according in places where we want the type to start definition, like `PrepareContextToReceiveMembers` (I'm not aware of any other places, but this should be a simple call to `SymbolFileDWARF::FindDefinitionDIE`) Result It fixes the stack overflow of lldb for the internal binary built with simple template name. When constructing the fully qualified name built with `-gsimple-template-names`, it gets the name of the type parameter by resolving the referenced DIE, which might be a declaration (we won't try to search for the definition DIE to just get the name). I got rough measurement about the time using the same commands (set breakpoint, run, expr this, exit). For the binary built without `-gsimple-template-names`, this change has no impact on time, still taking 41 seconds to complete. When built with `-gsimple-template-names`, it also takes about 41 seconds to complete wit this change. Added: lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h index 66db396279e06..e144cf0f9bd94 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h @@ -60,6 +60,8 @@ class DWARFASTParser { virtual ConstString GetDIEClassTemplateParams(const DWARFDIE &die) = 0; + virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE &die) = 0; + static std::optional ParseChildArrayInfo(const DWARFDIE &parent_die, const ExecutionContext *exe_ctx = nullptr); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index f8101aba5c627..e0b1b430b266f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -154,6 +154,26 @@ static bool TagIsRecordType(dw_tag_t tag) { } } +static bool IsForwardDeclaration
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jeffreytan81 wrote: @jimingham, friendly ping. @clayborg mentioned that you are the sole domain expert on ThreadPlan. Can you help to review this change? Thanks https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 871f483 - [lldb-dap] Fix a race during shutdown (#91591)
Author: Pavel Labath Date: 2024-05-10T18:56:21+02:00 New Revision: 871f4839f988a1ef59ea0371e0f25c8651a899f2 URL: https://github.com/llvm/llvm-project/commit/871f4839f988a1ef59ea0371e0f25c8651a899f2 DIFF: https://github.com/llvm/llvm-project/commit/871f4839f988a1ef59ea0371e0f25c8651a899f2.diff LOG: [lldb-dap] Fix a race during shutdown (#91591) lldb-dap was setting a flag which was meant to shut it down as soon as it sent a terminated event. The problem with this flag is two-fold: - as far as I can tell (definitely not an expert here), there's no justification for this in the protocol spec. The only way I found to shut the server down was to send it a disconnect request. - the flag did not actually work most of the time, because it's only checked between requests so nothing will happen if the server starts listening for a new request before a different thread manages to send the terminated event. And since the next request is usually the disconnect request, everything will operate normally. The combination of these two things meant that the issue was largely unnoticable, except for rare flaky test failures, which happened when the handler thread was too slow, and checked the flag after it has already been said. This caused the test suite to complain as it did not get a response to the disconnect request. This situation could be s(t)imulated by adding a sleep to the and of the main loop, which delayed the flag check, and caused the DAP tests to fail reliably. This patch changes the shutdown condition to only trigger when the disconnect request has been received. Since the flag can now only be set from the handler thread, it no longer needs to be atomic. Added: Modified: lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/lldb-dap.cpp Removed: diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index b254ddfef0d5f..55ff1493c1011 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -39,8 +39,7 @@ DAP::DAP() {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC}, {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift}, {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}), - focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false), - stop_at_entry(false), is_attach(false), + focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), enable_auto_variable_summaries(false), enable_synthetic_child_debugging(false), restarting_process_id(LLDB_INVALID_PROCESS_ID), @@ -623,7 +622,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) { } llvm::Error DAP::Loop() { - while (!sent_terminated_event) { + while (!disconnecting) { llvm::json::Object object; lldb_dap::PacketStatus status = GetNextObject(object); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 5c70a056fea4b..bbd9d46ba3a04 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -168,7 +168,7 @@ struct DAP { // arguments if we get a RestartRequest. std::optional last_launch_or_attach_request; lldb::tid_t focus_tid; - std::atomic sent_terminated_event; + bool disconnecting = false; bool stop_at_entry; bool is_attach; bool enable_auto_variable_summaries; diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index f35abd665e844..96da458be21d1 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -226,26 +226,14 @@ void SendContinuedEvent() { // Send a "terminated" event to indicate the process is done being // debugged. void SendTerminatedEvent() { - // If an inferior exits prior to the processing of a disconnect request, then - // the threads executing EventThreadFunction and request_discontinue - // respectively may call SendTerminatedEvent simultaneously. Without any - // synchronization, the thread executing EventThreadFunction may set - // g_dap.sent_terminated_event before the thread executing - // request_discontinue has had a chance to test it, in which case the latter - // would move ahead to issue a response to the disconnect request. Said - // response may get dispatched ahead of the terminated event compelling the - // client to terminate the debug session without consuming any console output - // that might've been generated by the execution of terminateCommands. So, - // synchronize simultaneous calls to SendTerminatedEvent. + // Prevent races if the process exits while we're being asked to disconnect. static std::mutex mutex; std::lock_guard locker(mutex); - if (!g_dap.sent_terminated_event) { -g_dap.sent_terminated_event = true; -g_dap.RunTerminateCommands(); -// Send a "terminated" event -llvm::json::Object event(CreateTerminatedEventObject()); -g_dap.SendJSON(llvm::j
[Lldb-commits] [lldb] [lldb-dap] Fix a race during shutdown (PR #91591)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/91591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3bde798 - [lldb] Put SBSourceLanguageName in lldb namespace (#91685)
Author: Alex Langford Date: 2024-05-10T10:00:36-07:00 New Revision: 3bde7983986d8ce637f6bb506860859249787751 URL: https://github.com/llvm/llvm-project/commit/3bde7983986d8ce637f6bb506860859249787751 DIFF: https://github.com/llvm/llvm-project/commit/3bde7983986d8ce637f6bb506860859249787751.diff LOG: [lldb] Put SBSourceLanguageName in lldb namespace (#91685) Added: Modified: lldb/scripts/generate-sbapi-dwarf-enum.py Removed: diff --git a/lldb/scripts/generate-sbapi-dwarf-enum.py b/lldb/scripts/generate-sbapi-dwarf-enum.py index f7a13e5efffef..7fd6037986317 100755 --- a/lldb/scripts/generate-sbapi-dwarf-enum.py +++ b/lldb/scripts/generate-sbapi-dwarf-enum.py @@ -15,6 +15,8 @@ #ifndef LLDB_API_SBLANGUAGE_H #define LLDB_API_SBLANGUAGE_H + +namespace lldb { /// Used by \\ref SBExpressionOptions. /// These enumerations use the same language enumerations as the DWARF /// specification for ease of use and consistency. @@ -24,6 +26,8 @@ FOOTER = """\ }; +} // namespace lldb + #endif """ ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Put SBSourceLanguageName in lldb namespace (PR #91685)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/91685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jimingham wrote: Thanks for working up this patch. I should be able to get time to look at this next week. https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -254,6 +280,8 @@ class ProcessInstanceInfo : public ProcessInfo { struct timespec m_system_time {}; struct timespec m_cumulative_user_time {}; struct timespec m_cumulative_system_time {}; + std::optional m_priority_value = std::nullopt; + bool m_zombie = false; feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
https://github.com/feg208 updated https://github.com/llvm/llvm-project/pull/91544 >From dbf56b2ebe93d2f3ef6e41bceb7359f6e10ae38d Mon Sep 17 00:00:00 2001 From: Fred Grim Date: Wed, 8 May 2024 15:36:16 -0700 Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo To implement SaveCore for elf binaries we need to populate some additional fields in the prpsinfo struct. Those fields are the nice value of the process whose core is to be taken as well as a boolean flag indicating whether or not that process is a zombie. This commit adds those as well as tests to ensure that the values are consistent with expectations --- lldb/include/lldb/Utility/ProcessInfo.h | 94 +++-- lldb/source/Host/linux/Host.cpp | 32 ++--- lldb/source/Utility/ProcessInfo.cpp | 11 ++- lldb/unittests/Host/linux/HostTest.cpp | 21 ++ 4 files changed, 109 insertions(+), 49 deletions(-) diff --git a/lldb/include/lldb/Utility/ProcessInfo.h b/lldb/include/lldb/Utility/ProcessInfo.h index 54ac000dc7fc2..995873a6869e0 100644 --- a/lldb/include/lldb/Utility/ProcessInfo.h +++ b/lldb/include/lldb/Utility/ProcessInfo.h @@ -15,6 +15,7 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/NameMatches.h" #include "lldb/Utility/StructuredData.h" +#include #include namespace lldb_private { @@ -147,72 +148,71 @@ class ProcessInstanceInfo : public ProcessInfo { ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) - : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX), -m_parent_pid(LLDB_INVALID_PROCESS_ID) {} + : ProcessInfo(name, arch, pid) {} void Clear() { ProcessInfo::Clear(); -m_euid = UINT32_MAX; -m_egid = UINT32_MAX; -m_parent_pid = LLDB_INVALID_PROCESS_ID; +m_euid = std::nullopt; +m_egid = std::nullopt; +m_parent_pid = std::nullopt; } - uint32_t GetEffectiveUserID() const { return m_euid; } + uint32_t GetEffectiveUserID() const { return m_euid.value(); } - uint32_t GetEffectiveGroupID() const { return m_egid; } + uint32_t GetEffectiveGroupID() const { return m_egid.value(); } - bool EffectiveUserIDIsValid() const { return m_euid != UINT32_MAX; } + bool EffectiveUserIDIsValid() const { return m_euid.has_value(); } - bool EffectiveGroupIDIsValid() const { return m_egid != UINT32_MAX; } + bool EffectiveGroupIDIsValid() const { return m_egid.has_value(); } void SetEffectiveUserID(uint32_t uid) { m_euid = uid; } void SetEffectiveGroupID(uint32_t gid) { m_egid = gid; } - lldb::pid_t GetParentProcessID() const { return m_parent_pid; } + lldb::pid_t GetParentProcessID() const { return m_parent_pid.value(); } void SetParentProcessID(lldb::pid_t pid) { m_parent_pid = pid; } - bool ParentProcessIDIsValid() const { -return m_parent_pid != LLDB_INVALID_PROCESS_ID; - } + bool ParentProcessIDIsValid() const { return m_parent_pid.has_value(); } - lldb::pid_t GetProcessGroupID() const { return m_process_group_id; } + lldb::pid_t GetProcessGroupID() const { return m_process_group_id.value(); } void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; } - bool ProcessGroupIDIsValid() const { -return m_process_group_id != LLDB_INVALID_PROCESS_ID; - } + bool ProcessGroupIDIsValid() const { return m_process_group_id.has_value(); } - lldb::pid_t GetProcessSessionID() const { return m_process_session_id; } + lldb::pid_t GetProcessSessionID() const { +return m_process_session_id.value(); + } void SetProcessSessionID(lldb::pid_t session) { m_process_session_id = session; } bool ProcessSessionIDIsValid() const { -return m_process_session_id != LLDB_INVALID_PROCESS_ID; +return m_process_session_id.has_value(); } - struct timespec GetUserTime() const { return m_user_time; } + struct timespec GetUserTime() const { return m_user_time.value(); } void SetUserTime(struct timespec utime) { m_user_time = utime; } bool UserTimeIsValid() const { -return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0; +return m_user_time.has_value() && + (m_user_time->tv_sec > 0 || m_user_time->tv_usec > 0); } - struct timespec GetSystemTime() const { return m_system_time; } + struct timespec GetSystemTime() const { return m_system_time.value(); } void SetSystemTime(struct timespec stime) { m_system_time = stime; } bool SystemTimeIsValid() const { -return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0; +return m_system_time.has_value() && + (m_system_time->tv_sec > 0 || m_system_time->tv_usec > 0); } struct timespec GetCumulativeUserTime() const { -return m_cumulative_user_time; +return m_cumulative_user_time.value(); } void SetCumulativeUserTime(struct timespec cutime) { @@ -220,12 +220,13 @@ class ProcessInstanceInfo : public ProcessInfo { } bool CumulativeUserTimeIsValid() cons
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -91,14 +87,16 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, if (Rest.empty()) return false; StatFields stat_fields; - if (sscanf(Rest.data(), - "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld", - &stat_fields.pid, stat_fields.comm, &stat_fields.state, - &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session, - &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags, - &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt, - &stat_fields.cmajflt, &stat_fields.utime, &stat_fields.stime, - &stat_fields.cutime, &stat_fields.cstime) < 0) { + if (sscanf( + Rest.data(), + "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld", + &stat_fields.pid, stat_fields.comm, &stat_fields.state, + &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session, + &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags, + &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt, + &stat_fields.cmajflt, &stat_fields.utime, &stat_fields.stime, + &stat_fields.cutime, &stat_fields.cstime, + &stat_fields.realtime_priority, &stat_fields.priority) < 0) { feg208 wrote: from proc_pid_stat(5) ``` (18) priority %ld (Explanation for Linux 2.6) For processes running a real-time scheduling policy (policy below; see sched_setscheduler(2)), this is the negated scheduling priority, minus one; that is, a number in the range -2 to -100, corresponding to real-time priorities 1 to 99. For processes running under a non-real-time scheduling policy, this is the raw nice value (setpriority(2)) as represented in the kernel. The kernel stores nice values as numbers in the range 0 (high) to 39 (low), corre‐ sponding to the user-visible nice range of -20 to 19. Before Linux 2.6, this was a scaled value based on the scheduler weighting given to this process. ``` so yes https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to show enum as name and value at the same time (PR #90059)
https://github.com/clayborg requested changes to this pull request. So each `ValueObject` has the ability to show its value as an enumeration if its format is set to `eFormatEnum`. If the format is set to `eFormatHex`, `eFormatUnsigned`, or `eFormatSigned`, then we show the numeric value. Can you show some sample output for this? I would like the `const char *SBValue::GetValue()` function to obey the format that is selected and not return something that has both inside of it. It would be ok for the `const char *SBValue::GetSummary()` to return the enumeration if the format isn't set to `eFormatEnum`. This makes me question the need for the `ValueObject::SetEnumsAlwaysShowValue()` and `ValueObject::GetEnumsAlwaysShowValue()` methods, the ivars they use and many of the other changes here. https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to show enum as name and value at the same time (PR #90059)
clayborg wrote: If your register has fields, you can set its format to be eFormatEnum by default. Each register defines how it gets displayed when we query the dynamic register information from the `lldb-server` or `debugserver` https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo { ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) - : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX), -m_parent_pid(LLDB_INVALID_PROCESS_ID) {} + : ProcessInfo(name, arch, pid) {} void Clear() { ProcessInfo::Clear(); -m_euid = UINT32_MAX; -m_egid = UINT32_MAX; -m_parent_pid = LLDB_INVALID_PROCESS_ID; +m_euid = std::nullopt; +m_egid = std::nullopt; +m_parent_pid = std::nullopt; } - uint32_t GetEffectiveUserID() const { return m_euid; } + uint32_t GetEffectiveUserID() const { return m_euid.value(); } clayborg wrote: can we actually return `std::optional` here? Otherwise if someone calls this and `m_euid` doesn't have a value, then it will throw a `std::bad_optional_access` exception. There is a `std::optional::value_or(T)`, but that will defeat the reason for adding a std::optional in the first place. So I see two options: ``` uint32_t GetEffectiveUserID(uint32_t default_value = 0) const { return m_euid.value_or(default_value); } ``` or ``` std::optional GetEffectiveUserID() const { return m_euid; } ``` I would prefer the latter, but now sure now many locations this will affect. https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
https://github.com/hawkinsw commented: Sorry for the nits! I am not smart enough to contribute on the technical merits, so I am trying to find some way to help! https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
https://github.com/hawkinsw edited https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, return ts; }; + // priority (nice) values run from 19 to -20 inclusive (in linux). In the hawkinsw wrote: ```suggestion // Priority (nice) values run from 19 to -20 inclusive (in Linux). In the ``` https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -70,6 +71,12 @@ struct StatFields { long unsigned stime; long cutime; long cstime; + // in proc_pid_stat(5) this field is specified as priority + // but documented as realtime priority. To keep with the adopted + // nomenclature in ProcessInstanceInfo we adopt the documented hawkinsw wrote: ```suggestion // nomenclature in ProcessInstanceInfo, we adopt the documented ``` https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -86,4 +89,22 @@ TEST_F(HostTest, GetProcessInfo) { ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime(); ASSERT_TRUE(user_time.tv_sec <= next_user_time.tv_sec || user_time.tv_usec <= next_user_time.tv_usec); + + struct rlimit rlim; + EXPECT_EQ(getrlimit(RLIMIT_NICE, &rlim), 0); + // getpriority can return -1 so we zero errno first + errno = 0; + int prio = getpriority(PRIO_PROCESS, PRIO_PROCESS); + ASSERT_TRUE((prio < 0 && errno == 0) || prio >= 0); + ASSERT_EQ(Info.GetPriorityValue(), prio); + // If we can't raise our nice level then this test can't be performed hawkinsw wrote: ```suggestion // If we can't raise our nice level then this test can't be performed. ``` https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -70,6 +71,12 @@ struct StatFields { long unsigned stime; long cutime; long cstime; + // in proc_pid_stat(5) this field is specified as priority + // but documented as realtime priority. To keep with the adopted + // nomenclature in ProcessInstanceInfo we adopt the documented + // naming here hawkinsw wrote: ```suggestion // naming here. ``` https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -70,6 +71,12 @@ struct StatFields { long unsigned stime; long cutime; long cstime; + // in proc_pid_stat(5) this field is specified as priority hawkinsw wrote: ```suggestion // In proc_pid_stat(5) this field is specified as priority ``` https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, return ts; }; + // priority (nice) values run from 19 to -20 inclusive (in linux). In the + // prpsinfo struct pr_nice is a char hawkinsw wrote: ```suggestion // prpsinfo struct pr_nice is a char. ``` https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -144,6 +144,19 @@ class ProcessInstanceInfo : public ProcessInfo { long int tv_usec = 0; }; + enum class ProcessState { +Unknown, +Dead, +DiskSleep, +Idle, +Paging, +Parked, +Running, +Sleeping, +TracedOrStopped, +Zombie, + }; + feg208 wrote: moved back https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
clayborg wrote: This is causing a clang assertion due: ``` (lldb) type lookup std::ios_base Assertion failed: (DD && "queried property of class with no definition"), function data, file DeclCXX.h, line 464. bt (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert frame #0: 0x000180acaa60 libsystem_kernel.dylib`__pthread_kill + 8 frame #1: 0x000180b02c20 libsystem_pthread.dylib`pthread_kill + 288 frame #2: 0x000180a0fa20 libsystem_c.dylib`abort + 180 frame #3: 0x000180a0ed10 libsystem_c.dylib`__assert_rtn + 284 frame #4: 0x00012223ba64 LLDB`clang::CXXRecordDecl::data(this=0x000119ec5518) const at DeclCXX.h:464:5 frame #5: 0x0001223b7914 LLDB`clang::CXXRecordDecl::hasUserDeclaredMoveConstructor(this=0x000119ec5518) const at DeclCXX.h:858:12 frame #6: 0x000122399c0c LLDB`lldb_private::TypeSystemClang::CompleteTagDeclarationDefinition(type=0x00050296fd10) at TypeSystemClang.cpp:8370:28 * frame #7: 0x0001e4ac LLDB`DWARFASTParserClang::CompleteRecordType(this=0x00050296faa0, die=0x0001761932d0 0x0004be848030 (0x0090cdd5), type=0x00050296fca0, clang_type=0x00050296fd10) at DWARFASTParserClang.cpp:2291:3 frame #8: 0x0001f150 LLDB`DWARFASTParserClang::CompleteTypeFromDWARF(this=0x00050296faa0, die=0x0001761932d0 0x0004be848030 (0x0090cdd5), type=0x00050296fca0, clang_type=0x00050296fd10) at DWARFASTParserClang.cpp:2356:12 frame #9: 0x0001222b6eac LLDB`lldb_private::plugin::dwarf::SymbolFileDWARF::CompleteType(this=0x00011912e218, compiler_type=0x00050296fd10) at SymbolFileDWARF.cpp:1659:23 frame #10: 0x0001218952e0 LLDB`lldb_private::Type::ResolveCompilerType(this=0x00050296fca0, compiler_type_resolve_state=Full) at Type.cpp:715:24 frame #11: 0x000121895520 LLDB`lldb_private::Type::GetFullCompilerType(this=0x00050296fca0) at Type.cpp:755:3 frame #12: 0x0001218e49c8 LLDB`lldb_private::Language::ImageListTypeScavenger::Find_Impl(this=0x000118ebd000, exe_scope=0x00011a810e00, key="std::ios_base", results=size=0) at Language.cpp:473:43 frame #13: 0x0001218e47f8 LLDB`lldb_private::Language::TypeScavenger::Find(this=0x000118ebd000, exe_scope=0x00011a810e00, key="std::ios_base", results=size=0, append=true) at Language.cpp:456:13 frame #14: 0x00012418c958 LLDB`CommandObjectTypeLookup::DoExecute(this=0x000118e7a200, raw_command_line="std::ios_base", result=0x00016fdfe2f0) at CommandObjectType.cpp:2667:24 frame #15: 0x00012175fe3c LLDB`lldb_private::CommandObjectRaw::Execute(this=0x000118e7a200, args_string="std::ios_base", result=0x00016fdfe2f0) at CommandObject.cpp:850:7 frame #16: 0x000121733c74 LLDB`lldb_private::CommandInterpreter::HandleCommand(this=0x000100739130, command_line="type lookup std::ios_base", lazy_add_to_history=eLazyBoolCalculate, result=0x00016fdfe2f0, force_repeat_command=false) at CommandInterpreter.cpp:2038:14 frame #17: 0x000121738594 LLDB`lldb_private::CommandInterpreter::IOHandlerInputComplete(this=0x000100739130, io_handler=0x000118eb5918, line="type lookup std::ios_base") at CommandInterpreter.cpp:3118:3 frame #18: 0x0001214c9004 LLDB`lldb_private::IOHandlerEditline::Run(this=0x000118eb5918) at IOHandler.cpp:600:22 frame #19: 0x00012145982c LLDB`lldb_private::Debugger::RunIOHandlers(this=0x00010185fe00) at Debugger.cpp:1092:16 frame #20: 0x000121739bc4 LLDB`lldb_private::CommandInterpreter::RunCommandInterpreter(this=0x000100739130, options=0x000118e94340) at CommandInterpreter.cpp:3380:16 frame #21: 0x0001211496f0 LLDB`lldb::SBDebugger::RunCommandInterpreter(this=0x00016fdfeb90, options=0x00016fdfe878) at SBDebugger.cpp:1288:14 frame #22: 0x00016338 lldb`Driver::MainLoop(this=0x00016fdfeb70) at Driver.cpp:558:20 frame #23: 0x00016e54 lldb`main(argc=7, argv=0x00016fdff308) at Driver.cpp:807:26 frame #24: 0x00018077a0e0 dyld`start + 2360 ``` The DIE being passed to `DWARFASTParserClang::CompleteRecordType()` looks like: ``` 0x0090cdbf: DW_TAG_compile_unit DW_AT_producer("clang version 15.0.7 (mononoke://mononoke.internal.tfbnw.net/fbsource 79a342ef1fab2184ef28a1c26022c7cbaec90e83)") DW_AT_language(DW_LANG_C_plus_plus_14) DW_AT_name("fbcode/folly/detail/UniqueInstance.cpp") DW_AT_dwo_name ("buck-out/v2/gen/fbcode/0765d56217397ee3/hphp/hhvm/__hhvm_link__/libunique_instance.a/UniqueInstance.cpp.o.opt.o") 0x0090cdc7: DW_TAG_namespace DW_AT_name ("std") 0x0090cdd5: DW_TAG_class_type DW_AT_name("ios_base") DW_AT_declaration (true) 0x0090cdd7: DW_TAG_class_type DW_AT_name
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
clayborg wrote: Is `SymbolFileDWARF::CompleteType(...)` responsible for trying to find a non-declaration DIE first? The issue might arise from having a .debug_names table that has `DW_IDX_parent` entries that means that there might be forward declarations included in the DWARF index. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > So this DIE is just a declaration. Shouldn't this code have tried to find a > non declaration DIE for "std::ios_base"? Yes, I suppose `SymbolFileDWARF::CompleteType` will try to find the definition DIE for it before calling `DWARFASTParserClang::CompleteTypeFromDWARF`. If the definition DIE is not found, it's expected to do an early exit because Type is nullptr: https://github.com/llvm/llvm-project/blob/d8e73752a5f4f79ef4293d8f446c03062010233d/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1643-L1644. If found, we should have the definition DIE in the GetForwardDeclCompilerTypeToDIE map: https://github.com/llvm/llvm-project/blob/d8e73752a5f4f79ef4293d8f446c03062010233d/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1646-L1651 Can you confirm that there exists a definition DIE for it? How does it look like? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > The issue might arise from having a .debug_names table that has DW_IDX_parent > entries that means that there might be forward declarations included in the > DWARF index. Do you mean that the searching in the type index returns a declaration DIE (but I expected it to always return a definition DIE if exists: https://github.com/llvm/llvm-project/blob/91feb130d5cd3cafce94bbaf7ad67d1542623a75/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3135)? If that's the case, we get a type created from declaration DIE `SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext`, which makes it falls through the check. A quick fix I have in mind is to do a double-check if the dwarf_die is a declaration or not before: https://github.com/llvm/llvm-project/blob/d8e73752a5f4f79ef4293d8f446c03062010233d/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1659-L1661, so we won't complete it. But if there truly exists a definition DIE, it will never be completed because it relies on index to find definition DIE. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
clayborg wrote: Ok, I found the issue. `.debug_names` tables with `DW_IDX_parent` entries, might contain tons of entries for forward declared classes because in my example `std::ios_base` is the parent declaration context for `seekdir`, `openmode`, and `iostate` so `.debug_names` entries for these types will refer to another `.debug_names` for `std::ios_base` as the parent, buit this is messing up the table itself as now it contains entries that are not all full definitions. The `.debug_names` spec states that only entries with definitions should be in the .debug_names table... That being said, an easy fix is this: ```bool DebugNamesDWARFIndex::ProcessEntry( const DebugNames::Entry &entry, llvm::function_ref callback) { std::optional ref = ToDIERef(entry); if (!ref) return true; SymbolFileDWARF &dwarf = *llvm::cast( m_module.GetSymbolFile()->GetBackingSymbolFile()); DWARFDIE die = dwarf.GetDIE(*ref); if (!die) return true; // Watch out for forward declarations that appear in the .debug_names tables // only due to being there for a DW_IDX_parent. if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) /// <<< newly added for fix return true;/// <<< newly added for fix return callback(die); } ``` https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
clayborg wrote: See the `/// <<< newly added for fix` comments for the new lines https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/91799 Fix the problem: https://github.com/llvm/llvm-project/pull/90663#issuecomment-2105164917 by enhancing a double-check for #90663 >From 1f6bf890dbfce07b6ab531597e876ab83cfd1298 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 10 May 2024 16:00:22 -0400 Subject: [PATCH] [lldb][DWARF] Do not complete type from declaration die. --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e0b1b430b266f..315ba4cc0ccdf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,7 +2343,10 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); - if (!die) + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (!die || is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) Changes Fix the problem: https://github.com/llvm/llvm-project/pull/90663#issuecomment-2105164917 by enhancing a double-check for #90663 --- Full diff: https://github.com/llvm/llvm-project/pull/91799.diff 1 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+4-1) ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e0b1b430b266f..315ba4cc0ccdf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,7 +2343,10 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); - if (!die) + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (!die || is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); `` https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: I sent an alternative fix at https://github.com/llvm/llvm-project/pull/91799. > The .debug_names spec states that only entries with definitions should be in > the .debug_names table... Do it mean the .debug_names is implemented incorrectly? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/clayborg commented: Let me verify this works. I would also like this to fix: ``` bool DebugNamesDWARFIndex::ProcessEntry( const DebugNames::Entry &entry, llvm::function_ref callback) { std::optional ref = ToDIERef(entry); if (!ref) return true; SymbolFileDWARF &dwarf = *llvm::cast( m_module.GetSymbolFile()->GetBackingSymbolFile()); DWARFDIE die = dwarf.GetDIE(*ref); if (!die) return true; // Watch out for forward declarations that appear in the .debug_names tables // only due to being there for a DW_IDX_parent. if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) return true; return callback(die); } ``` This adds: ``` // Watch out for forward declarations that appear in the .debug_names tables // only due to being there for a DW_IDX_parent. if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) return true; ``` To the above function to ensure we don't waste any time trying to parse any DIE information for forward declaration when using .debug_names. This will cause a TON of churn on our DWARF parser and cause us to pull in and parse way too much. https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
felipepiovezan wrote: AFAICT we never added new entries -- definitely not forward declarations -- to the table when doing the idx_parent work. Either they were already there, or the entry would have no parent. Would be nice to have an example to see this in action. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
ZequanWu wrote: > To the above function to ensure we don't waste any time trying to parse any > DIE information for forward declaration when using .debug_names. This will > cause a TON of churn on our DWARF parser and cause us to pull in and parse > way too much. That sounds like a better fix. https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/royitaqi updated https://github.com/llvm/llvm-project/pull/90703 >From 0fd67e2de7e702ce6f7353845454ea7ff9f980d6 Mon Sep 17 00:00:00 2001 From: Roy Shi Date: Tue, 30 Apr 2024 21:35:49 -0700 Subject: [PATCH 01/12] Add SBCommandInterpreter::GetTranscript() --- lldb/include/lldb/API/SBCommandInterpreter.h | 12 +--- lldb/source/API/SBCommandInterpreter.cpp | 7 ++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h b/lldb/include/lldb/API/SBCommandInterpreter.h index ba2e049204b8e..d65f06d676f91 100644 --- a/lldb/include/lldb/API/SBCommandInterpreter.h +++ b/lldb/include/lldb/API/SBCommandInterpreter.h @@ -247,13 +247,13 @@ class SBCommandInterpreter { lldb::SBStringList &matches, lldb::SBStringList &descriptions); - /// Returns whether an interrupt flag was raised either by the SBDebugger - + /// Returns whether an interrupt flag was raised either by the SBDebugger - /// when the function is not running on the RunCommandInterpreter thread, or /// by SBCommandInterpreter::InterruptCommand if it is. If your code is doing - /// interruptible work, check this API periodically, and interrupt if it + /// interruptible work, check this API periodically, and interrupt if it /// returns true. bool WasInterrupted() const; - + /// Interrupts the command currently executing in the RunCommandInterpreter /// thread. /// @@ -318,6 +318,12 @@ class SBCommandInterpreter { SBStructuredData GetStatistics(); + /// Returns a list of handled commands, output and error. Each element in + /// the list is a dictionary with three keys: "command" (string), "output" + /// (list of strings) and optionally "error" (list of strings). Each string + /// in "output" and "error" is a line (without EOL characteres). + SBStructuredData GetTranscript(); + protected: friend class lldb_private::CommandPluginInterfaceImplementation; diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp index 83c0951c56db6..242b3f8f09c48 100644 --- a/lldb/source/API/SBCommandInterpreter.cpp +++ b/lldb/source/API/SBCommandInterpreter.cpp @@ -150,7 +150,7 @@ bool SBCommandInterpreter::WasInterrupted() const { bool SBCommandInterpreter::InterruptCommand() { LLDB_INSTRUMENT_VA(this); - + return (IsValid() ? m_opaque_ptr->InterruptCommand() : false); } @@ -571,6 +571,11 @@ SBStructuredData SBCommandInterpreter::GetStatistics() { return data; } +SBStructuredData SBCommandInterpreter::GetTranscript() { + LLDB_INSTRUMENT_VA(this); + return SBStructuredData(); +} + lldb::SBCommand SBCommandInterpreter::AddMultiwordCommand(const char *name, const char *help) { LLDB_INSTRUMENT_VA(this, name, help); >From a1c948ceabaccdc3407e0c4eae0ebc594a9b68b7 Mon Sep 17 00:00:00 2001 From: Roy Shi Date: Wed, 1 May 2024 13:45:47 -0700 Subject: [PATCH 02/12] Implement the new API --- .../lldb/Interpreter/CommandInterpreter.h | 12 +-- lldb/include/lldb/Utility/StructuredData.h| 11 +++--- lldb/source/API/SBCommandInterpreter.cpp | 8 - .../source/Interpreter/CommandInterpreter.cpp | 21 ++- lldb/source/Utility/StructuredData.cpp| 35 +++ 5 files changed, 79 insertions(+), 8 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 70a55a77465bf..9474c41c0dced 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -22,6 +22,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StringList.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private.h" @@ -241,7 +242,7 @@ class CommandInterpreter : public Broadcaster, eCommandTypesAllThem = 0x //< all commands }; - // The CommandAlias and CommandInterpreter both have a hand in + // The CommandAlias and CommandInterpreter both have a hand in // substituting for alias commands. They work by writing special tokens // in the template form of the Alias command, and then detecting them when the // command is executed. These are the special tokens: @@ -576,7 +577,7 @@ class CommandInterpreter : public Broadcaster, void SetEchoCommentCommands(bool enable); bool GetRepeatPreviousCommand() const; - + bool GetRequireCommandOverwrite() const; const CommandObject::CommandMap &GetUserCommands() const { @@ -647,6 +648,7 @@ class CommandInterpreter : public Broadcaster, } llvm::json::Value GetStatistics(); + StructuredData::ArraySP GetTranscript() const; protected: friend class Debugger; @@ -766,6 +768,12 @@ class CommandInterpreter : public Broadcaster
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/royitaqi updated https://github.com/llvm/llvm-project/pull/90703 >From 0fd67e2de7e702ce6f7353845454ea7ff9f980d6 Mon Sep 17 00:00:00 2001 From: Roy Shi Date: Tue, 30 Apr 2024 21:35:49 -0700 Subject: [PATCH 01/13] Add SBCommandInterpreter::GetTranscript() --- lldb/include/lldb/API/SBCommandInterpreter.h | 12 +--- lldb/source/API/SBCommandInterpreter.cpp | 7 ++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h b/lldb/include/lldb/API/SBCommandInterpreter.h index ba2e049204b8e..d65f06d676f91 100644 --- a/lldb/include/lldb/API/SBCommandInterpreter.h +++ b/lldb/include/lldb/API/SBCommandInterpreter.h @@ -247,13 +247,13 @@ class SBCommandInterpreter { lldb::SBStringList &matches, lldb::SBStringList &descriptions); - /// Returns whether an interrupt flag was raised either by the SBDebugger - + /// Returns whether an interrupt flag was raised either by the SBDebugger - /// when the function is not running on the RunCommandInterpreter thread, or /// by SBCommandInterpreter::InterruptCommand if it is. If your code is doing - /// interruptible work, check this API periodically, and interrupt if it + /// interruptible work, check this API periodically, and interrupt if it /// returns true. bool WasInterrupted() const; - + /// Interrupts the command currently executing in the RunCommandInterpreter /// thread. /// @@ -318,6 +318,12 @@ class SBCommandInterpreter { SBStructuredData GetStatistics(); + /// Returns a list of handled commands, output and error. Each element in + /// the list is a dictionary with three keys: "command" (string), "output" + /// (list of strings) and optionally "error" (list of strings). Each string + /// in "output" and "error" is a line (without EOL characteres). + SBStructuredData GetTranscript(); + protected: friend class lldb_private::CommandPluginInterfaceImplementation; diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp index 83c0951c56db6..242b3f8f09c48 100644 --- a/lldb/source/API/SBCommandInterpreter.cpp +++ b/lldb/source/API/SBCommandInterpreter.cpp @@ -150,7 +150,7 @@ bool SBCommandInterpreter::WasInterrupted() const { bool SBCommandInterpreter::InterruptCommand() { LLDB_INSTRUMENT_VA(this); - + return (IsValid() ? m_opaque_ptr->InterruptCommand() : false); } @@ -571,6 +571,11 @@ SBStructuredData SBCommandInterpreter::GetStatistics() { return data; } +SBStructuredData SBCommandInterpreter::GetTranscript() { + LLDB_INSTRUMENT_VA(this); + return SBStructuredData(); +} + lldb::SBCommand SBCommandInterpreter::AddMultiwordCommand(const char *name, const char *help) { LLDB_INSTRUMENT_VA(this, name, help); >From a1c948ceabaccdc3407e0c4eae0ebc594a9b68b7 Mon Sep 17 00:00:00 2001 From: Roy Shi Date: Wed, 1 May 2024 13:45:47 -0700 Subject: [PATCH 02/13] Implement the new API --- .../lldb/Interpreter/CommandInterpreter.h | 12 +-- lldb/include/lldb/Utility/StructuredData.h| 11 +++--- lldb/source/API/SBCommandInterpreter.cpp | 8 - .../source/Interpreter/CommandInterpreter.cpp | 21 ++- lldb/source/Utility/StructuredData.cpp| 35 +++ 5 files changed, 79 insertions(+), 8 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 70a55a77465bf..9474c41c0dced 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -22,6 +22,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StringList.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private.h" @@ -241,7 +242,7 @@ class CommandInterpreter : public Broadcaster, eCommandTypesAllThem = 0x //< all commands }; - // The CommandAlias and CommandInterpreter both have a hand in + // The CommandAlias and CommandInterpreter both have a hand in // substituting for alias commands. They work by writing special tokens // in the template form of the Alias command, and then detecting them when the // command is executed. These are the special tokens: @@ -576,7 +577,7 @@ class CommandInterpreter : public Broadcaster, void SetEchoCommentCommands(bool enable); bool GetRepeatPreviousCommand() const; - + bool GetRequireCommandOverwrite() const; const CommandObject::CommandMap &GetUserCommands() const { @@ -647,6 +648,7 @@ class CommandInterpreter : public Broadcaster, } llvm::json::Value GetStatistics(); + StructuredData::ArraySP GetTranscript() const; protected: friend class Debugger; @@ -766,6 +768,12 @@ class CommandInterpreter : public Broadcaster
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
@@ -2044,6 +2052,15 @@ bool CommandInterpreter::HandleCommand(const char *command_line, m_transcript_stream << result.GetOutputData(); m_transcript_stream << result.GetErrorData(); + // Add output and error to the transcript item after splitting lines. In the + // future, other aspects of the command (e.g. perf) can be added, too. + transcript_item->AddItem( + "output", StructuredData::Array::SplitString(result.GetOutputData(), '\n', + -1, false)); + transcript_item->AddItem( + "error", StructuredData::Array::SplitString(result.GetErrorData(), '\n', + -1, false)); royitaqi wrote: Removed the split, and the utility function. Now `StructuredData.h/.cpp` are unchanged. https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
@@ -290,6 +290,31 @@ class StructuredData { void GetDescription(lldb_private::Stream &s) const override; +/// Creates an Array of substrings by splitting a string around the +/// occurrences of a separator character. +/// +/// \param[in] s +/// The input string. +/// +/// \param[in] separator +/// The character to split on. +/// +/// \param[in] maxSplit +/// The maximum number of times the string is split. If \a maxSplit is >= +/// 0, at most \a maxSplit splits are done and consequently <= \a maxSplit +/// + 1 elements are returned. +/// +/// \param[in] keepEmpty +/// True if empty substrings should be returned. Empty substrings still +/// count when considering \a maxSplit. +/// +/// \return +/// An array containing the substrings. If \a maxSplit == -1 and \a +/// keepEmpty == true, then the concatination of the array forms the input +/// string. +static ArraySP SplitString(llvm::StringRef s, char separator, + int maxSplit = -1, bool keepEmpty = true); + royitaqi wrote: Same as the above. > Removed the split, and the utility function. Now `StructuredData.h/.cpp` are > unchanged. https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/clayborg approved this pull request. This does fix things on your side. I will take care of a new PR for not searching all definition DIEs from the .debug_names. https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
@@ -2343,7 +2343,10 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); - if (!die) + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (!die || is_forward_declaration) clayborg wrote: Maybe this should first check if `die` is valid, then do this?: ``` if (!die) return false; ParsedDWARFTypeAttributes attrs(die); if (IsForwardDeclaration(die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU( return false; ``` https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
@@ -85,3 +86,91 @@ def test_command_output(self): self.assertEqual(res.GetOutput(), "") self.assertIsNotNone(res.GetError()) self.assertEqual(res.GetError(), "") + +def test_structured_transcript(self): +"""Test structured transcript generation and retrieval.""" +# Get command interpreter and create a target +self.build() +exe = self.getBuildArtifact("a.out") + +target = self.dbg.CreateTarget(exe) +self.assertTrue(target, VALID_TARGET) + +ci = self.dbg.GetCommandInterpreter() +self.assertTrue(ci, VALID_COMMAND_INTERPRETER) + +# Send a few commands through the command interpreter +res = lldb.SBCommandReturnObject() +ci.HandleCommand("version", res) +ci.HandleCommand("an-unknown-command", res) +ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res) +ci.HandleCommand("r", res) +ci.HandleCommand("p a", res) royitaqi wrote: Added the "statistics dump" test command. Validated that 1\ the output is valid JSON (through `json.loads`) and 2\ the output contains expected fields, like "commands" and "modules". https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/ZequanWu updated https://github.com/llvm/llvm-project/pull/91799 >From 1f6bf890dbfce07b6ab531597e876ab83cfd1298 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 10 May 2024 16:00:22 -0400 Subject: [PATCH 1/2] [lldb][DWARF] Do not complete type from declaration die. --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e0b1b430b266f..315ba4cc0ccdf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,7 +2343,10 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); - if (!die) + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (!die || is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); >From 3e2791d99be0a21351ee4bf57beaf2c30ba5256e Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 10 May 2024 16:38:54 -0400 Subject: [PATCH 2/2] address comment --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 315ba4cc0ccdf..2a46be9216121 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,10 +2343,12 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); + if (!die) +return false; ParsedDWARFTypeAttributes attrs(die); bool is_forward_declaration = IsForwardDeclaration( die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); - if (!die || is_forward_declaration) + if (is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a7eff59 - [lldb][DWARF] Do not complete type from declaration die. (#91799)
Author: Zequan Wu Date: 2024-05-10T16:39:20-04:00 New Revision: a7eff59f78f08f8ef0487dfe2a136fb311af4fd0 URL: https://github.com/llvm/llvm-project/commit/a7eff59f78f08f8ef0487dfe2a136fb311af4fd0 DIFF: https://github.com/llvm/llvm-project/commit/a7eff59f78f08f8ef0487dfe2a136fb311af4fd0.diff LOG: [lldb][DWARF] Do not complete type from declaration die. (#91799) Fix the problem: https://github.com/llvm/llvm-project/pull/90663#issuecomment-2105164917 by enhancing a double-check for #90663 Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e0b1b430b266f..2a46be9216121 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2345,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, if (!die) return false; + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (is_forward_declaration) +return false; const dw_tag_t tag = die.Tag(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
https://github.com/royitaqi closed https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
royitaqi wrote: Gentle ping. :) @jim https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
https://github.com/royitaqi reopened https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
https://github.com/clayborg created https://github.com/llvm/llvm-project/pull/91808 When a .debug_names table has entries that use the DW_IDX_parent attributes, we can end up with entries in the .debug_names table that are not full definitions. This is because a class that is foward declared, can contain types. For example: 0x0090cdbf: DW_TAG_compile_unit DW_AT_producer("clang version 15.0.7") DW_AT_language(DW_LANG_C_plus_plus_14) DW_AT_name("UniqueInstance.cpp") 0x0090cdc7: DW_TAG_namespace DW_AT_name ("std") 0x0090cdd5: DW_TAG_class_type DW_AT_name("ios_base") DW_AT_declaration (true) 0x0090cdd7: DW_TAG_class_type DW_AT_name ("Init") DW_AT_declaration (true) 0x0090cdda: DW_TAG_typedef DW_AT_type (0x0090ce4e "std::_Ios_Seekdir") DW_AT_name ("seekdir") DW_AT_decl_file (0x11) DW_AT_decl_line (479) 0x0090cde4: DW_TAG_typedef DW_AT_type (0x0090ce45 "std::_Ios_Openmode") DW_AT_name ("openmode") DW_AT_decl_file (0x11) DW_AT_decl_line (447) 0x0090cdee: DW_TAG_typedef DW_AT_type (0x0090ce3c "std::_Ios_Iostate") DW_AT_name ("iostate") DW_AT_decl_file (0x11) DW_AT_decl_line (416) 0x0090cdf8: NULL "std::ios_base" is forward declared and it contains typedefs whose entries in the .debug_names table will point to the DIE at offset 0x0090cdd5. These entries cause our type lookups to try and parse a TON of forward declarations and waste time and resources. This fix makes sure when/if we find an entry in the .debug_names table, we don't process it if it has a DW_AT_declaration(true) attribute. >From 0b4bc90db373d9ccae686f326d9eedceed68394e Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Fri, 10 May 2024 13:49:22 -0700 Subject: [PATCH] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used. When a .debug_names table has entries that use the DW_IDX_parent attributes, we can end up with entries in the .debug_names table that are not full definitions. This is because a class that is foward declared, can contain types. For example: 0x0090cdbf: DW_TAG_compile_unit DW_AT_producer("clang version 15.0.7") DW_AT_language(DW_LANG_C_plus_plus_14) DW_AT_name("UniqueInstance.cpp") 0x0090cdc7: DW_TAG_namespace DW_AT_name ("std") 0x0090cdd5: DW_TAG_class_type DW_AT_name("ios_base") DW_AT_declaration (true) 0x0090cdd7: DW_TAG_class_type DW_AT_name ("Init") DW_AT_declaration (true) 0x0090cdda: DW_TAG_typedef DW_AT_type (0x0090ce4e "std::_Ios_Seekdir") DW_AT_name ("seekdir") DW_AT_decl_file (0x11) DW_AT_decl_line (479) 0x0090cde4: DW_TAG_typedef DW_AT_type (0x0090ce45 "std::_Ios_Openmode") DW_AT_name ("openmode") DW_AT_decl_file (0x11) DW_AT_decl_line (447) 0x0090cdee: DW_TAG_typedef DW_AT_type (0x0090ce3c "std::_Ios_Iostate") DW_AT_name ("iostate") DW_AT_decl_file (0x11) DW_AT_decl_line (416) 0x0090cdf8: NULL "std::ios_base" is forward declared and it contains typedefs whose entries in the .debug_names table will point to the DIE at offset 0x0090cdd5. These entries cause our type lookups to try and parse a TON of forward declarations and waste time and resources. This fix makes sure when/if we find an entry in the .debug_names table, we don't process it if it has a DW_AT_declaration(true) attribute. --- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 4 1 file changed, 4 insertions(+) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 4da0d56fdcacb..f54e8071d97b6 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -83,6 +83,10 @@ bool DebugNamesDWARFIndex::ProcessEntry( DWARFDIE die = dwarf.GetDIE(*ref); if (!die) return true; + // Watch out for forward declarations that appear in the .debug_names tables + // only due to being there for a DW_IDX_parent. + // if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) + // return true; return callback(die); } ___ lldb-co
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Greg Clayton (clayborg) Changes When a .debug_names table has entries that use the DW_IDX_parent attributes, we can end up with entries in the .debug_names table that are not full definitions. This is because a class that is foward declared, can contain types. For example: ``` 0x0090cdbf: DW_TAG_compile_unit DW_AT_producer("clang version 15.0.7") DW_AT_language(DW_LANG_C_plus_plus_14) DW_AT_name("UniqueInstance.cpp") 0x0090cdc7: DW_TAG_namespace DW_AT_name ("std") 0x0090cdd5: DW_TAG_class_type DW_AT_name("ios_base") DW_AT_declaration (true) 0x0090cdd7: DW_TAG_class_type DW_AT_name ("Init") DW_AT_declaration (true) 0x0090cdda: DW_TAG_typedef DW_AT_type (0x0090ce4e "std::_Ios_Seekdir") DW_AT_name ("seekdir") DW_AT_decl_file (0x11) DW_AT_decl_line (479) 0x0090cde4: DW_TAG_typedef DW_AT_type (0x0090ce45 "std::_Ios_Openmode") DW_AT_name ("openmode") DW_AT_decl_file (0x11) DW_AT_decl_line (447) 0x0090cdee: DW_TAG_typedef DW_AT_type (0x0090ce3c "std::_Ios_Iostate") DW_AT_name ("iostate") DW_AT_decl_file (0x11) DW_AT_decl_line (416) 0x0090cdf8: NULL ``` "std::ios_base" is forward declared and it contains typedefs whose entries in the .debug_names table will point to the DIE at offset 0x0090cdd5. These entries cause our type lookups to try and parse a TON of forward declarations and waste time and resources. This fix makes sure when/if we find an entry in the .debug_names table, we don't process it if it has a DW_AT_declaration(true) attribute. --- Full diff: https://github.com/llvm/llvm-project/pull/91808.diff 1 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp (+4) ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 4da0d56fdcacb..f54e8071d97b6 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -83,6 +83,10 @@ bool DebugNamesDWARFIndex::ProcessEntry( DWARFDIE die = dwarf.GetDIE(*ref); if (!die) return true; + // Watch out for forward declarations that appear in the .debug_names tables + // only due to being there for a DW_IDX_parent. + // if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) + // return true; return callback(die); } `` https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/91808 >From 0cc1be6988e6ab5498151f32485f525a66133be2 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Fri, 10 May 2024 13:49:22 -0700 Subject: [PATCH] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used. When a .debug_names table has entries that use the DW_IDX_parent attributes, we can end up with entries in the .debug_names table that are not full definitions. This is because a class that is foward declared, can contain types. For example: 0x0090cdbf: DW_TAG_compile_unit DW_AT_producer("clang version 15.0.7") DW_AT_language(DW_LANG_C_plus_plus_14) DW_AT_name("UniqueInstance.cpp") 0x0090cdc7: DW_TAG_namespace DW_AT_name ("std") 0x0090cdd5: DW_TAG_class_type DW_AT_name("ios_base") DW_AT_declaration (true) 0x0090cdd7: DW_TAG_class_type DW_AT_name ("Init") DW_AT_declaration (true) 0x0090cdda: DW_TAG_typedef DW_AT_type (0x0090ce4e "std::_Ios_Seekdir") DW_AT_name ("seekdir") DW_AT_decl_file (0x11) DW_AT_decl_line (479) 0x0090cde4: DW_TAG_typedef DW_AT_type (0x0090ce45 "std::_Ios_Openmode") DW_AT_name ("openmode") DW_AT_decl_file (0x11) DW_AT_decl_line (447) 0x0090cdee: DW_TAG_typedef DW_AT_type (0x0090ce3c "std::_Ios_Iostate") DW_AT_name ("iostate") DW_AT_decl_file (0x11) DW_AT_decl_line (416) 0x0090cdf8: NULL "std::ios_base" is forward declared and it contains typedefs whose entries in the .debug_names table will point to the DIE at offset 0x0090cdd5. These entries cause our type lookups to try and parse a TON of forward declarations and waste time and resources. This fix makes sure when/if we find an entry in the .debug_names table, we don't process it if it has a DW_AT_declaration(true) attribute. --- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 4 1 file changed, 4 insertions(+) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 4da0d56fdcacb..eaa9f591ffd41 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -83,6 +83,10 @@ bool DebugNamesDWARFIndex::ProcessEntry( DWARFDIE die = dwarf.GetDIE(*ref); if (!die) return true; + // Watch out for forward declarations that appear in the .debug_names tables + // only due to being there for a DW_IDX_parent. + if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) +return true; return callback(die); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
jeffreytan81 wrote: The change/explanation looks intuitive, but I remember having seen DIE entry with `DW_AT_declaration (true)` but is not a forward declaration (it is a definition and has other attributes) . Will we cause regression in that case? https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: we should probably fix the underlying issue instead: https://github.com/llvm/llvm-project/issues/77696 https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
clayborg wrote: > The change/explanation looks intuitive, but I remember having seen DIE entry > with `DW_AT_declaration (true)` but is not a forward declaration (it is a > definition and has other attributes) . Will we cause regression in that case? No, it is ok for `DW_AT_declaration(true)` DIEs to be in the DWARF and contain children. They just won't declare the type itself. In the example above, you can see that "ios_base" contains typedefs. https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
clayborg wrote: > we should probably fix the underlying issue instead: #77696 This is one fix that is needed. Quoted from an e-mail chain: > I need to find my notes from those days, but I don't think we did. As Greg > points out, the standard forbids forward declarations in debug_names; entries > whose parents are forward declarations should be marked as having a parent > that is not indexed. > The addition of IDX_parent_entries should not cause the addition of _entries_ > in the table, if it does then it is a bug in the implementation. > In fact, in the original PR we have this bit of code: > https://github.com/llvm/llvm-project/pull/77457/files#diff-587587ad06ddb6f99f9ad8d8deffbc2ea59fde9d62d1b5ff58ace1f52cc75752R405 ``` std::optional DWARF5AccelTableData::getDefiningParentDieOffset(const DIE &Die) { if (auto *Parent = Die.getParent(); Parent && !Parent->findAttribute(dwarf::Attribute::DW_AT_declaration)) return Parent->getOffset(); return {}; } ``` https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
clayborg wrote: > we should probably fix the underlying issue instead: #77696 We still have binaries that are floating around for now that contain this issue and this was causing crashes. So it would be nice to fix this in LLDB for now and back this out after we have a stable and trustable .debug_names section? https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
@@ -1689,35 +1689,56 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, void SBDebugger::SetDestroyCallback( lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) { LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + if (m_opaque_sp) { -return m_opaque_sp->SetDestroyCallback( -destroy_callback, baton); +m_opaque_sp->SetDestroyCallback(destroy_callback, baton); } } +lldb::destroy_callback_token_t +SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, + void *baton) { + LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + + if (m_opaque_sp) { +return m_opaque_sp->AddDestroyCallback(destroy_callback, baton); + } + return LLDB_INVALID_DESTROY_CALLBACK_TOKEN; +} + +bool SBDebugger::RemoveDestroyCallback(lldb::destroy_callback_token_t token) { + LLDB_INSTRUMENT_VA(this, token); + + if (m_opaque_sp) { +return m_opaque_sp->RemoveDestroyCallback(token); + } clayborg wrote: remove braces for single line if statements per llvm coding guidelines. https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
@@ -1689,35 +1689,56 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, void SBDebugger::SetDestroyCallback( lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) { LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + if (m_opaque_sp) { -return m_opaque_sp->SetDestroyCallback( -destroy_callback, baton); +m_opaque_sp->SetDestroyCallback(destroy_callback, baton); } } +lldb::destroy_callback_token_t +SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, + void *baton) { + LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + + if (m_opaque_sp) { +return m_opaque_sp->AddDestroyCallback(destroy_callback, baton); + } clayborg wrote: remove braces for single line `if` statements per llvm coding guidelines. https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
@@ -731,8 +747,11 @@ class Debugger : public std::enable_shared_from_this, lldb::TargetSP m_dummy_target_sp; Diagnostics::CallbackID m_diagnostics_callback_id; - lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr; - void *m_destroy_callback_baton = nullptr; + std::recursive_mutex m_destroy_callback_mutex; clayborg wrote: Just change this to a std::mutex? https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
https://github.com/clayborg requested changes to this pull request. We should make this thread safe. It can only help and this isn't an API which gets called all of the time, so performance isn't an issue. A few inline comments https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: > I have fixed/worked around the mach exception issue in a [followup > commit](https://github.com/llvm/llvm-project/commit/b903badd73a2467fdd4e363231f2bf9b0704b546) > with a `settings set platform.plugin.darwin.ignored-exceptions > EXC_BAD_INSTRUCTION`. Now the process gets a SIGILL as expected, but it still > fails at the backtrace step (the second one, after stopping inside the > handler). Oh, that's a clever idea, I forgot about that setting. If you add `process handle -p true -s false SIGILL` (lldb was stopping on the SIGILL signal before sigtramp / the registered handler ran), then sigill_handler is called, ``` (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00013f4c a.out`sigill_handler(signo=4) + 20 at signal-in-leaf-function-aarch64.c:10 frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56 frame #2: 0x00013f7c a.out`main + 44 at signal-in-leaf-function-aarch64.c:14 frame #3: 0x000197bda0e0 dyld`start + 2360 ``` https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
chelcassanova wrote: Hm, so in that case should we focus on adding an `SBStream::GetUseColor` so that the stream's colour settings can take precedence here? https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
adrian-prantl wrote: Could this commit have broken the bots? https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1897/ https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: Ah, I misunderstood what the nature of the failure was. I tried running the shell test, and it's failing for different reasons. I almost never touch shell tests, I find them really hard to debug so I'm not sure what the problem is. If I run it by hand, ``` (lldb) settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_INSTRUCTION (lldb) b sigill_handler Breakpoint 1: where = a.out`sigill_handler + 20 at signal-in-leaf-function-aarch64.c:10:34, address = 0x00013f4c (lldb) r Process 25854 launched: '/tmp/a.out' (arm64) Process 25854 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGILL frame #0: 0x00013f2c a.out`signal_generating_add at signal-in-leaf-function-aarch64.c:5:3 2#include 3 4int __attribute__((naked)) signal_generating_add(int a, int b) { -> 5 asm("add w0, w1, w0\n\t" 6 "udf #0xdead\n\t" 7 "ret"); 8} (lldb) c Process 25854 resuming Process 25854 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 frame #0: 0x00013f4c a.out`sigill_handler(signo=4) at signal-in-leaf-function-aarch64.c:10:34 7 "ret"); 8} 9 -> 10 void sigill_handler(int signo) { _exit(0); } 11 12 int main() { 13 signal(SIGILL, sigill_handler); (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00013f4c a.out`sigill_handler(signo=4) at signal-in-leaf-function-aarch64.c:10:34 frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56 frame #2: 0x00013f7c a.out`main at signal-in-leaf-function-aarch64.c:14:3 frame #3: 0x000197bda0e0 dyld`start + 2360 (lldb) ``` which all looks good to me, but it the shell test fails with ``` /Volumes/work/llvm/llvm-project/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test:26:10: error: CHECK: expected string not found in input # CHECK: frame #{{[0-9]+}}: [[ADD]] {{.*}}`signal_generating_add ^ :32:86: note: scanning from here frame #0: 0x00013f38 signal-in-leaf-function-aarch64.test.tmp`sigill_handler ^ :32:86: note: with "ADD" equal to "0x00013f2c" frame #0: 0x00013f38 signal-in-leaf-function-aarch64.test.tmp`sigill_handler ^ :33:23: note: possible intended match here signal-in-leaf-function-aarch64.test.tmp`sigill_handler: ^ Input file: Check file: /Volumes/work/llvm/llvm-project/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test -dump-input=help explains the following input dump. Input was: << . . . 27: frame #2: 0x000197bda0e0 dyld`start + 2360 28: (lldb) continue 29: Process 23467 resuming 30: Process 23467 stopped 31: * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 32: frame #0: 0x00013f38 signal-in-leaf-function-aarch64.test.tmp`sigill_handler check:26'0 X error: no match found check:26'1 with "ADD" equal to "0x00013f2c" 33: signal-in-leaf-function-aarch64.test.tmp`sigill_handler: check:26'0 ~ check:26'2 ? possible intended match 34: -> 0x13f38 <+0>: sub sp, sp, #0x20 check:26'0 ~~~ 35: 0x13f3c <+4>: stp x29, x30, [sp, #0x10] check:26'0 ~ 36: 0x13f40 <+8>: add x29, sp, #0x10 check:26'0 ~~ 37: 0x13f44 <+12>: stur w0, [x29, #-0x4] check:26'0 ~~ 38: (lldb) thread backtrace check:26'0 . . . >> ``` https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: maybe the shell test is building without debug info, I am surprised to see assembly there. If I build it like that and run it by hand, ``` (lldb) settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_INSTRUCTION (lldb) b sigill_handler Breakpoint 1: where = a.out`sigill_handler, address = 0x00013f38 (lldb) r Process 25891 launched: '/tmp/a.out' (arm64) Process 25891 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGILL frame #0: 0x00013f2c a.out`signal_generating_add + 4 a.out`signal_generating_add: -> 0x13f2c <+4>: udf#0xdead 0x13f30 <+8>: ret 0x13f34 <+12>: brk#0x1 a.out`sigill_handler: 0x13f38 <+0>: subsp, sp, #0x20 (lldb) c Process 25891 resuming Process 25891 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 frame #0: 0x00013f38 a.out`sigill_handler a.out`sigill_handler: -> 0x13f38 <+0>: subsp, sp, #0x20 0x13f3c <+4>: stpx29, x30, [sp, #0x10] 0x13f40 <+8>: addx29, sp, #0x10 0x13f44 <+12>: stur w0, [x29, #-0x4] (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00013f38 a.out`sigill_handler frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56 frame #2: 0x00013f7c a.out`main + 44 frame #3: 0x000197bda0e0 dyld`start + 2360 (lldb) ``` https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add CMake dependency tracking for SBLanguages generation script (PR #91686)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/91686 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > Could this commit have broken the bots? > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1897/ Looks like so, but I cannot repro the test failure locally. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
jimingham wrote: > Hm, so in that case should we focus on adding an `SBStream::GetUseColor` so > that the stream's colour settings can take precedence here? That's the only way it makes sense to me. But if we are going to rely on the stream, we also have to be sure that we're setting the Streams "use color" correctly when we make them. That might be an annoying accounting exercise. I think an easier way allow streams to both get their "use color" from context and also override it for special purposes is to replace the use color bool with a "use color, don't use color, no opinion" enum. We'd make streams creation default to "no opinion". The SB API's would instead create them with "no use color" - and we should add an accessor to the SB API's. And then internally, "no opinion" means "consult the debugger" - but more generally it means "determine my use color value from context". That saves you from having to track down all the cases where streams were made and get their "use color" right. https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > > Could this commit have broken the bots? > > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1897/ > > Looks like so, but I cannot repro the test failure locally. The error message is different in current latest build (https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1907/testReport/junit/lldb-api/lang_c_forward/TestForwardDeclaration_py/) which makes me thinking it's related to clang module being out of date? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: > > we should probably fix the underlying issue instead: #77696 > > We still have binaries that are floating around for now that contain this > issue and this was causing crashes. So it would be nice to fix this in LLDB > for now and back this out after we have a stable and trustable .debug_names > section? That's ok by me, but I still don't understand how the problem being solved here is related to IDX_parent. As I said in the email you quoted, we don't add links to parents that are forward declarations. When the parent chain of a DIE is built, if we see an `AT_declaration` (just like what is being done in this PR), we pretend the parent is not indexed. In other others, the queries involving IDX_parent will _never_ reach the ProcessEntry function. Note that only `DebugNamesDWARFIndex::GetFullyQualifiedType` makes use of IDX_parent, but there are tons of other queries that call `ProcessEntry`. Maybe the slow path you are reaching is from one of those other queries? https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: Ok, minor correction: if there is no complete parent chain, it just defaults to the older implementation (which calls ProcessEntry). But my point still stands: I don't believe this is related to the presence of IDX_parent. ``` void DebugNamesDWARFIndex::GetFullyQualifiedType( const DWARFDeclContext &context, llvm::function_ref callback) { if (context.GetSize() == 0) return; llvm::StringRef leaf_name = context[0].name; llvm::SmallVector parent_names; for (auto idx : llvm::seq(1, context.GetSize())) parent_names.emplace_back(context[idx].name); // For each entry, grab its parent chain and check if we have a match. for (const DebugNames::Entry &entry : m_debug_names_up->equal_range(leaf_name)) { if (!isType(entry.tag())) continue; // Grab at most one extra parent, subsequent parents are not necessary to // test equality. std::optional> parent_chain = getParentChain(entry, parent_names.size() + 1); if (!parent_chain) { // Fallback: use the base class implementation. if (!ProcessEntry(entry, [&](DWARFDIE die) { return GetFullyQualifiedTypeImpl(context, die, callback); })) return; continue; } if (SameParentChain(parent_names, *parent_chain) && !ProcessEntry(entry, callback)) return; } } ``` https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [BOLT] Use disambiguated local names in BAT YAML (PR #91773)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91773 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [BOLT] Use disambiguated local names in BAT YAML (PR #91773)
https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/91773 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [BOLT] Set entry counts in BAT YAML profile (PR #91775)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91775 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [BOLT] Set entry counts in BAT YAML profile (PR #91775)
https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/91775 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits