date:20231110

[Lldb-commits] [lldb] 2f8e3d5 - [lldb][AArch64][Linux] Add field information for the mte_ctrl register (#71808)

2023-11-10 Thread via lldb-commits


Author: David Spickett
Date: 2023-11-10T09:01:22Z
New Revision: 2f8e3d55da68b08f63186fa9f98245145a180449

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

LOG: [lldb][AArch64][Linux] Add field information for the mte_ctrl register 
(#71808)

This is a Linux pseudo register provided by the NT_ARM_TAGGED_ADDR_CTRL
register set. It reflects the value passed to prctl
PR_SET_TAGGED_ADDR_CTRL.

https://docs.kernel.org/arch/arm64/memory-tagging-extension.html

The fields are made from the #defines the kernel provides for setting
the value. Its contents are constant so no runtime detection is needed
(once we've decided we have this register in the first place).

The permitted generated tags is technically a bitfield but at this time
we don't have a way to mark a field as preferring hex formatting.

```
(lldb) register read mte_ctrl
mte_ctrl = 0x0007fffb
 = (TAGS = 65535, TCF_ASYNC = 0, TCF_SYNC = 1, TAGGED_ADDR_ENABLE = 1)
```

(4 bit tags mean 16 possible tags, 16 bit bitfield)

Testing has been added to TestMTECtrlRegister.py, which needed a more
granular way to check for XML support, so I've added hasXMLSupport that
can be used within a test case instead of skipping whole tests if XML
isn't supported.

Same for the core file tests.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h

lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py

lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 15e8ba21266c896..19ba0e8c133edcf 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1224,6 +1224,17 @@ def dumpSessionInfo(self):
 # (enables reading of the current test configuration)
 # 
 
+def hasXMLSupport(self):
+"""Returns True if lldb was built with XML support. Use this check to
+enable parts of tests, if you want to skip a whole test use 
skipIfXmlSupportMissing
+instead."""
+return (
+lldb.SBDebugger.GetBuildConfiguration()
+.GetValueForKey("xml")
+.GetValueForKey("value")
+.GetBooleanValue(False)
+)
+
 def isMIPS(self):
 """Returns true if the architecture is MIPS."""
 arch = self.getArchitecture()

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 87f2464a9eb4868..ca247b628abe78f 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -24,6 +24,19 @@
 
 using namespace lldb_private;
 
+LinuxArm64RegisterFlags::Fields
+LinuxArm64RegisterFlags::DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2) {
+  (void)hwcap;
+  (void)hwcap2;
+  // Represents the contents of NT_ARM_TAGGED_ADDR_CTRL and the value passed
+  // to prctl(PR_TAGGED_ADDR_CTRL...). Fields are derived from the defines
+  // used to build the value.
+  return {{"TAGS", 3, 18}, // 16 bit bitfield shifted up by PR_MTE_TAG_SHIFT.
+  {"TCF_ASYNC", 2},
+  {"TCF_SYNC", 1},
+  {"TAGGED_ADDR_ENABLE", 0}};
+}
+
 LinuxArm64RegisterFlags::Fields
 LinuxArm64RegisterFlags::DetectFPCRFields(uint64_t hwcap, uint64_t hwcap2) {
   std::vector fpcr_fields{

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index 651a8c86f7c86a9..deff977a03d0a12 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -58,6 +58,7 @@ class LinuxArm64RegisterFlags {
   static Fields DetectCPSRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectFPSRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectFPCRFields(uint64_t hwcap, uint64_t hwcap2);
+  static Fields DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
 RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
@@ -67,10 +68,11 @@ class LinuxArm64RegisterFlags {
 llvm::StringRef m_name;
 RegisterFlags m_flags;
 DetectorFn m_detector;
-  } m_registers[3] = {
+  } m_registers[4] = {
   RegisterEntry("cpsr", 4, DetectCPSRFields),
   RegisterEntry("fpsr", 4, DetectFPSRFields),
   RegisterEntry("fpcr", 4, Detect

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add field information for the mte_ctrl register (PR #71808)

2023-11-10 Thread David Spickett via lldb-commits


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

[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

2023-11-10 Thread David Spickett via lldb-commits

=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 

DavidSpickett wrote:

> We've re-compiled the latest llvm project (without adding our implementation) 
> but that one is also giving us the same error:

This was going to be my first suggestion, good instinct here!

> We've used the following command for building lldb:
cmake -G Ninja -DLLVM_ENABLE_PROJECTS="lldb;clang" -DLLVM_USE_LINKER=lld 
-DLLVM_BUILD_TESTS=true -DCMAKE_BUILD_TYPE=Release 
/home/talha/llvm-latest/llvm-project-main/llvm

This seems fine, though usually I add `lld` to the enabled projects. Our test 
suite *should* tell you it's missing if that was the case.

For known good CMake confuigurations we have (go to "cmake-configure" -> "view 
all  lines" -> go to the top):
Ubuntu AArch64 (should work just as well for x86): 
https://lab.llvm.org/buildbot/#/builders/96/builds/48271
Debian X86 (again, should be fine on Ubuntu also): 
https://lab.llvm.org/buildbot/#/builders/68/builds/63272

> Can you please confirm if there's something wrong in our build. Also I've the 
> complete log of ninja check-lldb but it's pretty long. Do you need it to 
> figure out the issue?

Ultimately if I do it, then yes. However, maybe you'll be able to figure it out 
yourself.

For the `lldb-shell` tests you can run them individually with:
```
./bin/llvm-lit /lldb/test/Shell/ -a
```

If as I suspect this is one underlying issue causing all those failures, it may 
be evident from the output. It may also be relevant whether this is a physical 
machine or a docker container, but let's see what that output looks like first.

To get a decent log you'll need to add `-DLLVM_LIT_ARGS="-v"` to your CMake 
command. This prints every test run and when one fails, it prints its output.

Perhaps post the output of one of the individual tests, and attach the full log 
as a backup if we can't figure it out from the one test.

> Plus there's one more thing we are not sure of as we are new to github. There 
> have been some updates in a file we've changed in our draft 
> (source/Commands/CommandObjectTarget.cpp). So do we need a fresh build and 
> add our implementation there before the final review? As the draft also says 
> that

This is normal as `main` is where you'd land this change eventually, and that's 
moved on and yes, before a final review you'd update the change.

If this were in active review and only changing in small ways, I'd tell you not 
to rebase it (which is where you put your changes on top of the new `main`, 
https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase).

However, as this is in draft, you're still making a lot of changes, and I'm 
happy reading it through each time, feel free to rebase now if you wish to. 
When you do, you just push changes to this branch as normal and it'll work as 
expected.

Now might be a good time. Figure out the test issues without your changes, then 
rebase your changes onto that, check again.

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

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add register field information for SME's SVCR register (PR #71809)

2023-11-10 Thread David Spickett via lldb-commits


https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/71809

>From 67fb6c09c5b948e1f39800586880be1d28782cd0 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Mon, 9 Oct 2023 11:05:47 +0100
Subject: [PATCH] [lldb][AArch64][Linux] Add register field information for
 SME's SVCR register

This register is a pseudo register but mirrors the architectural
register's contents. See:
https://developer.arm.com/documentation/ddi0616/latest/

For the full details. Example output:
```
(lldb) register read svcr
svcr = 0x0002
 = (ZA = 1, SM = 0)
```
---
 .../Process/Utility/RegisterFlagsLinux_arm64.cpp| 13 +
 .../Process/Utility/RegisterFlagsLinux_arm64.h  |  4 +++-
 .../save_restore/TestSMEZRegistersSaveRestore.py| 12 
 .../sme_core_file/TestAArch64LinuxSMECoreFile.py| 13 +
 4 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index ca247b628abe78f..77c7116d3c624ae 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -24,6 +24,19 @@
 
 using namespace lldb_private;
 
+LinuxArm64RegisterFlags::Fields
+LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
+  (void)hwcap;
+  (void)hwcap2;
+  // Represents the pseudo register that lldb-server builds, which itself
+  // matches the architectural register SCVR. The fields match SVCR in the Arm
+  // manual.
+  return {
+  {"ZA", 1},
+  {"SM", 0},
+  };
+}
+
 LinuxArm64RegisterFlags::Fields
 LinuxArm64RegisterFlags::DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2) {
   (void)hwcap;
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index deff977a03d0a12..660bef08700f4c8 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -59,6 +59,7 @@ class LinuxArm64RegisterFlags {
   static Fields DetectFPSRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectFPCRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2);
+  static Fields DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
 RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
@@ -68,11 +69,12 @@ class LinuxArm64RegisterFlags {
 llvm::StringRef m_name;
 RegisterFlags m_flags;
 DetectorFn m_detector;
-  } m_registers[4] = {
+  } m_registers[5] = {
   RegisterEntry("cpsr", 4, DetectCPSRFields),
   RegisterEntry("fpsr", 4, DetectFPSRFields),
   RegisterEntry("fpcr", 4, DetectFPCRFields),
   RegisterEntry("mte_ctrl", 8, DetectMTECtrlFields),
+  RegisterEntry("svcr", 8, DetectSVCRFields),
   };
 
   // Becomes true once field detection has been run for all registers.
diff --git 
a/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
 
b/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
index a0949d80c56640a..9433aae0c53c45e 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
+++ 
b/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
@@ -180,9 +180,12 @@ def za_expr_test_impl(self, sve_mode, za_state, 
swap_start_vl):
 self.runCmd("register read " + sve_reg_names)
 sve_values = self.res.GetOutput()
 
-svcr_value = 1 if sve_mode == Mode.SSVE else 0
-if za_state == ZA.Enabled:
-svcr_value += 2
+za = 1 if za_state == ZA.Enabled else 0
+sm = 1 if sve_mode == Mode.SSVE else 0
+svcr_value = "0x{:016x}".format((za << 1) | sm)
+expected_svcr = [svcr_value]
+if self.hasXMLSupport():
+expected_svcr.append("(ZA = {}, SM = {})".format(za, sm))
 
 has_zt0 = self.isAArch64SME2()
 
@@ -201,7 +204,8 @@ def check_regs():
 self.assertEqual(start_vg, self.read_vg())
 
 self.expect("register read " + sve_reg_names, substrs=[sve_values])
-self.expect("register read svcr", 
substrs=["0x{:016x}".format(svcr_value)])
+
+self.expect("register read svcr", substrs=expected_svcr)
 
 for expr in exprs:
 expr_cmd = "expression {}()".format(expr)
diff --git 
a/lldb/test/API/linux/aarch64/sme_core_file/TestAArch64LinuxSMECoreFile.py 
b/lldb/test/API/linux/aarch64/sme_core_file/TestAArch64LinuxSMECoreFile.py
index a5fdd0ab2068cbb..ee699aad2982607 100644
--- a/lldb/test/API/linux/aarch64/sme_core_file/TestAArch64LinuxSMECoreFile.py
+++ b/lldb/test/

[Lldb-commits] [lldb] 9ad25f2 - [lldb][AArch64][Linux] Add register field information for SME's SVCR register (#71809)

2023-11-10 Thread via lldb-commits


Author: David Spickett
Date: 2023-11-10T09:30:11Z
New Revision: 9ad25f235262bf9e215fdcb68706966b19b915ef

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

LOG: [lldb][AArch64][Linux] Add register field information for SME's SVCR 
register (#71809)

This register is a pseudo register but mirrors the architectural
register's contents. See:
https://developer.arm.com/documentation/ddi0616/latest/

For the full details. Example output:
```
(lldb) register read svcr
svcr = 0x0002
 = (ZA = 1, SM = 0)
```

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h

lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
lldb/test/API/linux/aarch64/sme_core_file/TestAArch64LinuxSMECoreFile.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index ca247b628abe78f..77c7116d3c624ae 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -24,6 +24,19 @@
 
 using namespace lldb_private;
 
+LinuxArm64RegisterFlags::Fields
+LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
+  (void)hwcap;
+  (void)hwcap2;
+  // Represents the pseudo register that lldb-server builds, which itself
+  // matches the architectural register SCVR. The fields match SVCR in the Arm
+  // manual.
+  return {
+  {"ZA", 1},
+  {"SM", 0},
+  };
+}
+
 LinuxArm64RegisterFlags::Fields
 LinuxArm64RegisterFlags::DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2) {
   (void)hwcap;

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index deff977a03d0a12..660bef08700f4c8 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -59,6 +59,7 @@ class LinuxArm64RegisterFlags {
   static Fields DetectFPSRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectFPCRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2);
+  static Fields DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
 RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
@@ -68,11 +69,12 @@ class LinuxArm64RegisterFlags {
 llvm::StringRef m_name;
 RegisterFlags m_flags;
 DetectorFn m_detector;
-  } m_registers[4] = {
+  } m_registers[5] = {
   RegisterEntry("cpsr", 4, DetectCPSRFields),
   RegisterEntry("fpsr", 4, DetectFPSRFields),
   RegisterEntry("fpcr", 4, DetectFPCRFields),
   RegisterEntry("mte_ctrl", 8, DetectMTECtrlFields),
+  RegisterEntry("svcr", 8, DetectSVCRFields),
   };
 
   // Becomes true once field detection has been run for all registers.

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
 
b/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
index a0949d80c56640a..9433aae0c53c45e 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
+++ 
b/lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py
@@ -180,9 +180,12 @@ def za_expr_test_impl(self, sve_mode, za_state, 
swap_start_vl):
 self.runCmd("register read " + sve_reg_names)
 sve_values = self.res.GetOutput()
 
-svcr_value = 1 if sve_mode == Mode.SSVE else 0
-if za_state == ZA.Enabled:
-svcr_value += 2
+za = 1 if za_state == ZA.Enabled else 0
+sm = 1 if sve_mode == Mode.SSVE else 0
+svcr_value = "0x{:016x}".format((za << 1) | sm)
+expected_svcr = [svcr_value]
+if self.hasXMLSupport():
+expected_svcr.append("(ZA = {}, SM = {})".format(za, sm))
 
 has_zt0 = self.isAArch64SME2()
 
@@ -201,7 +204,8 @@ def check_regs():
 self.assertEqual(start_vg, self.read_vg())
 
 self.expect("register read " + sve_reg_names, substrs=[sve_values])
-self.expect("register read svcr", 
substrs=["0x{:016x}".format(svcr_value)])
+
+self.expect("register read svcr", substrs=expected_svcr)
 
 for expr in exprs:
 expr_cmd = "expression {}()".format(expr)

diff  --git 
a/lldb/test/API/linux/aarch64/sme_core_file/TestAArch64LinuxSMECoreFile.py 
b/lldb/test/API/linu

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add register field information for SME's SVCR register (PR #71809)

2023-11-10 Thread David Spickett via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Haojian Wu via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Haojian Wu via lldb-commits


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

[Lldb-commits] [lldb] 66acd1e - [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (#70779)

2023-11-10 Thread via lldb-commits


Author: Haojian Wu
Date: 2023-11-10T10:53:03+01:00
New Revision: 66acd1e4dc1080015fe6b234226f1d30d6577f04

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

LOG: [LLDB] Ignore actual-needed artificial members in 
DWARFASTParserClang::ParseSingleMember (#70779)

Address the FIXME, this will allow lldb to print all fields of the
generated coroutine frame structure.

Fixes #69309.

Added: 
lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test

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 3174c18c97d888c..a70dc9832f425c6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -133,6 +133,14 @@ static lldb::ModuleSP GetContainingClangModule(const 
DWARFDIE &die) {
   return lldb::ModuleSP();
 }
 
+// Returns true if the given artificial field name should be ignored when
+// parsing the DWARF.
+static bool ShouldIgnoreArtificialField(llvm::StringRef FieldName) {
+  return FieldName.starts_with("_vptr$")
+ // gdb emit vtable pointer as "_vptr.classname"
+ || FieldName.starts_with("_vptr.");
+}
+
 TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
  const DWARFDIE &die,
  Log *log) {
@@ -3059,9 +3067,7 @@ void DWARFASTParserClang::ParseSingleMember(
   // This needs to be done after updating FieldInfo which keeps track of where
   // field start/end so we don't later try to fill the space of this
   // artificial member with (unnamed bitfield) padding.
-  // FIXME: This check should verify that this is indeed an artificial member
-  // we are supposed to ignore.
-  if (attrs.is_artificial) {
+  if (attrs.is_artificial && ShouldIgnoreArtificialField(attrs.name)) {
 last_field_info.SetIsArtificial(true);
 return;
   }

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test 
b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
new file mode 100644
index 000..e7d3bc4b796224a
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
@@ -0,0 +1,17 @@
+# UNSUPPORTED: system-darwin, system-windows
+
+# Make sure the artifical field `vptr.ClassName` from gcc debug info is 
ignored.
+# RUN: %build --compiler=gcc %S/Inputs/debug-types-expressions.cpp -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+breakpoint set -n foo
+process launch
+
+# CHECK: Process {{.*}} stopped
+
+frame variable *a
+# CHECK-LABEL: frame variable *a
+# CHECK:  (B) *a = {
+# CHECK-NEXT:   A = (i = 47)
+# CHECK-NEXT:   j = 42
+# CHECK-NEXT: }



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

[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Make MemberAttributes const when parsing member DIEs (PR #71921)

2023-11-10 Thread Michael Buch via lldb-commits


https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/71921

This patch removes the Objective-C accessibility workaround added in 
https://github.com/llvm/llvm-project/commit/5a477cfd90a8a12510518973fa450ae67372f199
 (rdar://8492646). This allows us to make the local `MemberAttributes` variable 
immutable, which is useful for some other work around this function I was 
planning on doing.

We don't need the workaround anymore since compiler-support for giving 
debuggers access to private ivars was done couple of years later in 
https://github.com/llvm/llvm-project/commit/d6cb4a858db0592f6f946fd99a10a9dfcbea6ee9
 (rdar://10997647).

**Testing**

* Test-suite runs cleanly

>From 57c208d41f450636c8873b0c0c077dc6255ee104 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 10 Nov 2023 10:50:23 +
Subject: [PATCH] [lldb][DWARFASTParserClang] Make MemberAttributes const when
 parsing member DIEs

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 36 ---
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..4d87117cfe9c195 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2869,15 +2869,7 @@ void DWARFASTParserClang::ParseSingleMember(
   const uint64_t parent_bit_size =
   parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
 
-  // FIXME: Remove the workarounds below and make this const.
-  MemberAttributes attrs(die, parent_die, module_sp);
-
-  const bool class_is_objc_object_or_interface =
-  TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
-
-  // FIXME: Make Clang ignore Objective-C accessibility for expressions
-  if (class_is_objc_object_or_interface)
-attrs.accessibility = eAccessNone;
+  const MemberAttributes attrs(die, parent_die, module_sp);
 
   // Handle static members, which are typically members without
   // locations. However, GCC doesn't emit DW_AT_data_member_location
@@ -2892,13 +2884,13 @@ void DWARFASTParserClang::ParseSingleMember(
   if (attrs.member_byte_offset == UINT32_MAX &&
   attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
-
 if (var_type) {
-  if (attrs.accessibility == eAccessNone)
-attrs.accessibility = eAccessPublic;
+  const auto accessibility = attrs.accessibility == eAccessNone
+ ? eAccessPublic
+ : attrs.accessibility;
   CompilerType ct = var_type->GetForwardCompilerType();
   clang::VarDecl *v = TypeSystemClang::AddVariableToRecordType(
-  class_clang_type, attrs.name, ct, attrs.accessibility);
+  class_clang_type, attrs.name, ct, accessibility);
   if (!v) {
 LLDB_LOG(log, "Failed to add variable to the record type");
 return;
@@ -2942,8 +2934,9 @@ void DWARFASTParserClang::ParseSingleMember(
   const uint64_t word_width = 32;
   CompilerType member_clang_type = member_type->GetLayoutCompilerType();
 
-  if (attrs.accessibility == eAccessNone)
-attrs.accessibility = default_accessibility;
+  const auto accessibility = attrs.accessibility == eAccessNone
+ ? default_accessibility
+ : attrs.accessibility;
 
   uint64_t field_bit_offset = (attrs.member_byte_offset == UINT32_MAX
? 0
@@ -2957,12 +2950,13 @@ void DWARFASTParserClang::ParseSingleMember(
 if (attrs.data_bit_offset != UINT64_MAX) {
   this_field_info.bit_offset = attrs.data_bit_offset;
 } else {
-  if (!attrs.byte_size)
-attrs.byte_size = member_type->GetByteSize(nullptr);
+  auto byte_size = attrs.byte_size;
+  if (!byte_size)
+byte_size = member_type->GetByteSize(nullptr);
 
   ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
   if (objfile->GetByteOrder() == eByteOrderLittle) {
-this_field_info.bit_offset += attrs.byte_size.value_or(0) * 8;
+this_field_info.bit_offset += byte_size.value_or(0) * 8;
 this_field_info.bit_offset -= (attrs.bit_offset + attrs.bit_size);
   } else {
 this_field_info.bit_offset += attrs.bit_offset;
@@ -3001,7 +2995,7 @@ void DWARFASTParserClang::ParseSingleMember(
 // unnamed bitfields if we have a new enough clang.
 bool detect_unnamed_bitfields = true;
 
-if (class_is_objc_object_or_interface)
+if (TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type))
   detect_unnamed_bitfields =
   die.GetCU()->Supports_unnamed_objc_bitfields();
 
@@ -3033,7 +3027,7 @@ void DWARFASTParserClang::ParseSingleMember(
 class_clang_type, llvm::StringRef(),
 m_ast.GetBuiltinTypeFo

[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Make MemberAttributes const when parsing member DIEs (PR #71921)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

This patch removes the Objective-C accessibility workaround added in 
https://github.com/llvm/llvm-project/commit/5a477cfd90a8a12510518973fa450ae67372f199
 (rdar://8492646). This allows us to make the local `MemberAttributes` variable 
immutable, which is useful for some other work around this function I was 
planning on doing.

We don't need the workaround anymore since compiler-support for giving 
debuggers access to private ivars was done couple of years later in 
https://github.com/llvm/llvm-project/commit/d6cb4a858db0592f6f946fd99a10a9dfcbea6ee9
 (rdar://10997647).

**Testing**

* Test-suite runs cleanly

---
Full diff: https://github.com/llvm/llvm-project/pull/71921.diff


1 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(+15-21) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..4d87117cfe9c195 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2869,15 +2869,7 @@ void DWARFASTParserClang::ParseSingleMember(
   const uint64_t parent_bit_size =
   parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
 
-  // FIXME: Remove the workarounds below and make this const.
-  MemberAttributes attrs(die, parent_die, module_sp);
-
-  const bool class_is_objc_object_or_interface =
-  TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
-
-  // FIXME: Make Clang ignore Objective-C accessibility for expressions
-  if (class_is_objc_object_or_interface)
-attrs.accessibility = eAccessNone;
+  const MemberAttributes attrs(die, parent_die, module_sp);
 
   // Handle static members, which are typically members without
   // locations. However, GCC doesn't emit DW_AT_data_member_location
@@ -2892,13 +2884,13 @@ void DWARFASTParserClang::ParseSingleMember(
   if (attrs.member_byte_offset == UINT32_MAX &&
   attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
-
 if (var_type) {
-  if (attrs.accessibility == eAccessNone)
-attrs.accessibility = eAccessPublic;
+  const auto accessibility = attrs.accessibility == eAccessNone
+ ? eAccessPublic
+ : attrs.accessibility;
   CompilerType ct = var_type->GetForwardCompilerType();
   clang::VarDecl *v = TypeSystemClang::AddVariableToRecordType(
-  class_clang_type, attrs.name, ct, attrs.accessibility);
+  class_clang_type, attrs.name, ct, accessibility);
   if (!v) {
 LLDB_LOG(log, "Failed to add variable to the record type");
 return;
@@ -2942,8 +2934,9 @@ void DWARFASTParserClang::ParseSingleMember(
   const uint64_t word_width = 32;
   CompilerType member_clang_type = member_type->GetLayoutCompilerType();
 
-  if (attrs.accessibility == eAccessNone)
-attrs.accessibility = default_accessibility;
+  const auto accessibility = attrs.accessibility == eAccessNone
+ ? default_accessibility
+ : attrs.accessibility;
 
   uint64_t field_bit_offset = (attrs.member_byte_offset == UINT32_MAX
? 0
@@ -2957,12 +2950,13 @@ void DWARFASTParserClang::ParseSingleMember(
 if (attrs.data_bit_offset != UINT64_MAX) {
   this_field_info.bit_offset = attrs.data_bit_offset;
 } else {
-  if (!attrs.byte_size)
-attrs.byte_size = member_type->GetByteSize(nullptr);
+  auto byte_size = attrs.byte_size;
+  if (!byte_size)
+byte_size = member_type->GetByteSize(nullptr);
 
   ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
   if (objfile->GetByteOrder() == eByteOrderLittle) {
-this_field_info.bit_offset += attrs.byte_size.value_or(0) * 8;
+this_field_info.bit_offset += byte_size.value_or(0) * 8;
 this_field_info.bit_offset -= (attrs.bit_offset + attrs.bit_size);
   } else {
 this_field_info.bit_offset += attrs.bit_offset;
@@ -3001,7 +2995,7 @@ void DWARFASTParserClang::ParseSingleMember(
 // unnamed bitfields if we have a new enough clang.
 bool detect_unnamed_bitfields = true;
 
-if (class_is_objc_object_or_interface)
+if (TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type))
   detect_unnamed_bitfields =
   die.GetCU()->Supports_unnamed_objc_bitfields();
 
@@ -3033,7 +3027,7 @@ void DWARFASTParserClang::ParseSingleMember(
 class_clang_type, llvm::StringRef(),
 m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
   word_width),
-attrs.accessibility, unnamed_field_info-

[Lldb-commits] [lldb] bd61126 - [lldb][docs] Update Discord invite link

2023-11-10 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2023-11-10T11:18:30Z
New Revision: bd611264993f64decbce178d460caf1d1cb05f59

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

LOG: [lldb][docs] Update Discord invite link

The previous one must have been an expiring one, this new
one is a non-expiring one that Chandler generated some time ago.

Added: 


Modified: 
lldb/docs/index.rst

Removed: 




diff  --git a/lldb/docs/index.rst b/lldb/docs/index.rst
index 88ed245b7b27472..c378ab97d97bbb4 100644
--- a/lldb/docs/index.rst
+++ b/lldb/docs/index.rst
@@ -103,7 +103,7 @@ See the :doc:`LLDB Build Page ` for build 
instructions.
 
 Discussions about LLDB should go to the `LLDB forum
 `__ or the ``lldb`` channel on
-the `LLVM Discord server `__.
+the `LLVM Discord server `__.
 
 For contributions follow the
 `LLVM contribution process `__. Commit



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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Haojian Wu via lldb-commits


https://github.com/hokein created 
https://github.com/llvm/llvm-project/pull/71928

See the discussion in #69309.

>From 1ac7e612bf6917af4e347407fb98affa9bb296c6 Mon Sep 17 00:00:00 2001
From: Haojian Wu 
Date: Fri, 10 Nov 2023 12:35:10 +0100
Subject: [PATCH] [LLDB] Display artificial __promise and __coro_frame
 variables.

See the discussion in #69309.
---
 .../CPlusPlus/CPPLanguageRuntime.cpp|  6 +-
 .../generic/coroutine_handle/TestCoroutineHandle.py | 13 -
 .../generic/coroutine_handle/main.cpp   |  2 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git 
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index c2488eaa9f5b50d..b5dfd07bdff2453 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.
+  return name == g_this ||
+  // Artificial coroutine-related variables emitted by clang.
+ name == ConstString("__promise") ||
+ name == ConstString("__coro_frame");
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
index 42ee32f9ccca58d..bcb1da6dc3838c8 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -78,8 +78,19 @@ def do_test(self, stdlib_type):
 ],
 )
 
-# Run until after the `co_yield`
 process = self.process()
+
+# Break at a coroutine body
+lldbutil.continue_to_source_breakpoint(
+  self, process, "// Break at co_yield", lldb.SBFileSpec("main.cpp", 
False)
+)
+# Expect artificial variables to be displayed
+self.expect(
+  "frame variable",
+  substrs=['__promise', '__coro_frame']
+)
+
+# Run until after the `co_yield`
 lldbutil.continue_to_source_breakpoint(
 self, process, "// Break after co_yield", 
lldb.SBFileSpec("main.cpp", False)
 )
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
index 8cb81c3bc9f4c4e..4523b7c7baf80aa 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -33,7 +33,7 @@ struct int_generator {
   ~int_generator() { hdl.destroy(); }
 };
 
-int_generator my_generator_func() { co_yield 42; }
+int_generator my_generator_func() { co_yield 42; } // Break at co_yield
 
 // This is an empty function which we call just so the debugger has
 // a place to reliably set a breakpoint on.

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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Haojian Wu via lldb-commits


hokein wrote:

Is there a way to run the tests under `lldb/test/API/` directory? `ninja 
check-lldb` doesn't seem to run these tests on my linux machine.

```
 ./bin/llvm-lit -sv  
/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/


Testing Time: 0.02s

Total Discovered Tests: 1
  Unsupported: 1 (100.00%)
```

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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Haojian Wu via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Haojian Wu (hokein)


Changes

See the discussion in #69309.

---
Full diff: https://github.com/llvm/llvm-project/pull/71928.diff


3 Files Affected:

- (modified) 
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp (+5-1) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
 (+12-1) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
 (+1-1) 


``diff
diff --git 
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index c2488eaa9f5b50d..b5dfd07bdff2453 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.
+  return name == g_this ||
+  // Artificial coroutine-related variables emitted by clang.
+ name == ConstString("__promise") ||
+ name == ConstString("__coro_frame");
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
index 42ee32f9ccca58d..bcb1da6dc3838c8 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -78,8 +78,19 @@ def do_test(self, stdlib_type):
 ],
 )
 
-# Run until after the `co_yield`
 process = self.process()
+
+# Break at a coroutine body
+lldbutil.continue_to_source_breakpoint(
+  self, process, "// Break at co_yield", lldb.SBFileSpec("main.cpp", 
False)
+)
+# Expect artificial variables to be displayed
+self.expect(
+  "frame variable",
+  substrs=['__promise', '__coro_frame']
+)
+
+# Run until after the `co_yield`
 lldbutil.continue_to_source_breakpoint(
 self, process, "// Break after co_yield", 
lldb.SBFileSpec("main.cpp", False)
 )
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
index 8cb81c3bc9f4c4e..4523b7c7baf80aa 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -33,7 +33,7 @@ struct int_generator {
   ~int_generator() { hdl.destroy(); }
 };
 
-int_generator my_generator_func() { co_yield 42; }
+int_generator my_generator_func() { co_yield 42; } // Break at co_yield
 
 // This is an empty function which we call just so the debugger has
 // a place to reliably set a breakpoint on.

``




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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread via lldb-commits


github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
bd611264993f64decbce178d460caf1d1cb05f59..1ac7e612bf6917af4e347407fb98affa9bb296c6
 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
``





View the diff from darker here.


``diff
--- TestCoroutineHandle.py  2023-11-10 11:44:20.00 +
+++ TestCoroutineHandle.py  2023-11-10 11:58:11.960396 +
@@ -80,17 +80,14 @@
 
 process = self.process()
 
 # Break at a coroutine body
 lldbutil.continue_to_source_breakpoint(
-  self, process, "// Break at co_yield", lldb.SBFileSpec("main.cpp", 
False)
+self, process, "// Break at co_yield", lldb.SBFileSpec("main.cpp", 
False)
 )
 # Expect artificial variables to be displayed
-self.expect(
-  "frame variable",
-  substrs=['__promise', '__coro_frame']
-)
+self.expect("frame variable", substrs=["__promise", "__coro_frame"])
 
 # Run until after the `co_yield`
 lldbutil.continue_to_source_breakpoint(
 self, process, "// Break after co_yield", 
lldb.SBFileSpec("main.cpp", False)
 )

``




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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread via lldb-commits


github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff bd611264993f64decbce178d460caf1d1cb05f59 
1ac7e612bf6917af4e347407fb98affa9bb296c6 -- 
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
``





View the diff from clang-format here.


``diff
diff --git 
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index b5dfd07bdff2..eba9324410d7 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -43,7 +43,7 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
   // FIXME: use a list when the list grows more.
   return name == g_this ||
-  // Artificial coroutine-related variables emitted by clang.
+ // Artificial coroutine-related variables emitted by clang.
  name == ConstString("__promise") ||
  name == ConstString("__coro_frame");
 }

``




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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Michael Buch via lldb-commits


Michael137 wrote:

> Is there a way to run the tests under `lldb/test/API/` directory? `ninja 
> check-lldb` doesn't seem to run these tests on my linux machine.
> 
> ```
>  ./bin/llvm-lit -sv  
> /lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/
> 
> 
> Testing Time: 0.02s
> 
> Total Discovered Tests: 1
>   Unsupported: 1 (100.00%)
> ```

Did you configure your build with `LLDB_INCLUDE_TESTS=ON` and 
`LLDB_INCLUDE_PYTHON=ON`? Those CMake variables are necessary to get the API 
tests to run

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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Michael Buch via lldb-commits



@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.

Michael137 wrote:

Don't think we need this FIXME

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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread David Spickett via lldb-commits


DavidSpickett wrote:

Hi, this is failing on Linaro's Linux LLDB bots: 
https://lab.llvm.org/buildbot/#/builders/96/builds/48277

Looks like the `-m64` (or `-m32`) is being passed to an AArch64 (or ARM) g++, 
and those are X86 only flags.

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

[Lldb-commits] [lldb] 343eb4b - [lldb] Remove the newly-added test in 66acd1e4dc1080015fe6b234226f1d30d6577f04

2023-11-10 Thread Haojian Wu via lldb-commits


Author: Haojian Wu
Date: 2023-11-10T13:46:36+01:00
New Revision: 343eb4b4253fea31767f6a98e1cf77a7d69c856a

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

LOG: [lldb] Remove the newly-added test in 
66acd1e4dc1080015fe6b234226f1d30d6577f04

The test added causes some buildbot failures:
- https://lab.llvm.org/buildbot/#/builders/17/builds/45077
https://lab.llvm.org/buildbot/#/builders/96/builds/48277

Remove the test for now to make the builtbots happy, and will re-add it after
investigation.

Added: 


Modified: 


Removed: 
lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test



diff  --git a/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test 
b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
deleted file mode 100644
index e7d3bc4b796224a..000
--- a/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
+++ /dev/null
@@ -1,17 +0,0 @@
-# UNSUPPORTED: system-darwin, system-windows
-
-# Make sure the artifical field `vptr.ClassName` from gcc debug info is 
ignored.
-# RUN: %build --compiler=gcc %S/Inputs/debug-types-expressions.cpp -o %t
-# RUN: %lldb %t -s %s -o exit | FileCheck %s
-
-breakpoint set -n foo
-process launch
-
-# CHECK: Process {{.*}} stopped
-
-frame variable *a
-# CHECK-LABEL: frame variable *a
-# CHECK:  (B) *a = {
-# CHECK-NEXT:   A = (i = 47)
-# CHECK-NEXT:   j = 42
-# CHECK-NEXT: }



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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread David Spickett via lldb-commits


DavidSpickett wrote:

I think I have a fix for this, will commit it soon.

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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Haojian Wu via lldb-commits


hokein wrote:

> Hi, this is failing on Linaro's Linux LLDB bots: 
> https://lab.llvm.org/buildbot/#/builders/96/builds/48277

> Looks like the -m64 (or -m32) is being passed to an AArch64 (or ARM) g++, and 
> those are X86 only flags.

Sorry for the breakage. I removed this test temporarily in 
343eb4b4253fea31767f6a98e1cf77a7d69c856a to make the builtbots happy again 
while doing the investigation.


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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Haojian Wu via lldb-commits


hokein wrote:

> I think I have a fix for this, will commit it soon.

Ah, I just saw your new message. If you can fix that, that would be fantastic 
(I don't know too much about lldb).

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

[Lldb-commits] [lldb] 7c3603e - [lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 platforms

2023-11-10 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2023-11-10T12:52:36Z
New Revision: 7c3603e1c162382b5c038b99e482e0689e1505aa

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

LOG: [lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 platforms

This option is definitely needed for x86_64, and is valid for PowerPC
and s390x too.

I'm using "in" because on Armv8 Linux the uname is actually "armv8l"
not just "arm".

Added: 


Modified: 
lldb/test/Shell/helper/build.py

Removed: 




diff  --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index 2a04967c89bc305..561d9ba2363c7ae 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -747,7 +747,10 @@ def _get_compilation_command(self, source, obj):
 args = []
 
 args.append(self.compiler)
-args.append("-m" + self.arch)
+
+uname = platform.uname().machine.lower()
+if not "arm" in uname and not "aarch64" in uname:
+args.append("-m" + self.arch)
 
 args.append("-g")
 if self.opt == "none":
@@ -784,7 +787,11 @@ def _get_compilation_command(self, source, obj):
 def _get_link_command(self):
 args = []
 args.append(self.compiler)
-args.append("-m" + self.arch)
+
+uname = platform.uname().machine.lower()
+if not "arm" in uname and not "aarch64" in uname:
+args.append("-m" + self.arch)
+
 if self.nodefaultlib:
 args.append("-nostdlib")
 args.append("-static")



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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread David Spickett via lldb-commits


DavidSpickett wrote:

I just pushed a fix for the build script, I'll put the test case back once I 
know the script change is good.

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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Haojian Wu via lldb-commits


hokein wrote:

Thank you very much for the fix!

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

[Lldb-commits] [lldb] fd5206c - Revert "[lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 platforms"

2023-11-10 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2023-11-10T13:17:01Z
New Revision: fd5206cc55c820598d5145d799b18d66cc193356

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

LOG: Revert "[lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 
platforms"

This reverts commit 7c3603e1c162382b5c038b99e482e0689e1505aa.

Turns out when you ask for "clang" it also uses the GCC Builder class,
so I need to update some tests.

Added: 


Modified: 
lldb/test/Shell/helper/build.py

Removed: 




diff  --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index 561d9ba2363c7ae..2a04967c89bc305 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -747,10 +747,7 @@ def _get_compilation_command(self, source, obj):
 args = []
 
 args.append(self.compiler)
-
-uname = platform.uname().machine.lower()
-if not "arm" in uname and not "aarch64" in uname:
-args.append("-m" + self.arch)
+args.append("-m" + self.arch)
 
 args.append("-g")
 if self.opt == "none":
@@ -787,11 +784,7 @@ def _get_compilation_command(self, source, obj):
 def _get_link_command(self):
 args = []
 args.append(self.compiler)
-
-uname = platform.uname().machine.lower()
-if not "arm" in uname and not "aarch64" in uname:
-args.append("-m" + self.arch)
-
+args.append("-m" + self.arch)
 if self.nodefaultlib:
 args.append("-nostdlib")
 args.append("-static")



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

[Lldb-commits] [lldb] 6b4ac76 - Reland "[lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 platforms"

2023-11-10 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2023-11-10T13:36:23Z
New Revision: 6b4ac76504d120d44023618ef8240d4f907d08ca

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

LOG: Reland "[lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 
platforms"

This reverts commit fd5206cc55c820598d5145d799b18d66cc193356.

Fixing the test case would require some awkard %if use that I'm not
sure would even work, or splitting it into 2 copies that are almost
identical.

Instead, always add -m for clang, which allows it for all targets,
but not for gcc which does not.

Added: 


Modified: 
lldb/test/Shell/helper/build.py

Removed: 




diff  --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index 2a04967c89bc305..073198a6df2df36 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -743,11 +743,21 @@ def __init__(self, toolchain_type, args):
 cmd = ["xcrun", "--sdk", args.apple_sdk, "--show-sdk-path"]
 self.apple_sdk = 
subprocess.check_output(cmd).strip().decode("utf-8")
 
+def _add_m_option_if_needed(self, args):
+# clang allows -m(32|64) for any target, gcc does not.
+uname = platform.uname().machine.lower()
+if self.toolchain_type != "gcc" or (
+not "arm" in uname and not "aarch64" in uname
+):
+args.append("-m" + self.arch)
+
+return args
+
 def _get_compilation_command(self, source, obj):
 args = []
 
 args.append(self.compiler)
-args.append("-m" + self.arch)
+args = self._add_m_option_if_needed(args)
 
 args.append("-g")
 if self.opt == "none":
@@ -784,7 +794,8 @@ def _get_compilation_command(self, source, obj):
 def _get_link_command(self):
 args = []
 args.append(self.compiler)
-args.append("-m" + self.arch)
+args = self._add_m_option_if_needed(args)
+
 if self.nodefaultlib:
 args.append("-nostdlib")
 args.append("-static")



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

[Lldb-commits] [lldb] d96ea27 - Revert "[lldb] Remove the newly-added test in 66acd1e4dc1080015fe6b234226f1d30d6577f04"

2023-11-10 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2023-11-10T13:47:41Z
New Revision: d96ea279734fd9105a0ed7bad898ed84d79ed308

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

LOG: Revert "[lldb] Remove the newly-added test in 
66acd1e4dc1080015fe6b234226f1d30d6577f04"

This reverts commit 343eb4b4253fea31767f6a98e1cf77a7d69c856a.

This test should work now that the build script doesn't add -m(32|64) on 
Arm/AArch64
builders.

Added: 
lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test 
b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
new file mode 100644
index 000..e7d3bc4b796224a
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
@@ -0,0 +1,17 @@
+# UNSUPPORTED: system-darwin, system-windows
+
+# Make sure the artifical field `vptr.ClassName` from gcc debug info is 
ignored.
+# RUN: %build --compiler=gcc %S/Inputs/debug-types-expressions.cpp -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+breakpoint set -n foo
+process launch
+
+# CHECK: Process {{.*}} stopped
+
+frame variable *a
+# CHECK-LABEL: frame variable *a
+# CHECK:  (B) *a = {
+# CHECK-NEXT:   A = (i = 47)
+# CHECK-NEXT:   j = 42
+# CHECK-NEXT: }



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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Haojian Wu via lldb-commits


https://github.com/hokein updated 
https://github.com/llvm/llvm-project/pull/71928

>From 1ac7e612bf6917af4e347407fb98affa9bb296c6 Mon Sep 17 00:00:00 2001
From: Haojian Wu 
Date: Fri, 10 Nov 2023 12:35:10 +0100
Subject: [PATCH 1/2] [LLDB] Display artificial __promise and __coro_frame
 variables.

See the discussion in #69309.
---
 .../CPlusPlus/CPPLanguageRuntime.cpp|  6 +-
 .../generic/coroutine_handle/TestCoroutineHandle.py | 13 -
 .../generic/coroutine_handle/main.cpp   |  2 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git 
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index c2488eaa9f5b50d..b5dfd07bdff2453 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.
+  return name == g_this ||
+  // Artificial coroutine-related variables emitted by clang.
+ name == ConstString("__promise") ||
+ name == ConstString("__coro_frame");
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
index 42ee32f9ccca58d..bcb1da6dc3838c8 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -78,8 +78,19 @@ def do_test(self, stdlib_type):
 ],
 )
 
-# Run until after the `co_yield`
 process = self.process()
+
+# Break at a coroutine body
+lldbutil.continue_to_source_breakpoint(
+  self, process, "// Break at co_yield", lldb.SBFileSpec("main.cpp", 
False)
+)
+# Expect artificial variables to be displayed
+self.expect(
+  "frame variable",
+  substrs=['__promise', '__coro_frame']
+)
+
+# Run until after the `co_yield`
 lldbutil.continue_to_source_breakpoint(
 self, process, "// Break after co_yield", 
lldb.SBFileSpec("main.cpp", False)
 )
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
index 8cb81c3bc9f4c4e..4523b7c7baf80aa 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -33,7 +33,7 @@ struct int_generator {
   ~int_generator() { hdl.destroy(); }
 };
 
-int_generator my_generator_func() { co_yield 42; }
+int_generator my_generator_func() { co_yield 42; } // Break at co_yield
 
 // This is an empty function which we call just so the debugger has
 // a place to reliably set a breakpoint on.

>From d67d907cedbf20d656a4db0658f918e156714170 Mon Sep 17 00:00:00 2001
From: Haojian Wu 
Date: Fri, 10 Nov 2023 15:16:08 +0100
Subject: [PATCH 2/2] Remove an unneeded FIXME.

---
 .../Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp   | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git 
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index b5dfd07bdff2453..2d14cf0d7a62add 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -41,9 +41,8 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  // FIXME: use a list when the list grows more.
   return name == g_this ||
-  // Artificial coroutine-related variables emitted by clang.
+ // Artificial coroutine-related variables emitted by clang.
  name == ConstString("__promise") ||
  name == ConstString("__coro_frame");
 }

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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Haojian Wu via lldb-commits



@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.

hokein wrote:

Sure, removed. 

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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Haojian Wu via lldb-commits

hokein wrote:

> Did you configure your build with `LLDB_INCLUDE_TESTS=ON` and 
> `LLDB_INCLUDE_PYTHON=ON`? Those CMake variables are necessary to get the API 
> tests to run

Thanks! I was able to run the test locally now with `-DLLDB_INCLUDE_TESTS=On`,  
`-DLLDB_ENABLE_PYTHON=On`, and all deps installed (`sudo apt-get install 
build-essential swig python3-dev libedit-dev`).

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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread David Spickett via lldb-commits


DavidSpickett wrote:

All fixed now.

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

[Lldb-commits] [lldb] Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" (PR #71800)

2023-11-10 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl approved this pull request.


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

[Lldb-commits] [clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Adrian Prantl via lldb-commits



@@ -5596,6 +5587,42 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl 
*VD, const APValue &Init) {
   TemplateParameters, Align));
 }
 
+void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
+  assert(VD->hasInit());
+  assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+  if (VD->hasAttr())
+return;
+
+  auto &GV = DeclCache[VD];

adrian-prantl wrote:

is it intentional that we cache nullptr here if the next condition fails?

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

[Lldb-commits] [clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl approved this pull request.

Minor comments inside.

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

[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Adrian Prantl via lldb-commits



@@ -5800,6 +5827,18 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
 }
 
 void CGDebugInfo::finalize() {
+  for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
+assert(VD->isStaticDataMember());
+
+if (DeclCache.contains(VD))
+  continue;
+
+if (!VD->hasInit())

adrian-prantl wrote:

it's non-obvious why this skip exists, can you add a comment here?

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

[Lldb-commits] [clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Michael Buch via lldb-commits



@@ -5596,6 +5587,42 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl 
*VD, const APValue &Init) {
   TemplateParameters, Align));
 }
 
+void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
+  assert(VD->hasInit());
+  assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+  if (VD->hasAttr())
+return;
+
+  auto &GV = DeclCache[VD];

Michael137 wrote:

Good catch, I'll err on the side of caution and not do that

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

[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Michael Buch via lldb-commits



@@ -5800,6 +5827,18 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
 }
 
 void CGDebugInfo::finalize() {
+  for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
+assert(VD->isStaticDataMember());
+
+if (DeclCache.contains(VD))
+  continue;
+
+if (!VD->hasInit())

Michael137 wrote:

I'll update the `EmitGlobalVariable` docs to clarify that we need an 
initialiser, otherwise behaviour is undefined

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

[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)

2023-11-10 Thread Kevin Frei via lldb-commits



@@ -0,0 +1,154 @@
+//===-- SymbolLocatorDebuginfod.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolLocatorDebuginfod.h"
+
+#include 
+#include 
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+#include "lldb/Utility/UUID.h"
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Debuginfod/HTTPClient.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ThreadPool.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(SymbolLocatorDebuginfod)
+
+namespace {
+
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+  static llvm::StringRef GetSettingName() {
+return SymbolLocatorDebuginfod::GetPluginNameStatic();
+  }
+
+  PluginProperties() {
+m_collection_sp = 
std::make_shared(GetSettingName());
+m_collection_sp->Initialize(g_symbollocatordebuginfod_properties);
+m_collection_sp->SetValueChangedCallback(
+ePropertyURLs, [this] { URLsChangedCallback(); }
+);
+  }
+
+  Args GetDebugInfoDURLs() const {
+Args urls;
+m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyURLs, urls);
+return urls;
+  }
+
+private:
+  void URLsChangedCallback() {
+Args urls = GetDebugInfoDURLs();
+llvm::SmallVector dbginfod_urls;
+llvm::transform(urls, dbginfod_urls.end(),
+[](const auto &obj) { return obj.ref(); });
+llvm::setDefaultDebuginfodUrls(dbginfod_urls);
+  }
+};
+
+} // namespace
+
+
+SymbolLocatorDebuginfod::SymbolLocatorDebuginfod() : SymbolLocator() {}
+
+void SymbolLocatorDebuginfod::Initialize() {
+  PluginManager::RegisterPlugin(
+  GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+  LocateExecutableObjectFile, LocateExecutableSymbolFile,
+  DownloadObjectAndSymbolFile);
+  // There's a "safety" concern on this:
+  // Does plugin initialization occur while things are still single threaded?

kevinfrei wrote:

I'll update the comment, as I verified it was correct for normal LLDB. Is there 
somewhere we could document the requirement somehow for hosts? (or is there 
some mechanism for hosts that would allow this to be handled safely?)

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

[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)

2023-11-10 Thread Kevin Frei via lldb-commits



@@ -0,0 +1,154 @@
+//===-- SymbolLocatorDebuginfod.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolLocatorDebuginfod.h"
+
+#include 
+#include 
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+#include "lldb/Utility/UUID.h"

kevinfrei wrote:

Whoops. You're right. Copy pasta is delicious...
I'll trim this down...a lot :/

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

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread Alex Langford via lldb-commits


https://github.com/bulbazord created 
https://github.com/llvm/llvm-project/pull/71961

Similar to my previous patch (#71613) where I changed `GetItemAtIndexAsString`, 
this patch makes the same change to `GetItemAtIndexAsDictionary`.

`GetItemAtIndexAsDictionary` now returns a std::optional that is either 
`std::nullopt` or is a valid pointer. Therefore, if the optional is populated, 
we consider the pointer to always be valid (i.e. no need to check pointer 
validity).

>From d8e76ef15d431135003379e768bd69b98da1f4e7 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Thu, 9 Nov 2023 15:07:22 -0800
Subject: [PATCH] [lldb] Change interface of
 StructuredData::Array::GetItemAtIndexAsDictionary

Similar to my previous patch (#71613) where I changed
`GetItemAtIndexAsString`, this patch makes the same change to
`GetItemAtIndexAsDictionary`.

`GetItemAtIndexAsDictionary` now returns a std::optional that is either
`std::nullopt` or is a valid pointer. Therefore, if the optional is
populated, we consider the pointer to always be valid (i.e. no need to
check pointer validity).
---
 lldb/include/lldb/Utility/StructuredData.h| 24 +--
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp |  6 +++--
 .../GDBRemoteCommunicationClient.cpp  |  6 +++--
 .../Process/scripted/ScriptedThread.cpp   |  6 +++--
 lldb/source/Target/DynamicRegisterInfo.cpp|  6 +++--
 .../lldb-server/tests/MessageObjects.cpp  |  7 +++---
 6 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 6e39bcff2c0af0b..8d0ae372f43c6bf 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -256,14 +256,24 @@ class StructuredData {
   return {};
 }
 
-bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *&result) const {
-  result = nullptr;
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
-result = value_sp->GetAsDictionary();
-return (result != nullptr);
+/// Retrieves the element at index \a idx from a StructuredData::Array if 
it
+/// is a Dictionary.
+///
+/// \param[in] idx
+///   The index of the element to retrieve.
+///
+/// \return
+///   If the element at index \a idx is a Dictionary, this method returns a
+///   valid pointer to the Dictionary wrapped in a std::optional. If the
+///   element is not a Dictionary or the index is invalid, this returns
+///   std::nullopt. Note that the underlying Dictionary pointer is never
+///   nullptr.
+std::optional GetItemAtIndexAsDictionary(size_t idx) const {
+  if (auto item_sp = GetItemAtIndex(idx)) {
+if (auto *dict = item_sp->GetAsDictionary())
+  return dict;
   }
-  return false;
+  return {};
 }
 
 bool GetItemAtIndexAsArray(size_t idx, Array *&result) const {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3714e37ec5c57d0..1ea4f649427727f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5688,14 +5688,16 @@ bool 
ObjectFileMachO::GetCorefileThreadExtraInfos(std::vector &tids) {
   }
   const size_t num_threads = threads->GetSize();
   for (size_t i = 0; i < num_threads; i++) {
-StructuredData::Dictionary *thread;
-if (!threads->GetItemAtIndexAsDictionary(i, thread) || !thread) {
+std::optional maybe_thread =
+threads->GetItemAtIndexAsDictionary(i);
+if (!maybe_thread) {
   LLDB_LOGF(log,
 "Unable to read 'process metadata' LC_NOTE, threads "
 "array does not have a dictionary at index %zu.",
 i);
   return false;
 }
+StructuredData::Dictionary *thread = *maybe_thread;
 tid_t tid = LLDB_INVALID_THREAD_ID;
 if (thread->GetValueForKeyAsInteger("thread_id", tid))
   if (tid == 0)
diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 04d98b96acd6839..2cf8c29bf9d2fe9 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2649,10 +2649,12 @@ size_t GDBRemoteCommunicationClient::QueryGDBServer(
 return 0;
 
   for (size_t i = 0, count = array->GetSize(); i < count; ++i) {
-StructuredData::Dictionary *element = nullptr;
-if (!array->GetItemAtIndexAsDictionary(i, element))
+std::optional maybe_element =
+array->GetItemAtIndexAsDictionary(i);
+if (!maybe_element)
   continue;
 
+StructuredData::Dictionary *element = *maybe_element;
 uint16_t port = 0;
 if (Structure

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)


Changes

Similar to my previous patch (#71613) where I changed 
`GetItemAtIndexAsString`, this patch makes the same change to 
`GetItemAtIndexAsDictionary`.

`GetItemAtIndexAsDictionary` now returns a std::optional that is either 
`std::nullopt` or is a valid pointer. Therefore, if the optional is populated, 
we consider the pointer to always be valid (i.e. no need to check pointer 
validity).

---
Full diff: https://github.com/llvm/llvm-project/pull/71961.diff


6 Files Affected:

- (modified) lldb/include/lldb/Utility/StructuredData.h (+17-7) 
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+4-2) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+4-2) 
- (modified) lldb/source/Plugins/Process/scripted/ScriptedThread.cpp (+4-2) 
- (modified) lldb/source/Target/DynamicRegisterInfo.cpp (+4-2) 
- (modified) lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp (+4-3) 


``diff
diff --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 6e39bcff2c0af0b..8d0ae372f43c6bf 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -256,14 +256,24 @@ class StructuredData {
   return {};
 }
 
-bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *&result) const {
-  result = nullptr;
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
-result = value_sp->GetAsDictionary();
-return (result != nullptr);
+/// Retrieves the element at index \a idx from a StructuredData::Array if 
it
+/// is a Dictionary.
+///
+/// \param[in] idx
+///   The index of the element to retrieve.
+///
+/// \return
+///   If the element at index \a idx is a Dictionary, this method returns a
+///   valid pointer to the Dictionary wrapped in a std::optional. If the
+///   element is not a Dictionary or the index is invalid, this returns
+///   std::nullopt. Note that the underlying Dictionary pointer is never
+///   nullptr.
+std::optional GetItemAtIndexAsDictionary(size_t idx) const {
+  if (auto item_sp = GetItemAtIndex(idx)) {
+if (auto *dict = item_sp->GetAsDictionary())
+  return dict;
   }
-  return false;
+  return {};
 }
 
 bool GetItemAtIndexAsArray(size_t idx, Array *&result) const {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3714e37ec5c57d0..1ea4f649427727f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5688,14 +5688,16 @@ bool 
ObjectFileMachO::GetCorefileThreadExtraInfos(std::vector &tids) {
   }
   const size_t num_threads = threads->GetSize();
   for (size_t i = 0; i < num_threads; i++) {
-StructuredData::Dictionary *thread;
-if (!threads->GetItemAtIndexAsDictionary(i, thread) || !thread) {
+std::optional maybe_thread =
+threads->GetItemAtIndexAsDictionary(i);
+if (!maybe_thread) {
   LLDB_LOGF(log,
 "Unable to read 'process metadata' LC_NOTE, threads "
 "array does not have a dictionary at index %zu.",
 i);
   return false;
 }
+StructuredData::Dictionary *thread = *maybe_thread;
 tid_t tid = LLDB_INVALID_THREAD_ID;
 if (thread->GetValueForKeyAsInteger("thread_id", tid))
   if (tid == 0)
diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 04d98b96acd6839..2cf8c29bf9d2fe9 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2649,10 +2649,12 @@ size_t GDBRemoteCommunicationClient::QueryGDBServer(
 return 0;
 
   for (size_t i = 0, count = array->GetSize(); i < count; ++i) {
-StructuredData::Dictionary *element = nullptr;
-if (!array->GetItemAtIndexAsDictionary(i, element))
+std::optional maybe_element =
+array->GetItemAtIndexAsDictionary(i);
+if (!maybe_element)
   continue;
 
+StructuredData::Dictionary *element = *maybe_element;
 uint16_t port = 0;
 if (StructuredData::ObjectSP port_osp =
 element->GetValueForKey(llvm::StringRef("port")))
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index aa2796db15cd00a..88a4ca3b0389fb9 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -176,8 +176,9 @@ bool ScriptedThread::LoadArtificialStackFrames() {

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread Jonas Devlieghere via lldb-commits


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


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

[Lldb-commits] [compiler-rt] [clang-tools-extra] [llvm] [flang] [mlir] [clang] [lldb] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)

2023-11-10 Thread Zequan Wu via lldb-commits


ZequanWu wrote:

There are some discussions about this outside the PR:
https://github.com/llvm/llvm-project/pull/70856#issuecomment-1791465183
https://discourse.llvm.org/t/rfc-add-binary-profile-correlation-to-not-load-profile-metadata-sections-into-memory-at-runtime/74565/8

Some further discussion/pre-work needed before proceeding with this PR.

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

[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)

2023-11-10 Thread Augusto Noronha via lldb-commits



@@ -2210,9 +2216,16 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
   !layout_info.vbase_offsets.empty()) {
 if (type)
   layout_info.bit_size = type->GetByteSize(nullptr).value_or(0) * 8;
-if (layout_info.bit_size == 0)
-  layout_info.bit_size =
-  die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8;
+if (layout_info.bit_size == 0) {
+  auto byte_size =
+  die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
+
+  if (byte_size != UINT64_MAX)
+layout_info.bit_size = byte_size;

augusto2112 wrote:

Thanks for catching this.

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

[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)

2023-11-10 Thread Augusto Noronha via lldb-commits



@@ -2866,8 +2879,12 @@ void DWARFASTParserClang::ParseSingleMember(
   // Get the parent byte size so we can verify any members will fit
   const uint64_t parent_byte_size =
   parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
-  const uint64_t parent_bit_size =
-  parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
+  uint64_t parent_bit_size;
+  if (parent_byte_size != UINT64_MAX)
+parent_bit_size = parent_byte_size * 8;
+  else
+parent_bit_size =
+parent_die.GetAttributeValueAsUnsigned(DW_AT_bit_size, UINT64_MAX);

augusto2112 wrote:

Hmm, I'll definitely have a downstream test that exercises this behavior, but I 
don't think on the clang side this would be easy to test, do you have any 
suggestions on how to do that?

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

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread Med Ismail Bennani via lldb-commits


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

LGTM!

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

[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)

2023-11-10 Thread Augusto Noronha via lldb-commits


https://github.com/augusto2112 updated 
https://github.com/llvm/llvm-project/pull/69741

>From 997da625fda1efebde43ec965c23c1a8ef9c0132 Mon Sep 17 00:00:00 2001
From: Augusto Noronha 
Date: Fri, 10 Nov 2023 10:40:05 -0800
Subject: [PATCH] Emit DIE's size in bits when size is not a multiple of 8

The existing code will always round down the size in bits when
calculating the size in bytes (for example, a types with 1-7 bits will
be emitted as 0 bytes long). Fix this by emitting the size in bits if
the size is not aligned to a byte.
---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 25 +--
 llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | 30 +---
 .../AArch64/non_standard_bit_sizes.ll | 69 +++
 3 files changed, 110 insertions(+), 14 deletions(-)
 create mode 100644 llvm/test/DebugInfo/AArch64/non_standard_bit_sizes.ll

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..928033ea48e890c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -298,6 +298,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const 
DWARFDIE &die) {
   byte_size = form_value.Unsigned();
   break;
 
+case DW_AT_bit_size:
+  // Convert the bit size to byte size, and round it up to the minimum
+  // amount of bytes that will fit the bits.
+  byte_size = (form_value.Unsigned() + 7) / 8;
+  break;
+
 case DW_AT_byte_stride:
   byte_stride = form_value.Unsigned();
   break;
@@ -2134,6 +2140,16 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
  template_param_infos.hasParameterPack();
 }
 
+/// Reads the bit size from a die, by trying both the byte size and bit size
+/// attributes.
+static uint64_t GetSizeInBitsFromDie(const DWARFDIE &die) {
+  const uint64_t byte_size =
+  die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
+  if (byte_size != UINT64_MAX)
+return byte_size * 8;
+  return die.GetAttributeValueAsUnsigned(DW_AT_bit_size, UINT64_MAX);
+}
+
 bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
  lldb_private::Type *type,
  CompilerType &clang_type) {
@@ -2211,8 +2227,7 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 if (type)
   layout_info.bit_size = type->GetByteSize(nullptr).value_or(0) * 8;
 if (layout_info.bit_size == 0)
-  layout_info.bit_size =
-  die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8;
+  layout_info.bit_size = GetSizeInBitsFromDie(die);
 
 clang::CXXRecordDecl *record_decl =
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
@@ -2864,10 +2879,8 @@ void DWARFASTParserClang::ParseSingleMember(
   ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
   const dw_tag_t tag = die.Tag();
   // Get the parent byte size so we can verify any members will fit
-  const uint64_t parent_byte_size =
-  parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
-  const uint64_t parent_bit_size =
-  parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
+  uint64_t parent_bit_size = GetSizeInBitsFromDie(parent_die);
+  ;
 
   // FIXME: Remove the workarounds below and make this const.
   MemberAttributes attrs(die, parent_die, module_sp);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp 
b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index a5960a5d4a09a17..9633e3b19ec0d63 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -687,6 +687,16 @@ std::string DwarfUnit::getParentContextString(const 
DIScope *Context) const {
   return CS;
 }
 
+/// Returns the most appropriate dwarf size attribute (bits or bytes) and size
+/// to be used with it, given the input size in bits.
+static std::pair
+getMostAppropriateRepresentationAndSize(uint64_t SizeInBits) {
+  if (SizeInBits % 8 == 0) {
+return {dwarf::DW_AT_byte_size, SizeInBits / 8};
+  }
+  return {dwarf::DW_AT_bit_size, SizeInBits};
+}
+
 void DwarfUnit::constructTypeDIE(DIE &Buffer, const DIBasicType *BTy) {
   // Get core information.
   StringRef Name = BTy->getName();
@@ -702,8 +712,9 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const 
DIBasicType *BTy) {
 addUInt(Buffer, dwarf::DW_AT_encoding, dwarf::DW_FORM_data1,
 BTy->getEncoding());
 
-  uint64_t Size = BTy->getSizeInBits() >> 3;
-  addUInt(Buffer, dwarf::DW_AT_byte_size, std::nullopt, Size);
+  auto [SizeAttribute, Size] =
+  getMostAppropriateRepresentationAndSize(BTy->getSizeInBits());
+  addUInt(Buffer, SizeAttribute, std::nullopt, Size);
 
   if (BTy->isBigEndian())
 addUInt(Buffer, dwarf::DW_AT_endianity, std::nullopt, dwarf::DW_END_big);
@@ -731,8 +742,9 @@ void DwarfUnit::constructTypeDIE

[Lldb-commits] [llvm] [lldb] Emit DIE's size in bits when size is not a multiple of 8 (PR #69741)

2023-11-10 Thread Augusto Noronha via lldb-commits


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

[Lldb-commits] [libc] [lldb] [compiler-rt] [clang-tools-extra] [flang] [lld] [libcxx] [llvm] [clang] [asan] Report executable/DSO name for report_globals=2 and odr-violation checking (PR #71879)

2023-11-10 Thread Fangrui Song via lldb-commits


https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/71879

>From 573a5c0ea74284f572cb1cff7e8d3e2d9cac210b Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Thu, 9 Nov 2023 15:17:25 -0800
Subject: [PATCH 1/3] [asan] Report executable/DSO name for report_globals=2
 and odr-violation checking

For an odr-violation error due to a source file linked into two DSOs, or
one DSO and the main executable, it can be difficult to identify the DSO
name. Let's print the module name in the error report.

```
echo 'extern long var; int main() { return var; }' > a.cc
echo 'long var;' > b.cc
clang++ -fpic -fsanitize=address -shared b.cc -o b.so
clang++ -fsanitize=address a.cc b.cc ./b.so -o a
```

w/o this patch:
```
==1375386==ERROR: AddressSanitizer: odr-violation (0x56067cb06240):
  [1] size=8 'var' b.cc
  [2] size=8 'var' b.cc
...
```
w/ this patch:
```
==1375386==ERROR: AddressSanitizer: odr-violation (0x56067cb06240):
  [1] size=8 'var' b.cc in /tmp/c/a
  [2] size=8 'var' b.cc in ./b.so
```

In addition, update the `report_globals=2` message to include the module
name
```
==1451005==Added Global[0x7fcfe59ae040]: beg=0x7fcfe59ae140 size=8/32 name=var 
source=b.cc module=./b.so dyn_init=0 odr_indicator=0x55754f939260
```
---
 compiler-rt/lib/asan/asan_descriptions.cpp|  2 +-
 compiler-rt/lib/asan/asan_errors.cpp  |  4 ++--
 compiler-rt/lib/asan/asan_globals.cpp | 15 ++-
 compiler-rt/lib/asan/asan_report.h|  3 ++-
 .../test/asan/TestCases/Linux/odr-violation.cpp   |  2 +-
 .../test/asan/TestCases/Linux/odr_indicators.cpp  |  6 +++---
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_descriptions.cpp 
b/compiler-rt/lib/asan/asan_descriptions.cpp
index 5f3d15bcc9e2b0b..ef6f3e0a096f828 100644
--- a/compiler-rt/lib/asan/asan_descriptions.cpp
+++ b/compiler-rt/lib/asan/asan_descriptions.cpp
@@ -291,7 +291,7 @@ static void DescribeAddressRelativeToGlobal(uptr addr, uptr 
access_size,
   }
   str.AppendF(" global variable '%s' defined in '",
   MaybeDemangleGlobalName(g.name));
-  PrintGlobalLocation(&str, g);
+  PrintGlobalLocation(&str, g, /*print_module_name=*/false);
   str.AppendF("' (0x%zx) of size %zu\n", g.beg, g.size);
   str.Append(d.Default());
   PrintGlobalNameIfASCII(&str, g);
diff --git a/compiler-rt/lib/asan/asan_errors.cpp 
b/compiler-rt/lib/asan/asan_errors.cpp
index 49f5b38c3338f61..3f2d13e3146406d 100644
--- a/compiler-rt/lib/asan/asan_errors.cpp
+++ b/compiler-rt/lib/asan/asan_errors.cpp
@@ -362,8 +362,8 @@ void ErrorODRViolation::Print() {
   Printf("%s", d.Default());
   InternalScopedString g1_loc;
   InternalScopedString g2_loc;
-  PrintGlobalLocation(&g1_loc, global1);
-  PrintGlobalLocation(&g2_loc, global2);
+  PrintGlobalLocation(&g1_loc, global1, /*print_module_name=*/true);
+  PrintGlobalLocation(&g2_loc, global2, /*print_module_name=*/true);
   Printf("  [1] size=%zd '%s' %s\n", global1.size,
  MaybeDemangleGlobalName(global1.name), g1_loc.data());
   Printf("  [2] size=%zd '%s' %s\n", global2.size,
diff --git a/compiler-rt/lib/asan/asan_globals.cpp 
b/compiler-rt/lib/asan/asan_globals.cpp
index 14c36bdc8e64296..9c95dd58661bd97 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -80,14 +80,16 @@ static bool IsAddressNearGlobal(uptr addr, const 
__asan_global &g) {
 }
 
 static void ReportGlobal(const Global &g, const char *prefix) {
+  DataInfo info;
+  bool symbolized = Symbolizer::GetOrInit()->SymbolizeData(g.beg, &info);
   Report(
-  "%s Global[%p]: beg=%p size=%zu/%zu name=%s module=%s dyn_init=%zu "
+  "%s Global[%p]: beg=%p size=%zu/%zu name=%s source=%s module=%s "
+  "dyn_init=%zu "
   "odr_indicator=%p\n",
   prefix, (void *)&g, (void *)g.beg, g.size, g.size_with_redzone, g.name,
-  g.module_name, g.has_dynamic_init, (void *)g.odr_indicator);
+  g.module_name, info.module, g.has_dynamic_init, (void *)g.odr_indicator);
 
-  DataInfo info;
-  if (Symbolizer::GetOrInit()->SymbolizeData(g.beg, &info) && info.line != 0) {
+  if (symbolized && info.line != 0) {
 Report("  location: name=%s, %d\n", info.file, 
static_cast(info.line));
   } else if (g.gcc_location != 0) {
 // Fallback to Global::gcc_location
@@ -297,7 +299,8 @@ void PrintGlobalNameIfASCII(InternalScopedString *str, 
const __asan_global &g) {
(char *)g.beg);
 }
 
-void PrintGlobalLocation(InternalScopedString *str, const __asan_global &g) {
+void PrintGlobalLocation(InternalScopedString *str, const __asan_global &g,
+ bool print_module_name) {
   DataInfo info;
   if (Symbolizer::GetOrInit()->SymbolizeData(g.beg, &info) && info.line != 0) {
 str->AppendF("%s:%d", info.file, static_cast(info.line));
@@ -312,6 +315,8 @@ void PrintGlobalLocation(InternalScopedString *str, const 
__asan_global &g) {
   } else {
 str->AppendF("%s", g.module_name);
   }
+  if (

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread Greg Clayton via lldb-commits


clayborg wrote:

A great future patch would be to have StructuredData backed by the llvm::json 
stuff instead of our own custom version of this a JSON style object hiearchy

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Felipe de Azevedo Piovezan via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

felipepiovezan wrote:

@clayborg what do you think about moving ownership of the data to inside the 
AppleAcceleratorTable class?

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Felipe de Azevedo Piovezan via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Caslyn Tonelli via lldb-commits


Caslyn wrote:

Hi @hokein - ignored_artificial_fields.test breaks if gcc is unavailable. For 
ex we purposely exclude gcc on our downstream bots so as not to implicitly 
depend on it. Maybe all that is needed is adding `# REQUIRES: gcc`.

Would you be able to revert the test and/or patch it to only run when gcc is 
present?

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Greg Clayton via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

clayborg wrote:

Fine with me!

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Greg Clayton via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

clayborg wrote:

> These asserts might not always pass? I don't know if it's guaranteed that if 
> one of those sections is present, then the rest of them are as well.

You can check the size first and if non-zero then check the buffer:
```
lldbassert(apple_names.GetByteSize() == 0 || apple_names.GetSharedDataBuffer());
lldbassert(apple_namespaces.GetByteSize() == 0 || 
apple_namespaces.GetSharedDataBuffer());
lldbassert(apple_types.GetByteSize() == 0 || apple_types.GetSharedDataBuffer());
lldbassert(apple_objc.GetByteSize() == 0 || apple_objc.GetSharedDataBuffer());
```

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

[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)

2023-11-10 Thread Greg Clayton via lldb-commits


https://github.com/clayborg updated 
https://github.com/llvm/llvm-project/pull/71772

>From feea395f4ef165dfc057dfdc0649c6948895eeb3 Mon Sep 17 00:00:00 2001
From: Greg Clayton 
Date: Wed, 8 Nov 2023 21:14:49 -0800
Subject: [PATCH 1/2] Centralize the code that figures out which memory ranges
 to save into core files.

Prior to this patch, each core file plugin (ObjectFileMachO.cpp and 
ObjectFileMinindump.cpp) would calculate the address ranges to save in 
different ways. This patch adds a new function to Process.h/.cpp:

```
Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, 
CoreFileMemoryRanges &ranges);
```

The patch updates the ObjectFileMachO::SaveCore(...) and 
ObjectFileMinindump::SaveCore(...) to use same code. This will allow core files 
to be consistent with the lldb::SaveCoreStyle across different core file 
creators and will allow us to add new core file saving features that do more 
complex things in future patches.
---
 lldb/include/lldb/Target/MemoryRegionInfo.h   |   8 +-
 lldb/include/lldb/Target/Process.h|  46 -
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 137 +++--
 .../Minidump/MinidumpFileBuilder.cpp  |  37 +---
 .../ObjectFile/Minidump/MinidumpFileBuilder.h |   3 +-
 .../Minidump/ObjectFileMinidump.cpp   |   9 +-
 lldb/source/Target/Process.cpp| 189 +-
 lldb/source/Target/TraceDumper.cpp|   7 +-
 8 files changed, 267 insertions(+), 169 deletions(-)

diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index 47d4c9d6398728c..66a4b3ed1b42d5f 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -81,11 +81,11 @@ class MemoryRegionInfo {
   // lldb::Permissions
   uint32_t GetLLDBPermissions() const {
 uint32_t permissions = 0;
-if (m_read)
+if (m_read == eYes)
   permissions |= lldb::ePermissionsReadable;
-if (m_write)
+if (m_write == eYes)
   permissions |= lldb::ePermissionsWritable;
-if (m_execute)
+if (m_execute == eYes)
   permissions |= lldb::ePermissionsExecutable;
 return permissions;
   }
@@ -151,7 +151,7 @@ class MemoryRegionInfo {
   int m_pagesize = 0;
   std::optional> m_dirty_pages;
 };
-  
+
 inline bool operator<(const MemoryRegionInfo &lhs,
   const MemoryRegionInfo &rhs) {
   return lhs.GetRange() < rhs.GetRange();
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..08e3c60f7c324e6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -54,6 +54,7 @@
 #include "lldb/Utility/UserIDResolver.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/ADT/AddressRanges.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VersionTuple.h"
@@ -354,12 +355,10 @@ class Process : public 
std::enable_shared_from_this,
   };
   // This is all the event bits the public process broadcaster broadcasts.
   // The process shadow listener signs up for all these bits...
-  static constexpr int g_all_event_bits = eBroadcastBitStateChanged 
-| eBroadcastBitInterrupt
-| eBroadcastBitSTDOUT 
-| eBroadcastBitSTDERR
-| eBroadcastBitProfileData 
-| eBroadcastBitStructuredData;
+  static constexpr int g_all_event_bits =
+  eBroadcastBitStateChanged | eBroadcastBitInterrupt | eBroadcastBitSTDOUT 
|
+  eBroadcastBitSTDERR | eBroadcastBitProfileData |
+  eBroadcastBitStructuredData;
 
   enum {
 eBroadcastInternalStateControlStop = (1 << 0),
@@ -390,7 +389,7 @@ class Process : public 
std::enable_shared_from_this,
   ConstString &GetBroadcasterClass() const override {
 return GetStaticBroadcasterClass();
   }
-  
+
 /// A notification structure that can be used by clients to listen
 /// for changes in a process's lifetime.
 ///
@@ -579,7 +578,7 @@ class Process : public 
std::enable_shared_from_this,
   /// of CommandObject like CommandObjectRaw, CommandObjectParsed,
   /// or CommandObjectMultiword.
   virtual CommandObject *GetPluginCommandObject() { return nullptr; }
-  
+
   /// The underlying plugin might store the low-level communication history for
   /// this session.  Dump it into the provided stream.
   virtual void DumpPluginHistory(Stream &s) { return; }
@@ -614,7 +613,7 @@ class Process : public 
std::enable_shared_from_this,
 return error;
   }
 
-  /// The "ShadowListener" for a process is just an ordinary Listener that 
+  /// The "ShadowListener" for a process is just an ordinary Listener that
   /// listens for all the Process event bits.  It's convenient because you can
   /// specify it in the LaunchInfo or AttachInfo, so it will get events fr

[Lldb-commits] [lldb] 81a7690 - [lldb] Only run ignored_artificial_fields.test when gcc is available.

2023-11-10 Thread Haojian Wu via lldb-commits


Author: Haojian Wu
Date: 2023-11-10T20:56:27+01:00
New Revision: 81a76902ae0fc08138e37212239c5c704eec2f26

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

LOG: [lldb] Only run ignored_artificial_fields.test when gcc is available.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test 
b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
index e7d3bc4b796224a..21af535368ee943 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
+++ b/lldb/test/Shell/SymbolFile/DWARF/ignored_artificial_fields.test
@@ -1,4 +1,5 @@
 # UNSUPPORTED: system-darwin, system-windows
+# REQUIRES: gcc
 
 # Make sure the artifical field `vptr.ClassName` from gcc debug info is 
ignored.
 # RUN: %build --compiler=gcc %S/Inputs/debug-types-expressions.cpp -o %t



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

[Lldb-commits] [lldb] [LLDB] Ignore actual-needed artificial members in DWARFASTParserClang::ParseSingleMember (PR #70779)

2023-11-10 Thread Haojian Wu via lldb-commits


hokein wrote:

@Caslyn , I added the `# REQUIRES: gcc` to the test in 
https://github.com/llvm/llvm-project/commit/81a76902ae0fc08138e37212239c5c704eec2f26

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

[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)

2023-11-10 Thread via lldb-commits


github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
d0da3d8393939790bb1a6b3b5a36daeeef000c9b..50e81f16a6c09d19af53e64818d3b67ebb8dbfd0
 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
``





View the diff from darker here.


``diff
--- TestProcessSaveCoreMinidump.py  2023-11-10 19:53:18.00 +
+++ TestProcessSaveCoreMinidump.py  2023-11-10 20:06:23.404845 +
@@ -9,13 +9,13 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class ProcessSaveCoreMinidumpTestCase(TestBase):
-
-def verify_core_file(self, core_path, expected_pid, expected_modules,
- expected_threads):
+def verify_core_file(
+self, core_path, expected_pid, expected_modules, expected_threads
+):
 # To verify, we'll launch with the mini dump
 target = self.dbg.CreateTarget(None)
 process = target.LoadCore(core_path)
 
 # check if the core is in desired state
@@ -75,56 +75,59 @@
 thread_id = thread.GetThreadID()
 expected_threads.append(thread_id)
 
 # save core and, kill process and verify corefile existence
 self.runCmd(
-"process save-core --plugin-name=minidump --style=stack '%s'" 
% (core_stack)
+"process save-core --plugin-name=minidump --style=stack '%s'"
+% (core_stack)
 )
 self.assertTrue(os.path.isfile(core_stack))
-self.verify_core_file(core_stack, expected_pid, expected_modules,
-  expected_threads)
+self.verify_core_file(
+core_stack, expected_pid, expected_modules, expected_threads
+)
 
 self.runCmd(
-"process save-core --plugin-name=minidump 
--style=modified-memory '%s'" (core_dirty)
+"process save-core --plugin-name=minidump 
--style=modified-memory '%s'"(
+core_dirty
+)
 )
 self.assertTrue(os.path.isfile(core_dirty))
-self.verify_core_file(core_dirty, expected_pid, expected_modules,
-  expected_threads)
-
+self.verify_core_file(
+core_dirty, expected_pid, expected_modules, expected_threads
+)
 
 self.runCmd(
-"process save-core --plugin-name=minidump --style=full '%s'" 
(core_full)
+"process save-core --plugin-name=minidump --style=full 
'%s'"(core_full)
 )
 self.assertTrue(os.path.isfile(core_full))
-self.verify_core_file(core_full, expected_pid, expected_modules,
-  expected_threads)
-
+self.verify_core_file(
+core_full, expected_pid, expected_modules, expected_threads
+)
 
 # validate saving via SBProcess
-error = process.SaveCore(core_sb_stack, "minidump",
- lldb.eSaveCoreStackOnly)
+error = process.SaveCore(core_sb_stack, "minidump", 
lldb.eSaveCoreStackOnly)
 self.assertTrue(error.Success())
 self.assertTrue(os.path.isfile(core_sb_stack))
-self.verify_core_file(core_sb_stack, expected_pid, 
expected_modules,
-  expected_threads)
+self.verify_core_file(
+core_sb_stack, expected_pid, expected_modules, expected_threads
+)
 
-
-error = process.SaveCore(core_sb_dirty, "minidump",
- lldb.eSaveCoreDirtyOnly)
+error = process.SaveCore(core_sb_dirty, "minidump", 
lldb.eSaveCoreDirtyOnly)
 self.assertTrue(error.Success())
 self.assertTrue(os.path.isfile(core_sb_dirty))
-self.verify_core_file(core_sb_dirty, expected_pid, 
expected_modules,
-  expected_threads)
+self.verify_core_file(
+core_sb_dirty, expected_pid, expected_modules, expected_threads
+)
 
 # Minidump can now save full core files, but they will be huge and
 # they might cause this test to timeout.
-error = process.SaveCore(core_sb_full, "minidump",
- lldb.eSaveCoreFull)
+error = process.SaveCore(core_sb_full, "minidump", 
lldb.eSaveCoreFull)
 self.assertTrue(error.Success())
 self.assertTrue(os.path.isfile(core_sb_full))
-self.verify_core_file(core_sb_full, expected_pid, expected_modules,
-  expected_threads)
+self.verify_core_file(
+core_sb_full, expected_pid, expected_modules,

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread Alex Langford via lldb-commits

bulbazord wrote:

> A great future patch would be to have StructuredData backed by the llvm::json 
> stuff instead of our own custom version of this a JSON style object hiearchy

I think that I agree. Unfortunately I don't have time to drive that right now 
and it might appear to not make sense why I'm doing these interface changes now 
if I think we should refactor this class more thoroughly in the future. That 
being said, part of my goal in refactoring these interfaces is to force folks 
to think about error handling more explicitly. The other goal is so I can do 
other refactors in a safer/nicer way. 🙂 

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

[Lldb-commits] [lldb] 133bcac - [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (#71961)

2023-11-10 Thread via lldb-commits


Author: Alex Langford
Date: 2023-11-10T12:47:43-08:00
New Revision: 133bcacecfb70e8b1692f9c2c0a44ec640a0422a

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

LOG: [lldb] Change interface of 
StructuredData::Array::GetItemAtIndexAsDictionary (#71961)

Similar to my previous patch (#71613) where I changed
`GetItemAtIndexAsString`, this patch makes the same change to
`GetItemAtIndexAsDictionary`.

`GetItemAtIndexAsDictionary` now returns a std::optional that is either
`std::nullopt` or is a valid pointer. Therefore, if the optional is
populated, we consider the pointer to always be valid (i.e. no need to
check pointer validity).

Added: 


Modified: 
lldb/include/lldb/Utility/StructuredData.h
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/source/Target/DynamicRegisterInfo.cpp
lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 6e39bcff2c0af0b..8d0ae372f43c6bf 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -256,14 +256,24 @@ class StructuredData {
   return {};
 }
 
-bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *&result) const {
-  result = nullptr;
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
-result = value_sp->GetAsDictionary();
-return (result != nullptr);
+/// Retrieves the element at index \a idx from a StructuredData::Array if 
it
+/// is a Dictionary.
+///
+/// \param[in] idx
+///   The index of the element to retrieve.
+///
+/// \return
+///   If the element at index \a idx is a Dictionary, this method returns a
+///   valid pointer to the Dictionary wrapped in a std::optional. If the
+///   element is not a Dictionary or the index is invalid, this returns
+///   std::nullopt. Note that the underlying Dictionary pointer is never
+///   nullptr.
+std::optional GetItemAtIndexAsDictionary(size_t idx) const {
+  if (auto item_sp = GetItemAtIndex(idx)) {
+if (auto *dict = item_sp->GetAsDictionary())
+  return dict;
   }
-  return false;
+  return {};
 }
 
 bool GetItemAtIndexAsArray(size_t idx, Array *&result) const {

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3714e37ec5c57d0..1ea4f649427727f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5688,14 +5688,16 @@ bool 
ObjectFileMachO::GetCorefileThreadExtraInfos(std::vector &tids) {
   }
   const size_t num_threads = threads->GetSize();
   for (size_t i = 0; i < num_threads; i++) {
-StructuredData::Dictionary *thread;
-if (!threads->GetItemAtIndexAsDictionary(i, thread) || !thread) {
+std::optional maybe_thread =
+threads->GetItemAtIndexAsDictionary(i);
+if (!maybe_thread) {
   LLDB_LOGF(log,
 "Unable to read 'process metadata' LC_NOTE, threads "
 "array does not have a dictionary at index %zu.",
 i);
   return false;
 }
+StructuredData::Dictionary *thread = *maybe_thread;
 tid_t tid = LLDB_INVALID_THREAD_ID;
 if (thread->GetValueForKeyAsInteger("thread_id", tid))
   if (tid == 0)

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 04d98b96acd6839..2cf8c29bf9d2fe9 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2649,10 +2649,12 @@ size_t GDBRemoteCommunicationClient::QueryGDBServer(
 return 0;
 
   for (size_t i = 0, count = array->GetSize(); i < count; ++i) {
-StructuredData::Dictionary *element = nullptr;
-if (!array->GetItemAtIndexAsDictionary(i, element))
+std::optional maybe_element =
+array->GetItemAtIndexAsDictionary(i);
+if (!maybe_element)
   continue;
 
+StructuredData::Dictionary *element = *maybe_element;
 uint16_t port = 0;
 if (StructuredData::ObjectSP port_osp =
 element->GetValueForKey(llvm::StringRef("port")))

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index aa2796db15cd00a..88

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary (PR #71961)

2023-11-10 Thread Alex Langford via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Display artificial promise and coro_frame variables. (PR #71928)

2023-11-10 Thread Jonas Devlieghere via lldb-commits



@@ -41,7 +41,10 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  return name == g_this ||
+ // Artificial coroutine-related variables emitted by clang.
+ name == ConstString("__promise") ||
+ name == ConstString("__coro_frame");

JDevlieghere wrote:

These should be static constants too, like `g_this`. Otherwise we lose the 
(little) benefit of these being `ConstStrings`. 

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

[Lldb-commits] [lldb] [llvm] Emit DIE's size in bits when size is not a multiple of 8 (PR #69741)

2023-11-10 Thread Michael Buch via lldb-commits



@@ -2866,8 +2879,12 @@ void DWARFASTParserClang::ParseSingleMember(
   // Get the parent byte size so we can verify any members will fit
   const uint64_t parent_byte_size =
   parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
-  const uint64_t parent_bit_size =
-  parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
+  uint64_t parent_bit_size;
+  if (parent_byte_size != UINT64_MAX)
+parent_bit_size = parent_byte_size * 8;
+  else
+parent_bit_size =
+parent_die.GetAttributeValueAsUnsigned(DW_AT_bit_size, UINT64_MAX);

Michael137 wrote:

DWARFASTParserClang has unittests where we sometimes use Yaml2Obj and directly 
invoke the parser. But it's quite finicky. Let me know if you run into troubles 
with it

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

[Lldb-commits] [clang-tools-extra] [libcxx] [lldb] [flang] [lld] [llvm] [libc] [clang] [compiler-rt] [asan] Report executable/DSO name for report_globals=2 and odr-violation checking (PR #71879)

2023-11-10 Thread Fangrui Song via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (PR #71984)

2023-11-10 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/71984

This fixes a subtle and previously harmless off-by-one bug in 
ParseSupportFilesFromPrologue. The function accounts for the start index being 
one-based for DWARF v4 and earlier and zero-based for DWARF v5 and later. 
However, the same care wasn't taken for the end index.

This bug existed unnoticed because GetFileByIndex gracefully handles an invalid 
index. However, the bug manifested itself after #71458, which added a call to 
getFileNameEntry which requires the index to be valid.

No test as the bug cannot be observed without the changes from #71458. One that 
PR is merged, this will be covered by all the DWARF v5 tests.

>From c6e6d30da2e67b2de53211fd70a7077a7dab62ef Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 10 Nov 2023 12:44:18 -0800
Subject: [PATCH] [lldb] Fix a off-by-one error in
 ParseSupportFilesFromPrologue

This fixes a subtle and previously harmless off-by-one bug in
ParseSupportFilesFromPrologue. The function accounts for the start index
being one-based for DWARF v4 and earlier and zero-based for DWARF v5 and
later. However, the same care wasn't taken for the end index.

This bug existed unnoticed because GetFileByIndex gracefully handles an
invalid index. However, the bug manifested itself after #71458, which
added a call to getFileNameEntry which requires the index to be valid.

No test as the bug cannot be observed without the changes from #71458.
One that PR is merged, this will be covered by all the DWARF v5 tests.
---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 25 ---
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index aabd83a194932ff..67ea17b2e1fb1bd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -210,17 +210,24 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP 
&module,
   FileSpec::Style style,
   llvm::StringRef compile_dir = {}) {
   FileSpecList support_files;
-  size_t first_file = 0;
-  if (prologue.getVersion() <= 4) {
-// File index 0 is not valid before DWARF v5. Add a dummy entry to ensure
-// support file list indices match those we get from the debug info and 
line
-// tables.
+
+  // Handle the case where there are no files first to avoid having to special
+  // case this later.
+  if (prologue.FileNames.empty())
+return support_files;
+
+  // Before DWARF v5, the line table indexes were one based.
+  const bool is_one_based = prologue.getVersion() < 5;
+  const size_t file_names = prologue.FileNames.size();
+  const size_t first_file_idx = is_one_based ? 1 : 0;
+  const size_t last_file_idx = is_one_based ? file_names : file_names - 1;
+
+  // Add a dummy entry to ensure the support file list indices match those we
+  // get from the debug info and line tables.
+  if (is_one_based)
 support_files.Append(FileSpec());
-first_file = 1;
-  }
 
-  const size_t number_of_files = prologue.FileNames.size();
-  for (size_t idx = first_file; idx <= number_of_files; ++idx) {
+  for (size_t idx = first_file_idx; idx <= last_file_idx; ++idx) {
 std::string remapped_file;
 if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) {
   if (auto remapped = module->RemapSourceFile(llvm::StringRef(*file_path)))

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

[Lldb-commits] [lldb] [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (PR #71984)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

This fixes a subtle and previously harmless off-by-one bug in 
ParseSupportFilesFromPrologue. The function accounts for the start index being 
one-based for DWARF v4 and earlier and zero-based for DWARF v5 and later. 
However, the same care wasn't taken for the end index.

This bug existed unnoticed because GetFileByIndex gracefully handles an invalid 
index. However, the bug manifested itself after #71458, which added a 
call to getFileNameEntry which requires the index to be valid.

No test as the bug cannot be observed without the changes from #71458. 
One that PR is merged, this will be covered by all the DWARF v5 tests.

---
Full diff: https://github.com/llvm/llvm-project/pull/71984.diff


1 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+16-9) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index aabd83a194932ff..67ea17b2e1fb1bd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -210,17 +210,24 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP 
&module,
   FileSpec::Style style,
   llvm::StringRef compile_dir = {}) {
   FileSpecList support_files;
-  size_t first_file = 0;
-  if (prologue.getVersion() <= 4) {
-// File index 0 is not valid before DWARF v5. Add a dummy entry to ensure
-// support file list indices match those we get from the debug info and 
line
-// tables.
+
+  // Handle the case where there are no files first to avoid having to special
+  // case this later.
+  if (prologue.FileNames.empty())
+return support_files;
+
+  // Before DWARF v5, the line table indexes were one based.
+  const bool is_one_based = prologue.getVersion() < 5;
+  const size_t file_names = prologue.FileNames.size();
+  const size_t first_file_idx = is_one_based ? 1 : 0;
+  const size_t last_file_idx = is_one_based ? file_names : file_names - 1;
+
+  // Add a dummy entry to ensure the support file list indices match those we
+  // get from the debug info and line tables.
+  if (is_one_based)
 support_files.Append(FileSpec());
-first_file = 1;
-  }
 
-  const size_t number_of_files = prologue.FileNames.size();
-  for (size_t idx = first_file; idx <= number_of_files; ++idx) {
+  for (size_t idx = first_file_idx; idx <= last_file_idx; ++idx) {
 std::string remapped_file;
 if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) {
   if (auto remapped = module->RemapSourceFile(llvm::StringRef(*file_path)))

``




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

[Lldb-commits] [lldb] [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (PR #71984)

2023-11-10 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo updated 
https://github.com/llvm/llvm-project/pull/71828

>From 48ba23b4c0ef9c698b26d06e5b8b337a774563b8 Mon Sep 17 00:00:00 2001
From: walter erquinigo 
Date: Thu, 9 Nov 2023 11:40:23 -0500
Subject: [PATCH] [LLDB] Ensure the data of apple accelerator tables is kept
 around

lldb's AppleDWARFIndex is created with a few `llvm::AppleAcceleratorTable` 
variables, but they only hold pointers to the underlying data, which doesn't 
prevent the data owner (lldb's DataBufferHeap) from being disposed.

Because of this, an asan build of lldb was showing an error in which a jitted 
accelerator table was being fred before read, which made my lldb fall in an 
infinite loop trying to parse corrupted accel table data. This issue only 
happens when I'm using a jitted execution and not when debugging a precompiled 
binary, probably because in the jit case the underlying data has to necessarily 
be copied via gdb-remote instead of mmaping a debug info file.

The correct fix seems to also store the underlying storage along with the 
accelerator tables in AppleDWARFIndex.
---
 .../SymbolFile/DWARF/AppleDWARFIndex.cpp  | 11 +-
 .../SymbolFile/DWARF/AppleDWARFIndex.h| 20 +--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 325517ca1d2499b..cf96938f1fc6473 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -51,13 +51,22 @@ std::unique_ptr AppleDWARFIndex::Create(
   extract_and_check(apple_namespaces_table_up);
   extract_and_check(apple_types_table_up);
   extract_and_check(apple_objc_table_up);
+  lldbassert(apple_names.GetByteSize() == 0 ||
+ apple_names.GetSharedDataBuffer());
+  lldbassert(apple_namespaces.GetByteSize() == 0 ||
+ apple_namespaces.GetSharedDataBuffer());
+  lldbassert(apple_types.GetByteSize() == 0 ||
+ apple_types.GetSharedDataBuffer());
+  lldbassert(apple_objc.GetByteSize() == 0 || 
apple_objc.GetSharedDataBuffer());
 
   if (apple_names_table_up || apple_namespaces_table_up ||
   apple_types_table_up || apple_objc_table_up)
 return std::make_unique(
 module, std::move(apple_names_table_up),
 std::move(apple_namespaces_table_up), std::move(apple_types_table_up),
-std::move(apple_objc_table_up));
+std::move(apple_objc_table_up), apple_names.GetSharedDataBuffer(),
+apple_namespaces.GetSharedDataBuffer(),
+apple_types.GetSharedDataBuffer(), apple_objc.GetSharedDataBuffer());
 
   return nullptr;
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
index a1fb99700d10a21..73de75b583bd419 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -25,11 +25,19 @@ class AppleDWARFIndex : public DWARFIndex {
   std::unique_ptr apple_names,
   std::unique_ptr 
apple_namespaces,
   std::unique_ptr apple_types,
-  std::unique_ptr apple_objc)
+  std::unique_ptr apple_objc,
+  lldb::DataBufferSP apple_names_storage,
+  lldb::DataBufferSP apple_namespaces_storage,
+  lldb::DataBufferSP apple_types_storage,
+  lldb::DataBufferSP apple_objc_storage)
   : DWARFIndex(module), m_apple_names_up(std::move(apple_names)),
 m_apple_namespaces_up(std::move(apple_namespaces)),
 m_apple_types_up(std::move(apple_types)),
-m_apple_objc_up(std::move(apple_objc)) {}
+m_apple_objc_up(std::move(apple_objc)),
+m_apple_names_storage(apple_names_storage),
+m_apple_namespaces_storage(apple_namespaces_storage),
+m_apple_types_storage(apple_types_storage),
+m_apple_objc_storage(apple_objc_storage) {}
 
   void Preload() override {}
 
@@ -67,6 +75,14 @@ class AppleDWARFIndex : public DWARFIndex {
   std::unique_ptr m_apple_namespaces_up;
   std::unique_ptr m_apple_types_up;
   std::unique_ptr m_apple_objc_up;
+  /// The following storage variables hold the data that the apple accelerator
+  /// tables tables above point to.
+  /// {
+  lldb::DataBufferSP m_apple_names_storage;
+  lldb::DataBufferSP m_apple_namespaces_storage;
+  lldb::DataBufferSP m_apple_types_storage;
+  lldb::DataBufferSP m_apple_objc_storage;
+  /// }
 
   /// Search for entries whose name is `name` in `table`, calling `callback` 
for
   /// each match. If `search_for_tag` is provided, ignore entries whose tag is

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Jonas Devlieghere via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

JDevlieghere wrote:

Without expressing an opinion, the [contributing 
guidelines](https://lldb.llvm.org/resources/contributing.html) say not to add 
new `lldbasserts`. This is currently being discussed on 
[Discourse](https://discourse.llvm.org/t/rfc-error-handling-in-release-builds-aka-can-i-use-lldbassert-or-not/74738/).
 

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Walter Erquinigo via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

walter-erquinigo wrote:

@JDevlieghere , what do you suggest in this case? Plain assert or better not to 
put anything? I'd rather follow the guidelines

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Jonas Devlieghere via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

JDevlieghere wrote:

This checks internal consistency so an `assert` is totally appropriate imho. 

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

[Lldb-commits] [lldb] [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (PR #71984)

2023-11-10 Thread Felipe de Azevedo Piovezan via lldb-commits


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

LGTM! really like the `_idx` suffix of the new induction variable and its bounds

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo updated 
https://github.com/llvm/llvm-project/pull/71828

>From b7fadbbe01d626fbffe2523c601b2e7ea727b591 Mon Sep 17 00:00:00 2001
From: walter erquinigo 
Date: Thu, 9 Nov 2023 11:40:23 -0500
Subject: [PATCH] [LLDB] Ensure the data of apple accelerator tables is kept
 around

lldb's AppleDWARFIndex is created with a few `llvm::AppleAcceleratorTable` 
variables, but they only hold pointers to the underlying data, which doesn't 
prevent the data owner (lldb's DataBufferHeap) from being disposed.

Because of this, an asan build of lldb was showing an error in which a jitted 
accelerator table was being fred before read, which made my lldb fall in an 
infinite loop trying to parse corrupted accel table data. This issue only 
happens when I'm using a jitted execution and not when debugging a precompiled 
binary, probably because in the jit case the underlying data has to necessarily 
be copied via gdb-remote instead of mmaping a debug info file.

The correct fix seems to also store the underlying storage along with the 
accelerator tables in AppleDWARFIndex.
---
 .../SymbolFile/DWARF/AppleDWARFIndex.cpp  |  9 -
 .../SymbolFile/DWARF/AppleDWARFIndex.h| 20 +--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 325517ca1d2499b..33537df4f507624 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -51,13 +51,20 @@ std::unique_ptr AppleDWARFIndex::Create(
   extract_and_check(apple_namespaces_table_up);
   extract_and_check(apple_types_table_up);
   extract_and_check(apple_objc_table_up);
+  assert(apple_names.GetByteSize() == 0 || apple_names.GetSharedDataBuffer());
+  assert(apple_namespaces.GetByteSize() == 0 ||
+ apple_namespaces.GetSharedDataBuffer());
+  assert(apple_types.GetByteSize() == 0 || apple_types.GetSharedDataBuffer());
+  assert(apple_objc.GetByteSize() == 0 || apple_objc.GetSharedDataBuffer());
 
   if (apple_names_table_up || apple_namespaces_table_up ||
   apple_types_table_up || apple_objc_table_up)
 return std::make_unique(
 module, std::move(apple_names_table_up),
 std::move(apple_namespaces_table_up), std::move(apple_types_table_up),
-std::move(apple_objc_table_up));
+std::move(apple_objc_table_up), apple_names.GetSharedDataBuffer(),
+apple_namespaces.GetSharedDataBuffer(),
+apple_types.GetSharedDataBuffer(), apple_objc.GetSharedDataBuffer());
 
   return nullptr;
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
index a1fb99700d10a21..73de75b583bd419 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -25,11 +25,19 @@ class AppleDWARFIndex : public DWARFIndex {
   std::unique_ptr apple_names,
   std::unique_ptr 
apple_namespaces,
   std::unique_ptr apple_types,
-  std::unique_ptr apple_objc)
+  std::unique_ptr apple_objc,
+  lldb::DataBufferSP apple_names_storage,
+  lldb::DataBufferSP apple_namespaces_storage,
+  lldb::DataBufferSP apple_types_storage,
+  lldb::DataBufferSP apple_objc_storage)
   : DWARFIndex(module), m_apple_names_up(std::move(apple_names)),
 m_apple_namespaces_up(std::move(apple_namespaces)),
 m_apple_types_up(std::move(apple_types)),
-m_apple_objc_up(std::move(apple_objc)) {}
+m_apple_objc_up(std::move(apple_objc)),
+m_apple_names_storage(apple_names_storage),
+m_apple_namespaces_storage(apple_namespaces_storage),
+m_apple_types_storage(apple_types_storage),
+m_apple_objc_storage(apple_objc_storage) {}
 
   void Preload() override {}
 
@@ -67,6 +75,14 @@ class AppleDWARFIndex : public DWARFIndex {
   std::unique_ptr m_apple_namespaces_up;
   std::unique_ptr m_apple_types_up;
   std::unique_ptr m_apple_objc_up;
+  /// The following storage variables hold the data that the apple accelerator
+  /// tables tables above point to.
+  /// {
+  lldb::DataBufferSP m_apple_names_storage;
+  lldb::DataBufferSP m_apple_namespaces_storage;
+  lldb::DataBufferSP m_apple_types_storage;
+  lldb::DataBufferSP m_apple_objc_storage;
+  /// }
 
   /// Search for entries whose name is `name` in `table`, calling `callback` 
for
   /// each match. If `search_for_tag` is provided, ignore entries whose tag is

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Walter Erquinigo via lldb-commits



@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 return std::make_unique(

walter-erquinigo wrote:

Sure, I've just changed it to assert

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

[Lldb-commits] [openmp] [clang-tools-extra] [libcxx] [libc] [mlir] [lldb] [llvm] [flang] [clang] GlobalISel: Guide return in llvm::getIConstantSplatVal (PR #71989)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-llvm-globalisel

Author: Changpeng Fang (changpeng)


Changes

getIConstantVRegValWithLookThrough could return NULL.

---
Full diff: https://github.com/llvm/llvm-project/pull/71989.diff


1 Files Affected:

- (modified) llvm/lib/CodeGen/GlobalISel/Utils.cpp (+3-3) 


``diff
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp 
b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 473c3f452f8b1d9..eaf829f562b2dc9 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -1116,9 +1116,9 @@ std::optional
 llvm::getIConstantSplatVal(const Register Reg, const MachineRegisterInfo &MRI) 
{
   if (auto SplatValAndReg =
   getAnyConstantSplat(Reg, MRI, /* AllowUndef */ false)) {
-std::optional ValAndVReg =
-getIConstantVRegValWithLookThrough(SplatValAndReg->VReg, MRI);
-return ValAndVReg->Value;
+if (std::optional ValAndVReg =
+getIConstantVRegValWithLookThrough(SplatValAndReg->VReg, MRI))
+  return ValAndVReg->Value;
   }
 
   return std::nullopt;

``




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

[Lldb-commits] [lldb] fa7e07e - [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (#71984)

2023-11-10 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2023-11-10T14:03:29-08:00
New Revision: fa7e07ed9995637d84f40b3d29e0449154e73dd5

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

LOG: [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (#71984)

This fixes a subtle and previously harmless off-by-one bug in
ParseSupportFilesFromPrologue. The function accounts for the start index
being one-based for DWARF v4 and earlier and zero-based for DWARF v5 and
later. However, the same care wasn't taken for the end index.

This bug existed unnoticed because GetFileByIndex gracefully handles an
invalid index. However, the bug manifested itself after #71458, which
added a call to getFileNameEntry which requires the index to be valid.

No test as the bug cannot be observed without the changes from #71458.
Once that PR is merged, this will be covered by all the DWARF v5 tests.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index aabd83a194932ff..67ea17b2e1fb1bd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -210,17 +210,24 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP 
&module,
   FileSpec::Style style,
   llvm::StringRef compile_dir = {}) {
   FileSpecList support_files;
-  size_t first_file = 0;
-  if (prologue.getVersion() <= 4) {
-// File index 0 is not valid before DWARF v5. Add a dummy entry to ensure
-// support file list indices match those we get from the debug info and 
line
-// tables.
+
+  // Handle the case where there are no files first to avoid having to special
+  // case this later.
+  if (prologue.FileNames.empty())
+return support_files;
+
+  // Before DWARF v5, the line table indexes were one based.
+  const bool is_one_based = prologue.getVersion() < 5;
+  const size_t file_names = prologue.FileNames.size();
+  const size_t first_file_idx = is_one_based ? 1 : 0;
+  const size_t last_file_idx = is_one_based ? file_names : file_names - 1;
+
+  // Add a dummy entry to ensure the support file list indices match those we
+  // get from the debug info and line tables.
+  if (is_one_based)
 support_files.Append(FileSpec());
-first_file = 1;
-  }
 
-  const size_t number_of_files = prologue.FileNames.size();
-  for (size_t idx = first_file; idx <= number_of_files; ++idx) {
+  for (size_t idx = first_file_idx; idx <= last_file_idx; ++idx) {
 std::string remapped_file;
 if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) {
   if (auto remapped = module->RemapSourceFile(llvm::StringRef(*file_path)))



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

[Lldb-commits] [lldb] [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (PR #71984)

2023-11-10 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] [mlir] [flang] [libc] [openmp] [llvm] [clang] [libcxx] [clang-tools-extra] GlobalISel: Guide return in llvm::getIConstantSplatVal (PR #71989)

2023-11-10 Thread via lldb-commits


github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 0448a1c0dc131835301a59db7138d504b836a591 
23d34569183f405fdae0a8a665be8b7772addebe -- 
llvm/lib/CodeGen/GlobalISel/Utils.cpp
``





View the diff from clang-format here.


``diff
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp 
b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index eaf829f562b2..e4a69a1600c5 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -1117,7 +1117,7 @@ llvm::getIConstantSplatVal(const Register Reg, const 
MachineRegisterInfo &MRI) {
   if (auto SplatValAndReg =
   getAnyConstantSplat(Reg, MRI, /* AllowUndef */ false)) {
 if (std::optional ValAndVReg =
-getIConstantVRegValWithLookThrough(SplatValAndReg->VReg, MRI))
+getIConstantVRegValWithLookThrough(SplatValAndReg->VReg, MRI))
   return ValAndVReg->Value;
   }
 

``




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

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsInteger (PR #71993)

2023-11-10 Thread Alex Langford via lldb-commits


https://github.com/bulbazord created 
https://github.com/llvm/llvm-project/pull/71993

This is a follow-up to (#71613) and (#71961).

>From c6b3f622c32634db628cb76eaca556ae3d8ad6f4 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Fri, 10 Nov 2023 14:06:48 -0800
Subject: [PATCH] [lldb] Change interface of
 StructuredData::Array::GetItemAtIndexAsInteger

This is a follow-up to (#71613) and (#71961).
---
 lldb/include/lldb/Utility/StructuredData.h| 28 +--
 .../Breakpoint/BreakpointResolverName.cpp |  8 +++---
 .../TSan/InstrumentationRuntimeTSan.cpp   |  5 ++--
 lldb/source/Target/DynamicRegisterInfo.cpp| 15 +-
 4 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 8d0ae372f43c6bf..35fcfca8dd98c67 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -221,31 +221,17 @@ class StructuredData {
 }
 
 template 
-bool GetItemAtIndexAsInteger(size_t idx, IntType &result) const {
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
+std::optional GetItemAtIndexAsInteger(size_t idx) const {
+  if (auto item_sp = GetItemAtIndex(idx)) {
 if constexpr (std::numeric_limits::is_signed) {
-  if (auto signed_value = value_sp->GetAsSignedInteger()) {
-result = static_cast(signed_value->GetValue());
-return true;
-  }
+  if (auto *signed_value = item_sp->GetAsSignedInteger())
+return static_cast(signed_value->GetValue());
 } else {
-  if (auto unsigned_value = value_sp->GetAsUnsignedInteger()) {
-result = static_cast(unsigned_value->GetValue());
-return true;
-  }
+  if (auto *unsigned_value = item_sp->GetAsUnsignedInteger())
+return static_cast(unsigned_value->GetValue());
 }
   }
-  return false;
-}
-
-template 
-bool GetItemAtIndexAsInteger(size_t idx, IntType &result,
- IntType default_val) const {
-  bool success = GetItemAtIndexAsInteger(idx, result);
-  if (!success)
-result = default_val;
-  return success;
+  return {};
 }
 
 std::optional GetItemAtIndexAsString(size_t idx) const {
diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp 
b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 0097046cf511b5d..07ff597e10a5ca0 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -161,14 +161,14 @@ BreakpointResolverSP 
BreakpointResolverName::CreateFromStructuredData(
 error.SetErrorString("BRN::CFSD: name entry is not a string.");
 return nullptr;
   }
-  std::underlying_type::type fnt;
-  success = names_mask_array->GetItemAtIndexAsInteger(i, fnt);
-  if (!success) {
+  auto maybe_fnt = names_mask_array->GetItemAtIndexAsInteger<
+  std::underlying_type::type>(i);
+  if (!maybe_fnt) {
 error.SetErrorString("BRN::CFSD: name mask entry is not an integer.");
 return nullptr;
   }
   names.push_back(std::string(*maybe_name));
-  name_masks.push_back(static_cast(fnt));
+  name_masks.push_back(static_cast(*maybe_fnt));
 }
 
 std::shared_ptr resolver_sp =
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
index 2a35256a6fb0bf3..72293c5331f40da 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -592,9 +592,10 @@ addr_t 
InstrumentationRuntimeTSan::GetFirstNonInternalFramePc(
 if (skip_one_frame && i == 0)
   continue;
 
-addr_t addr;
-if (!trace_array->GetItemAtIndexAsInteger(i, addr))
+auto maybe_addr = trace_array->GetItemAtIndexAsInteger(i);
+if (!maybe_addr)
   continue;
+addr_t addr = *maybe_addr;
 
 lldb_private::Address so_addr;
 if (!process_sp->GetTarget().GetSectionLoadList().ResolveLoadAddress(
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp 
b/lldb/source/Target/DynamicRegisterInfo.cpp
index 7469c1d4259afc2..1a817449fa95896 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -349,10 +349,8 @@ DynamicRegisterInfo::SetRegisterInfo(const 
StructuredData::Dictionary &dict,
   const size_t num_regs = invalidate_reg_list->GetSize();
   if (num_regs > 0) {
 for (uint32_t idx = 0; idx < num_regs; ++idx) {
-  uint64_t invalidate_reg_num;
-  std::optional maybe_invalidate_reg_name =
-  invalidate_reg_list->GetItemAtIndexAsString(idx);
-  if (maybe_invalidate_reg_name) {
+  if (auto

[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsInteger (PR #71993)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)


Changes

This is a follow-up to (#71613) and (#71961).

---
Full diff: https://github.com/llvm/llvm-project/pull/71993.diff


4 Files Affected:

- (modified) lldb/include/lldb/Utility/StructuredData.h (+7-21) 
- (modified) lldb/source/Breakpoint/BreakpointResolverName.cpp (+4-4) 
- (modified) 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp 
(+3-2) 
- (modified) lldb/source/Target/DynamicRegisterInfo.cpp (+7-8) 


``diff
diff --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 8d0ae372f43c6bf..35fcfca8dd98c67 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -221,31 +221,17 @@ class StructuredData {
 }
 
 template 
-bool GetItemAtIndexAsInteger(size_t idx, IntType &result) const {
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
+std::optional GetItemAtIndexAsInteger(size_t idx) const {
+  if (auto item_sp = GetItemAtIndex(idx)) {
 if constexpr (std::numeric_limits::is_signed) {
-  if (auto signed_value = value_sp->GetAsSignedInteger()) {
-result = static_cast(signed_value->GetValue());
-return true;
-  }
+  if (auto *signed_value = item_sp->GetAsSignedInteger())
+return static_cast(signed_value->GetValue());
 } else {
-  if (auto unsigned_value = value_sp->GetAsUnsignedInteger()) {
-result = static_cast(unsigned_value->GetValue());
-return true;
-  }
+  if (auto *unsigned_value = item_sp->GetAsUnsignedInteger())
+return static_cast(unsigned_value->GetValue());
 }
   }
-  return false;
-}
-
-template 
-bool GetItemAtIndexAsInteger(size_t idx, IntType &result,
- IntType default_val) const {
-  bool success = GetItemAtIndexAsInteger(idx, result);
-  if (!success)
-result = default_val;
-  return success;
+  return {};
 }
 
 std::optional GetItemAtIndexAsString(size_t idx) const {
diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp 
b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 0097046cf511b5d..07ff597e10a5ca0 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -161,14 +161,14 @@ BreakpointResolverSP 
BreakpointResolverName::CreateFromStructuredData(
 error.SetErrorString("BRN::CFSD: name entry is not a string.");
 return nullptr;
   }
-  std::underlying_type::type fnt;
-  success = names_mask_array->GetItemAtIndexAsInteger(i, fnt);
-  if (!success) {
+  auto maybe_fnt = names_mask_array->GetItemAtIndexAsInteger<
+  std::underlying_type::type>(i);
+  if (!maybe_fnt) {
 error.SetErrorString("BRN::CFSD: name mask entry is not an integer.");
 return nullptr;
   }
   names.push_back(std::string(*maybe_name));
-  name_masks.push_back(static_cast(fnt));
+  name_masks.push_back(static_cast(*maybe_fnt));
 }
 
 std::shared_ptr resolver_sp =
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
index 2a35256a6fb0bf3..72293c5331f40da 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -592,9 +592,10 @@ addr_t 
InstrumentationRuntimeTSan::GetFirstNonInternalFramePc(
 if (skip_one_frame && i == 0)
   continue;
 
-addr_t addr;
-if (!trace_array->GetItemAtIndexAsInteger(i, addr))
+auto maybe_addr = trace_array->GetItemAtIndexAsInteger(i);
+if (!maybe_addr)
   continue;
+addr_t addr = *maybe_addr;
 
 lldb_private::Address so_addr;
 if (!process_sp->GetTarget().GetSectionLoadList().ResolveLoadAddress(
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp 
b/lldb/source/Target/DynamicRegisterInfo.cpp
index 7469c1d4259afc2..1a817449fa95896 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -349,10 +349,8 @@ DynamicRegisterInfo::SetRegisterInfo(const 
StructuredData::Dictionary &dict,
   const size_t num_regs = invalidate_reg_list->GetSize();
   if (num_regs > 0) {
 for (uint32_t idx = 0; idx < num_regs; ++idx) {
-  uint64_t invalidate_reg_num;
-  std::optional maybe_invalidate_reg_name =
-  invalidate_reg_list->GetItemAtIndexAsString(idx);
-  if (maybe_invalidate_reg_name) {
+  if (auto maybe_invalidate_reg_name =
+  invalidate_reg_list->GetItemAtIndexAsString(idx)) {
 const RegisterInfo *invalidate_reg_info =

[Lldb-commits] [lldb] [lldb] Remove StructuredData::Array::GetItemAtIndexAsArray (PR #71994)

2023-11-10 Thread Alex Langford via lldb-commits


https://github.com/bulbazord created 
https://github.com/llvm/llvm-project/pull/71994

This method is completely unused.

>From 96d7701aa01312e1decdf43c1c1287c9a4e3da91 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Fri, 10 Nov 2023 14:40:21 -0800
Subject: [PATCH] [lldb] Remove StructuredData::Array::GetItemAtIndexAsArray

This method is completely unused.
---
 lldb/include/lldb/Utility/StructuredData.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 8d0ae372f43c6bf..e7ee12868512f4a 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -276,16 +276,6 @@ class StructuredData {
   return {};
 }
 
-bool GetItemAtIndexAsArray(size_t idx, Array *&result) const {
-  result = nullptr;
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
-result = value_sp->GetAsArray();
-return (result != nullptr);
-  }
-  return false;
-}
-
 void Push(const ObjectSP &item) { m_items.push_back(item); }
 
 void AddItem(const ObjectSP &item) { m_items.push_back(item); }

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

[Lldb-commits] [lldb] [lldb] Remove StructuredData::Array::GetItemAtIndexAsArray (PR #71994)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)


Changes

This method is completely unused.

---
Full diff: https://github.com/llvm/llvm-project/pull/71994.diff


1 Files Affected:

- (modified) lldb/include/lldb/Utility/StructuredData.h (-10) 


``diff
diff --git a/lldb/include/lldb/Utility/StructuredData.h 
b/lldb/include/lldb/Utility/StructuredData.h
index 8d0ae372f43c6bf..e7ee12868512f4a 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -276,16 +276,6 @@ class StructuredData {
   return {};
 }
 
-bool GetItemAtIndexAsArray(size_t idx, Array *&result) const {
-  result = nullptr;
-  ObjectSP value_sp = GetItemAtIndex(idx);
-  if (value_sp.get()) {
-result = value_sp->GetAsArray();
-return (result != nullptr);
-  }
-  return false;
-}
-
 void Push(const ObjectSP &item) { m_items.push_back(item); }
 
 void AddItem(const ObjectSP &item) { m_items.push_back(item); }

``




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

[Lldb-commits] [lldb] 64f62de - [lldb] Read Checksum from DWARF line tables (#71458)

2023-11-10 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-11-10T14:43:47-08:00
New Revision: 64f62de96609dc3ea9a8a914a9e9445b7f4d625d

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

LOG: [lldb] Read Checksum from DWARF line tables (#71458)

Read the MD5 checksum from DWARF line tables and store it in the
corresponding support files.

This is a re-land after fixing an off-by-one error in LLDB's
ParseSupportFilesFromPrologue (fixed in #71984).

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 67ea17b2e1fb1bd..5bd4665dff7da7f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -236,8 +236,15 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP &module,
 remapped_file = std::move(*file_path);
 }
 
+Checksum checksum;
+if (prologue.ContentTypes.HasMD5) {
+  const llvm::DWARFDebugLine::FileNameEntry &file_name_entry =
+  prologue.getFileNameEntry(idx);
+  checksum = file_name_entry.Checksum;
+}
+
 // Unconditionally add an entry, so the indices match up.
-support_files.EmplaceBack(remapped_file, style);
+support_files.EmplaceBack(remapped_file, style, checksum);
   }
 
   return support_files;



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

[Lldb-commits] [lldb] [lldb] Remove StructuredData::Array::GetItemAtIndexAsArray (PR #71994)

2023-11-10 Thread Jonas Devlieghere via lldb-commits


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


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

[Lldb-commits] [lldb] Fix a little thinko in sending events to secondary listeners. (PR #71997)

2023-11-10 Thread via lldb-commits


https://github.com/jimingham created 
https://github.com/llvm/llvm-project/pull/71997

If `unique` is true, I was redoing the uniqueness check for the secondary 
listeners, which is racy since a "duplicate" event could come in between 
deciding to send the event to the main listener and checking for the shadow 
listener, and then the two streams would get out of sync.

There's not a good way to write a direct test for this, but the 
test_shadow_listeners test has been flakey and this is the only racy part of 
that system I can find.  So the test would be that that test_shadow_listeners 
becomes not flakey.

>From d76a7dec707e484e501b45a32066f852cba8de95 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Fri, 10 Nov 2023 15:08:19 -0800
Subject: [PATCH] Fix a little thinko in sending events to secondary listeners.

If `unique` is true, I was redoing the uniqueness check for the
secondary listeners, which is racy since a "duplicate" event could
come in between deciding to send the event to the main listener and
checking for the shadow listener, and then the two streams would
get out of sync.

There's not a good way to write a direct test for this, but the
test_shadow_listeners test has been flakey and this is the only
racy part of that system I can find.  So the test would be that
that test_shadow_listeners becomes not flakey.
---
 lldb/source/Utility/Broadcaster.cpp | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index 5192502addbdd72..914812d7857779f 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -280,13 +280,13 @@ void 
Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp,
 // Make sure to do this before adding the event to the primary or it might
 // start handling the event before we're done adding all the pending
 // listeners.
+// Also, don't redo the check for unique here, since otherwise that could
+// be racy, and if we send the event to the primary listener then we 
SHOULD 
+// send it to the secondary listeners or they will get out of sync with the
+// primary listener.
 if (!hijacking_listener_sp) {
-  for (auto &pair : GetListeners(event_type, false)) {
-if (unique && pair.first->PeekAtNextEventForBroadcasterWithType(
-  &m_broadcaster, event_type))
-  continue;
+  for (auto &pair : GetListeners(event_type, false))
 event_sp->AddPendingListener(pair.first);
-  }
 }
 primary_listener_sp->AddEvent(event_sp);
   } else {

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

[Lldb-commits] [lldb] Fix a little thinko in sending events to secondary listeners. (PR #71997)

2023-11-10 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes

If `unique` is true, I was redoing the uniqueness check for the secondary 
listeners, which is racy since a "duplicate" event could come in between 
deciding to send the event to the main listener and checking for the shadow 
listener, and then the two streams would get out of sync.

There's not a good way to write a direct test for this, but the 
test_shadow_listeners test has been flakey and this is the only racy part of 
that system I can find.  So the test would be that that test_shadow_listeners 
becomes not flakey.

---
Full diff: https://github.com/llvm/llvm-project/pull/71997.diff


1 Files Affected:

- (modified) lldb/source/Utility/Broadcaster.cpp (+5-5) 


``diff
diff --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index 5192502addbdd72..914812d7857779f 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -280,13 +280,13 @@ void 
Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp,
 // Make sure to do this before adding the event to the primary or it might
 // start handling the event before we're done adding all the pending
 // listeners.
+// Also, don't redo the check for unique here, since otherwise that could
+// be racy, and if we send the event to the primary listener then we 
SHOULD 
+// send it to the secondary listeners or they will get out of sync with the
+// primary listener.
 if (!hijacking_listener_sp) {
-  for (auto &pair : GetListeners(event_type, false)) {
-if (unique && pair.first->PeekAtNextEventForBroadcasterWithType(
-  &m_broadcaster, event_type))
-  continue;
+  for (auto &pair : GetListeners(event_type, false))
 event_sp->AddPendingListener(pair.first);
-  }
 }
 primary_listener_sp->AddEvent(event_sp);
   } else {

``




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

[Lldb-commits] [lldb] Fix a little thinko in sending events to secondary listeners. (PR #71997)

2023-11-10 Thread via lldb-commits


github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 64f62de96609dc3ea9a8a914a9e9445b7f4d625d 
d76a7dec707e484e501b45a32066f852cba8de95 -- lldb/source/Utility/Broadcaster.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index 914812d78..64e21c0bd 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -281,7 +281,7 @@ void 
Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp,
 // start handling the event before we're done adding all the pending
 // listeners.
 // Also, don't redo the check for unique here, since otherwise that could
-// be racy, and if we send the event to the primary listener then we 
SHOULD 
+// be racy, and if we send the event to the primary listener then we SHOULD
 // send it to the secondary listeners or they will get out of sync with the
 // primary listener.
 if (!hijacking_listener_sp) {

``




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

[Lldb-commits] [clang-tools-extra] [mlir] [libcxx] [llvm] [libc] [flang] [openmp] [clang] [lldb] GlobalISel: Guide return in llvm::getIConstantSplatVal (PR #71989)

2023-11-10 Thread Stanislav Mekhanoshin via lldb-commits


rampitec wrote:

Any tests?

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

[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)

2023-11-10 Thread Michael Buch via lldb-commits


https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/71780

>From e5bc858c35b479d29174c9945c6c67f4d2dc085b Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Thu, 9 Nov 2023 01:13:21 +
Subject: [PATCH 1/4] Reland "[clang][DebugInfo] Emit global variable
 definitions for static data members with constant initializers"

---
 clang/lib/CodeGen/CGDebugInfo.cpp | 58 +++
 clang/lib/CodeGen/CGDebugInfo.h   |  6 ++
 clang/test/CodeGenCXX/debug-info-class.cpp| 13 +++--
 .../CodeGenCXX/debug-info-static-member.cpp   | 52 ++---
 .../TestConstStaticIntegralMember.py  |  7 ++-
 .../lldb-dap/variables/TestDAP_variables.py   |  9 +--
 6 files changed, 103 insertions(+), 42 deletions(-)

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 84a166d3ac3659c..245f7516640d098 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1677,22 +1677,13 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl 
*Var, llvm::DIType *RecordTy,
 
   unsigned LineNumber = getLineNumber(Var->getLocation());
   StringRef VName = Var->getName();
-  llvm::Constant *C = nullptr;
-  if (Var->getInit()) {
-const APValue *Value = Var->evaluateValue();
-if (Value) {
-  if (Value->isInt())
-C = llvm::ConstantInt::get(CGM.getLLVMContext(), Value->getInt());
-  if (Value->isFloat())
-C = llvm::ConstantFP::get(CGM.getLLVMContext(), Value->getFloat());
-}
-  }
 
   llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);
   auto Align = getDeclAlignIfRequired(Var, CGM.getContext());
   llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
-  RecordTy, VName, VUnit, LineNumber, VTy, Flags, C, Align);
+  RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, 
Align);
   StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
+  StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
   return GV;
 }
 
@@ -5596,6 +5587,39 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl 
*VD, const APValue &Init) {
   TemplateParameters, Align));
 }
 
+void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
+  assert(VD->hasInit());
+  assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+  if (VD->hasAttr())
+return;
+
+  auto &GV = DeclCache[VD];
+  if (GV)
+return;
+
+  auto const *InitVal = VD->evaluateValue();
+  if (!InitVal)
+return;
+
+  llvm::DIFile *Unit = nullptr;
+  llvm::DIScope *DContext = nullptr;
+  unsigned LineNo;
+  StringRef DeclName, LinkageName;
+  QualType T;
+  llvm::MDTuple *TemplateParameters = nullptr;
+  collectVarDeclProps(VD, Unit, LineNo, T, DeclName, LinkageName,
+  TemplateParameters, DContext);
+
+  auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
+  llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(VD);
+  llvm::DIExpression *InitExpr = createConstantValueExpression(VD, *InitVal);
+
+  GV.reset(DBuilder.createGlobalVariableExpression(
+  TheCU, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
+  true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VD),
+  TemplateParameters, Align, Annotations));
+}
+
 void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
const VarDecl *D) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
@@ -5800,6 +5824,18 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
 }
 
 void CGDebugInfo::finalize() {
+  for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
+assert(VD->isStaticDataMember());
+
+if (DeclCache.contains(VD))
+  continue;
+
+if (!VD->hasInit())
+  continue;
+
+EmitGlobalVariable(VD);
+  }
+
   // Creating types might create further types - invalidating the current
   // element and the size(), so don't cache/reference them.
   for (size_t i = 0; i != ObjCInterfaceCache.size(); ++i) {
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d0608..3e4c133b7f2b9f1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -161,6 +161,9 @@ class CGDebugInfo {
   llvm::DenseMap>
   StaticDataMemberCache;
 
+  /// Keeps track of static data members for which we should emit a definition.
+  std::vector StaticDataMemberDefinitionsToEmit;
+
   using ParamDecl2StmtTy = llvm::DenseMap;
   using Param2DILocTy =
   llvm::DenseMap;
@@ -526,6 +529,9 @@ class CGDebugInfo {
   /// Emit a constant global variable's debug info.
   void EmitGlobalVariable(const ValueDecl *VD, const APValue &Init);
 
+  /// Emit debug-info for a variable with a constant initializer.
+  void EmitGlobalVariable(const VarDecl *VD);
+
   /// Emit information about an external variable.
   void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
diff --git a/clang/test/CodeGenCXX/debug-info-class.cpp 
b/clang/

[Lldb-commits] [lldb] Fix a little thinko in sending events to secondary listeners. (PR #71997)

2023-11-10 Thread Felipe de Azevedo Piovezan via lldb-commits


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

LGTM! I will keep an eye on the bots once we merge this to make sure we don't 
see the test failure anymore

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

[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)

2023-11-10 Thread Felipe de Azevedo Piovezan via lldb-commits


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

LGTM. I'm ok postponing the lifetime change to a separate task

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

[Lldb-commits] [lldb] 0f8d3e6 - Fix a little thinko in sending events to secondary listeners. (#71997)

2023-11-10 Thread via lldb-commits


Author: jimingham
Date: 2023-11-10T16:12:51-08:00
New Revision: 0f8d3e62d3fe3db905ac94e5d11fee87553c7794

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

LOG: Fix a little thinko in sending events to secondary listeners. (#71997)

If `unique` is true, I was redoing the uniqueness check for the
secondary listeners, which is racy since a "duplicate" event could come
in between deciding to send the event to the main listener and checking
for the shadow listener, and then the two streams would get out of sync.

There's not a good way to write a direct test for this, but the
test_shadow_listeners test has been flakey and this is the only racy
part of that system I can find. So the test would be that that
test_shadow_listeners becomes not flakey.

Added: 


Modified: 
lldb/source/Utility/Broadcaster.cpp

Removed: 




diff  --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index 5192502addbdd72..914812d7857779f 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -280,13 +280,13 @@ void 
Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp,
 // Make sure to do this before adding the event to the primary or it might
 // start handling the event before we're done adding all the pending
 // listeners.
+// Also, don't redo the check for unique here, since otherwise that could
+// be racy, and if we send the event to the primary listener then we 
SHOULD 
+// send it to the secondary listeners or they will get out of sync with the
+// primary listener.
 if (!hijacking_listener_sp) {
-  for (auto &pair : GetListeners(event_type, false)) {
-if (unique && pair.first->PeekAtNextEventForBroadcasterWithType(
-  &m_broadcaster, event_type))
-  continue;
+  for (auto &pair : GetListeners(event_type, false))
 event_sp->AddPendingListener(pair.first);
-  }
 }
 primary_listener_sp->AddEvent(event_sp);
   } else {



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

[Lldb-commits] [lldb] Fix a little thinko in sending events to secondary listeners. (PR #71997)

2023-11-10 Thread via lldb-commits


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

[Lldb-commits] [lldb] [mlir] [flang] [llvm] [libc] [libcxx] [openmp] [clang-tools-extra] [clang] GlobalISel: Guide return in llvm::getIConstantSplatVal (PR #71989)

2023-11-10 Thread Changpeng Fang via lldb-commits


changpeng wrote:

> Any tests?
Encountered this issue during a downstream branch testing. No test for trunk 
yet but think the issue should be here.  

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

1 2 >

1 - 100 of 106 matches

Mail list logo