[Lldb-commits] [lldb] [WIP] [lldb][Progress] Report progress when completing types from DWARF (PR #91452)

2024-05-10 Thread Michael Buch via lldb-commits

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)

2024-05-10 Thread David Spickett via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread David Spickett via lldb-commits

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)

2024-05-10 Thread David Spickett via lldb-commits

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)

2024-05-10 Thread David Spickett via lldb-commits

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)

2024-05-10 Thread David Spickett via lldb-commits


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

2024-05-10 Thread David Spickett via lldb-commits

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)

2024-05-10 Thread David Spickett via lldb-commits


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

2024-05-10 Thread David Spickett via lldb-commits

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

2024-05-10 Thread David Spickett via lldb-commits

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)

2024-05-10 Thread Kevin Frei via lldb-commits

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)

2024-05-10 Thread Ed Maste via lldb-commits


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

2024-05-10 Thread Walter Erquinigo via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Pavel Labath via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Alex Langford via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Fred Grim via lldb-commits


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

2024-05-10 Thread Fred Grim via lldb-commits

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)

2024-05-10 Thread Fred Grim via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Will Hawkins via lldb-commits

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)

2024-05-10 Thread Will Hawkins via lldb-commits

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)

2024-05-10 Thread Will Hawkins via lldb-commits


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

2024-05-10 Thread Will Hawkins via lldb-commits


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

2024-05-10 Thread Will Hawkins via lldb-commits


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

2024-05-10 Thread Will Hawkins via lldb-commits


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

2024-05-10 Thread Will Hawkins via lldb-commits


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

2024-05-10 Thread Will Hawkins via lldb-commits


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

2024-05-10 Thread Fred Grim via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Felipe de Azevedo Piovezan via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread via lldb-commits


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

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread via lldb-commits


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

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread via lldb-commits


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

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Felipe de Azevedo Piovezan via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits


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

2024-05-10 Thread Greg Clayton via lldb-commits

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)

2024-05-10 Thread Jason Molenda via lldb-commits

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)

2024-05-10 Thread Chelsea Cassanova via lldb-commits

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)

2024-05-10 Thread Adrian Prantl via lldb-commits

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)

2024-05-10 Thread Jason Molenda via lldb-commits

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)

2024-05-10 Thread Jason Molenda via lldb-commits

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)

2024-05-10 Thread Adrian Prantl via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread via lldb-commits

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)

2024-05-10 Thread Zequan Wu via lldb-commits

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)

2024-05-10 Thread Felipe de Azevedo Piovezan via lldb-commits

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)

2024-05-10 Thread Felipe de Azevedo Piovezan via lldb-commits

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)

2024-05-10 Thread Amir Ayupov via lldb-commits

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)

2024-05-10 Thread Amir Ayupov via lldb-commits

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)

2024-05-10 Thread Amir Ayupov via lldb-commits

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)

2024-05-10 Thread Amir Ayupov via lldb-commits

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


  1   2   >