[Lldb-commits] [lldb] [lldb] Avoid calling DataLayout constructor accepting Module pointer (NFC) (PR #102839)
https://github.com/nikic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/102839 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] Extending LLDB to work on AIX (PR #102601)
https://github.com/Dhruv-Srivastava-IBM converted_to_draft https://github.com/llvm/llvm-project/pull/102601 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath approved this pull request. This looks fine now, just please check what happened with one of the suggestions I highlighted. Thanks for your patience. https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,92 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors) {
+ return Memory64Iterator(Storage, Descriptors);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator->() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0) {
labath wrote:
Github says this has been applied, but I don't see that in the code. Did you
maybe forget to upload of force-overwrite something by mistake?
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #102570)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/102570 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #102570)
labath wrote: Looks good, thanks. The structuring made it really easy to review. https://github.com/llvm/llvm-project/pull/102570 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] Extending LLDB to work on AIX (PR #102601)
https://github.com/Dhruv-Srivastava-IBM edited https://github.com/llvm/llvm-project/pull/102601 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 32a62eb - [lldb] Tolerate multiple compile units with the same DWO ID (#100577)
Author: Pavel Labath
Date: 2024-08-12T11:29:08+02:00
New Revision: 32a62ebdeab0c10d5311cf812e021717636d4514
URL:
https://github.com/llvm/llvm-project/commit/32a62ebdeab0c10d5311cf812e021717636d4514
DIFF:
https://github.com/llvm/llvm-project/commit/32a62ebdeab0c10d5311cf812e021717636d4514.diff
LOG: [lldb] Tolerate multiple compile units with the same DWO ID (#100577)
I ran into this when LTO completely emptied two compile units, so they
ended up with the same hash (see #100375). Although, ideally, the
compiler would try to ensure we don't end up with a hash collision even
in this case, guaranteeing their absence is practically impossible. This
patch ensures this situation does not bring down lldb.
Added:
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 66a762bf9b6854..0a52159d055bb4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -97,12 +97,14 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
*m_dwo_id, m_first_die.GetOffset()));
return; // Can't fetch the compile unit from the dwo file.
}
- // If the skeleton compile unit gets its unit DIE parsed first, then this
- // will fill in the DWO file's back pointer to this skeleton compile unit.
- // If the DWO files get parsed on their own first the skeleton back link
- // can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
- // do a reverse lookup and cache the result.
- dwo_cu->SetSkeletonUnit(this);
+
+ // Link the DWO unit to this object, if it hasn't been linked already (this
+ // can happen when we have an index, and the DWO unit is parsed first).
+ if (!dwo_cu->LinkToSkeletonUnit(*this)) {
+SetDwoError(Status::createWithFormat(
+"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
+return;
+ }
DWARFBaseDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
if (!dwo_cu_die.IsValid()) {
@@ -718,13 +720,11 @@ DWARFCompileUnit *DWARFUnit::GetSkeletonUnit() {
return llvm::dyn_cast_or_null(m_skeleton_unit);
}
-void DWARFUnit::SetSkeletonUnit(DWARFUnit *skeleton_unit) {
- // If someone is re-setting the skeleton compile unit backlink, make sure
- // it is setting it to a valid value when it wasn't valid, or if the
- // value in m_skeleton_unit was valid, it should be the same value.
- assert(skeleton_unit);
- assert(m_skeleton_unit == nullptr || m_skeleton_unit == skeleton_unit);
- m_skeleton_unit = skeleton_unit;
+bool DWARFUnit::LinkToSkeletonUnit(DWARFUnit &skeleton_unit) {
+ if (m_skeleton_unit && m_skeleton_unit != &skeleton_unit)
+return false;
+ m_skeleton_unit = &skeleton_unit;
+ return true;
}
bool DWARFUnit::Supports_DW_AT_APPLE_objc_complete_type() {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 85c37971ced8e0..209104fe3a054e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -170,7 +170,7 @@ class DWARFUnit : public UserID {
/// both cases correctly and avoids crashes.
DWARFCompileUnit *GetSkeletonUnit();
- void SetSkeletonUnit(DWARFUnit *skeleton_unit);
+ bool LinkToSkeletonUnit(DWARFUnit &skeleton_unit);
bool Supports_DW_AT_APPLE_objc_complete_type();
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
new file mode 100644
index 00..d626b4602ad58f
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
@@ -0,0 +1,142 @@
+## Test that lldb handles (mainly, that it doesn't crash) the situation where
+## two skeleton compile units have the same DWO ID (and try to claim the same
+## split unit from the DWP file. This can sometimes happen when the compile
unit
+## is nearly empty (e.g. because LTO has optimized all of it away).
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp
+# RUN: %lldb %t -o "image lookup -t my_enum_type" \
+# RUN: -o "image dump separate-debug-info" -o exit | FileCheck %s
+
+## Check that we're able to access the type within the split unit (no matter
+## which skeleton unit it ends up associated with). Completely ignoring the
unit
+## might also be reasonable.
+# CHECK: image lookup -t my_enum_type
+# CHECK: 1 match found
+# CHECK: name = "my_enum_type", byte-size = 4, compiler_type = "enum
my_enum_type {
+# CHECK-NEXT: }"
+#
+## Check that we get some indication of the error.
+# CHECK: image dump separate-debug-info
+# CHECK: Dwo ID
[Lldb-commits] [lldb] [lldb] Tolerate multiple compile units with the same DWO ID (PR #100577)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/100577 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Tolerate multiple compile units with the same DWO ID (PR #100577)
@@ -97,12 +97,14 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
*m_dwo_id, m_first_die.GetOffset()));
return; // Can't fetch the compile unit from the dwo file.
}
- // If the skeleton compile unit gets its unit DIE parsed first, then this
- // will fill in the DWO file's back pointer to this skeleton compile unit.
- // If the DWO files get parsed on their own first the skeleton back link
- // can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
- // do a reverse lookup and cache the result.
- dwo_cu->SetSkeletonUnit(this);
+
+ // Link the DWO unit to this object, if it hasn't been linked already (this
+ // can happen when we have an index, and the DWO unit is parsed first).
+ if (!dwo_cu->LinkToSkeletonUnit(*this)) {
+SetDwoError(Status::createWithFormat(
+"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
labath wrote:
Sounds good. With #100375, I think the only way this can show up is with old
compiler versions. New versions should no longer generate identical hashes even
in these cases (of course, it's still possible to create a hash collision
deliberately, but accidental ones should not be happening).
https://github.com/llvm/llvm-project/pull/100577
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
@@ -482,6 +482,13 @@ size_t ObjectFile::CopyData(lldb::offset_t offset, size_t
length,
return m_data.CopyData(offset, length, dst);
}
+llvm::ArrayRef ObjectFile::PeekData(lldb::addr_t offset,
+ size_t length) const {
+ const uint8_t *data = m_data.PeekData(offset, length);
+ llvm::ArrayRef array(data, length);
+ return array;
+}
+
labath wrote:
Could we avoid adding this function? You should be able access the same data
using the `DataExtractor` overload of `GetData` (line 471, just call `GetData`
on the returned DataExtractor), and this function is basically unimplementable
for in-memory object files, which read their data on demand (as there's noone
who can own the returned data).
(I realize this PR is not interested in those, but this API is going to be
available for all object files)
https://github.com/llvm/llvm-project/pull/102536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
https://github.com/xusheng6 created
https://github.com/llvm/llvm-project/pull/102873
This fixes https://github.com/llvm/llvm-project/issues/56125 and
https://github.com/vadimcn/codelldb/issues/666, as well as the downstream issue
in our binary ninja debugger: https://github.com/Vector35/debugger/issues/535
Basically, lldb does not claim to support the `swbreak` packet so the gdbserver
would not use it. As a result, it always sends the unmodified program counter
value which, on systems like x86, causes the program counter to be off-by-off
(or otherwise wrong).
No new code is added to add support `swbreak`, since the way lldb works already
expects the remote to have adjusted the program counter. The change just lets
the gdbserver know that lldb supports it, and it will send the adjusted program
counter.
I am unable to add a unit test for this. I browsed the gdbserver unit test code
and it seems to be testing against the gdbserver in lldb, which also does not
use the `swbreak` packet. But as discussed in the issue, there is no reason to
add support for sending swbreak in lldb-server.
To test this PR, you can use lldb to connect to a gdbserver running on e.g.,
Ubuntu 22.04, and see the program counter is off-by-one without the patch. With
the patch, things work as expected
>From 73c98df4baef99f96d9c67113ba2ed0d972e5a04 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 20 Mar 2023 20:24:11 +0800
Subject: [PATCH] [lldb] Claim to support swbreak and hwbreak packets when
debugging a gdbremote
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp| 2 +-
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..d8b17a8ff59baf 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
"multiprocess+", "fork-events+",
- "vfork-events+"};
+ "vfork-events+", "swbreak+",
"hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..b8fe8fdc9b8742 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,6 +2349,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// There is nothing needs to be done for swbreak or hwbreak since
+// the value is expected to be empty
} else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: xusheng (xusheng6)
Changes
This fixes https://github.com/llvm/llvm-project/issues/56125 and
https://github.com/vadimcn/codelldb/issues/666, as well as the downstream issue
in our binary ninja debugger: https://github.com/Vector35/debugger/issues/535
Basically, lldb does not claim to support the `swbreak` packet so the gdbserver
would not use it. As a result, it always sends the unmodified program counter
value which, on systems like x86, causes the program counter to be off-by-off
(or otherwise wrong).
No new code is added to add support `swbreak`, since the way lldb works already
expects the remote to have adjusted the program counter. The change just lets
the gdbserver know that lldb supports it, and it will send the adjusted program
counter.
I am unable to add a unit test for this. I browsed the gdbserver unit test code
and it seems to be testing against the gdbserver in lldb, which also does not
use the `swbreak` packet. But as discussed in the issue, there is no reason to
add support for sending swbreak in lldb-server.
To test this PR, you can use lldb to connect to a gdbserver running on e.g.,
Ubuntu 22.04, and see the program counter is off-by-one without the patch. With
the patch, things work as expected
---
Full diff: https://github.com/llvm/llvm-project/pull/102873.diff
2 Files Affected:
- (modified)
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+1-1)
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+3)
``diff
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..d8b17a8ff59baf 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
"multiprocess+", "fork-events+",
- "vfork-events+"};
+ "vfork-events+", "swbreak+",
"hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..b8fe8fdc9b8742 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,6 +2349,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// There is nothing needs to be done for swbreak or hwbreak since
+// the value is expected to be empty
} else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
``
https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
https://github.com/xusheng6 edited https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
@@ -2349,6 +2349,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// There is nothing needs to be done for swbreak or hwbreak since
+// the value is expected to be empty
DavidSpickett wrote:
This code is run by lldb when parsing a stop packet from the gdbserver, so this
comment should also note that lldb's default is to act in the same way as
swbreak or hwbreak does, that's why we take no action on it here.
I think making this whole thing a comment is also a good idea, so we don't trip
up static analysis tools. Best place for it I think is after the last else if,
something like:
```
// swbreak and hwbreak are also expected keys, but we don't need to change our
behaviour for them because...
```
https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
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 55d7e59023bc48f97321970cda5e400c07de59fa
73c98df4baef99f96d9c67113ba2ed0d972e5a04 --extensions cpp --
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
``
View the diff from clang-format here.
``diff
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d8b17a8ff5..83ba27783d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -352,8 +352,11 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
- "multiprocess+", "fork-events+",
- "vfork-events+", "swbreak+",
"hwbreak+"};
+ "multiprocess+",
+ "fork-events+",
+ "vfork-events+",
+ "swbreak+",
+ "hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
``
https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
https://github.com/xusheng6 updated
https://github.com/llvm/llvm-project/pull/102873
>From 73c98df4baef99f96d9c67113ba2ed0d972e5a04 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 20 Mar 2023 20:24:11 +0800
Subject: [PATCH 1/3] [lldb] Claim to support swbreak and hwbreak packets when
debugging a gdbremote
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp| 2 +-
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..d8b17a8ff59baf 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
"multiprocess+", "fork-events+",
- "vfork-events+"};
+ "vfork-events+", "swbreak+",
"hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..b8fe8fdc9b8742 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,6 +2349,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// There is nothing needs to be done for swbreak or hwbreak since
+// the value is expected to be empty
} else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
>From 1db7c14528bb709921a0d6607c2118477c8de500 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 12 Aug 2024 18:37:40 +0800
Subject: [PATCH 2/3] Fix format
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp| 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d8b17a8ff59baf..83ba27783da471 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -352,8 +352,11 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
- "multiprocess+", "fork-events+",
- "vfork-events+", "swbreak+",
"hwbreak+"};
+ "multiprocess+",
+ "fork-events+",
+ "vfork-events+",
+ "swbreak+",
+ "hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
>From 2d085e46c50f1c909ce5b90dd34d6c47832564d0 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 12 Aug 2024 18:43:36 +0800
Subject: [PATCH 3/3] Put the condition as the last one and update comment
---
.../source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index b8fe8fdc9b8742..e467577ab2b4f0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,13 +2349,14 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
- } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
-// There is nothing needs to be done for swbreak or hwbreak since
-// the value is expected to be empty
} else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
+ } else if (key.compare("swbrea
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
@@ -2349,6 +2349,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// There is nothing needs to be done for swbreak or hwbreak since
+// the value is expected to be empty
xusheng6 wrote:
Hi @DavidSpickett, thx for the reply! I have address your comments in the last
commits
https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
DavidSpickett wrote: On the testing, lldb sends this qsupported value regardless of what the remote sends. So there's nothing to do there unless you just repeat the feature list. There are some tests that look at lldb-server's qsupported values, but I don't see any for the client lldb. You could check we don't crash when seeing swbreak/hwbreak in a stop response, but it's the same as checking we don't crash on any other unknown key in there. Since you can't observe any behaviour change from swbreak/hwbreak, so I don't think that has much value either. What you could do is write a test where lldb connects to a mock gdbserver that claims to be an architecture where the correction to addresses would be done by a gdb server (GDB's gdbserver I mean). Then via lldb check that the stopped location is unmodified. You could then run the test with swbreak / hwbreak / neither of those in the stop info and each time lldb will report the value unchanged. So in the unlikely case that we decided to change how lldb behaves, the test would fail. `lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py` might be a good starting point. https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
DavidSpickett wrote: Also there is probably a qsupported parsing function in lldb-server, please add a comment there to say we consume lldb's swbreak/hwbreak feature but it doesn't change the behaviour of lldb-server. https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
@@ -2353,6 +2353,10 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// swbreak and hwbreak are also expected keys, but we don't need to
+// change our behaviour for them because lldb always expects the remote
+// to adjust the program counter (if relevant, e.g., for x86 targets)
DavidSpickett wrote:
I don't think we need the actual `else if`. Since `if condition do nothing`
looks like a programming mistake. The comment is fine on its own here.
https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] afe019c - [lldb][test][AArch64] Regex match field values in register test
Author: David Spickett
Date: 2024-08-12T11:03:06Z
New Revision: afe019ca93a72a5969d82cfff5018f3dd79dc75a
URL:
https://github.com/llvm/llvm-project/commit/afe019ca93a72a5969d82cfff5018f3dd79dc75a
DIFF:
https://github.com/llvm/llvm-project/commit/afe019ca93a72a5969d82cfff5018f3dd79dc75a.diff
LOG: [lldb][test][AArch64] Regex match field values in register test
As these are flags they can be set or not depending on what the system
libraries did prior to loading the program.
Added:
Modified:
lldb/test/API/commands/register/register/register_command/TestRegisters.py
Removed:
diff --git
a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
index d1fc3e100af332..bfd7a382064e9d 100644
--- a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -628,11 +628,16 @@ def test_register_read_fields(self):
self.common_setup()
# N/Z/C/V bits will always be present, so check only for those.
-self.expect("register read cpsr", substrs=["= (N = 0, Z = 1, C = 1, V
= 0"])
-self.expect("register read fpsr", substrs=["= (QC = 0, IDC = 0, IXC =
0"])
-# AHP/DN/FZ/RMode always present, others may vary.
self.expect(
-"register read fpcr", substrs=["= (AHP = 0, DN = 0, FZ = 0, RMode
= RN"]
+"register read cpsr",
+patterns=["= \(N = [0|1], Z = [0|1], C = [0|1], V = [0|1]"],
+)
+self.expect(
+"register read fpsr", patterns=["= \(QC = [0|1], IDC = [0|1], IXC
= [0|1]"]
+)
+# AHP/DN/FZ always present, others may vary.
+self.expect(
+"register read fpcr", patterns=["= \(AHP = [0|1], DN = [0|1], FZ =
[0|1]"]
)
# Should get enumerator descriptions for RMode.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
@@ -0,0 +1,560 @@
+//===-- DILAST.cpp
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/DILAST.h"
+#include "lldb/API/SBType.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/RegisterContext.h"
+#include "llvm/ADT/StringRef.h"
+
+#include
+
+namespace lldb_private {
+
+namespace DIL {
+
+lldb::ValueObjectSP
+GetDynamicOrSyntheticValue(lldb::ValueObjectSP in_valobj_sp,
+ lldb::DynamicValueType use_dynamic,
+ bool use_synthetic) {
+ Status error;
+
+ if (!in_valobj_sp) {
+error.SetErrorString("invalid value object");
+return in_valobj_sp;
+ }
+
+ lldb::ValueObjectSP value_sp = in_valobj_sp;
+
+ Target *target = value_sp->GetTargetSP().get();
+ // If this ValueObject holds an error, then it is valuable for that.
+ if (value_sp->GetError().Fail())
+return value_sp;
+
+ if (!target)
+return lldb::ValueObjectSP();
+
+ if (use_dynamic != lldb::eNoDynamicValues) {
+lldb::ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(use_dynamic);
+if (dynamic_sp)
+ value_sp = dynamic_sp;
+ }
+
+ if (use_synthetic) {
+lldb::ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue();
+if (synthetic_sp)
+ value_sp = synthetic_sp;
+ }
+
+ if (!value_sp)
+error.SetErrorString("invalid value object");
+
+ return value_sp;
+}
+
+CompilerType DILASTNode::GetDereferencedResultType() const {
+ auto type = result_type();
+ return type.IsReferenceType() ? type.GetNonReferenceType() : type;
+}
+
+std::optional
labath wrote:
Why do we actually want this function to work on something like `C[2]`? That's
definitely not what I would have expected from the function signature (nor from
its implementation, actually).
Even that was the behavior we want (which I really hope isn't the case), the
other implementation might still make more sense, because it would give you a
chance to explain what you're doing and why. Right now, this function looks
like it's trying to reimplement
`TypeSystemClang::GetIndexOfChildMemberWithName` without any indication of why.
I wouldn't be surprised if it didn't work with other languages due to its
reliance on how TypeSystemClang or C++ represent certain constructs.
(This extraction of information from the types could be part of why you're
needing to detect if something is a smart pointer, etc. We currently don't have
a way to tell if a type represents a smart pointer -- we can only do that for a
specific value of a that type)
https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
https://github.com/labath commented: Even though you've removed the smart pointer checking function (the most obvious example of language-specific behavior), I'm still worried about whether the overall design of this code will allow you to avoid it. Looking at the `IdentifierInfo` class, I see that it can be constructed with only a type (the value is optional). I think this is going to be very hard to make work, since a type does not know if its a smart pointer (only a value does). Have you already figured out a way to make that work? IndentifierInfo isn't the only suspicious place though. All of the places that are doing something with bitfield offsets are very suspicious to me, as that's something that the Values should already know. I haven't seen the parser, but I wouldn't be surprised if this has something to do with the vexing parse issues of C languages, where one needs to know if something is a type just to parse the expression correctly. Now, if that's the case, and if really need to know if something is a smart pointer (i.e. whether it is dereferencable) without having access to a value of that type, then we may need to have a bigger discussion about what to do about that. https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
@@ -0,0 +1,426 @@
+//===-- DILAST.h *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include
+#include
+#include
+#include
+#include
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/Support/Casting.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct MemberInfo {
+ std::optional name;
+ CompilerType type;
+ std::optional bitfield_size_in_bits;
+ bool is_synthetic;
+ bool is_dynamic;
+ lldb::ValueObjectSP val_obj_sp;
+};
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed.
+lldb::ValueObjectSP GetDynamicOrSyntheticValue(
+lldb::ValueObjectSP valobj_sp,
+lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class NodeKind {
+ eErrorNode,
+ eScalarLiteralNode,
+ eStringLiteralNode,
+ eIdentifierNode,
+ eCStyleCastNode,
+ eMemberOfNode,
+ eArraySubscriptNode,
+ eUnaryOpNode,
+};
+
+/// The C-Style casts for type promotion allowed by DIL.
+enum class TypePromotionCastKind {
+ eArithmetic,
+ ePointer,
+};
+
+/// The Unary operators recognized by DIL.
+enum class UnaryOpKind {
+ AddrOf, // "&"
+ Deref, // "*"
+ Minus, // "-"
+};
+
+/// Given a string representing a type, returns the CompilerType corresponding
+/// to the named type, if it exists.
+CompilerType
+ResolveTypeByName(const std::string &name,
+ std::shared_ptr ctx_scope);
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+private:
+
+public:
labath wrote:
```suggestion
public:
```
https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
@@ -0,0 +1,459 @@ +//===-- DILAST.h *- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLDB_CORE_DILAST_H +#define LLDB_CORE_DILAST_H + +#include +#include +#include +#include +#include + +#include "lldb/Core/ValueObject.h" +#include "lldb/Symbol/Type.h" +#include "lldb/Symbol/TypeList.h" +#include "lldb/Target/LanguageRuntime.h" +#include "lldb/Utility/ConstString.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/TokenKinds.h" labath wrote: What are these doing here? Can this be a language-agnostic implementation if it depends on clang? https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
@@ -0,0 +1,426 @@
+//===-- DILAST.h *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include
+#include
+#include
+#include
+#include
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/Support/Casting.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Struct to hold information about member fields. Used by the parser for the
+/// Data Inspection Language (DIL).
+struct MemberInfo {
+ std::optional name;
+ CompilerType type;
+ std::optional bitfield_size_in_bits;
+ bool is_synthetic;
+ bool is_dynamic;
+ lldb::ValueObjectSP val_obj_sp;
+};
+
+/// Get the appropriate ValueObjectSP, consulting the use_dynamic and
+/// use_synthetic options passed.
+lldb::ValueObjectSP GetDynamicOrSyntheticValue(
+lldb::ValueObjectSP valobj_sp,
+lldb::DynamicValueType use_dynamic = lldb::eNoDynamicValues,
+bool use_synthetic = false);
+
+/// The various types DIL AST nodes (used by the DIL parser).
+enum class NodeKind {
+ eErrorNode,
+ eScalarLiteralNode,
+ eStringLiteralNode,
+ eIdentifierNode,
+ eCStyleCastNode,
+ eMemberOfNode,
+ eArraySubscriptNode,
+ eUnaryOpNode,
+};
+
+/// The C-Style casts for type promotion allowed by DIL.
+enum class TypePromotionCastKind {
+ eArithmetic,
+ ePointer,
+};
+
+/// The Unary operators recognized by DIL.
+enum class UnaryOpKind {
+ AddrOf, // "&"
+ Deref, // "*"
+ Minus, // "-"
+};
+
+/// Given a string representing a type, returns the CompilerType corresponding
+/// to the named type, if it exists.
+CompilerType
+ResolveTypeByName(const std::string &name,
+ std::shared_ptr ctx_scope);
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+private:
+
+public:
+ enum class Kind {
+eValue,
+eContextArg,
+eMemberPath,
+ };
+
+ static std::unique_ptr FromValue(ValueObject &valobj) {
+CompilerType type;
+type = valobj.GetCompilerType();
+return std::unique_ptr(
+new IdentifierInfo(Kind::eValue, type, valobj.GetSP(), {}));
+ }
+
+ static std::unique_ptr FromContextArg(CompilerType type) {
+lldb::ValueObjectSP empty_value;
+return std::unique_ptr(
+new IdentifierInfo(Kind::eContextArg, type, empty_value, {}));
+ }
+
+ static std::unique_ptr
+ FromMemberPath(CompilerType type, std::vector path) {
+lldb::ValueObjectSP empty_value;
+return std::unique_ptr(new IdentifierInfo(
+Kind::eMemberPath, type, empty_value, std::move(path)));
+ }
+
+ Kind GetKind() const { return m_kind; }
+ lldb::ValueObjectSP GetValue() const { return m_value; }
+ const std::vector &GetPath() const { return m_path; }
+
+ CompilerType GetType() { return m_type; }
+ bool IsValid() const { return m_type.IsValid(); }
labath wrote:
Could we remove this function and use an null IdentifierInfo pointer to denote
invalidness?
https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
@@ -0,0 +1,560 @@
+//===-- DILAST.cpp
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/DILAST.h"
+#include "lldb/API/SBType.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/RegisterContext.h"
+#include "llvm/ADT/StringRef.h"
+
+#include
+
+namespace lldb_private {
+
+namespace DIL {
+
+lldb::ValueObjectSP
+GetDynamicOrSyntheticValue(lldb::ValueObjectSP in_valobj_sp,
+ lldb::DynamicValueType use_dynamic,
+ bool use_synthetic) {
+ Status error;
+
+ if (!in_valobj_sp) {
+error.SetErrorString("invalid value object");
+return in_valobj_sp;
+ }
+
+ lldb::ValueObjectSP value_sp = in_valobj_sp;
+
+ Target *target = value_sp->GetTargetSP().get();
+ // If this ValueObject holds an error, then it is valuable for that.
+ if (value_sp->GetError().Fail())
+return value_sp;
+
+ if (!target)
+return lldb::ValueObjectSP();
+
+ if (use_dynamic != lldb::eNoDynamicValues) {
+lldb::ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(use_dynamic);
+if (dynamic_sp)
+ value_sp = dynamic_sp;
+ }
+
+ if (use_synthetic) {
+lldb::ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue();
+if (synthetic_sp)
+ value_sp = synthetic_sp;
+ }
+
+ if (!value_sp)
+error.SetErrorString("invalid value object");
+
+ return value_sp;
+}
+
+CompilerType DILASTNode::GetDereferencedResultType() const {
+ auto type = result_type();
+ return type.IsReferenceType() ? type.GetNonReferenceType() : type;
+}
+
+std::optional
+GetFieldWithNameIndexPath(lldb::ValueObjectSP lhs_val_sp, CompilerType type,
+ const std::string &name, std::vector *idx,
+ CompilerType empty_type, bool use_synthetic,
+ bool is_dynamic) {
+ bool is_synthetic = false;
+ // Go through the fields first.
+ uint32_t num_fields = type.GetNumFields();
+ lldb::ValueObjectSP empty_valobj_sp;
+ for (uint32_t i = 0; i < num_fields; ++i) {
+uint64_t bit_offset = 0;
+uint32_t bitfield_bit_size = 0;
+bool is_bitfield = false;
+std::string name_sstr;
+CompilerType field_type(type.GetFieldAtIndex(
+i, name_sstr, &bit_offset, &bitfield_bit_size, &is_bitfield));
+auto field_name =
+name_sstr.length() == 0 ? std::optional() : name_sstr;
+if (field_type.IsValid()) {
+ std::optional size_in_bits;
+ if (is_bitfield)
+size_in_bits = bitfield_bit_size;
+ struct MemberInfo field = {field_name, field_type, size_in_bits,
+ is_synthetic, is_dynamic, empty_valobj_sp};
+
+ // Name can be null if this is a padding field.
+ if (field.name == name) {
+if (lhs_val_sp) {
+ lldb::ValueObjectSP child_valobj_sp =
+ lhs_val_sp->GetChildMemberWithName(name);
+ if (child_valobj_sp)
+field.val_obj_sp = child_valobj_sp;
+}
+
+if (idx) {
+ assert(idx->empty());
+ // Direct base classes are located before fields, so field members
+ // needs to be offset by the number of base classes.
+ idx->push_back(i + type.GetNumberOfNonEmptyBaseClasses());
+}
+return field;
+ } else if (field.type.IsAnonymousType()) {
+// Every member of an anonymous struct is considered to be a member of
+// the enclosing struct or union. This applies recursively if the
+// enclosing struct or union is also anonymous.
+
+assert(!field.name && "Field should be unnamed.");
+
+std::optional field_in_anon_type =
+GetFieldWithNameIndexPath(lhs_val_sp, field.type, name, idx,
+ empty_type, use_synthetic, is_dynamic);
+if (field_in_anon_type) {
+ if (idx) {
+idx->push_back(i + type.GetNumberOfNonEmptyBaseClasses());
+ }
+ return field_in_anon_type.value();
+}
+ }
+}
+ }
+
+ // LLDB can't access inherited fields of anonymous struct members.
+ if (type.IsAnonymousType()) {
+return {};
+ }
+
+ // Go through the base classes and look for the field there.
+ uint32_t num_non_empty_bases = 0;
+ uint32_t num_direct_bases = type.GetNumDirectBaseClasses();
+ for (uint32_t i = 0; i < num_direct_bases; ++i) {
+uint32_t bit_offset;
+auto base = type.GetDirectBaseClassAtIndex(i, &bit_offset);
+auto field = GetFieldWithNameIndexPath(
+lhs_val_sp, base
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/labath approved this pull request. This looks good. Thanks for your patience. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 21ef272 - [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (#102123)
Author: Pavel Labath
Date: 2024-08-12T14:31:14+02:00
New Revision: 21ef272ec1974244710fc639f98674eae3f8b02c
URL:
https://github.com/llvm/llvm-project/commit/21ef272ec1974244710fc639f98674eae3f8b02c
DIFF:
https://github.com/llvm/llvm-project/commit/21ef272ec1974244710fc639f98674eae3f8b02c.diff
LOG: [lldb/DWARF] Search fallback to the manual index in GetFullyQualified…
(#102123)
…Type
This is needed to ensure we find a type if its definition is in a CU
that wasn't indexed. This can happen if the definition is in some
precompiled code (e.g. the c++ standard library) which was built with
different flags than the rest of the binary.
Added:
lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 7e66b3dccf97fa..32d8a92305aafa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -371,6 +371,7 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
!ProcessEntry(entry, callback))
return;
}
+ m_fallback.GetFullyQualifiedType(context, callback);
}
bool DebugNamesDWARFIndex::SameParentChain(
diff --git
a/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
new file mode 100644
index 00..71da8fad001652
--- /dev/null
+++
b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
@@ -0,0 +1,35 @@
+REQUIRES: lld, python
+
+RUN: split-file %s %t
+RUN: %clang --target=x86_64-pc-linux -g -gpubnames -c %t/file1.c -o %t-1.o
+RUN: %clang --target=x86_64-pc-linux -g -gno-pubnames -c %t/file2.c -o %t-2.o
+RUN: llvm-dwarfdump %t-1.o --debug-names | FileCheck %s --check-prefix=PUBNAMES
+RUN: llvm-dwarfdump %t-2.o --debug-names | FileCheck %s
--check-prefix=NOPUBNAMES
+RUN: ld.lld %t-1.o %t-2.o -o %t.out
+RUN: %lldb %t.out -s %t/commands -o exit | FileCheck %s
+
+// Precondition check: The first file should contain a debug_names index, but
no
+// entries for MYSTRUCT.
+PUBNAMES: Name Index @ 0x0 {
+PUBNAMES-NOT: MYSTRUCT
+
+// The second file should not contain an index.
+NOPUBNAMES-NOT: Name Index
+
+// Starting from the variable in the first file, we should be able to find the
+// declaration of the type in the first unit, and then match that with the
+// definition in the second unit.
+CHECK: (lldb) script
+CHECK: struct MYSTRUCT {
+CHECK-NEXT: int x;
+CHECK-NEXT: }
+
+#--- commands
+script
lldb.target.FindFirstGlobalVariable("struct_ptr").GetType().GetPointeeType()
+#--- file1.c
+struct MYSTRUCT *struct_ptr;
+#--- file2.c
+struct MYSTRUCT {
+ int x;
+};
+struct MYSTRUCT struct_;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/labath milestoned https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
labath wrote: /cherry-pick [57cd100](https://github.com/llvm/llvm-project/commit/57cd1000c9c93fd0e64352cfbc9fbbe5b8a8fcef) https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
llvmbot wrote: Failed to cherry-pick: [57cd100](https://github.com/llvm/llvm-project/commit/57cd1000c9c93fd0e64352cfbc9fbbe5b8a8fcef) https://github.com/llvm/llvm-project/actions/runs/10352006063 Please manually backport the fix and push it to your github fork. Once this is done, please create a [pull request](https://github.com/llvm/llvm-project/compare) https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f2991bd - [lldb][test] Disable procfile by thread ID test when LLVM_ENABLE_THREADS is not defined
Author: David Spickett
Date: 2024-08-12T12:41:49Z
New Revision: f2991bd93146162bcc30bc5e8da8707074f3fdef
URL:
https://github.com/llvm/llvm-project/commit/f2991bd93146162bcc30bc5e8da8707074f3fdef
DIFF:
https://github.com/llvm/llvm-project/commit/f2991bd93146162bcc30bc5e8da8707074f3fdef.diff
LOG: [lldb][test] Disable procfile by thread ID test when LLVM_ENABLE_THREADS
is not defined
When LLVM_ENABLE_THREADS is not defined, llvm::get_threadid returns 0 which
makes this test case fail.
This is a pretty niche setting, Linaro uses it to stop lld crashing our 32 bit
containers. So the test will get plenty of runs elsewhere.
In lldb's code it's not getting the current thread ID anyway, it's using
a value it got from ptrace. So even if that copy of lldb was built with
LLVM_ENABLE_THREADS off, it should still be able to debug threads.
Added:
Modified:
lldb/unittests/Host/linux/SupportTest.cpp
Removed:
diff --git a/lldb/unittests/Host/linux/SupportTest.cpp
b/lldb/unittests/Host/linux/SupportTest.cpp
index 680893c03d0a20..6d1d28cd4caad5 100644
--- a/lldb/unittests/Host/linux/SupportTest.cpp
+++ b/lldb/unittests/Host/linux/SupportTest.cpp
@@ -18,8 +18,10 @@ TEST(Support, getProcFile_Pid) {
ASSERT_TRUE(*BufferOrError);
}
+#ifdef LLVM_ENABLE_THREADING
TEST(Support, getProcFile_Tid) {
auto BufferOrError = getProcFile(getpid(), llvm::get_threadid(), "comm");
ASSERT_TRUE(BufferOrError);
ASSERT_TRUE(*BufferOrError);
}
+#endif /*ifdef LLVM_ENABLE_THREADING */
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
labath wrote: /cherry-pick 57cd100 https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
llvmbot wrote: /pull-request llvm/llvm-project#102895 https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 513c372 - [lldb][test] Break early when walking backtrace in concurrent tests
Author: David Spickett Date: 2024-08-12T13:18:10Z New Revision: 513c3726ebc0a324f7e5a11d25617bb9557324d6 URL: https://github.com/llvm/llvm-project/commit/513c3726ebc0a324f7e5a11d25617bb9557324d6 DIFF: https://github.com/llvm/llvm-project/commit/513c3726ebc0a324f7e5a11d25617bb9557324d6.diff LOG: [lldb][test] Break early when walking backtrace in concurrent tests We only need to see that 1 frame of the stack is in user code. No need to carry on looking. Doing so actually caused a test failure on Armv8 Ubuntu Jammy where a libc function does not have a display name. I'm sure I'm going to get stung by this elsewhere, but for this test, breaking early sidesteps the problem. Added: Modified: lldb/packages/Python/lldbsuite/test/concurrent_base.py Removed: diff --git a/lldb/packages/Python/lldbsuite/test/concurrent_base.py b/lldb/packages/Python/lldbsuite/test/concurrent_base.py index 46d71666d06977..a45b4207a834b4 100644 --- a/lldb/packages/Python/lldbsuite/test/concurrent_base.py +++ b/lldb/packages/Python/lldbsuite/test/concurrent_base.py @@ -290,6 +290,10 @@ def do_thread_actions( if funcname in f.GetDisplayFunctionName(): thread_has_user_code = True break + +if thread_has_user_code: +break + if thread_has_user_code: num_threads_with_usercode += 1 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
labath wrote: The new test is still hanging occasionally (e.g. https://lab.llvm.org/buildbot/#/builders/162/builds/3718/steps/6/logs/stdio ). I don't know if it's a problem with the code being tested, or the test itself, but it looks like there's still some issues here. https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
https://github.com/xusheng6 updated
https://github.com/llvm/llvm-project/pull/102873
>From 73c98df4baef99f96d9c67113ba2ed0d972e5a04 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 20 Mar 2023 20:24:11 +0800
Subject: [PATCH 1/5] [lldb] Claim to support swbreak and hwbreak packets when
debugging a gdbremote
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp| 2 +-
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..d8b17a8ff59baf 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
"multiprocess+", "fork-events+",
- "vfork-events+"};
+ "vfork-events+", "swbreak+",
"hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..b8fe8fdc9b8742 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,6 +2349,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// There is nothing needs to be done for swbreak or hwbreak since
+// the value is expected to be empty
} else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
>From 1db7c14528bb709921a0d6607c2118477c8de500 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 12 Aug 2024 18:37:40 +0800
Subject: [PATCH 2/5] Fix format
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp| 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d8b17a8ff59baf..83ba27783da471 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -352,8 +352,11 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector features = {"xmlRegisters=i386,arm,mips,arc",
- "multiprocess+", "fork-events+",
- "vfork-events+", "swbreak+",
"hwbreak+"};
+ "multiprocess+",
+ "fork-events+",
+ "vfork-events+",
+ "swbreak+",
+ "hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
>From 2d085e46c50f1c909ce5b90dd34d6c47832564d0 Mon Sep 17 00:00:00 2001
From: Xusheng
Date: Mon, 12 Aug 2024 18:43:36 +0800
Subject: [PATCH 3/5] Put the condition as the last one and update comment
---
.../source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index b8fe8fdc9b8742..e467577ab2b4f0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,13 +2349,14 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!value.getAsInteger(0, addressing_bits)) {
addressable_bits.SetHighmemAddressableBits(addressing_bits);
}
- } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
-// There is nothing needs to be done for swbreak or hwbreak since
-// the value is expected to be empty
} else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
+ } else if (key.compare("swbrea
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
xusheng6 wrote: > Also there is probably a qsupported parsing function in lldb-server, please > add a comment there to say we consume lldb's swbreak/hwbreak feature but it > doesn't change the behaviour of lldb-server. Fixed in https://github.com/llvm/llvm-project/pull/102873/commits/d0851d4ff7e049d0c7537f5bd6b72bd66b29474a https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
@@ -2353,6 +2353,10 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
uint32_t reg = UINT32_MAX;
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+// swbreak and hwbreak are also expected keys, but we don't need to
+// change our behaviour for them because lldb always expects the remote
+// to adjust the program counter (if relevant, e.g., for x86 targets)
xusheng6 wrote:
Fixed in
https://github.com/llvm/llvm-project/pull/102873/commits/d61ea562c82eab27ae2cc460b37f2f0d27961c1c
https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
xusheng6 wrote: > On the testing, lldb sends this qsupported value regardless of what the > remote sends. So there's nothing to do there unless you just repeat the > feature list. There are some tests that look at lldb-server's qsupported > values, but I don't see any for the client lldb. > > You could check we don't crash when seeing swbreak/hwbreak in a stop > response, but it's the same as checking we don't crash on any other unknown > key in there. Since you can't observe any behaviour change from > swbreak/hwbreak, so I don't think that has much value either. > > What you could do is write a test where lldb connects to a mock gdbserver > that claims to be an architecture where the correction to addresses would be > done by a gdb server (GDB's gdbserver I mean). Then via lldb check that the > stopped location is unmodified. You could then run the test with swbreak / > hwbreak / neither of those in the stop info and each time lldb will report > the value unchanged. > > So in the unlikely case that we decided to change how lldb behaves, the test > would fail. `lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py` > might be a good starting point. I totally agree with you that the first two cases, even if added, would not provide much value. For the third case, i.e., writing a mock gdbserver, I think that is quite a bit of work -- or do we already have some infra that I can use? By the way, do I always need some unit test change to get a PR accepted? In this particular case I do not see a compelling reason to add a new test, though if this is a LLVM development policy then I will start looking into it. https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
mbucko wrote:
potential division by 0
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+~SummaryInvocation() {
+ m_summary.OnInvoked();
+}
+ private:
+SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
mbucko wrote:
this should be `noexcept` if you're going to call with from dtor
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
https://github.com/mbucko requested changes to this pull request. https://github.com/llvm/llvm-project/pull/102708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+~SummaryInvocation() {
+ m_summary.OnInvoked();
+}
+ private:
+SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ SummaryStatisticsCache() = default;
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ lldb_private::SummaryStatistics
&GetSummaryStatisticsForProviderName(lldb_private::ConstString
summary_provider_name) {
+m_map_mutex.lock();
mbucko wrote:
Might be safer to use a lock guard here
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -408,3 +410,23 @@ llvm::json::Value DebuggerStats::ReportStatistics(
return std::move(global_stats);
}
+
+llvm::json::Value SummaryStatistics::ToJSON() const {
+ json::Object body {{
+ {"invocationCount", GetSummaryCount()},
+ {"totalTime", GetTotalTime()},
+ {"averageTime", GetAverageTime()}
+}};
+ return json::Object{{GetName().AsCString(), std::move(body)}};
+}
+
+
+json::Value SummaryStatisticsCache::ToJSON() {
+ m_map_mutex.lock();
mbucko wrote:
ditto
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
mbucko wrote:
`SummaryStatistics(const SummaryStatistics &rhs)`
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 82ee31f - [lldb] Updated lldb-server to spawn the child process and share socket (#101283)
Author: Dmitry Vasilyev
Date: 2024-08-12T18:50:23+04:00
New Revision: 82ee31f75ac1316006fa9e21dddfddec37cf7072
URL:
https://github.com/llvm/llvm-project/commit/82ee31f75ac1316006fa9e21dddfddec37cf7072
DIFF:
https://github.com/llvm/llvm-project/commit/82ee31f75ac1316006fa9e21dddfddec37cf7072.diff
LOG: [lldb] Updated lldb-server to spawn the child process and share socket
(#101283)
`lldb-server platform --server` works on Windows now w/o multithreading.
The rest functionality remains unchanged.
Fixes #90923, fixes #56346.
This is the part 1 of the replacement of #100670.
In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and
listen a common gdb port for all gdbserver connections. Then we can
remove gdb port mapping to fiх #97537.
Added:
Modified:
lldb/tools/lldb-server/lldb-platform.cpp
Removed:
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 7148a1d2a30941..82a3a0d6b4e51c 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,6 +22,7 @@
#include
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
@@ -32,8 +33,10 @@
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostGetOpt.h"
#include "lldb/Host/OptionParser.h"
+#include "lldb/Host/Socket.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Status.h"
using namespace lldb;
@@ -44,6 +47,108 @@ using namespace llvm;
// option descriptors for getopt_long_only()
+#ifdef _WIN32
+typedef pipe_t shared_fd_t;
+const shared_fd_t kInvalidSharedFD = LLDB_INVALID_PIPE;
+#else
+typedef NativeSocket shared_fd_t;
+const shared_fd_t kInvalidSharedFD = Socket::kInvalidSocketValue;
+#endif
+
+class SharedSocket {
+public:
+ SharedSocket(Connection *conn, Status &error) {
+m_fd = kInvalidSharedFD;
+
+const Socket *socket =
+static_cast(conn->GetReadObject().get());
+if (socket == nullptr) {
+ error = Status("invalid conn socket");
+ return;
+}
+
+#ifdef _WIN32
+m_socket = socket->GetNativeSocket();
+
+// Create a pipe to transfer WSAPROTOCOL_INFO to the child process.
+error = m_socket_pipe.CreateNew(true);
+if (error.Fail())
+ return;
+
+m_fd = m_socket_pipe.GetReadPipe();
+#else
+m_fd = socket->GetNativeSocket();
+error = Status();
+#endif
+ }
+
+ shared_fd_t GetSendableFD() { return m_fd; }
+
+ Status CompleteSending(lldb::pid_t child_pid) {
+#ifdef _WIN32
+// Transfer WSAPROTOCOL_INFO to the child process.
+m_socket_pipe.CloseReadFileDescriptor();
+
+WSAPROTOCOL_INFO protocol_info;
+if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) ==
+SOCKET_ERROR) {
+ int last_error = ::WSAGetLastError();
+ return Status("WSADuplicateSocket() failed, error: %d", last_error);
+}
+
+size_t num_bytes;
+Status error =
+m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info),
+ std::chrono::seconds(10), num_bytes);
+if (error.Fail())
+ return error;
+if (num_bytes != sizeof(protocol_info))
+ return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes",
+num_bytes);
+#endif
+return Status();
+ }
+
+ static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket) {
+#ifdef _WIN32
+socket = Socket::kInvalidSocketValue;
+// Read WSAPROTOCOL_INFO from the parent process and create NativeSocket.
+WSAPROTOCOL_INFO protocol_info;
+{
+ Pipe socket_pipe(fd, LLDB_INVALID_PIPE);
+ size_t num_bytes;
+ Status error =
+ socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info),
+ std::chrono::seconds(10), num_bytes);
+ if (error.Fail())
+return error;
+ if (num_bytes != sizeof(protocol_info)) {
+return Status(
+"socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes",
+num_bytes);
+ }
+}
+socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO,
+ FROM_PROTOCOL_INFO, &protocol_info, 0, 0);
+if (socket == INVALID_SOCKET) {
+ return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d",
+::WSAGetLastError());
+}
+return Status();
+#else
+socket = fd;
+return Status();
+#endif
+ }
+
+private:
+#ifdef _WIN32
+ Pipe m_socket_pipe;
+ NativeSocket m_socket;
+#endif
+ shared_fd_t m_fd;
+};
+
static int g_debug = 0;
static int g_verbose = 0;
static int g_server = 0;
@@ -60,6 +165,7 @@ static struct option g_long_options[] = {
{"max-gdbserver-port", required_argument, nullptr, 'M'},
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
DavidSpickett wrote: > or do we already have some infra that I can use? `lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py` is an example of that infrastructure. The `threadStopInfo` is where you would insert the swbreak hwbreak key. In fact, if i386 is an architecture where this correction of PC might happen (or not) you could just extend that test. The result should be the same whether the mock gdbserver returns "swbreak" "hwbreak" or leaves that field out completely. If i386 isn't an architecture where correction would be done, you should be able to find some other target xml stubs elsewhere in tests. I don't think this test case is i386 specific. It can be tricky because that XML needs to include a minimal set of registers before lldb will function, so if in doubt, copy paste a working one. > By the way, do I always need some unit test change to get a PR accepted? In > this particular case I do not see a compelling reason to add a new test, > though if this is a LLVM development policy then I will start looking into it. Not 100% of the time but you will always have to justify why it does not have tests, or has the sort of tests that it has. This justification is useful for auditing code later, it also serves as a guide to anyone trying to reproduce an old bug that has no test cases. https://llvm.org/docs/DeveloperPolicy.html#test-cases is written from the llvm perspective but applies to lldb as well https://lldb.llvm.org/resources/contributing.html#test-infrastructure. In this case because no one does lldb -> gdbserver testing within the llvm project, so my worry is that this "easy fix" will get forgotten over time and someone will remove it as dead code or forget it in a refactoring. This is not a case of "if it has no tests it will get deleted" but it will certainly be at higher risk. We have had cases like this before for example, someone fixed a bug handling an msp430 simulator and the test for that replays the packets that it sent to lldb so we don't have to have the actual simulator to hand. https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
DavidSpickett wrote: AArch64 for example would be: ``` aarch64 ``` https://github.com/llvm/llvm-project/pull/102873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Tolerate multiple compile units with the same DWO ID (PR #100577)
DavidSpickett wrote: The new test appears to be flaky on Windows: ``` # .---command stderr # | C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-hash-collision.s:22:10: error: CHECK: expected string not found in input # | # CHECK: 0xdeadbeefbaadf00d E multiple compile units with Dwo ID 0xdeadbeefbaadf00d # | ^ # | :14:20: note: scanning from here # | Dwo ID Err Dwo Path # |^ # | # | Input file: # | Check file: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-hash-collision.s # | # | -dump-input=help explains the following input dump. # | # | Input was: # | << # | . # | . # | . # | 9: }" # | 10: # | 11: (lldb) image dump separate-debug-info # | 12: Symbol file: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-hash-collision.s.tmp # | 13: Type: "dwo" # | 14: Dwo ID Err Dwo Path # | check:22X error: no match found # | 15: -- --- - # | check:22 ~ # | 16: 0xdeadbeefbaadf00d C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-hash-collision.s.tmp.dwp(foo0.dwo) # | check:22 ~ # | 17: 0xdeadbeefbaadf00d C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-hash-collision.s.tmp.dwp(foo1.dwo) # | check:22 ~ # | 18: (lldb) exit # | check:22 # | >> # `- # error: command failed with exit status: 1 -- ``` https://lab.llvm.org/buildbot/#/builders/141/builds/1497 It passed when it was first added https://lab.llvm.org/buildbot/#/builders/141/builds/1493. I'm going to disable it there and look into it tomorrow. https://github.com/llvm/llvm-project/pull/100577 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7027cc6 - [lldb][test] Diable dwp hash collision test on Windows
Author: David Spickett Date: 2024-08-12T15:09:18Z New Revision: 7027cc6a073cb5ae7a0ce04fa4a2dbe714615da9 URL: https://github.com/llvm/llvm-project/commit/7027cc6a073cb5ae7a0ce04fa4a2dbe714615da9 DIFF: https://github.com/llvm/llvm-project/commit/7027cc6a073cb5ae7a0ce04fa4a2dbe714615da9.diff LOG: [lldb][test] Diable dwp hash collision test on Windows This has been flaky on our Windows on Arm bot: https://lab.llvm.org/buildbot/#/builders/141/builds/1497 Despite passing when first landed. Added: Modified: lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s Removed: diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s index d626b4602ad58..7e106d6e9c2de 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s @@ -3,6 +3,9 @@ ## split unit from the DWP file. This can sometimes happen when the compile unit ## is nearly empty (e.g. because LTO has optimized all of it away). +# Is flaky on Windows on Arm. +# UNSUPPORTED: system-windows + # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp # RUN: %lldb %t -o "image lookup -t my_enum_type" \ ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid calling DataLayout constructor accepting Module pointer (NFC) (PR #102839)
https://github.com/s-barannikov updated
https://github.com/llvm/llvm-project/pull/102839
>From e3cdec63767738ca0e5fe640f9e01996ec1ecc22 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov
Date: Mon, 12 Aug 2024 03:53:28 +0300
Subject: [PATCH] [lldb] Avoid calling DataLayout constructor accepting Module
pointer (NFC)
The constructor initializes `*this` with a copy of `M->getDataLayout()`,
which can just be spelled as `DataLayout DL = M->getDataLayout()`.
In all places where the constructor is used, Module outlives DataLayout,
so store a reference to it instead of cloning.
---
lldb/source/Expression/IRInterpreter.cpp | 6 +++---
.../Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp | 7 +++
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp | 2 +-
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h | 4 ++--
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Expression/IRInterpreter.cpp
b/lldb/source/Expression/IRInterpreter.cpp
index 5b670067b5c433..030b30f070ee35 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -96,7 +96,7 @@ class InterpreterStackFrame {
typedef std::map ValueMap;
ValueMap m_values;
- DataLayout &m_target_data;
+ const DataLayout &m_target_data;
lldb_private::IRExecutionUnit &m_execution_unit;
const BasicBlock *m_bb = nullptr;
const BasicBlock *m_prev_bb = nullptr;
@@ -110,7 +110,7 @@ class InterpreterStackFrame {
lldb::ByteOrder m_byte_order;
size_t m_addr_byte_size;
- InterpreterStackFrame(DataLayout &target_data,
+ InterpreterStackFrame(const DataLayout &target_data,
lldb_private::IRExecutionUnit &execution_unit,
lldb::addr_t stack_frame_bottom,
lldb::addr_t stack_frame_top)
@@ -703,7 +703,7 @@ bool IRInterpreter::Interpret(llvm::Module &module,
llvm::Function &function,
s.c_str());
}
- DataLayout data_layout(&module);
+ const DataLayout &data_layout = module.getDataLayout();
InterpreterStackFrame frame(data_layout, execution_unit, stack_frame_bottom,
stack_frame_top);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
index bc0f5993aad0d6..defd72bbd93106 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
@@ -280,10 +280,9 @@ class Instrumenter {
IntegerType *GetIntptrTy() {
if (!m_intptr_ty) {
- llvm::DataLayout data_layout(&m_module);
-
- m_intptr_ty = llvm::Type::getIntNTy(m_module.getContext(),
- data_layout.getPointerSizeInBits());
+ m_intptr_ty = llvm::Type::getIntNTy(
+ m_module.getContext(),
+ m_module.getDataLayout().getPointerSizeInBits());
}
return m_intptr_ty;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index cc9bd14c6194e4..34461da46dfc7b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -1606,7 +1606,7 @@ bool IRForTarget::runOnModule(Module &llvm_module) {
lldb_private::Log *log(GetLog(LLDBLog::Expressions));
m_module = &llvm_module;
- m_target_data = std::make_unique(m_module);
+ m_target_data = &m_module->getDataLayout();
m_intptr_ty = llvm::Type::getIntNTy(m_module->getContext(),
m_target_data->getPointerSizeInBits());
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
index a924187ba04c06..45027fcd6fa492 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -331,9 +331,9 @@ class IRForTarget {
lldb_private::TypeFromParser m_result_type;
/// The module being processed, or NULL if that has not been determined yet.
llvm::Module *m_module = nullptr;
- /// The target data for the module being processed, or NULL if there is no
+ /// The target data for the module being processed, or nullptr if there is no
/// module.
- std::unique_ptr m_target_data;
+ const llvm::DataLayout *m_target_data = nullptr;
/// The DeclMap containing the Decls
lldb_private::ClangExpressionDeclMap *m_decl_map;
/// The address of the function CFStringCreateWithBytes, cast to the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+~SummaryInvocation() {
+ m_summary.OnInvoked();
+}
+ private:
+SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ SummaryStatisticsCache() = default;
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ lldb_private::SummaryStatistics
&GetSummaryStatisticsForProviderName(lldb_private::ConstString
summary_provider_name) {
+m_map_mutex.lock();
+if (m_summary_stats_map.count(summary_provider_name) == 0) {
+ m_summary_stats_map.emplace(summary_provider_name,
SummaryStatistics(summary_provider_name));
+}
+
+SummaryStatistics &summary_stats =
m_summary_stats_map.at(summary_provider_name);
+m_map_mutex.unlock();
+return summary_stats;
+ }
+
+ llvm::json::Value ToJSON();
+
+private:
+ std::map
m_summary_stats_map;
Michael137 wrote:
Does this need to be a `ConstString`? I guess the choice of `std::map` here is
to avoid iterator invalidations?
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
Michael137 wrote:
```suggestion
explicit SummaryStatistics(lldb_private::ConstString name) :
```
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+~SummaryInvocation() {
+ m_summary.OnInvoked();
+}
+ private:
+SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ SummaryStatisticsCache() = default;
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ lldb_private::SummaryStatistics
&GetSummaryStatisticsForProviderName(lldb_private::ConstString
summary_provider_name) {
+m_map_mutex.lock();
+if (m_summary_stats_map.count(summary_provider_name) == 0) {
Michael137 wrote:
I think you can just replace this whole implementation with a call to
`m_summary_stats_map.emplace(summary_provider_name, summary_provider_name)`.
Does feel a b it odd to use this name as the key but also have it on the
object. Could probably avoid having the name on the `SummaryStatistics` by just
passing the name through to `ToJSON`?
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
Michael137 wrote:
I think technically you don't need the copy-constructor here?
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -615,7 +615,16 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl
*summary_ptr,
m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
// the synthetic children being
// up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+SummaryStatistics &summary_stats = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryStatisticsForProvider(GetTypeName());
+/// Construct RAII types to time and collect data on summary creation.
+SummaryStatistics::SummaryInvocation summary_inv(summary_stats);
Michael137 wrote:
Do we strictly need `SummaryInvocation`? It just increments the type summary
call count, but in a somewhat convoluted way (in my opinion). I guess if we
ever needed to add more things to these stats it would be useful?
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+~SummaryInvocation() {
+ m_summary.OnInvoked();
+}
+ private:
+SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ SummaryStatisticsCache() = default;
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ lldb_private::SummaryStatistics
&GetSummaryStatisticsForProviderName(lldb_private::ConstString
summary_provider_name) {
+m_map_mutex.lock();
+if (m_summary_stats_map.count(summary_provider_name) == 0) {
+ m_summary_stats_map.emplace(summary_provider_name,
SummaryStatistics(summary_provider_name));
+}
+
+SummaryStatistics &summary_stats =
m_summary_stats_map.at(summary_provider_name);
+m_map_mutex.unlock();
+return summary_stats;
Michael137 wrote:
You're handing out non-const references to objects stored inside this map
that's accessed from multiple threads. I guess it's fine for now since you're
only mutating these objects under the reference using atomics.
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -25,6 +26,7 @@ namespace lldb_private {
using StatsClock = std::chrono::high_resolution_clock;
using StatsTimepoint = std::chrono::time_point;
+using Duration = std::chrono::duration;
Michael137 wrote:
Is this used anywhere?
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name),
m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+return m_total_time.get().count() /
m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+~SummaryInvocation() {
+ m_summary.OnInvoked();
+}
+ private:
+SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ SummaryStatisticsCache() = default;
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ lldb_private::SummaryStatistics
&GetSummaryStatisticsForProviderName(lldb_private::ConstString
summary_provider_name) {
+m_map_mutex.lock();
Michael137 wrote:
`std::scoped_lock`
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
felipepiovezan wrote: Isn't this going to degrade the performance of _all_ negative queries? I don't remember off the top of my head what is "m_fallback" when we have an accelerator table, but this worries me. https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -918,3 +920,24 @@ def test_order_of_options_do_not_matter(self):
debug_stats_1,
f"The order of options '{options[0]}' and '{options[1]}'
should not matter",
)
+
+def test_summary_statistics_providers(self):
+"""
+Test summary timing statistics is included in statistics dump when
+a type with a summary provider exists, and is evaluated.
+"""
+
+self.build()
+target = self.createTestTarget()
+lldbutil.run_to_source_breakpoint(
+self, "// stop here", lldb.SBFileSpec("main.cpp")
+)
+self.expect("frame var", substrs=["hello world"])
+stats = self.get_target_stats(self.get_stats())
+self.assertIn("summaryProviderStatistics", stats)
+summary_providers = stats["summaryProviderStatistics"]
+# We don't want to take a dependency on the type name, so we just look
+# for string and that it was called once.
+summary_provider_str = str(summary_providers)
+self.assertIn("string", summary_provider_str)
+self.assertIn("'invocationCount': 1", summary_provider_str)
Michael137 wrote:
can we check existence of `totalTime` and `averageTime`?
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -1,7 +1,13 @@
// Test that the lldb command `statistics` works.
+#include
+
+void foo() {
+ std::string str = "hello world";
Michael137 wrote:
instead of relying on `std::string` formatter should we add a type summary for
a dummy type? And test against that? Feels a bit more hermetic
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid calling DataLayout constructor accepting Module pointer (NFC) (PR #102839)
https://github.com/Michael137 approved this pull request. Lifetimes look correct to me, thanks! https://github.com/llvm/llvm-project/pull/102839 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -615,7 +615,16 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl
*summary_ptr,
m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
// the synthetic children being
// up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+SummaryStatistics &summary_stats = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
Michael137 wrote:
This should be:
```suggestion
if (auto target_sp = GetTargetSP())
SummaryStatistics &summary_stats =
target_sp->GetSummaryStatisticsForProvider(GetTypeName())
```
etc.
Not sure if we're guaranteed a `ProcessSP` and `TargetSP` here (which might be
the cause of the PR test failures)
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
labath wrote: Sort of yes (*), but only so much as it is necessary to make it correct, and the level of slowdown depends on how much non-indexed code you have. The fallback index will only index those units that aren't covered by the debug_names index, so if debug_names covers everything, the fallback is a noop -- if there's one non-indexed unit, you only pay the cost of indexing that unit. (*) except not really. The most expensive fallback operation is building the manual index (afterwards, it may even be faster than debug_names), and this happens first time that anything queries it. Since all of the other debug_names methods are already calling the fallback index, it will most likely be already built by the time we get to this point and the fallback call should be fast. https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
Michael137 wrote: Stats around this area could definitely be interesting to explore! https://github.com/llvm/llvm-project/pull/102708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Tolerate multiple compile units with the same DWO ID (PR #100577)
adrian-prantl wrote: @labath This test fails in two configs on the matrix bot: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/602/testReport/junit/lldb-shell/SymbolFile_DWARF_x86/Test_Clang_17_0_6___dwp_hash_collision_s/ https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/602/testReport/junit/lldb-shell/SymbolFile_DWARF_x86/Test_DWARF2___dwp_hash_collision_s/ https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/602/ Can you figure out a way to skip the test there or otherwise make it robust against these config changes? https://github.com/llvm/llvm-project/pull/100577 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Emit declarations along with variables (PR #74865)
vogelsgesang wrote: FYI @walter-erquinigo: There is a proposal under discussion to add first-class support for `declarationLocation` (and also `valueLocation`) to the debug adapter protocol. See https://github.com/microsoft/debug-adapter-protocol/issues/343 https://github.com/llvm/llvm-project/pull/74865 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
felipepiovezan wrote: > The fallback index will only index those units that aren't covered by the > debug_names index, so if debug_names covers everything, the fallback is a noop Got it, this is what I was hoping that would happen! Thanks for explaining it. (future ideas) This makes me wonder if a better design would have been to make whoever owns this instances of Index classes have instead a collection of these indices, instead of having an _implementation_ of an index have to remember to call a fallback mechanism. To answer this question with another question: since Index classes are not allowed to have false negatives / positives, we would never need more than one index (for a give CU). Which begs the question of why we have a design that allows us to reach the problem this PR is fixing? It feels off that we have two objects: a manual index and a dwarf index, and that the dwarf index has to remember to call the manual index when the dwarf index is empty. https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Emit declarations along with variables (PR #74865)
walter-erquinigo wrote: > FYI @walter-erquinigo: There is a proposal under discussion to add > first-class support for `declarationLocation` (and also `valueLocation`) to > the debug adapter protocol. See > [microsoft/debug-adapter-protocol#343](https://github.com/microsoft/debug-adapter-protocol/issues/343) Man, that's amazing. I really hope that get merged https://github.com/llvm/llvm-project/pull/74865 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
labath wrote: It just so happens that I got a `memory find` bug today, which made me realize just how broken the current behavior is -- it will effectively abort the search as soon as it runs into any unreadable block of memory (plus it also reads one byte at a time, which makes it excruciatingly slow for large ranges). Now, this patch fixes both of those problems for core files (which implement the required APIs), but it still leaves the operation for live processes in its current, broken, state. This got me thinking whether it would be possible to implement something like this PR in a way that benefits all implementations -- and I think the answer to that question is "yes". The part where you skip over the holes in memory is implementable using the existing GetMemoryRegion API which all processes implement (or should implement). And we can certainly do something to avoid copying in the ReadMemory calls. Probably the best think to do would be to have a `ReadMemory` overload which returns a DataExtractor. This will give individual implementations the choice of how to return the memory. Implementations (like the core files, but maybe not all of them) which have the memory available already, can just return a DataExtractor referencing existing memory, while others (most live processes) can create a new DataBuffer object to back the DataExtractor. I think that would give us most of the performance of the current implementation for core files, and also significantly improve the live process situation. (And we'd also have a single implementation). What to you (all) think? Note that this isn't something I expect you must do on your own. Since this is something that I now need to do on my own anyway, I'd be happy to do some or all of this work myself. https://github.com/llvm/llvm-project/pull/102536 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
labath wrote: > So the main idea is: if either ObjectFile or PostMortemProcess can return a > reference to the data in the Peek calls, then should as long as they have > this data mapped into the LLDB address space and can hand out a pointer to > the data. If they can't, we should fall back to something that can copy the > data into a buffer as mentioned with `CopyData` or `ReadMemory`. Core files > or object files might compress up their data, in which case we wouldn't be > able to hande out a buffer from calls to Peek, but we would need to unzip the > buffer into the buffer supplied by CopyData and ReadMemory on the fly. BTW, we have another bug where the person wants to use lldb to open `/proc/kcore` (which is a special file, which pretends to be a (ELF) core file, but actually points to the live memory of the system). This core file cannot be mmapped, because a) the kernel won't let us, b) it's as big as the virtual address space (128TB for me) of the kernel. It works fine with gdb, but lldb chokes when it tries to mmap it. Anyway, I'm not going to work on this any time soon, just thought you might want to hear about another use case where its not possible to return a reference to existing data. https://github.com/llvm/llvm-project/pull/102536 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Provide `declarationLocation` for variables (PR #102928)
https://github.com/vogelsgesang created
https://github.com/llvm/llvm-project/pull/102928
`declarationLocation` is about to become part of the upstream debug adapter
protocol (see microsoft/debug-adapter-protocol#343). This is a draft
implementation, to be finalized and merged after the corresponding changes were
merged into DAP.
TODO:
* Adjust comment on `CreateVariable` function with updated jsonschema as soon
as the upstream changes were merged to DAP.
* Update based on final protocol merged to DAP
>From 5c3b3458eb3426e58ead2d66f0cc9530eb368dd3 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang
Date: Sat, 10 Aug 2024 23:59:55 +
Subject: [PATCH] [lldb-dap] Provide `declarationLocation` for variables
TODO:
* Adjust comment on `CreateVariable` function with updated jsonschema as
soon as the upstream changes were merged to DAP.
`declarationLocation` is about to become part of the upstream
debug-adapter-protocol. This is a draft implementation, to be finalized
and merged after the corresponding changes were merged into DAP.
---
.../lldb-dap/variables/TestDAP_variables.py| 4
lldb/tools/lldb-dap/JSONUtils.cpp | 18 --
lldb/tools/lldb-dap/JSONUtils.h| 14 --
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 3c6901b2fd99a5..7d982e801f741e 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -168,6 +168,10 @@ def do_test_scopes_variables_setVariable_evaluate(
"type": "int",
"value": "1",
},
+"declarationLocation": {
+"equals": {"line": 12, "column": 14},
+"contains": {"path": ["lldb-dap", "variables",
"main.cpp"]},
+},
"$__lldb_extensions": {
"equals": {
"value": "1",
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp
b/lldb/tools/lldb-dap/JSONUtils.cpp
index a8b85f55939e17..3e0c1076643260 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -614,9 +614,8 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint
&bp) {
// }
// }
// }
-llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
+llvm::json::Value CreateSource(const lldb::SBFileSpec &file) {
llvm::json::Object object;
- lldb::SBFileSpec file = line_entry.GetFileSpec();
if (file.IsValid()) {
const char *name = file.GetFilename();
if (name)
@@ -630,6 +629,10 @@ llvm::json::Value CreateSource(lldb::SBLineEntry
&line_entry) {
return llvm::json::Value(std::move(object));
}
+llvm::json::Value CreateSource(const lldb::SBLineEntry &line_entry) {
+ return CreateSource(line_entry.GetFileSpec());
+}
+
llvm::json::Value CreateSource(llvm::StringRef source_path) {
llvm::json::Object source;
llvm::StringRef name = llvm::sys::path::filename(source_path);
@@ -1253,6 +1256,17 @@ llvm::json::Value CreateVariable(lldb::SBValue v,
int64_t variablesReference,
else
object.try_emplace("variablesReference", (int64_t)0);
+ if (lldb::SBDeclaration decl = v.GetDeclaration(); decl.IsValid()) {
+llvm::json::Object decl_obj;
+decl_obj.try_emplace("source", CreateSource(decl.GetFileSpec()));
+if (int line = decl.GetLine())
+ decl_obj.try_emplace("line", line);
+if (int column = decl.GetColumn())
+ decl_obj.try_emplace("column", column);
+
+object.try_emplace("declarationLocation", std::move(decl_obj));
+ }
+
object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
return llvm::json::Value(std::move(object));
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 1515f5ba2e5f4d..610f920952e77c 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -282,6 +282,16 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
int64_t variablesReference,
int64_t namedVariables, bool expensive);
+/// Create a "Source" JSON object as described in the debug adaptor definition.
+///
+/// \param[in] file
+/// The SBFileSpec to use when populating out the "Source" object
+///
+/// \return
+/// A "Source" JSON object that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateSource(const lldb::SBFileSpec &file);
+
/// Create a "Source" JSON object as described in the debug adaptor definition.
///
/// \param[in] line_entry
@@ -289,9 +299,9 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
/// object
///
/// \return
-/// A "Source" JSON object with that follows the formal JSON
+/// A "Source" JSON object that follows the formal JSON
///
[Lldb-commits] [lldb] [lldb-dap] Provide `declarationLocation` for variables (PR #102928)
vogelsgesang wrote: @walter-erquinigo I took a first stab at implementing the DAP proposal. Seemed rather straightforward. I would love to hear if I missed anything. Also, I am not sure how to implement `valueLocation`, i.e., a the source location referecend by the value. E.g., for a function pointer, this location should point to the source location where the corresponding function was implemented. If you have any idea on how to do so, I would be interested in hearing about it 🙂 https://github.com/llvm/llvm-project/pull/102928 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 38b67c5 - Revert "[lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (#102123)"
Author: Pavel Labath
Date: 2024-08-12T18:36:18+02:00
New Revision: 38b67c54ed858f60c0caebcfba4b61f9326684ca
URL:
https://github.com/llvm/llvm-project/commit/38b67c54ed858f60c0caebcfba4b61f9326684ca
DIFF:
https://github.com/llvm/llvm-project/commit/38b67c54ed858f60c0caebcfba4b61f9326684ca.diff
LOG: Revert "[lldb/DWARF] Search fallback to the manual index in
GetFullyQualified… (#102123)"
The test appears to be flaky. Revert it while I investigate.
This reverts commits 7027cc6a073cb5ae7a0ce04fa4a2dbe714615da9 and
21ef272ec1974244710fc639f98674eae3f8b02c.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
Removed:
lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 32d8a92305aafa..7e66b3dccf97fa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -371,7 +371,6 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
!ProcessEntry(entry, callback))
return;
}
- m_fallback.GetFullyQualifiedType(context, callback);
}
bool DebugNamesDWARFIndex::SameParentChain(
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
index 7e106d6e9c2de4..d626b4602ad58f 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
@@ -3,9 +3,6 @@
## split unit from the DWP file. This can sometimes happen when the compile
unit
## is nearly empty (e.g. because LTO has optimized all of it away).
-# Is flaky on Windows on Arm.
-# UNSUPPORTED: system-windows
-
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp
# RUN: %lldb %t -o "image lookup -t my_enum_type" \
diff --git
a/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
deleted file mode 100644
index 71da8fad001652..00
---
a/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
+++ /dev/null
@@ -1,35 +0,0 @@
-REQUIRES: lld, python
-
-RUN: split-file %s %t
-RUN: %clang --target=x86_64-pc-linux -g -gpubnames -c %t/file1.c -o %t-1.o
-RUN: %clang --target=x86_64-pc-linux -g -gno-pubnames -c %t/file2.c -o %t-2.o
-RUN: llvm-dwarfdump %t-1.o --debug-names | FileCheck %s --check-prefix=PUBNAMES
-RUN: llvm-dwarfdump %t-2.o --debug-names | FileCheck %s
--check-prefix=NOPUBNAMES
-RUN: ld.lld %t-1.o %t-2.o -o %t.out
-RUN: %lldb %t.out -s %t/commands -o exit | FileCheck %s
-
-// Precondition check: The first file should contain a debug_names index, but
no
-// entries for MYSTRUCT.
-PUBNAMES: Name Index @ 0x0 {
-PUBNAMES-NOT: MYSTRUCT
-
-// The second file should not contain an index.
-NOPUBNAMES-NOT: Name Index
-
-// Starting from the variable in the first file, we should be able to find the
-// declaration of the type in the first unit, and then match that with the
-// definition in the second unit.
-CHECK: (lldb) script
-CHECK: struct MYSTRUCT {
-CHECK-NEXT: int x;
-CHECK-NEXT: }
-
-#--- commands
-script
lldb.target.FindFirstGlobalVariable("struct_ptr").GetType().GetPointeeType()
-#--- file1.c
-struct MYSTRUCT *struct_ptr;
-#--- file2.c
-struct MYSTRUCT {
- int x;
-};
-struct MYSTRUCT struct_;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Tolerate multiple compile units with the same DWO ID (PR #100577)
labath wrote: This appears to be the same issue that David reported. I'm going to revert this while I investigate. I doubt it's a compiler thing as the test does not actually compile anything. It's more likely that something in the code is flaky (and I think I have an idea what could that be). https://github.com/llvm/llvm-project/pull/100577 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b3ed1d9 - Reapply "[lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (#102123)"
Author: Pavel Labath
Date: 2024-08-12T18:41:28+02:00
New Revision: b3ed1d92112e0f455f8ef0888ef4c5d0ca29096d
URL:
https://github.com/llvm/llvm-project/commit/b3ed1d92112e0f455f8ef0888ef4c5d0ca29096d
DIFF:
https://github.com/llvm/llvm-project/commit/b3ed1d92112e0f455f8ef0888ef4c5d0ca29096d.diff
LOG: Reapply "[lldb/DWARF] Search fallback to the manual index in
GetFullyQualified… (#102123)"
This reverts commit 38b67c54ed858f60c0caebcfba4b61f9326684ca.
I reverted the wrong patch -- sorry :(
Added:
lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 7e66b3dccf97fa..32d8a92305aafa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -371,6 +371,7 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
!ProcessEntry(entry, callback))
return;
}
+ m_fallback.GetFullyQualifiedType(context, callback);
}
bool DebugNamesDWARFIndex::SameParentChain(
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
index d626b4602ad58f..7e106d6e9c2de4 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
@@ -3,6 +3,9 @@
## split unit from the DWP file. This can sometimes happen when the compile
unit
## is nearly empty (e.g. because LTO has optimized all of it away).
+# Is flaky on Windows on Arm.
+# UNSUPPORTED: system-windows
+
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp
# RUN: %lldb %t -o "image lookup -t my_enum_type" \
diff --git
a/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
new file mode 100644
index 00..71da8fad001652
--- /dev/null
+++
b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test
@@ -0,0 +1,35 @@
+REQUIRES: lld, python
+
+RUN: split-file %s %t
+RUN: %clang --target=x86_64-pc-linux -g -gpubnames -c %t/file1.c -o %t-1.o
+RUN: %clang --target=x86_64-pc-linux -g -gno-pubnames -c %t/file2.c -o %t-2.o
+RUN: llvm-dwarfdump %t-1.o --debug-names | FileCheck %s --check-prefix=PUBNAMES
+RUN: llvm-dwarfdump %t-2.o --debug-names | FileCheck %s
--check-prefix=NOPUBNAMES
+RUN: ld.lld %t-1.o %t-2.o -o %t.out
+RUN: %lldb %t.out -s %t/commands -o exit | FileCheck %s
+
+// Precondition check: The first file should contain a debug_names index, but
no
+// entries for MYSTRUCT.
+PUBNAMES: Name Index @ 0x0 {
+PUBNAMES-NOT: MYSTRUCT
+
+// The second file should not contain an index.
+NOPUBNAMES-NOT: Name Index
+
+// Starting from the variable in the first file, we should be able to find the
+// declaration of the type in the first unit, and then match that with the
+// definition in the second unit.
+CHECK: (lldb) script
+CHECK: struct MYSTRUCT {
+CHECK-NEXT: int x;
+CHECK-NEXT: }
+
+#--- commands
+script
lldb.target.FindFirstGlobalVariable("struct_ptr").GetType().GetPointeeType()
+#--- file1.c
+struct MYSTRUCT *struct_ptr;
+#--- file2.c
+struct MYSTRUCT {
+ int x;
+};
+struct MYSTRUCT struct_;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a0c57a0 - Revert "[lldb] Tolerate multiple compile units with the same DWO ID (#100577)"
Author: Pavel Labath
Date: 2024-08-12T18:43:16+02:00
New Revision: a0c57a0f3c6b44ce8f2c7222d0932df85883cf06
URL:
https://github.com/llvm/llvm-project/commit/a0c57a0f3c6b44ce8f2c7222d0932df85883cf06
DIFF:
https://github.com/llvm/llvm-project/commit/a0c57a0f3c6b44ce8f2c7222d0932df85883cf06.diff
LOG: Revert "[lldb] Tolerate multiple compile units with the same DWO ID
(#100577)"
The test appears to be flaky. Revert it while I investigate.
This reverts commits 32a62ebdeab0c10d5311cf812e021717636d4514 and
7027cc6a073cb5ae7a0ce04fa4a2dbe714615da9.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
Removed:
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 0a52159d055bb4..66a762bf9b6854 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -97,14 +97,12 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
*m_dwo_id, m_first_die.GetOffset()));
return; // Can't fetch the compile unit from the dwo file.
}
-
- // Link the DWO unit to this object, if it hasn't been linked already (this
- // can happen when we have an index, and the DWO unit is parsed first).
- if (!dwo_cu->LinkToSkeletonUnit(*this)) {
-SetDwoError(Status::createWithFormat(
-"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
-return;
- }
+ // If the skeleton compile unit gets its unit DIE parsed first, then this
+ // will fill in the DWO file's back pointer to this skeleton compile unit.
+ // If the DWO files get parsed on their own first the skeleton back link
+ // can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
+ // do a reverse lookup and cache the result.
+ dwo_cu->SetSkeletonUnit(this);
DWARFBaseDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
if (!dwo_cu_die.IsValid()) {
@@ -720,11 +718,13 @@ DWARFCompileUnit *DWARFUnit::GetSkeletonUnit() {
return llvm::dyn_cast_or_null(m_skeleton_unit);
}
-bool DWARFUnit::LinkToSkeletonUnit(DWARFUnit &skeleton_unit) {
- if (m_skeleton_unit && m_skeleton_unit != &skeleton_unit)
-return false;
- m_skeleton_unit = &skeleton_unit;
- return true;
+void DWARFUnit::SetSkeletonUnit(DWARFUnit *skeleton_unit) {
+ // If someone is re-setting the skeleton compile unit backlink, make sure
+ // it is setting it to a valid value when it wasn't valid, or if the
+ // value in m_skeleton_unit was valid, it should be the same value.
+ assert(skeleton_unit);
+ assert(m_skeleton_unit == nullptr || m_skeleton_unit == skeleton_unit);
+ m_skeleton_unit = skeleton_unit;
}
bool DWARFUnit::Supports_DW_AT_APPLE_objc_complete_type() {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 209104fe3a054e..85c37971ced8e0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -170,7 +170,7 @@ class DWARFUnit : public UserID {
/// both cases correctly and avoids crashes.
DWARFCompileUnit *GetSkeletonUnit();
- bool LinkToSkeletonUnit(DWARFUnit &skeleton_unit);
+ void SetSkeletonUnit(DWARFUnit *skeleton_unit);
bool Supports_DW_AT_APPLE_objc_complete_type();
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
deleted file mode 100644
index 7e106d6e9c2de4..00
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
+++ /dev/null
@@ -1,145 +0,0 @@
-## Test that lldb handles (mainly, that it doesn't crash) the situation where
-## two skeleton compile units have the same DWO ID (and try to claim the same
-## split unit from the DWP file. This can sometimes happen when the compile
unit
-## is nearly empty (e.g. because LTO has optimized all of it away).
-
-# Is flaky on Windows on Arm.
-# UNSUPPORTED: system-windows
-
-# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t
-# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp
-# RUN: %lldb %t -o "image lookup -t my_enum_type" \
-# RUN: -o "image dump separate-debug-info" -o exit | FileCheck %s
-
-## Check that we're able to access the type within the split unit (no matter
-## which skeleton unit it ends up associated with). Completely ignoring the
unit
-## might also be reasonable.
-# CHECK: image lookup -t my_enum_type
-# CHECK: 1 match found
-# CHECK: name = "my_enum_type", byte-size = 4, compiler_type = "enum
my_enum_type {
-# CHECK-NEXT: }"
-#
-## Check that we get some indication of the error.
-# CHECK: image dump separate-debug-info
-# CHECK: Dwo ID Err Dwo Path
-# CHECK: 0xdeadbeefbaadf00d E multiple compile units with Dwo
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
Jlalond wrote: @labath those did get omitted due to github marking them as outdated due to moving the code around, I mixed that. I appreciate your long review on this one, I learned a lot from your feedback. I'm going to let CI run and merge when it's completed https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -615,7 +615,16 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl
*summary_ptr,
m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
// the synthetic children being
// up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+SummaryStatistics &summary_stats = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryStatisticsForProvider(GetTypeName());
+/// Construct RAII types to time and collect data on summary creation.
+SummaryStatistics::SummaryInvocation summary_inv(summary_stats);
Jlalond wrote:
This is actually the biggest reason I pushed these changes 'as is', because I
wanted feedback on this. The goal here was to make the increment of timing as
simple as possible.
Elapsed time handles it's scope by it's lifetime, and I wanted to mirror that.
In the future we may want to add more side-effects and data collection when a
summary-call is complete. Such as giving the user a warning if a formatter was
so many multiples off the average.
Where I'm not sure is if we should make this one RAII object with an elapsed
object internal to it's lifetime so you get all the scoping side effects rolled
up into one
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/102708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
labath wrote: The current setup makes sense to me, but I guess that's expected as I'm the one who created it. I can also imagine something like what you propose, but it doesn't seem like a clear win to me. These objects are owned by SymbolFileDWARF, and we probably don't want to have it do the work of juggling these (it has a lot on its plate already), so it would probably have to be a separate object (basically another implementation of the "dwarf index" interface). That would be a lot of boilerplate (though maybe we could use some template trickery to reduce it). We'd also need to come up with a less ad-hoc way communicate which things are supposed to be indexed by who, but we also wouldn't want to make it too generic (== more code), since there are basically only three index configurations we care about (and these could easily be reduced to two). https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
felipepiovezan wrote:
> The current setup makes sense to me, but I guess that's expected as I'm the
> one who created it. I can also imagine something like what you propose, but
> it doesn't seem like a clear win to me. These objects are owned by
> SymbolFileDWARF, and we probably don't want to have it do the work of
> juggling these (it has a lot on its plate already), so it would probably have
> to be a separate object (basically another implementation of the "dwarf
> index" interface). That would be a lot of boilerplate (though maybe we could
> use some template trickery to reduce it). We'd also need to come up with a
> less ad-hoc way communicate which things are supposed to be indexed by who,
> but we also wouldn't want to make it too generic (== more code), since there
> are basically only three index configurations we care about (and these could
> easily be reduced to two).
I guess what I was proposing was more about deleting code than adding it. We
must always have at most _one_ index, but we have a lot of code allowing for
the situation of two indices (manual & {apple, dwarf}).
https://github.com/llvm/llvm-project/pull/102123
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/augusto2112 requested changes to this pull request. I think overall this looks good! Just some small things to fix. Could you run `ninja check-lldb` from your build folder to make sure this change doesn't break anything? https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -799,20 +803,21 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
-}
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name,
lldb::eFunctionNameTypeFull,
- function_options, sc_list);
+ sc.module_sp->FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
-if (sc.target_sp) {
+{
augusto2112 wrote:
Since you removed the check here, you can remove the parenthesis as well.
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/augusto2112 edited https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const std::vector &names, return LLDB_INVALID_ADDRESS; } + ModuleList images = target->GetImages(); augusto2112 wrote: Check if `target` is null before accessing it. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const std::vector &names, return LLDB_INVALID_ADDRESS; } + ModuleList images = target->GetImages(); augusto2112 wrote: Oh, looks like target is already checked right before this, disregard this comment https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
DmT021 wrote:
It was already checked 4 lines above
```
if (!target) {
// We shouldn't be doing any symbol lookup at all without a target.
return LLDB_INVALID_ADDRESS;
}
ModuleList images = target->GetImages();
```
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
