[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

2023-05-12 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added a comment.
Herald added a subscriber: luke.

Hi! I wondering if someone knows what is the status of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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


[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

2023-05-12 Thread Ties Stuij via Phabricator via lldb-commits
stuij accepted this revision.
stuij added a comment.

Regarding this particular change, we at Arm are happy with the name change. I 
saw BFloat16 instead of BF16 as tech debt that we didn't get round to fixing.

I do wonder if we need two bfloat implementations, but for that I'll leave a 
comment on D149573 .

BTW, unfortunately @lenary doesn't work at Arm anymore.


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

https://reviews.llvm.org/D150291

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


[Lldb-commits] [PATCH] D150418: [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

2023-05-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Given the numbers I'm surprised you decided to stick with 8 rather than 4? 
Unless I'm reading them wrong, it seems that despite the number of allocations, 
it's about as fast and uses (a bit) less memory. Besides that this LGTM.




Comment at: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp:58
+  int64_t value = data.GetSLEB128(offset_ptr);
+  m_attributes.push_back(AttributeSpec(attr, form, value));
+  continue;

This could be an `emplace_back`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150418

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


[Lldb-commits] [PATCH] D150313: Fix libstdc++ data formatter for reference/pointer to std::string

2023-05-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

I'm surprised this didn't work but was able to reproduce the failure on Linux 
with libstdcpp. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150313

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


[Lldb-commits] [lldb] 5130e04 - [lldb] Don't write to source directory in test

2023-05-12 Thread Benjamin Kramer via lldb-commits

Author: Benjamin Kramer
Date: 2023-05-12T12:26:29+02:00
New Revision: 5130e049ff43eee4fcabdfda3c626f1bc051ba31

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

LOG: [lldb] Don't write to source directory in test

Added: 


Modified: 
lldb/test/Shell/Commands/command-disassemble-mixed.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-disassemble-mixed.test 
b/lldb/test/Shell/Commands/command-disassemble-mixed.test
index d27cdb1cb5a16..e078f341a694b 100644
--- a/lldb/test/Shell/Commands/command-disassemble-mixed.test
+++ b/lldb/test/Shell/Commands/command-disassemble-mixed.test
@@ -6,8 +6,8 @@ int main() {
   return abc;
 }
 
-// RUN: %clang_host -g -x c -o a.out %s
-// RUN: %lldb -b -o 'disassemble --mixed -n main' a.out | FileCheck %s
+// RUN: %clang_host -g -x c -o %t.out %s
+// RUN: %lldb -b -o 'disassemble --mixed -n main' %t.out | FileCheck %s
 
-// CHECK: a.out`main:
+// CHECK: .out`main:
 // CHECK-NOT: do_not_show



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


[Lldb-commits] [PATCH] D150299: [lldb][NFCI] Redefine dw_attr_t typedef with llvm::dwarf::Attribute

2023-05-12 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve accepted this revision.
fdeazeve added a comment.
This revision is now accepted and ready to land.

LGTM! Sorry for the delay!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150299

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


[Lldb-commits] [PATCH] D150402: [lldb][NFCI] Change return type of DWARFDebugInfoEntry::GetAttributes

2023-05-12 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve accepted this revision.
fdeazeve added a comment.
This revision is now accepted and ready to land.

Thanks for doing this! In particular, all the early returns are very welcome :)

I suspect you already have this in your radar, but `ExtractFormValueAtIndex` 
could probably return an `optional` to cleanup some code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150402

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


[Lldb-commits] [lldb] 81be680 - [lldb][nfc] Simplify DebugRanges class

2023-05-12 Thread Felipe de Azevedo Piovezan via lldb-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-05-12T08:48:31-04:00
New Revision: 81be68062f49dccfb0c28b98f04d88b5343c4ae6

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

LOG: [lldb][nfc] Simplify DebugRanges class

Most of the code changed here dates back to 2010, when LLDB was first
introduced upstream, as such it benefits from a slight cleanup.

The method "dump" is not used anywhere nor is it tested, so this commit removes
it.

The "findRanges" method returns a boolean which is never checked and indicates
whether the method found anything/assigned a range map to the out parameter.
This commit folds the out parameter into the return type of the method.

A handful of typedefs were also never used and therefore removed.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
index 6a0f11d2a1c49..d3598de5b3d31 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
@@ -8,7 +8,6 @@
 
 #include "DWARFDebugRanges.h"
 #include "DWARFUnit.h"
-#include "lldb/Utility/Stream.h"
 
 using namespace lldb_private;
 
@@ -72,55 +71,17 @@ bool DWARFDebugRanges::Extract(DWARFContext &context,
   return range_offset != *offset_ptr;
 }
 
-void DWARFDebugRanges::Dump(Stream &s,
-const DWARFDataExtractor &debug_ranges_data,
-lldb::offset_t *offset_ptr,
-dw_addr_t cu_base_addr) {
-  uint32_t addr_size = s.GetAddressByteSize();
-
-  dw_addr_t base_addr = cu_base_addr;
-  while (
-  debug_ranges_data.ValidOffsetForDataOfSize(*offset_ptr, 2 * addr_size)) {
-dw_addr_t begin = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
-dw_addr_t end = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
-// Extend 4 byte addresses that consists of 32 bits of 1's to be 64 bits of
-// ones
-if (begin == 0xull && addr_size == 4)
-  begin = LLDB_INVALID_ADDRESS;
-
-s.Indent();
-if (begin == 0 && end == 0) {
-  s.PutCString(" End");
-  break;
-} else if (begin == LLDB_INVALID_ADDRESS) {
-  // A base address selection entry
-  base_addr = end;
-  DumpAddress(s.AsRawOstream(), base_addr, sizeof(dw_addr_t),
-  " Base address = ");
-} else {
-  // Convert from offset to an address
-  dw_addr_t begin_addr = begin + base_addr;
-  dw_addr_t end_addr = end + base_addr;
-
-  DumpAddressRange(s.AsRawOstream(), begin_addr, end_addr,
-   sizeof(dw_addr_t), nullptr);
-}
-  }
-}
-
-bool DWARFDebugRanges::FindRanges(const DWARFUnit *cu,
-  dw_offset_t debug_ranges_offset,
-  DWARFRangeList &range_list) const {
+DWARFRangeList
+DWARFDebugRanges::FindRanges(const DWARFUnit *cu,
+ dw_offset_t debug_ranges_offset) const {
   dw_addr_t debug_ranges_address = cu->GetRangesBase() + debug_ranges_offset;
-  range_map_const_iterator pos = m_range_map.find(debug_ranges_address);
-  if (pos != m_range_map.end()) {
-range_list = pos->second;
-
-// All DW_AT_ranges are relative to the base address of the compile
-// unit. We add the compile unit base address to make sure all the
-// addresses are properly fixed up.
-range_list.Slide(cu->GetBaseAddress());
-return true;
-  }
-  return false;
+  auto pos = m_range_map.find(debug_ranges_address);
+  DWARFRangeList ans =
+  pos == m_range_map.end() ? DWARFRangeList() : pos->second;
+
+  // All DW_AT_ranges are relative to the base address of the compile
+  // unit. We add the compile unit base address to make sure all the
+  // addresses are properly fixed up.
+  ans.Slide(cu->GetBaseAddress());
+  return ans;
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
index b587845a67d91..5d5ddada6c2f9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
@@ -22,21 +22,14 @@ class DWARFDebugRanges {
   DWARFDebugRanges();
 
   void Extract(lldb_private::DWARFContext &context);
-  bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
-  DWARFRangeList &range_list) const;
-
-  static void Dump(lldb_private::Stream &s,
-   const lldb_private::DWARFDataExtractor &de

[Lldb-commits] [PATCH] D150363: [lldb][nfc] Simplify DebugRanges class

2023-05-12 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG81be68062f49: [lldb][nfc] Simplify DebugRanges class 
(authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150363

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1032,9 +1032,7 @@
 if (!debug_ranges)
   return llvm::make_error(
   "No debug_ranges section");
-DWARFRangeList ranges;
-debug_ranges->FindRanges(this, offset, ranges);
-return ranges;
+return debug_ranges->FindRanges(this, offset);
   }
 
   if (!GetRnglistTable())
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
@@ -22,21 +22,14 @@
   DWARFDebugRanges();
 
   void Extract(lldb_private::DWARFContext &context);
-  bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
-  DWARFRangeList &range_list) const;
-
-  static void Dump(lldb_private::Stream &s,
-   const lldb_private::DWARFDataExtractor &debug_ranges_data,
-   lldb::offset_t *offset_ptr, dw_addr_t cu_base_addr);
+  DWARFRangeList FindRanges(const DWARFUnit *cu,
+dw_offset_t debug_ranges_offset) const;
 
 protected:
   bool Extract(lldb_private::DWARFContext &context, lldb::offset_t *offset_ptr,
DWARFRangeList &range_list);
 
-  typedef std::map range_map;
-  typedef range_map::iterator range_map_iterator;
-  typedef range_map::const_iterator range_map_const_iterator;
-  range_map m_range_map;
+  std::map m_range_map;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGRANGES_H
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
@@ -8,7 +8,6 @@
 
 #include "DWARFDebugRanges.h"
 #include "DWARFUnit.h"
-#include "lldb/Utility/Stream.h"
 
 using namespace lldb_private;
 
@@ -72,55 +71,17 @@
   return range_offset != *offset_ptr;
 }
 
-void DWARFDebugRanges::Dump(Stream &s,
-const DWARFDataExtractor &debug_ranges_data,
-lldb::offset_t *offset_ptr,
-dw_addr_t cu_base_addr) {
-  uint32_t addr_size = s.GetAddressByteSize();
-
-  dw_addr_t base_addr = cu_base_addr;
-  while (
-  debug_ranges_data.ValidOffsetForDataOfSize(*offset_ptr, 2 * addr_size)) {
-dw_addr_t begin = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
-dw_addr_t end = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
-// Extend 4 byte addresses that consists of 32 bits of 1's to be 64 bits of
-// ones
-if (begin == 0xull && addr_size == 4)
-  begin = LLDB_INVALID_ADDRESS;
-
-s.Indent();
-if (begin == 0 && end == 0) {
-  s.PutCString(" End");
-  break;
-} else if (begin == LLDB_INVALID_ADDRESS) {
-  // A base address selection entry
-  base_addr = end;
-  DumpAddress(s.AsRawOstream(), base_addr, sizeof(dw_addr_t),
-  " Base address = ");
-} else {
-  // Convert from offset to an address
-  dw_addr_t begin_addr = begin + base_addr;
-  dw_addr_t end_addr = end + base_addr;
-
-  DumpAddressRange(s.AsRawOstream(), begin_addr, end_addr,
-   sizeof(dw_addr_t), nullptr);
-}
-  }
-}
-
-bool DWARFDebugRanges::FindRanges(const DWARFUnit *cu,
-  dw_offset_t debug_ranges_offset,
-  DWARFRangeList &range_list) const {
+DWARFRangeList
+DWARFDebugRanges::FindRanges(const DWARFUnit *cu,
+ dw_offset_t debug_ranges_offset) const {
   dw_addr_t debug_ranges_address = cu->GetRangesBase() + debug_ranges_offset;
-  range_map_const_iterator pos = m_range_map.find(debug_ranges_address);
-  if (pos != m_range_map.end()) {
-range_list = pos->second;
-
-// All DW_AT_ranges are relative to the base address of the compile
-// unit. We add the compile unit base address to make sure all the
-// addresses are properly fixed up.
-range_list.Slide(cu->GetBaseAddress());
-return true;
-  }
-  return false;
+  auto pos = m_range_map.find(debug_ranges_address);
+  DWARFRangeList ans =
+  pos == m_range_map.end() ? DWARFRangeList() : 

[Lldb-commits] [PATCH] D150366: [lldb][NFCI] Use llvm's libDebugInfo for DebugRanges

2023-05-12 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve added a comment.

In D150366#4336168 , @bulbazord wrote:

> Ok, this looks like it's doing the same thing to me which is good. My 
> understanding of this change is that you're changing 
> `lldb::DWARFDebugRanges::Extract` to use `llvm::DWARFDebugRangeList` instead 
> of `lldb::DWARFRangeList`.
>
> Out of curiosity, do you have an idea of the change to performance (if any)? 
> I wouldn't expect it to be very different if at all because I don't think the 
> algorithms between lldb and llvm are different but it would be nice to make 
> sure.

I ran an experiment with a C++ project that forced LLDB to parse roughly 30,000 
range entries and no significant differences were detected:

  // LLDB's implementation
   Time (mean ± σ): 695.2 ms ±   7.9 ms[User: 157.8 ms, System: 69.0 ms]
   Range (min … max):   678.7 ms … 703.1 ms10 runs
  // LLVM's implementation
   Time (mean ± σ): 694.9 ms ±   9.7 ms[User: 158.8 ms, System: 70.5 ms]
   Range (min … max):   675.2 ms … 711.7 ms10 runs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150366

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


[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

2023-05-12 Thread Tom Honermann via Phabricator via lldb-commits
tahonermann requested changes to this revision.
tahonermann added a comment.
This revision now requires changes to proceed.

> I do wonder if we need two bfloat implementations, but for that I'll leave a 
> comment on D149573 .

Given the discussions occurring in D149573 , 
let's hold off on landing this for now. It is sounding like we might have 
direction for repurposing `__bf16` for `std::bfloat16_t` (and a future 
`_BFloat16` C type; with `__bf16` retained as an alternate spelling). If we go 
in that direction, then we would presumably want a change that goes in the 
opposite direction of this patch; a change that migrates "bf16" names towards 
"bfloat16". Let's focus on confirming that direction first. I'll mark this as 
requesting changes for now while we figure this out.


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

https://reviews.llvm.org/D150291

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


[Lldb-commits] [lldb] b58dd92 - [lldb-vscode] Skip restart tests on ARM

2023-05-12 Thread Leandro Lupori via lldb-commits

Author: Leandro Lupori
Date: 2023-05-12T13:52:17-03:00
New Revision: b58dd9230e655feb0d904b8095f113a4f0246cbf

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

LOG: [lldb-vscode] Skip restart tests on ARM

These tests always time out on ARM buildbot. Disabling them for
now, until https://github.com/llvm/llvm-project/issues/62684 is
fixed.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/restart/TestVSCode_restart_runInTerminal.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-vscode/restart/TestVSCode_restart_runInTerminal.py 
b/lldb/test/API/tools/lldb-vscode/restart/TestVSCode_restart_runInTerminal.py
index f250e014cfe3d..48931f4e1bb5c 100644
--- 
a/lldb/test/API/tools/lldb-vscode/restart/TestVSCode_restart_runInTerminal.py
+++ 
b/lldb/test/API/tools/lldb-vscode/restart/TestVSCode_restart_runInTerminal.py
@@ -24,6 +24,7 @@ def isTestSupported(self):
 
 @skipIfWindows
 @skipIfRemote
+@skipIf(archs=["arm"]) # Always times out on buildbot
 def test_basic_functionality(self):
 '''
 Test basic restarting functionality when the process is running in
@@ -58,6 +59,7 @@ def test_basic_functionality(self):
 
 @skipIfWindows
 @skipIfRemote
+@skipIf(archs=["arm"]) # Always times out on buildbot
 def test_stopOnEntry(self):
 '''
 Check that stopOnEntry works correctly when using runInTerminal.



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


[Lldb-commits] [lldb] 25159ee - Fix libstdc++ data formatter for reference/pointer to std::string

2023-05-12 Thread Jeffrey Tan via lldb-commits

Author: Jeffrey Tan
Date: 2023-05-12T10:09:58-07:00
New Revision: 25159ee3af5cd1c05c010ea195b4b359df3fe820

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

LOG: Fix libstdc++ data formatter for reference/pointer to std::string

This patch fixes libstdc++ data formatter for reference/pointer to std::string.
The failure testcases are added which succeed with the patch.

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 90976fa053b8a..c7f1c79422246 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -233,8 +233,15 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   const bool scalar_is_load_addr = true;
   AddressType addr_type;
-  lldb::addr_t addr_of_string =
-  valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
+  lldb::addr_t addr_of_string = LLDB_INVALID_ADDRESS;
+  if (valobj.IsPointerOrReferenceType()) {
+Status error;
+ValueObjectSP pointee_sp = valobj.Dereference(error);
+if (pointee_sp && error.Success())
+  addr_of_string = pointee_sp->GetAddressOf(scalar_is_load_addr, 
&addr_type);
+  } else
+addr_of_string =
+valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
   if (addr_of_string != LLDB_INVALID_ADDRESS) {
 switch (addr_type) {
 case eAddressTypeLoad: {

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
index 6ce7235ff015d..34d39ee3e6b09 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -57,6 +57,11 @@ def cleanup():
 var_empty = self.frame().FindVariable('empty')
 var_q = self.frame().FindVariable('q')
 var_Q = self.frame().FindVariable('Q')
+var_rq = self.frame().FindVariable('rq')
+var_rQ = self.frame().FindVariable('rQ')
+var_pq = self.frame().FindVariable('pq')
+var_pQ = self.frame().FindVariable('pQ')
+
 var_uchar = self.frame().FindVariable('uchar')
 
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary 
wrong")
@@ -75,6 +80,18 @@ def cleanup():
 var_Q.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
 "Q summary wrong")
 self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
+self.assertEqual(
+var_rq.GetSummary(), '"hello world"',
+"rq summary wrong")
+self.assertEqual(
+var_rQ.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
+"rQ summary wrong")
+self.assertEqual(
+var_pq.GetSummary(), '"hello world"',
+"pq summary wrong")
+self.assertEqual(
+var_pQ.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
+"pQ summary wrong")
 
 self.runCmd("next")
 

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
index 73519197d8c1a..930f8c2afef21 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -10,6 +10,8 @@ int main()
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
 std::basic_string uchar(5, 'a');
+auto &rq = q, &rQ = Q;
+std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }



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


[Lldb-commits] [PATCH] D150313: Fix libstdc++ data formatter for reference/pointer to std::string

2023-05-12 Thread jeffrey tan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG25159ee3af5c: Fix libstdc++ data formatter for 
reference/pointer to std::string (authored by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150313

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -10,6 +10,8 @@
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
 std::basic_string uchar(5, 'a');
+auto &rq = q, &rQ = Q;
+std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -57,6 +57,11 @@
 var_empty = self.frame().FindVariable('empty')
 var_q = self.frame().FindVariable('q')
 var_Q = self.frame().FindVariable('Q')
+var_rq = self.frame().FindVariable('rq')
+var_rQ = self.frame().FindVariable('rQ')
+var_pq = self.frame().FindVariable('pq')
+var_pQ = self.frame().FindVariable('pQ')
+
 var_uchar = self.frame().FindVariable('uchar')
 
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary 
wrong")
@@ -75,6 +80,18 @@
 var_Q.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
 "Q summary wrong")
 self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
+self.assertEqual(
+var_rq.GetSummary(), '"hello world"',
+"rq summary wrong")
+self.assertEqual(
+var_rQ.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
+"rQ summary wrong")
+self.assertEqual(
+var_pq.GetSummary(), '"hello world"',
+"pq summary wrong")
+self.assertEqual(
+var_pQ.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
+"pQ summary wrong")
 
 self.runCmd("next")
 
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -233,8 +233,15 @@
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   const bool scalar_is_load_addr = true;
   AddressType addr_type;
-  lldb::addr_t addr_of_string =
-  valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
+  lldb::addr_t addr_of_string = LLDB_INVALID_ADDRESS;
+  if (valobj.IsPointerOrReferenceType()) {
+Status error;
+ValueObjectSP pointee_sp = valobj.Dereference(error);
+if (pointee_sp && error.Success())
+  addr_of_string = pointee_sp->GetAddressOf(scalar_is_load_addr, 
&addr_type);
+  } else
+addr_of_string =
+valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
   if (addr_of_string != LLDB_INVALID_ADDRESS) {
 switch (addr_type) {
 case eAddressTypeLoad: {


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -10,6 +10,8 @@
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
 std::basic_string uchar(5, 'a');
+auto &rq = q, &rQ = Q;
+std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ lldb/test/API/functionalities/data-for

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

2023-05-12 Thread M. Zeeshan Siddiqui via Phabricator via lldb-commits
codemzs added a comment.

In D150291#4338118 , @tahonermann 
wrote:

>> I do wonder if we need two bfloat implementations, but for that I'll leave a 
>> comment on D149573 .
>
> Given the discussions occurring in D149573 
> , let's hold off on landing this for now. 
> It is sounding like we might have direction for repurposing `__bf16` for 
> `std::bfloat16_t` (and a future `_BFloat16` C type; with `__bf16` retained as 
> an alternate spelling). If we go in that direction, then we would presumably 
> want a change that goes in the opposite direction of this patch; a change 
> that migrates "bf16" names towards "bfloat16". Let's focus on confirming that 
> direction first. I'll mark this as requesting changes for now while we figure 
> this out.

Thank you for your guidance, @tahonermann. I agree it's prudent to establish a 
confirmed direction for reusing `__bf16` to implement `std::bfloat16_t` as an 
arithmetic type before proceeding. If we decide to take this route, I 
understand that we'll be looking towards a change that aligns "bf16" names more 
closely with "bfloat16".

In the meantime, I will revert to my initial change in D149573 
 which repurposed `__bf16` type, and present 
it for further discussion. This, along with a summary of the RFC discussion, 
should help us reach a consensus. I welcome any further insights or 
considerations you might think pertinent to this process.


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

https://reviews.llvm.org/D150291

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


[Lldb-commits] [PATCH] D150470: [lldb-vscode] Make tests not wait for 'launch' process events if launch fails.

2023-05-12 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe created this revision.
jgorbe added reviewers: ted, luporl, wallace.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

After https://reviews.llvm.org/D147831, lldb-vscode doesn't send a
process event after launch/attach in case of failure. I believe this is
the right interpretation of the spec, because the description of the
event says:

> The event indicates that the debugger has begun debugging a new
> process.

and we haven't started debugging a process if there's no process because
we failed to launch it. This is also supported by the fact that
`request_launch` in vscode.py doesn't wait for the event if passed
`expectFailure=True`. However, this doesn't take into account
*unexpected* launch failures. In that case, `request_launch` will hang
indefinitely waiting for the process event.

This patch changes it so we'll only wait for these events if
`request["success"]` is true, independently of whether any failure was
expected or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150470

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py


Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -661,8 +661,7 @@
stopCommands=None, exitCommands=None,
terminateCommands=None ,sourcePath=None,
debuggerRoot=None, launchCommands=None, sourceMap=None,
-   runInTerminal=False, expectFailure=False,
-   postRunCommands=None):
+   runInTerminal=False, postRunCommands=None):
 args_dict = {
 'program': program
 }
@@ -712,7 +711,7 @@
 }
 response = self.send_recv(command_dict)
 
-if not expectFailure:
+if response['success']:
 # Wait for a 'process' and 'initialized' event in any order
 self.wait_for_event(filter=['process', 'initialized'])
 self.wait_for_event(filter=['process', 'initialized'])
Index: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -322,7 +322,6 @@
 launchCommands=launchCommands,
 sourceMap=sourceMap,
 runInTerminal=runInTerminal,
-expectFailure=expectFailure,
 postRunCommands=postRunCommands)
 
 if expectFailure:


Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -661,8 +661,7 @@
stopCommands=None, exitCommands=None,
terminateCommands=None ,sourcePath=None,
debuggerRoot=None, launchCommands=None, sourceMap=None,
-   runInTerminal=False, expectFailure=False,
-   postRunCommands=None):
+   runInTerminal=False, postRunCommands=None):
 args_dict = {
 'program': program
 }
@@ -712,7 +711,7 @@
 }
 response = self.send_recv(command_dict)
 
-if not expectFailure:
+if response['success']:
 # Wait for a 'process' and 'initialized' event in any order
 self.wait_for_event(filter=['process', 'initialized'])
 self.wait_for_event(filter=['process', 'initialized'])
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -322,7 +322,6 @@
 launchCommands=launchCommands,
 sourceMap=sourceMap,
 runInTerminal=runInTerminal,
-expectFailure=expectFailure,
 postRunCommands=postRunCommands)
 
 if expectFailure:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D150470: [lldb-vscode] Make tests not wait for 'launch' process events if launch fails.

2023-05-12 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe added a comment.

See also https://github.com/llvm/llvm-project/issues/62684


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150470

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


[Lldb-commits] [PATCH] D150418: [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

In D150418#4337243 , @JDevlieghere 
wrote:

> Given the numbers I'm surprised you decided to stick with 8 rather than 4? 
> Unless I'm reading them wrong, it seems that despite the number of 
> allocations, it's about as fast and uses (a bit) less memory. Besides that 
> this LGTM.

Yeah, I just forgot to update this before I published. Will update the patch 
before landing. Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150418

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


[Lldb-commits] [PATCH] D150402: [lldb][NFCI] Change return type of DWARFDebugInfoEntry::GetAttributes

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

In D150402#4337400 , @fdeazeve wrote:

> Thanks for doing this! In particular, all the early returns are very welcome 
> :)
>
> I suspect you already have this in your radar, but `ExtractFormValueAtIndex` 
> could probably return an `optional` to cleanup some code.

Yup, on the radar! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150402

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


[Lldb-commits] [lldb] 64f1fda - [lldb][NFCI] Redefine dw_attr_t typedef with llvm::dwarf::Attribute

2023-05-12 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-05-12T11:58:50-07:00
New Revision: 64f1fda29e2dd133c84f23474e29de02d7ed392d

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

LOG: [lldb][NFCI] Redefine dw_attr_t typedef with llvm::dwarf::Attribute

Similar to dw_form_t, dw_attr_t is typedef'd to be a uint16_t. LLVM
defines their type `llvm::dwarf::Attribute` as an enum backed by a
uint16_t. Switching to the LLVM type buys us type checking and the
requirement of explicit casts.

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

Added: 


Modified: 
lldb/include/lldb/Core/dwarf.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/dwarf.h b/lldb/include/lldb/Core/dwarf.h
index 6fcf2f8cc0c78..e162a090ba7c9 100644
--- a/lldb/include/lldb/Core/dwarf.h
+++ b/lldb/include/lldb/Core/dwarf.h
@@ -21,7 +21,7 @@ namespace dwarf {
 }
 }
 
-typedef uint16_t dw_attr_t;
+typedef llvm::dwarf::Attribute dw_attr_t;
 typedef llvm::dwarf::Form dw_form_t;
 typedef llvm::dwarf::Tag dw_tag_t;
 typedef uint64_t dw_addr_t; // Dwarf address define that must be big enough for

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 53068823ceac9..b8a517100524d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -274,6 +274,8 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const 
DWARFDIE &die) {
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_abstract_origin:
   abstract_origin = form_value;
   break;

diff  --git 
a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
index 5bd3b23dc95fe..8dd47e6da8d5f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -40,7 +40,7 @@ DWARFAbbreviationDeclaration::extract(const 
DWARFDataExtractor &data,
   m_has_children = data.GetU8(offset_ptr);
 
   while (data.ValidOffset(*offset_ptr)) {
-dw_attr_t attr = data.GetULEB128(offset_ptr);
+auto attr = static_cast(data.GetULEB128(offset_ptr));
 auto form = static_cast(data.GetULEB128(offset_ptr));
 
 // This is the last attribute for this abbrev decl, but there may still be

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 91be5e5b7bb6f..8e2e04d4176a0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -391,6 +391,8 @@ void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry 
&cu_die) {
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_loclists_base:
   SetLoclistsBase(form_value.Unsigned());
   break;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 79cbb1059349a..9be005ea06d52 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -239,6 +239,8 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit,
 dw_attr_t attr = attributes.AttributeAtIndex(i);
 DWARFFormValue form_value;
 switch (attr) {
+default:
+  break;
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
 name = form_value.AsCString();



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


[Lldb-commits] [PATCH] D150299: [lldb][NFCI] Redefine dw_attr_t typedef with llvm::dwarf::Attribute

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG64f1fda29e2d: [lldb][NFCI] Redefine dw_attr_t typedef with 
llvm::dwarf::Attribute (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150299

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -239,6 +239,8 @@
 dw_attr_t attr = attributes.AttributeAtIndex(i);
 DWARFFormValue form_value;
 switch (attr) {
+default:
+  break;
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
 name = form_value.AsCString();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -391,6 +391,8 @@
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_loclists_base:
   SetLoclistsBase(form_value.Unsigned());
   break;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -40,7 +40,7 @@
   m_has_children = data.GetU8(offset_ptr);
 
   while (data.ValidOffset(*offset_ptr)) {
-dw_attr_t attr = data.GetULEB128(offset_ptr);
+auto attr = static_cast(data.GetULEB128(offset_ptr));
 auto form = static_cast(data.GetULEB128(offset_ptr));
 
 // This is the last attribute for this abbrev decl, but there may still be
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -274,6 +274,8 @@
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_abstract_origin:
   abstract_origin = form_value;
   break;
Index: lldb/include/lldb/Core/dwarf.h
===
--- lldb/include/lldb/Core/dwarf.h
+++ lldb/include/lldb/Core/dwarf.h
@@ -21,7 +21,7 @@
 }
 }
 
-typedef uint16_t dw_attr_t;
+typedef llvm::dwarf::Attribute dw_attr_t;
 typedef llvm::dwarf::Form dw_form_t;
 typedef llvm::dwarf::Tag dw_tag_t;
 typedef uint64_t dw_addr_t; // Dwarf address define that must be big enough for


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -239,6 +239,8 @@
 dw_attr_t attr = attributes.AttributeAtIndex(i);
 DWARFFormValue form_value;
 switch (attr) {
+default:
+  break;
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
 name = form_value.AsCString();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -391,6 +391,8 @@
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_loclists_base:
   SetLoclistsBase(form_value.Unsigned());
   break;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -40,7 +40,7 @@
   m_has_children = data.GetU8(offset_ptr);
 
   while (data.ValidOffset(*offset_ptr)) {
-dw_attr_t attr = data.GetULEB128(offset_ptr);
+auto attr = static_cast(data.GetULEB128(offset_ptr));
 auto form = static_cast(data.GetULEB128(offset_ptr));
 
 // This is the last attribute for this abbrev decl, but there may still be
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAS

[Lldb-commits] [PATCH] D150470: [lldb-vscode] Make tests not wait for 'launch' process events if launch fails.

2023-05-12 Thread Leandro Lupori via Phabricator via lldb-commits
luporl added a comment.

LGTM. With this patch, if I re-enable the restart tests on ARM I get a failure 
after a few seconds, instead of timing out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150470

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


[Lldb-commits] [lldb] c909b49 - [lldb][NFCI] Change return type of DWARFDebugInfoEntry::GetAttributes

2023-05-12 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-05-12T12:17:21-07:00
New Revision: c909b491cb5f392147de4490749abc1e5372a571

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

LOG: [lldb][NFCI] Change return type of DWARFDebugInfoEntry::GetAttributes

The purpose of this method is to get the list of attributes of a
DebugInfoEntry. Prior to this change we were passing in a mutable
reference to a DWARFAttributes object and having the method fill it in
for us while returning the size of the filled out list. But
instead of doing that, we can just return a `DWARFAttributes` object
ourselves since every caller creates a new list before calling
GetAttributes.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
index dbece346b99a..a68b7cd110eb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
@@ -31,71 +31,70 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE 
&parent_die,
 if (tag != DW_TAG_subrange_type)
   continue;
 
-DWARFAttributes attributes;
-const size_t num_child_attributes = die.GetAttributes(attributes);
-if (num_child_attributes > 0) {
-  uint64_t num_elements = 0;
-  uint64_t lower_bound = 0;
-  uint64_t upper_bound = 0;
-  bool upper_bound_valid = false;
-  uint32_t i;
-  for (i = 0; i < num_child_attributes; ++i) {
-const dw_attr_t attr = attributes.AttributeAtIndex(i);
-DWARFFormValue form_value;
-if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-  switch (attr) {
-  case DW_AT_name:
-break;
-
-  case DW_AT_count:
-if (DWARFDIE var_die = die.GetReferencedDIE(DW_AT_count)) {
-  if (var_die.Tag() == DW_TAG_variable)
-if (exe_ctx) {
-  if (auto frame = exe_ctx->GetFrameSP()) {
-Status error;
-lldb::VariableSP var_sp;
-auto valobj_sp = frame->GetValueForVariableExpressionPath(
-var_die.GetName(), eNoDynamicValues, 0, var_sp, error);
-if (valobj_sp) {
-  num_elements = valobj_sp->GetValueAsUnsigned(0);
-  break;
-}
+DWARFAttributes attributes = die.GetAttributes();
+if (attributes.Size() == 0)
+  continue;
+
+uint64_t num_elements = 0;
+uint64_t lower_bound = 0;
+uint64_t upper_bound = 0;
+bool upper_bound_valid = false;
+for (size_t i = 0; i < attributes.Size(); ++i) {
+  const dw_attr_t attr = attributes.AttributeAtIndex(i);
+  DWARFFormValue form_value;
+  if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+switch (attr) {
+case DW_AT_name:
+  break;
+
+case DW_AT_count:
+  if (DWARFDIE var_die = die.GetReferencedDIE(DW_AT_count)) {
+if (var_die.Tag() == DW_TAG_variable)
+  if (exe_ctx) {
+if (auto frame = exe_ctx->GetFrameSP()) {
+  Status error;
+  lldb::VariableSP var_sp;
+  auto valobj_sp = frame->GetValueForVariableExpressionPath(
+  var_die.GetName(), eNoDynamicValues, 0, var_sp, error);
+  if (valobj_sp) {
+num_elements = valobj_sp->GetValueAsUnsigned(0);
+break;
   }
 }
-} else
-  num_elements = form_value.Unsigned();
-break;
-
-  case DW_AT_bit_stride:
-array_info.bit_stride = form_value.Unsigned();
-break;
-
-  case DW_AT_byte_stride:
-array_info.byte_stride = form_value.Unsigned();
-break;
-
-  case DW_AT_lower_bound:
-lower_bound = form_value.Unsigned();
-break;
-
-  case DW_AT_upper_bound:
-upper_bound_valid = true;
-upper_bound = form_value.Unsigned();
-break;
-
-  default:
-break;
-  }

[Lldb-commits] [PATCH] D150402: [lldb][NFCI] Change return type of DWARFDebugInfoEntry::GetAttributes

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc909b491cb5f: [lldb][NFCI] Change return type of 
DWARFDebugInfoEntry::GetAttributes (authored by bulbazord).

Changed prior to commit:
  https://reviews.llvm.org/D150402?vs=521453&id=521757#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150402

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3285,8 +3285,7 @@
   (tag != DW_TAG_formal_parameter || !sc.function))
 return nullptr;
 
-  DWARFAttributes attributes;
-  const size_t num_attributes = die.GetAttributes(attributes);
+  DWARFAttributes attributes = die.GetAttributes();
   const char *name = nullptr;
   const char *mangled = nullptr;
   Declaration decl;
@@ -3297,7 +3296,7 @@
   DWARFFormValue const_value_form, location_form;
   Variable::RangeList scope_ranges;
 
-  for (size_t i = 0; i < num_attributes; ++i) {
+  for (size_t i = 0; i < attributes.Size(); ++i) {
 dw_attr_t attr = attributes.AttributeAtIndex(i);
 DWARFFormValue form_value;
 
@@ -3895,8 +3894,7 @@
 std::optional LocationInCallee;
 std::optional LocationInCaller;
 
-DWARFAttributes attributes;
-const size_t num_attributes = child.GetAttributes(attributes);
+DWARFAttributes attributes = child.GetAttributes();
 
 // Parse the location at index \p attr_index within this call site parameter
 // DIE, or return std::nullopt on failure.
@@ -3915,7 +3913,7 @@
   child.GetCU());
 };
 
-for (size_t i = 0; i < num_attributes; ++i) {
+for (size_t i = 0; i < attributes.Size(); ++i) {
   dw_attr_t attr = attributes.AttributeAtIndex(i);
   if (attr == DW_AT_location)
 LocationInCallee = parse_simple_location(i);
@@ -3966,10 +3964,8 @@
 // Second DW_AT_low_pc may come from DW_TAG_subprogram referenced by
 // DW_TAG_GNU_call_site's DW_AT_abstract_origin overwriting our 'low_pc'.
 // So do not inherit attributes from DW_AT_abstract_origin.
-DWARFAttributes attributes;
-const size_t num_attributes =
-child.GetAttributes(attributes, DWARFDIE::Recurse::no);
-for (size_t i = 0; i < num_attributes; ++i) {
+DWARFAttributes attributes = child.GetAttributes(DWARFDIE::Recurse::no);
+for (size_t i = 0; i < attributes.Size(); ++i) {
   DWARFFormValue form_value;
   if (!attributes.ExtractFormValueAtIndex(i, form_value)) {
 LLDB_LOG(log, "CollectCallEdges: Could not extract TAG_call_site form");
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -223,62 +223,58 @@
   continue;
 }
 
-DWARFAttributes attributes;
 const char *name = nullptr;
 const char *mangled_cstr = nullptr;
 bool is_declaration = false;
-// bool is_artificial = false;
 bool has_address = false;
 bool has_location_or_const_value = false;
 bool is_global_or_static_variable = false;
 
 DWARFFormValue specification_die_form;
-const size_t num_attributes = die.GetAttributes(&unit, attributes);
-if (num_attributes > 0) {
-  for (uint32_t i = 0; i < num_attributes; ++i) {
-dw_attr_t attr = attributes.AttributeAtIndex(i);
-DWARFFormValue form_value;
-switch (attr) {
-default:
-  break;
-case DW_AT_name:
-  if (attributes.ExtractFormValueAtIndex(i, form_value))
-name = form_value.AsCString();
-  break;
-
-case DW_AT_declaration:
-  if (attributes.ExtractFormValueAtIndex(i, form_value))
-is_declaration = form_value.Unsigned() != 0;
-  break;
-
-case DW_AT_MIPS_linkage_name:
-case DW_AT_linkage_name:
-  if (attributes.ExtractFormValueAtIndex(i, form_value))
-mangled_cstr = form_value.AsCString();
-  break;
-
-case DW_AT_low_pc:
-case DW_AT_high_pc:
-case DW_AT_ranges:
-  has_address = true;
-  break;
-
-case DW_AT_entry_pc:
-  has_address = true;

[Lldb-commits] [lldb] be71d4c - [lldb][NFCI] Delete commented out method OptionValueProperties::GetQualifiedName

2023-05-12 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-05-12T12:33:25-07:00
New Revision: be71d4cc5010cf6e52aafc430691afd12a1c07c6

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

LOG: [lldb][NFCI] Delete commented out method 
OptionValueProperties::GetQualifiedName

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValueProperties.h
lldb/source/Interpreter/OptionValueProperties.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValueProperties.h 
b/lldb/include/lldb/Interpreter/OptionValueProperties.h
index 5a6bf2a81e4b8..e20ba2c3af71b 100644
--- a/lldb/include/lldb/Interpreter/OptionValueProperties.h
+++ b/lldb/include/lldb/Interpreter/OptionValueProperties.h
@@ -63,9 +63,6 @@ class OptionValueProperties
 
   void Initialize(const PropertyDefinitions &setting_definitions);
 
-  //bool
-  //GetQualifiedName (Stream &strm);
-
   // Subclass specific functions
 
   // Get the index of a property given its exact name in this property

diff  --git a/lldb/source/Interpreter/OptionValueProperties.cpp 
b/lldb/source/Interpreter/OptionValueProperties.cpp
index a061c41982fc9..36d732d2d25e1 100644
--- a/lldb/source/Interpreter/OptionValueProperties.cpp
+++ b/lldb/source/Interpreter/OptionValueProperties.cpp
@@ -51,25 +51,6 @@ void OptionValueProperties::AppendProperty(ConstString name,
   m_name_to_index.Sort();
 }
 
-// bool
-// OptionValueProperties::GetQualifiedName (Stream &strm)
-//{
-//bool dumped_something = false;
-lldb::OptionValuePropertiesSP parent_sp(GetParent ());
-if (parent_sp)
-{
-parent_sp->GetQualifiedName (strm);
-strm.PutChar('.');
-dumped_something = true;
-}
-//if (m_name)
-//{
-//strm << m_name;
-//dumped_something = true;
-//}
-//return dumped_something;
-//}
-//
 lldb::OptionValueSP
 OptionValueProperties::GetValueForKey(const ExecutionContext *exe_ctx,
   ConstString key) const {



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


[Lldb-commits] [PATCH] D150418: [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
bulbazord updated this revision to Diff 521771.
bulbazord added a comment.

- Use emplace_back
- llvm::SmallVector size 8 -> 4


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150418

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -31,10 +31,6 @@
 form = m_form;
 val = m_value;
   }
-  typedef std::vector collection;
-  typedef collection::iterator iterator;
-  typedef collection::const_iterator const_iterator;
-
 protected:
   dw_attr_t m_attr;
   dw_form_t m_form;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
@@ -16,6 +16,29 @@
 
 class DWARFAbbreviationDeclaration {
 public:
+  struct AttributeSpec {
+AttributeSpec(dw_attr_t attr, dw_form_t form, int64_t value)
+: m_attr(attr), m_form(form), m_value(value) {}
+
+AttributeSpec(dw_attr_t attr, dw_form_t form)
+: m_attr(attr), m_form(form), m_value(0) {}
+
+bool IsImplicitConst() const {
+  return m_form == lldb_private::dwarf::DW_FORM_implicit_const;
+}
+
+int64_t GetImplicitConstValue() const { return m_value; }
+
+dw_attr_t GetAttribute() const { return m_attr; }
+
+dw_form_t GetForm() const { return m_form; }
+
+  private:
+dw_attr_t m_attr;
+dw_form_t m_form;
+int64_t m_value;
+  };
+
   enum { InvalidCode = 0 };
   DWARFAbbreviationDeclaration();
 
@@ -28,17 +51,21 @@
   bool HasChildren() const { return m_has_children; }
   size_t NumAttributes() const { return m_attributes.size(); }
   dw_form_t GetFormByIndex(uint32_t idx) const {
-return m_attributes.size() > idx ? m_attributes[idx].get_form()
+return m_attributes.size() > idx ? m_attributes[idx].GetForm()
  : dw_form_t(0);
   }
 
   // idx is assumed to be valid when calling GetAttrAndFormByIndex()
   void GetAttrAndFormValueByIndex(uint32_t idx, dw_attr_t &attr,
   DWARFFormValue &form_value) const {
-m_attributes[idx].get(attr, form_value.FormRef(), form_value.ValueRef());
+const AttributeSpec &spec = m_attributes[idx];
+attr = spec.GetAttribute();
+form_value.FormRef() = spec.GetForm();
+if (spec.IsImplicitConst())
+  form_value.SetSigned(spec.GetImplicitConstValue());
   }
   dw_form_t GetFormByIndexUnchecked(uint32_t idx) const {
-return m_attributes[idx].get_form();
+return m_attributes[idx].GetForm();
   }
   uint32_t FindAttributeIndex(dw_attr_t attr) const;
 
@@ -59,7 +86,7 @@
   uint32_t m_code = InvalidCode;
   dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
   uint8_t m_has_children = 0;
-  DWARFAttribute::collection m_attributes;
+  llvm::SmallVector m_attributes;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFABBREVIATIONDECLARATION_H
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -53,12 +53,13 @@
   return llvm::make_error(
   "malformed abbreviation declaration attribute");
 
-DWARFFormValue::ValueType val;
+if (form == DW_FORM_implicit_const) {
+  int64_t value = data.GetSLEB128(offset_ptr);
+  m_attributes.emplace_back(attr, form, value);
+  continue;
+}
 
-if (form == DW_FORM_implicit_const)
-  val.value.sval = data.GetSLEB128(offset_ptr);
-
-m_attributes.push_back(DWARFAttribute(attr, form, val));
+m_attributes.emplace_back(attr, form);
   }
 
   return llvm::make_error(
@@ -72,10 +73,8 @@
 
 uint32_t
 DWARFAbbreviationDeclaration::FindAttributeIndex(dw_attr_t attr) const {
-  uint32_t i;
-  const uint32_t kNumAttributes = m_attributes.size();
-  for (i = 0; i < kNumAttributes; ++i) {
-if (m_attributes[i].get_attr() == attr)
+  for (size_t i = 0; i < m_attributes.size(); ++i) {
+if (m_attributes[i].GetAttribute() == attr)
   return i;
   }
   return DW_INVALID_INDEX;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D150418: [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG050c09f0bed6: [lldb][NFCI] Replace use of DWARFAttribute in 
DWARFAbbreviationDecl (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150418

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -31,10 +31,6 @@
 form = m_form;
 val = m_value;
   }
-  typedef std::vector collection;
-  typedef collection::iterator iterator;
-  typedef collection::const_iterator const_iterator;
-
 protected:
   dw_attr_t m_attr;
   dw_form_t m_form;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
@@ -16,6 +16,29 @@
 
 class DWARFAbbreviationDeclaration {
 public:
+  struct AttributeSpec {
+AttributeSpec(dw_attr_t attr, dw_form_t form, int64_t value)
+: m_attr(attr), m_form(form), m_value(value) {}
+
+AttributeSpec(dw_attr_t attr, dw_form_t form)
+: m_attr(attr), m_form(form), m_value(0) {}
+
+bool IsImplicitConst() const {
+  return m_form == lldb_private::dwarf::DW_FORM_implicit_const;
+}
+
+int64_t GetImplicitConstValue() const { return m_value; }
+
+dw_attr_t GetAttribute() const { return m_attr; }
+
+dw_form_t GetForm() const { return m_form; }
+
+  private:
+dw_attr_t m_attr;
+dw_form_t m_form;
+int64_t m_value;
+  };
+
   enum { InvalidCode = 0 };
   DWARFAbbreviationDeclaration();
 
@@ -28,17 +51,21 @@
   bool HasChildren() const { return m_has_children; }
   size_t NumAttributes() const { return m_attributes.size(); }
   dw_form_t GetFormByIndex(uint32_t idx) const {
-return m_attributes.size() > idx ? m_attributes[idx].get_form()
+return m_attributes.size() > idx ? m_attributes[idx].GetForm()
  : dw_form_t(0);
   }
 
   // idx is assumed to be valid when calling GetAttrAndFormByIndex()
   void GetAttrAndFormValueByIndex(uint32_t idx, dw_attr_t &attr,
   DWARFFormValue &form_value) const {
-m_attributes[idx].get(attr, form_value.FormRef(), form_value.ValueRef());
+const AttributeSpec &spec = m_attributes[idx];
+attr = spec.GetAttribute();
+form_value.FormRef() = spec.GetForm();
+if (spec.IsImplicitConst())
+  form_value.SetSigned(spec.GetImplicitConstValue());
   }
   dw_form_t GetFormByIndexUnchecked(uint32_t idx) const {
-return m_attributes[idx].get_form();
+return m_attributes[idx].GetForm();
   }
   uint32_t FindAttributeIndex(dw_attr_t attr) const;
 
@@ -59,7 +86,7 @@
   uint32_t m_code = InvalidCode;
   dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
   uint8_t m_has_children = 0;
-  DWARFAttribute::collection m_attributes;
+  llvm::SmallVector m_attributes;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFABBREVIATIONDECLARATION_H
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -53,12 +53,13 @@
   return llvm::make_error(
   "malformed abbreviation declaration attribute");
 
-DWARFFormValue::ValueType val;
+if (form == DW_FORM_implicit_const) {
+  int64_t value = data.GetSLEB128(offset_ptr);
+  m_attributes.emplace_back(attr, form, value);
+  continue;
+}
 
-if (form == DW_FORM_implicit_const)
-  val.value.sval = data.GetSLEB128(offset_ptr);
-
-m_attributes.push_back(DWARFAttribute(attr, form, val));
+m_attributes.emplace_back(attr, form);
   }
 
   return llvm::make_error(
@@ -72,10 +73,8 @@
 
 uint32_t
 DWARFAbbreviationDeclaration::FindAttributeIndex(dw_attr_t attr) const {
-  uint32_t i;
-  const uint32_t kNumAttributes = m_attributes.size();
-  for (i = 0; i < kNumAttributes; ++i) {
-if (m_attributes[i].get_attr() == attr)
+  for (size_t i = 0; i < m_attributes.size(); ++i) {
+if (m_attributes[i].GetAttribute() == attr)
   return i;
   }
   return DW_INVALID_INDEX;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 050c09f - [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

2023-05-12 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-05-12T13:11:56-07:00
New Revision: 050c09f0bed67f3135cb2ad99d4d35e241e5d61b

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

LOG: [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

DWARFAttribute is used in 2 classes: DWARFAbbreviationDecl and
DWARFAttributes. The former stores a std::vector of them and the latter
has a small structure called AttributeValue that contains a
DWARFAttribute. DWARFAttributes maintains a llvm::SmallVector of
AttributeValues.

My end goal is to have `DWARFAttributes` have a llvm::SmallVector
specialized on DWARFAttribute. In order to do that, we'll have to move
the other elements of AttributeValue into DWARFAttribute itself. But we
don't want to do this while DWARFAbbreviationDecl is using
DWARFAttribute because it will needlessly increase the size of
DWARFAbbreviationDecl. So instead I will create a small type containing
only what DWARFAbbreviationDecl needs and call it `AttributeSpec`. This
is the exact same thing that LLVM does today.

I've elected to swap std::vector for llvm::SmallVector here with a pre-allocated
size of 8. I've collected time and memory measurements before this change and
after it as well. Using a c++ project with 10,000 object files and no dSYM, I
place a breakpoint by file + lineno and see how long it takes to resolve.

Before this patch:
  Time (mean ± σ): 13.577 s ±  0.024 s[User: 12.418 s, System: 1.247 s]
Total number of bytes allocated: 1.38 GiB
Total number of allocations: 6.47 million allocations

After this patch:
  Time (mean ± σ): 13.287 s ±  0.020 s[User: 12.128 s, System: 1.250 s]
Total number of bytes allocated: 1.59 GiB
Total number of allocations: 4.61 million allocations

So we consume more memory than before, but we actually make less allocations on
average.

I also measured with an llvm::SmallVector with a pre-allocated size of 4 instead
of 8 to measure how well it performs:

  Time (mean ± σ): 13.246 s ±  0.048 s[User: 12.074 s, System: 1.268 s]
Total memory consumption: 1.50 GiB
Total number of allocations: 5.74 million

Of course this data may look very different depending on the actual program
being debugged, but each of the object files had 100+ AbbreviationDeclarations
each with between 0 and 10 Attributes, so I feel this was a fair example to
consider.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

Removed: 




diff  --git 
a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
index 8dd47e6da8d5..f88a604fd916 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -53,12 +53,13 @@ DWARFAbbreviationDeclaration::extract(const 
DWARFDataExtractor &data,
   return llvm::make_error(
   "malformed abbreviation declaration attribute");
 
-DWARFFormValue::ValueType val;
+if (form == DW_FORM_implicit_const) {
+  int64_t value = data.GetSLEB128(offset_ptr);
+  m_attributes.emplace_back(attr, form, value);
+  continue;
+}
 
-if (form == DW_FORM_implicit_const)
-  val.value.sval = data.GetSLEB128(offset_ptr);
-
-m_attributes.push_back(DWARFAttribute(attr, form, val));
+m_attributes.emplace_back(attr, form);
   }
 
   return llvm::make_error(
@@ -72,10 +73,8 @@ bool DWARFAbbreviationDeclaration::IsValid() {
 
 uint32_t
 DWARFAbbreviationDeclaration::FindAttributeIndex(dw_attr_t attr) const {
-  uint32_t i;
-  const uint32_t kNumAttributes = m_attributes.size();
-  for (i = 0; i < kNumAttributes; ++i) {
-if (m_attributes[i].get_attr() == attr)
+  for (size_t i = 0; i < m_attributes.size(); ++i) {
+if (m_attributes[i].GetAttribute() == attr)
   return i;
   }
   return DW_INVALID_INDEX;

diff  --git 
a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
index 1c69894f82ec..f67698dd9e62 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
@@ -16,6 +16,29 @@
 
 class DWARFAbbreviationDeclaration {
 public:
+  struct AttributeSpec {
+AttributeSpec(dw_attr_t attr, dw_form_t form, int64_t value)
+: m_attr(attr), m_form(form), m_value(value) {}
+
+AttributeSpec(dw_attr_t attr, dw_form_t form)
+: m_attr(attr), m_form(form), m_value(0) {}
+
+bool IsIm

[Lldb-commits] [PATCH] D150485: [lldb] Add support for negative integer to {SB, }StructuredData

2023-05-12 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: labath, bulbazord, JDevlieghere, jingham.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch refactors the `StructuredData::Integer` class to make it
templated, makes it private and adds 2 public specialization for both
`int64_t` & `uint64_t` with a public type aliases, respectively
`SignedInteger` & `UnsignedInteger`.

It adds new getter for signed and unsigned interger values to the
`StructuredData::Object` base class and changes the implementation of
`StructuredData::Array::GetItemAtIndexAsInteger` and
`StructuredData::Dictionary::GetValueForKeyAsInteger` to support signed
and unsigned integers.

This patch updates the `SBStructuredData::GetIntegerValue` by templating
it and specialize it for `int64_t` & `uint64_t` to avoid breaking the
current implementation. This way when called from python, the same
function will work for both signed and unsigned integers.

Finally, this patch audits all the caller of `StructuredData::Integer`
or `StructuredData::Integer::GetIntegerValue()` to use the proper type.

rdar://105575764

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150485

Files:
  lldb/bindings/interface/SBStructuredDataExtensions.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Utility/StructuredData.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Host/common/XML.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp

Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -1162,7 +1162,11 @@
 out.try_emplace(key_utf8, value.GetFloatValue());
 break;
   case lldb::eStructuredDataTypeInteger:
-out.try_emplace(key_utf8, value.GetIntegerValue());
+  case lldb::eStructuredDataTypeUnsignedInteger:
+out.try_emplace(key_utf8, value.GetIntegerValue((uint64_t)0));
+break;
+  case lldb::eStructuredDataTypeSignedInteger:
+out.try_emplace(key_utf8, value.GetIntegerValue((int64_t)0));
 break;
   case lldb::eStructuredDataTypeArray: {
 lldb::SBStream contents;
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -68,10 +68,10 @@
 return std::make_shared(*b);
 
   if (auto u = value.getAsUINT64())
-return std::make_shared(*u);
+return std::make_shared(*u);
 
   if (auto i = value.getAsInteger())
-return std::make_shared(*i);
+return std::make_shared(*i);
 
   if (auto d = value.getAsNumber())
 return std::make_shared(*d);
@@ -149,10 +149,6 @@
   s.arrayEnd();
 }
 
-void StructuredData::Integer::Serialize(json::OStream &s) const {
-  s.value(static_cast(m_value));
-}
-
 void StructuredData::Float::Serialize(json::OStream &s) const {
   s.value(m_value);
 }
@@ -183,10 +179,6 @@
   s.value(llvm::formatv("{0:X}", m_object));
 }
 
-void StructuredData::Integer::GetDescription(lldb_private::Stream &s) const {
-  s.Printf("%" PRId64, static_cast(m_value));
-}
-
 void StructuredData::Float::GetDescription(lldb_private::Stream &s) const {
   s.Printf("%f", m_value);
 }
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1807,7 +1807,7 @@
   id->GetType() == eStructuredDataTypeInteger) {
 strm.Format("  Activity '{0}', {1:x}\n",
 name->GetAsS

[Lldb-commits] [PATCH] D150485: [lldb] Add support for negative integer to {SB, }StructuredData

2023-05-12 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

This is going to need some tests... And because you're adding templates to the 
SBAPI we'll need multiple tests -- Not just on the python side but I think we 
should add some actual C++ tests here in `test/API/api/`




Comment at: lldb/include/lldb/API/SBStructuredData.h:12-13
 
+#include "lldb/Core/StructuredDataImpl.h"
+
 #include "lldb/API/SBDefines.h"

You can't include a private header in a public header.



Comment at: lldb/include/lldb/API/SBStructuredData.h:70-77
+  template  T GetIntegerValue(T fail_value = {}) const {
+if constexpr (!std::is_integral_v)
+  return fail_value;
+
+return m_impl_up->GetType() == eStructuredDataTypeSignedInteger
+   ? m_impl_up->GetIntegerValue(static_cast(fail_value))
+   : m_impl_up->GetIntegerValue(static_cast(fail_value));

> All the SB API classes are non-virtual, single inheritance classes. They 
> should only include SBDefines.h or other SB headers as needed. **There should 
> be no inlined method implementations in the header files, they should all be 
> in the implementation files**. And there should be no direct ivar access.

Emphasis mine, from: https://lldb.llvm.org/design/sbapi.html

We should be able to move this into the implementation file.



Comment at: lldb/include/lldb/API/SBStructuredData.h:71-72
+  template  T GetIntegerValue(T fail_value = {}) const {
+if constexpr (!std::is_integral_v)
+  return fail_value;
+

This implementation allows us to generate `GetIntegerValue` for things like 
`const char *` and will just return a `fail_value` for those things. I think 
the interface would be better if it could give users feedback when they 
accidentally do something wrong with types instead of just giving them back 
whatever they want the failure value to be. Is there a way we can use SFINAE to 
**only** generate `GetIntegerValue` for things that are integral type? Would 
such a thing be compatible with SWIG?



Comment at: lldb/include/lldb/Core/StructuredDataImpl.h:131
 
   uint64_t GetIntegerValue(uint64_t fail_value = 0) const {
+return (m_data_sp ? m_data_sp->GetUnsignedIntegerValue(fail_value)

Not related to your change, we should probably move StructuredDataImpl to 
Utility :P



Comment at: lldb/include/lldb/Utility/StructuredData.h:308-309
+template  void AddIntegerItem(T value) {
+  static_assert(std::is_integral::value,
+"value type should be integral");
+  if constexpr (std::numeric_limits::is_signed)

Check for floating type here in addition, just like below.



Comment at: lldb/include/lldb/lldb-enumerations.h:816-817
   eStructuredDataTypeInteger,
+  eStructuredDataTypeUnsignedInteger = eStructuredDataTypeInteger,
+  eStructuredDataTypeSignedInteger,
   eStructuredDataTypeFloat,

Is it a good idea to insert these in the middle? Not that people should do 
this, but I assume some people hardcode these values in their script


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150485

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