[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
DavidSpickett wrote: The test content is fine, but I was assuming that we'd extend the existing `TestStopPCs.py` to do the same thing in less code. If that test had 1 more thread, you could have normal / swbreak / hwbreak threads in one test case. 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: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 55d7e59023bc48f97321970cda5e400c07de59fa...55f9473b8832723b96db45103bf4d5aa0b10da90 lldb/test/API/functionalities/gdb_remote_client/TestSwbreak.py `` View the diff from darker here. ``diff --- TestSwbreak.py 2024-08-13 06:54:56.00 + +++ TestSwbreak.py 2024-08-13 09:10:48.011135 + @@ -46,10 +46,11 @@ self.assertEqual(th0.GetThreadID(), 0x1FF0D) self.assertEqual(th1.GetThreadID(), 0x2FF0D) self.assertEqual(th0.GetFrameAtIndex(0).GetPC(), 0x10001BC00) self.assertEqual(th1.GetFrameAtIndex(0).GetPC(), 0x10002BC00) + class TestHwbreak(GDBRemoteTestBase): @skipIfXmlSupportMissing def test(self): class MyResponder(MockGDBServerResponder): def haltReason(self): `` 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] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
slydiman wrote: See the replacement #101283. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/100670 ___ 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)
mbucko 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. This would be quite the rework of the FindInMemory() API. Would it be worth creating an Issue and move the discussion there to unblock this PR? 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] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -285,7 +285,7 @@ class ModuleList {
/// \see Module::FindFunctions ()
void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
const ModuleFunctionSearchOptions &options,
- SymbolContextList &sc_list) const;
+ const SymbolContext &sc, SymbolContextList &sc_list)
const;
DmT021 wrote:
> How about we call sc, search_first instead, or something more
> self-documenting like that? (which is what FindTypes also does).
The word "hint" came up a couple times in the discussion as well. Maybe
`search_hint` ?
> could we add documentation to the function describing what the parameter does?
Sure.
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/DmT021 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] [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 cdd4b8ce09fb536defb3df182e8b5ebf16ada685 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
---
.../gdb-remote/GDBRemoteCommunicationClient.cpp | 7 +--
.../GDBRemoteCommunicationServerLLGS.cpp | 4
.../Process/gdb-remote/ProcessGDBRemote.cpp | 3 +++
.../gdb_remote_client/TestStopPCs.py | 15 +++
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..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+"};
+ "multiprocess+",
+ "fork-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/GDBRemoteCommunicationServerLLGS.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index a0b08a219ae147..c0c49e0382f6bd 100644
---
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -4261,6 +4261,10 @@ std::vector
GDBRemoteCommunicationServerLLGS::HandleFeatures(
for (auto &x : m_debugged_processes)
SetEnabledExtensions(*x.second.process_up);
+
+ // We consume lldb's swbreak/hwbreak feature, but it doesn't change the
+ // behaviour of lldb-server. We always adjust the program counter for targets
+ // like x86
return ret;
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..c7ce368ab41ce2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2354,6 +2354,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
}
+ // 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)
}
if (stop_pid != LLDB_INVALID_PROCESS_ID && stop_pid != pid) {
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
index ef28cc95f7ad4b..3faae5fec38ba1 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
@@ -10,13 +10,17 @@ class TestStopPCs(GDBRemoteTestBase):
def test(self):
class MyResponder(MockGDBServerResponder):
def haltReason(self):
-return
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+# lldb should treat the default halt reason, hwbreak and
swbreak in the same way. Which is that it
+# expects the stub to have corrected the PC already, so lldb
should not modify it further.
+return
"T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
def threadStopInfo(self, threadnum):
if threadnum == 0x1FF0D:
-return
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+return
"T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
if threadnum == 0x2FF0D:
-return
"T00thread:2ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+return
"T00swbreak:;thread:2ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
+if threadnum == 0x3FF0D:
+return
"T00hwbreak:;thread:3ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
def qXferRead(self, obj, annex, offset, length):
[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 51a356b384f28cce3b2cd874fbba139414b0e207 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
---
.../gdb-remote/GDBRemoteCommunicationClient.cpp | 7 +--
.../GDBRemoteCommunicationServerLLGS.cpp | 4
.../Process/gdb-remote/ProcessGDBRemote.cpp | 3 +++
.../gdb_remote_client/TestStopPCs.py | 15 +++
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..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+"};
+ "multiprocess+",
+ "fork-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/GDBRemoteCommunicationServerLLGS.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index a0b08a219ae147..345f5cd5de8491 100644
---
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -4245,6 +4245,10 @@ std::vector
GDBRemoteCommunicationServerLLGS::HandleFeatures(
.Case("vfork-events+", Extension::vfork)
.Default({});
+ // We consume lldb's swbreak/hwbreak feature, but it doesn't change the
+ // behaviour of lldb-server. We always adjust the program counter for targets
+ // like x86
+
m_extensions_supported &= plugin_features;
// fork & vfork require multiprocess
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..c7ce368ab41ce2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2354,6 +2354,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
}
+ // 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)
}
if (stop_pid != LLDB_INVALID_PROCESS_ID && stop_pid != pid) {
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
index ef28cc95f7ad4b..3faae5fec38ba1 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
@@ -10,13 +10,17 @@ class TestStopPCs(GDBRemoteTestBase):
def test(self):
class MyResponder(MockGDBServerResponder):
def haltReason(self):
-return
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+# lldb should treat the default halt reason, hwbreak and
swbreak in the same way. Which is that it
+# expects the stub to have corrected the PC already, so lldb
should not modify it further.
+return
"T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
def threadStopInfo(self, threadnum):
if threadnum == 0x1FF0D:
-return
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+return
"T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
if threadnum == 0x2FF0D:
-return
"T00thread:2ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+return
"T00swbreak:;thread:2ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
+if threadnum == 0x3FF0D:
+return
"T00hwbreak:;thread:3ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
@@ -10,13 +10,15 @@ class TestStopPCs(GDBRemoteTestBase): def test(self): class MyResponder(MockGDBServerResponder): def haltReason(self): -return "T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;" +return "T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;" def threadStopInfo(self, threadnum): if threadnum == 0x1FF0D: xusheng6 wrote: Now fixed in https://github.com/llvm/llvm-project/commit/51a356b384f28cce3b2cd874fbba139414b0e207 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)
@@ -3594,6 +3594,9 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char
*p) {
reply << "SupportedWatchpointTypes=x86_64;";
#endif
+ // We consume lldb's swbreak/hwbreak feature, but it doesn't change the
+ // behaviour of lldb-server. We always adjust the program counter for targets
+ // like x86
xusheng6 wrote:
Now fixed in
https://github.com/llvm/llvm-project/commit/51a356b384f28cce3b2cd874fbba139414b0e207
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] 5dbec8c - [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (#102873)
Author: xusheng
Date: 2024-08-13T15:28:35+01:00
New Revision: 5dbec8c6ce473352cac2d89d6a5b81f65182df59
URL:
https://github.com/llvm/llvm-project/commit/5dbec8c6ce473352cac2d89d6a5b81f65182df59
DIFF:
https://github.com/llvm/llvm-project/commit/5dbec8c6ce473352cac2d89d6a5b81f65182df59.diff
LOG: [lldb] Claim to support swbreak and hwbreak packets when debugging a
gdbremote (#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, the gdbserver always sends the
unmodified program counter value which, on systems like x86, causes the
program counter to be off-by-one (or otherwise wrong). For reference,
the lldb-server always sends the modified program counter value so it
works perfectly with lldb.
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html#swbreak-stop-reason
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, so that it
will send the adjusted program counter.
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
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
Removed:
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..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+"};
+ "multiprocess+",
+ "fork-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/GDBRemoteCommunicationServerLLGS.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index a0b08a219ae147..345f5cd5de8491 100644
---
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -4245,6 +4245,10 @@ std::vector
GDBRemoteCommunicationServerLLGS::HandleFeatures(
.Case("vfork-events+", Extension::vfork)
.Default({});
+ // We consume lldb's swbreak/hwbreak feature, but it doesn't change the
+ // behaviour of lldb-server. We always adjust the program counter for targets
+ // like x86
+
m_extensions_supported &= plugin_features;
// fork & vfork require multiprocess
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..c7ce368ab41ce2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2354,6 +2354,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
}
+ // 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)
}
if (stop_pid != LLDB_INVALID_PROCESS_ID && stop_pid != pid) {
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
index ef28cc95f7ad4b..3faae5fec38ba1 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
@@ -10,13
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
https://github.com/DavidSpickett closed 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: Thanks for the contribution! The test especially, it will keep us honest and protect your downstream tools going forward. 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][DWARFASTParser] Don't pass CompilerType by non-const reference in the DWARFASTParserClang APIs (PR #103245)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/103245
The `CompilerType` is just a wrapper around two pointers, and there is no usage
of the `CompilerType` where those are expected to change underneath the caller.
To make the interface more straightforward to reason about, this patch changes
all instances of `CompilerType&` to `const CompilerType&` around the
`DWARFASTParserClang` APIs.
We could probably pass these by-value, but all other APIs don't, and this patch
just makes the parameter passing convention consistent with the rest of the
file.
>From e21ec8d96923b8f56899c6bfd28812694d9c2e12 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Tue, 13 Aug 2024 15:59:20 +0100
Subject: [PATCH] [lldb][DWARFASTParser] Don't pass CompilerType by non-const
reference in the DWARFASTParserClang APIs
The `CompilerType` is just a wrapper around two pointers,
and there is no usage of the `CompilerType` where those
are expected to change underneath the caller.
To make the interface more straightforward to reason about, this
patch changes all instances of `CompilerType&` to `const CompilerType&`
around the `DWARFASTParserClang` APIs.
We could probably pass these by-value, but all other APIs don't,
and this patch just makes the parameter passing convention consistent
with the rest of the file.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParser.h | 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 17 +
.../SymbolFile/DWARF/DWARFASTParserClang.h | 18 +-
3 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index abaeb2502cbbdc..636ff4b5cf11dc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -45,7 +45,7 @@ class DWARFASTParser {
const AddressRange &range) = 0;
virtual bool CompleteTypeFromDWARF(const DWARFDIE &die, Type *type,
- CompilerType &compiler_type) = 0;
+ const CompilerType &compiler_type) = 0;
virtual CompilerDecl GetDeclForUIDFromDWARF(const DWARFDIE &die) = 0;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 86b3117f361cd1..3c58be26c266bb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2057,7 +2057,7 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
lldb_private::Type *type,
- CompilerType &clang_type) {
+ const CompilerType &clang_type) {
const dw_tag_t tag = die.Tag();
SymbolFileDWARF *dwarf = die.GetDWARF();
@@ -2152,7 +2152,7 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
- CompilerType &clang_type) {
+ const CompilerType &clang_type) {
if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
if (die.HasChildren()) {
bool is_signed = false;
@@ -2165,9 +2165,9 @@ bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE
&die,
return (bool)clang_type;
}
-bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die,
-lldb_private::Type *type,
-CompilerType &clang_type) {
+bool DWARFASTParserClang::CompleteTypeFromDWARF(
+const DWARFDIE &die, lldb_private::Type *type,
+const CompilerType &clang_type) {
SymbolFileDWARF *dwarf = die.GetDWARF();
std::lock_guard guard(
@@ -2244,7 +2244,7 @@
DWARFASTParserClang::GetDeclContextContainingUIDFromDWARF(const DWARFDIE &die) {
}
size_t DWARFASTParserClang::ParseChildEnumerators(
-lldb_private::CompilerType &clang_type, bool is_signed,
+const lldb_private::CompilerType &clang_type, bool is_signed,
uint32_t enumerator_byte_size, const DWARFDIE &parent_die) {
if (!parent_die)
return 0;
@@ -3032,7 +3032,7 @@ void DWARFASTParserClang::ParseSingleMember(
}
bool DWARFASTParserClang::ParseChildMembers(
-const DWARFDIE &parent_die, CompilerType &class_clang_type,
+const DWARFDIE &parent_die, const CompilerType &class_clang_type,
std::vector> &base_classes,
std::vector &member_function_dies,
std::vector &contained_type_dies,
@@ -3763,7 +3763,8 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
}
void DWARFASTParserCla
[Lldb-commits] [lldb] [lldb][DWARFASTParser] Don't pass CompilerType by non-const reference in the DWARFASTParserClang APIs (PR #103245)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
The `CompilerType` is just a wrapper around two pointers, and there is no usage
of the `CompilerType` where those are expected to change underneath the caller.
To make the interface more straightforward to reason about, this patch changes
all instances of `CompilerType&` to `const CompilerType&` around the
`DWARFASTParserClang` APIs.
We could probably pass these by-value, but all other APIs don't, and this patch
just makes the parameter passing convention consistent with the rest of the
file.
---
Full diff: https://github.com/llvm/llvm-project/pull/103245.diff
3 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+9-8)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+9-9)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index abaeb2502cbbdc..636ff4b5cf11dc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -45,7 +45,7 @@ class DWARFASTParser {
const AddressRange &range) = 0;
virtual bool CompleteTypeFromDWARF(const DWARFDIE &die, Type *type,
- CompilerType &compiler_type) = 0;
+ const CompilerType &compiler_type) = 0;
virtual CompilerDecl GetDeclForUIDFromDWARF(const DWARFDIE &die) = 0;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 86b3117f361cd1..3c58be26c266bb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2057,7 +2057,7 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
lldb_private::Type *type,
- CompilerType &clang_type) {
+ const CompilerType &clang_type) {
const dw_tag_t tag = die.Tag();
SymbolFileDWARF *dwarf = die.GetDWARF();
@@ -2152,7 +2152,7 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
- CompilerType &clang_type) {
+ const CompilerType &clang_type) {
if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
if (die.HasChildren()) {
bool is_signed = false;
@@ -2165,9 +2165,9 @@ bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE
&die,
return (bool)clang_type;
}
-bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die,
-lldb_private::Type *type,
-CompilerType &clang_type) {
+bool DWARFASTParserClang::CompleteTypeFromDWARF(
+const DWARFDIE &die, lldb_private::Type *type,
+const CompilerType &clang_type) {
SymbolFileDWARF *dwarf = die.GetDWARF();
std::lock_guard guard(
@@ -2244,7 +2244,7 @@
DWARFASTParserClang::GetDeclContextContainingUIDFromDWARF(const DWARFDIE &die) {
}
size_t DWARFASTParserClang::ParseChildEnumerators(
-lldb_private::CompilerType &clang_type, bool is_signed,
+const lldb_private::CompilerType &clang_type, bool is_signed,
uint32_t enumerator_byte_size, const DWARFDIE &parent_die) {
if (!parent_die)
return 0;
@@ -3032,7 +3032,7 @@ void DWARFASTParserClang::ParseSingleMember(
}
bool DWARFASTParserClang::ParseChildMembers(
-const DWARFDIE &parent_die, CompilerType &class_clang_type,
+const DWARFDIE &parent_die, const CompilerType &class_clang_type,
std::vector> &base_classes,
std::vector &member_function_dies,
std::vector &contained_type_dies,
@@ -3763,7 +3763,8 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
}
void DWARFASTParserClang::ParseRustVariantPart(
-DWARFDIE &die, const DWARFDIE &parent_die, CompilerType &class_clang_type,
+DWARFDIE &die, const DWARFDIE &parent_die,
+const CompilerType &class_clang_type,
const lldb::AccessType default_accesibility,
ClangASTImporter::LayoutInfo &layout_info) {
assert(die.Tag() == llvm::dwarf::DW_TAG_variant_part);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 4b0ae026bce7e9..ae6720608121f5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang
[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" cmtice wrote: ok, then we've run into a very serious problem: the DIL implementation (and lldb-eval, on which it was based) makes extensive use of Clang for parsing & lexing. I'm *guessing* the best way to handle this is to try to bundle the parsing/lexing stuff into some kind of plugin, with different plugins for the different languages? OR...is it possible that the small language subset that DIL wants to parse/lex/evaluate is roughly the same across all the languages? I'm really not sure about the best way forward here... 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 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
labath wrote: > This would be quite the rework of the FindInMemory() API. Would it be worth > creating an Issue and move the discussion there to unblock this PR? I think the two are related because if we (I?) do something like I proposed, then this patch becomes unnecessary (or even gets in the way). I also don't think that would be as much work as you think. In fact, if we look at the number of new APIs added, I'm certain that implementation would end up being simpler. `GetMemoryRegionInfo` is an existing API, and it serves a similar purpose as `VMRangeToFileOffset` in this patch, so the change would mainly consist of rewriting `FindInMemory` to use that instead. The only new API being added is the DataExtractor version of the `ReadMemory` function, but that one can be naturally split into a separate patch (and it may not even be needed -- depending on exactly how much performance you want to get out of this). 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] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: Oh, wait a sec. I actually changed the behavior of the `IRExecutionUnit::FindInSymbols`. It used to exit early if the function was found in `module_sp`, but now we will always scan through the whole ModuleList. And we can't change the behavior `ModuleList::FindFunctions` to return after the first match, because it might be not what we want in general case. Maybe I should add more generic versions of these functions taking a callback to invoke on each match instead of SymbolContextList? Something like ``` void ModuleList::FindSymbolsWithNameAndType( ConstString name, lldb::SymbolType symbol_type, const SymbolContext *search_hint, llvm::function_ref callback ) ``` where the result of `callback` indicates whether the search should be continued or not. 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] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
@@ -4245,6 +4245,10 @@ std::vector
GDBRemoteCommunicationServerLLGS::HandleFeatures(
.Case("vfork-events+", Extension::vfork)
.Default({});
+ // We consume lldb's swbreak/hwbreak feature, but it doesn't change the
+ // behaviour of lldb-server. We always adjust the program counter for targets
+ // like x86
jasonmolenda wrote:
Please end the comment with a period.
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/jasonmolenda commented: Thanks for writing up the patch. We do see some handling of this behavior for x86 in lldb, e.g. ProcessWindowsNative, and KDP protocol support with x86 Darwin kernel debugging, also `breakpoint-pc-offset` in ProcessGDBRemote. But we do assume that the pc value reported by a thread that hit a breakpoint will be the start of the breakpoint instruction, not the end. The only question I have is -- if we are talking to a stub that supports `swbreak`/`hwbreak`, and we get a stop packet *without* those that hit a breakpoint, do we need to decr the pc value? I'm guessing gdbserver et al don't report a `reason:breakpoint` key-value pair in the stop packet. Should we treat the presence of `swbreak` or `hwbreak` as equivalent to receiving a `reason:breakpoint` in the packet (see the `reason` argument passed to `ProcessGDBRemote::SetThreadStopInfo`) to unambiguously indicate that a breakpoint was hit by this thread? 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/jasonmolenda 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)
@@ -2354,6 +2354,9 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
}
+ // 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)
jasonmolenda wrote:
Please end the comment with a period.
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] 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: Yeah, that does sound like a problem. Putting this stuff in a plugin would be possible, but it would be a departure from what we've talked about earlier, since we would essentially end up with a different flavour of DIL for every language. Saying that clang *is* the parser for the DIL might also be possible, but that would also warrant more discussion, because we have a (maybe undocumented?) rule of no clang dependencies on lldb core. A third option might be to roll our own parser. Unlike lldb-eval, I think the DIL does not have the ambition of matching c++ (or any other language) precisely, so maybe we have some leeway to remove/skip/alter the features which make parsing c(++) the mess that it is. 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] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
jasonmolenda wrote: oh lol I missed that this was already merged. But I am curious what you think about treating `swbreak`/`hwbreak` as equivalent to having received `reason:breakpoint`. 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] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
augusto2112 wrote: > > @clayborg I can see how a module could be used as a hint, but I'm not sure > > how we'd prioritize lookups using functions or compile unit. The code in > > `SymTab.cpp` does a binary search to find the symbol, how could we restrict > > this further by passing a function hint, for example? > > For symbol lookups, we would only check the Module. If there is not module, > we would just search through the target's image list. If there was a module, > then we search it first, and avoid that module if we search the module list > just by checking the pointer of the module when iterating over the target's > module list. The full symbol context isn't required, but it would be nice to > set the precedent that others can duplicate when doing lookups of other kinds. If that's the case, personally I think that it would make more sense if `FindFunctions` and `FindSymbolsWithNameAndType` took in a module because it wouldn't be possible to pass in something that doesn't make sense to those functions (like a symbol context with only a compile unit set, for example). 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] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
jasonmolenda wrote: Just to be clear, if I'm understanding the packet we'll be getting back, we have no indication that we hit the breakpoint, we only show that we are stopped at an address which has a breakpoint. Current lldb stepping behavior will work -- because the rule is, when we stop at a breakpoint address, we will say we hit the breakpoint. I am refining a patch https://github.com/llvm/llvm-project/pull/96260 which changes this behavior -- we handle "instruction stepped / stopped at a breakpoint" differently than "hit a breakpoint". I worry this difference will be lost with a stub that reports `swbreak`/`hwbreak`, stepping won't work correctly, and we won't capture it in the testsuite or on any of our CI bots. 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] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
Michael137 wrote: > Oh, wait a sec. I actually changed the behavior of the > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function was > found in `module_sp`, but now we will always scan through the whole > ModuleList. And we can't change the behavior of the > `ModuleList::FindFunctions` to return after the first match, because it might > be not what we want in general case. Maybe I should add more generic versions > of these functions taking a callback to invoke on each match instead of > SymbolContextList? Something like > > ``` > void ModuleList::FindSymbolsWithNameAndType( > ConstString name, > lldb::SymbolType symbol_type, > const SymbolContext *search_hint, > llvm::function_ref callback > ) > ``` > > where the result of `callback` indicates whether the search should be > continued or not. Can we make it configurable behind the `ModuleSearchOptions`? This is what the `FindTypes` API does via `TypeQuery` (which has a bunch of flags to configure your search, including `eFindOne`, etc.). There's already a `ModuleFunctionSearchOptions` that could be re-used for this? 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)
augusto2112 wrote: > Oh, wait a sec. I actually changed the behavior of the > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function was > found in `module_sp`, but now we will always scan through the whole > ModuleList. And we can't change the behavior of the > `ModuleList::FindFunctions` to return after the first match, because it might > be not what we want in general case. Maybe I should add more generic versions > of these functions taking a callback to invoke on each match instead of > SymbolContextList? Something like > > ``` > void ModuleList::FindSymbolsWithNameAndType( > ConstString name, > lldb::SymbolType symbol_type, > const SymbolContext *search_hint, > llvm::function_ref callback > ) > ``` > > where the result of `callback` indicates whether the search should be > continued or not. I think it'd be fine to return early if the type if found with the `search_hint`, as long as this is clearly documented in the API. My impression is that this would be a more useful behavior than just reordering the search results so the `search_hint` results are first, and I don't think we need the extra flexibility provided by that callback at the moment. I don't touch this code very often though so if @clayborg or @Michael137 disagree I'd defer to them. 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)
@@ -441,14 +441,20 @@ ModuleSP ModuleList::GetModuleAtIndexUnlocked(size_t idx)
const {
void ModuleList::FindFunctions(ConstString name,
FunctionNameType name_type_mask,
const ModuleFunctionSearchOptions &options,
+ const SymbolContext &sc,
SymbolContextList &sc_list) const {
const size_t old_size = sc_list.GetSize();
if (name_type_mask & eFunctionNameTypeAuto) {
Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
std::lock_guard guard(m_modules_mutex);
+if (sc.module_sp)
augusto2112 wrote:
Should we assert that the module_sp belongs to the module list?
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)
Michael137 wrote: > > Oh, wait a sec. I actually changed the behavior of the > > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function was > > found in `module_sp`, but now we will always scan through the whole > > ModuleList. And we can't change the behavior of the > > `ModuleList::FindFunctions` to return after the first match, because it > > might be not what we want in general case. Maybe I should add more generic > > versions of these functions taking a callback to invoke on each match > > instead of SymbolContextList? Something like > > ``` > > void ModuleList::FindSymbolsWithNameAndType( > > ConstString name, > > lldb::SymbolType symbol_type, > > const SymbolContext *search_hint, > > llvm::function_ref callback > > ) > > ``` > > > > > > > > > > > > > > > > > > > > > > > > where the result of `callback` indicates whether the search should be > > continued or not. > > I think it'd be fine to return early if the type if found with the > `search_hint`, as long as this is clearly documented in the API. My > impression is that this would be a more useful behavior than just reordering > the search results so the `search_hint` results are first, and I don't think > we need the extra flexibility provided by that callback at the moment. I > don't touch this code very often though so if @clayborg or @Michael137 > disagree I'd defer to them. Probably fine returning early unconditionally tbh, but only if the `search_hint` is specified. It shouldn'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] 7e23613 - [lldb] Skip libcxx tests with older versions of clang
Author: Adrian Prantl
Date: 2024-08-13T09:43:07-07:00
New Revision: 7e236136ab2896dee12bbe96d5994bb65c326e9f
URL:
https://github.com/llvm/llvm-project/commit/7e236136ab2896dee12bbe96d5994bb65c326e9f
DIFF:
https://github.com/llvm/llvm-project/commit/7e236136ab2896dee12bbe96d5994bb65c326e9f.diff
LOG: [lldb] Skip libcxx tests with older versions of clang
Added:
Modified:
lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
Removed:
diff --git
a/lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
b/lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
index f8e746e94730d7..d072d4f84d4544 100644
---
a/lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
+++
b/lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
@@ -10,6 +10,7 @@
class TestSharedPtr(TestBase):
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '17.0'])
def test(self):
self.build()
diff --git
a/lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
b/lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
index 5ba8acc6774472..3da93914f3456d 100644
---
a/lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
+++
b/lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
@@ -10,6 +10,7 @@
class TestDbgInfoContentWeakPtr(TestBase):
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '17.0'])
def test(self):
self.build()
diff --git
a/lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
b/lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
index f4a230ce26f3fd..3363c8c9dc87b2 100644
---
a/lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
+++
b/lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
@@ -10,6 +10,7 @@
class TestSharedPtr(TestBase):
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '17.0'])
def test(self):
self.build()
___
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)
DmT021 wrote: > > > Oh, wait a sec. I actually changed the behavior of the > > > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function > > > was found in `module_sp`, but now we will always scan through the whole > > > ModuleList. And we can't change the behavior of the > > > `ModuleList::FindFunctions` to return after the first match, because it > > > might be not what we want in general case. Maybe I should add more > > > generic versions of these functions taking a callback to invoke on each > > > match instead of SymbolContextList? Something like > > > ``` > > > void ModuleList::FindSymbolsWithNameAndType( > > > ConstString name, > > > lldb::SymbolType symbol_type, > > > const SymbolContext *search_hint, > > > llvm::function_ref callback > > > ) > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > where the result of `callback` indicates whether the search should be > > > continued or not. > > > > > > I think it'd be fine to return early if the type if found with the > > `search_hint`, as long as this is clearly documented in the API. My > > impression is that this would be a more useful behavior than just > > reordering the search results so the `search_hint` results are first, and I > > don't think we need the extra flexibility provided by that callback at the > > moment. I don't touch this code very often though so if @clayborg or > > @Michael137 disagree I'd defer to them. > > Probably fine returning early unconditionally tbh, but only if the > `search_hint` is specified. It shouldn't break anything? It wouldn't be that good API then, because the function takes an optional argument that changes the behavior significantly: - when it's provided the function returns a single result(which means we don't really need `SymbolContextList`) - when it's NULL, the function will find all the matches 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] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
@@ -2835,6 +2835,34 @@ void PruneThreadPlans();
AddressRanges &matches, size_t alignment,
size_t max_matches);
+ template
+ lldb::addr_t FindInMemoryGeneric(IT &&iterator, lldb::addr_t low,
+ lldb::addr_t high, const uint8_t *buf,
+ size_t size) {
+const size_t region_size = high - low;
+
+if (region_size < size)
+ return LLDB_INVALID_ADDRESS;
+
+std::vector bad_char_heuristic(256, size);
+
+for (size_t idx = 0; idx < size - 1; idx++) {
+ decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx];
+ bad_char_heuristic[bcu_idx] = size - idx - 1;
+}
+for (size_t s = 0; s <= (region_size - size);) {
+ int64_t j = size - 1;
+ while (j >= 0 && buf[j] == iterator[s + j])
+j--;
+ if (j < 0)
+return low + s;
+ else
+s += bad_char_heuristic[iterator[s + size - 1]];
+}
+
+return LLDB_INVALID_ADDRESS;
+ }
+
kevinfrei wrote:
This algorithm is probably a performance win, but obscures the simplicity of
the function. Obscuring simplicity for good reasons requires an explanation. At
least name drop BMH in the comments or link to the wikipedia page
(https://en.wikipedia.org/wiki/Boyer%E2%80%93Moore%E2%80%93Horspool_algorithm)
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] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
augusto2112 wrote: > > > > Oh, wait a sec. I actually changed the behavior of the > > > > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function > > > > was found in `module_sp`, but now we will always scan through the whole > > > > ModuleList. And we can't change the behavior of the > > > > `ModuleList::FindFunctions` to return after the first match, because it > > > > might be not what we want in general case. Maybe I should add more > > > > generic versions of these functions taking a callback to invoke on each > > > > match instead of SymbolContextList? Something like > > > > ``` > > > > void ModuleList::FindSymbolsWithNameAndType( > > > > ConstString name, > > > > lldb::SymbolType symbol_type, > > > > const SymbolContext *search_hint, > > > > llvm::function_ref callback > > > > ) > > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > where the result of `callback` indicates whether the search should be > > > > continued or not. > > > > > > > > > I think it'd be fine to return early if the type if found with the > > > `search_hint`, as long as this is clearly documented in the API. My > > > impression is that this would be a more useful behavior than just > > > reordering the search results so the `search_hint` results are first, and > > > I don't think we need the extra flexibility provided by that callback at > > > the moment. I don't touch this code very often though so if @clayborg or > > > @Michael137 disagree I'd defer to them. > > > > > > Probably fine returning early unconditionally tbh, but only if the > > `search_hint` is specified. It shouldn't break anything? > > It wouldn't be that good API then, because the function takes an optional > argument that changes the behavior significantly: > > * when it's provided the function returns a single result(which means we > don't really need `SymbolContextList`) > * when it's NULL, the function will find all the matches > > > > Oh, wait a sec. I actually changed the behavior of the > > > > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function > > > > was found in `module_sp`, but now we will always scan through the whole > > > > ModuleList. And we can't change the behavior of the > > > > `ModuleList::FindFunctions` to return after the first match, because it > > > > might be not what we want in general case. Maybe I should add more > > > > generic versions of these functions taking a callback to invoke on each > > > > match instead of SymbolContextList? Something like > > > > ``` > > > > void ModuleList::FindSymbolsWithNameAndType( > > > > ConstString name, > > > > lldb::SymbolType symbol_type, > > > > const SymbolContext *search_hint, > > > > llvm::function_ref callback > > > > ) > > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > where the result of `callback` indicates whether the search should be > > > > continued or not. > > > > > > > > > I think it'd be fine to return early if the type if found with the > > > `search_hint`, as long as this is clearly documented in the API. My > > > impression is that this would be a more useful behavior than just > > > reordering the search results so the `search_hint` results are first, and > > > I don't think we need the extra flexibility provided by that callback at > > > the moment. I don't touch this code very often though so if @clayborg or > > > @Michael137 disagree I'd defer to them. > > > > > > Probably fine returning early unconditionally tbh, but only if the > > `search_hint` is specified. It shouldn't break anything? > > It wouldn't be that good API then, because the function takes an optional > argument that changes the behavior significantly: > > * when it's provided the function returns a single result(which means we > don't really need `SymbolContextList`) > * when it's NULL, the function will find all the matches Then you probably want to implement something similar to `FindTypes` which takes in a `TypeQuery`, like @Michael137 said. 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] [lldb] Realpath symlinks for breakpoints (PR #102223)
royitaqi wrote: Gentle ping. @jimingham and @labath Is there anything else you want me to improve/fix in this PR? > I think I have addressed all of your comments. > When you have the time, I will appreciate you taking another look cc @clayborg https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 494eec0 - [lldb][NFCI] Simplify ProcessElfCore::GetAuxvData() (#102263)
Author: Igor Kudrin
Date: 2024-08-13T10:55:28-07:00
New Revision: 494eec0255d0e270ed877e960843177759f0ee73
URL:
https://github.com/llvm/llvm-project/commit/494eec0255d0e270ed877e960843177759f0ee73
DIFF:
https://github.com/llvm/llvm-project/commit/494eec0255d0e270ed877e960843177759f0ee73.diff
LOG: [lldb][NFCI] Simplify ProcessElfCore::GetAuxvData() (#102263)
There is no need to clone the content and set extraction properties
because `m_auxv` is already in the required form.
Added:
Modified:
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
Removed:
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 30af9345999c41..e73e31ca78d19f 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -1077,10 +1077,10 @@ ArchSpec ProcessElfCore::GetArchitecture() {
}
DataExtractor ProcessElfCore::GetAuxvData() {
- const uint8_t *start = m_auxv.GetDataStart();
- size_t len = m_auxv.GetByteSize();
- lldb::DataBufferSP buffer(new lldb_private::DataBufferHeap(start, len));
- return DataExtractor(buffer, GetByteOrder(), GetAddressByteSize());
+ assert(m_auxv.GetByteSize() == 0 ||
+ (m_auxv.GetByteOrder() == GetByteOrder() &&
+ m_auxv.GetAddressByteSize() == GetAddressByteSize()));
+ return DataExtractor(m_auxv);
}
bool ProcessElfCore::GetProcessInfo(ProcessInstanceInfo &info) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Simplify ProcessElfCore::GetAuxvData() (PR #102263)
https://github.com/igorkudrin closed https://github.com/llvm/llvm-project/pull/102263 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
https://github.com/kimgr created
https://github.com/llvm/llvm-project/pull/103388
When Clang is consumed as a library, the CLANG_RESOURCE_DIR definition is not
exported from the CMake system, so external clients will be unable to compute
the same resource dir as Clang itself would, because they don't know what to
pass for the optional CustomResourceDir argument.
All call sites except one would pass CLANG_RESOURCE_DIR to
Driver::GetResourcesPath. It seems the one exception in libclang CIndexer was
an oversight.
Move the use of CLANG_RESOURCE_DIR into GetResourcesPath and remove the
optional argument to avoid this inconsistency between internal and external
clients.
From 102517cd9d53695c9ae56135492bab09df9d90ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kim=20Gr=C3=A4sman?=
Date: Mon, 5 Aug 2024 11:49:32 +0200
Subject: [PATCH] Use CLANG_RESOURCE_DIR more consistently
When Clang is consumed as a library, the CLANG_RESOURCE_DIR definition
is not exported from the CMake system, so external clients will be
unable to compute the same resource dir as Clang itself would, because
they don't know what to pass for the optional CustomResourceDir
argument.
All call sites except one would pass CLANG_RESOURCE_DIR to
Driver::GetResourcesPath. It seems the one exception in libclang
CIndexer was an oversight.
Move the use of CLANG_RESOURCE_DIR into GetResourcesPath and remove the
optional argument to avoid this inconsistency between internal and
external clients.
---
clang/include/clang/Driver/Driver.h| 3 +--
clang/lib/Driver/Driver.cpp| 14 +++---
clang/lib/Frontend/CompilerInvocation.cpp | 2 +-
.../Plugins/ExpressionParser/Clang/ClangHost.cpp | 2 +-
lldb/unittests/Expression/ClangParserTest.cpp | 2 +-
5 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Driver/Driver.h
b/clang/include/clang/Driver/Driver.h
index 04b46782467d6a..0382c8cb155e51 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -379,8 +379,7 @@ class Driver {
/// Takes the path to a binary that's either in bin/ or lib/ and returns
/// the path to clang's resource directory.
- static std::string GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir = "");
+ static std::string GetResourcesPath(StringRef BinaryPath);
Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine &Diags, std::string Title = "clang LLVM compiler",
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e44d5afa40e05..dac8185693cf58 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -171,18 +171,18 @@ getHIPOffloadTargetTriple(const Driver &D, const ArgList
&Args) {
}
// static
-std::string Driver::GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir) {
+std::string Driver::GetResourcesPath(StringRef BinaryPath) {
// Since the resource directory is embedded in the module hash, it's
important
// that all places that need it call this function, so that they get the
// exact same string ("a/../b/" and "b/" get different hashes, for example).
// Dir is bin/ or lib/, depending on where BinaryPath is.
- std::string Dir = std::string(llvm::sys::path::parent_path(BinaryPath));
-
+ StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
SmallString<128> P(Dir);
- if (CustomResourceDir != "") {
-llvm::sys::path::append(P, CustomResourceDir);
+
+ StringRef ConfiguredResourceDir(CLANG_RESOURCE_DIR);
+ if (!ConfiguredResourceDir.empty()) {
+llvm::sys::path::append(P, ConfiguredResourceDir);
} else {
// On Windows, libclang.dll is in bin/.
// On non-Windows, libclang.so/.dylib is in lib/.
@@ -239,7 +239,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef
TargetTriple,
#endif
// Compute the path to the resource directory.
- ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ ResourceDir = GetResourcesPath(ClangExecutable);
}
void Driver::setDriverMode(StringRef Value) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index f6b6c44a4cab6a..6962cb4f32b06a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3121,7 +3121,7 @@ std::string CompilerInvocation::GetResourcesPath(const
char *Argv0,
void *MainAddr) {
std::string ClangExecutable =
llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
- return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ return Driver::GetResourcesPath(ClangExecutable);
}
static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
b/lldb/source/Plugins/ExpressionParser/Clan
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
llvmbot wrote:
@llvm/pr-subscribers-clang-driver
Author: Kim Gräsman (kimgr)
Changes
When Clang is consumed as a library, the CLANG_RESOURCE_DIR definition is not
exported from the CMake system, so external clients will be unable to compute
the same resource dir as Clang itself would, because they don't know what to
pass for the optional CustomResourceDir argument.
All call sites except one would pass CLANG_RESOURCE_DIR to
Driver::GetResourcesPath. It seems the one exception in libclang CIndexer was
an oversight.
Move the use of CLANG_RESOURCE_DIR into GetResourcesPath and remove the
optional argument to avoid this inconsistency between internal and external
clients.
---
Full diff: https://github.com/llvm/llvm-project/pull/103388.diff
5 Files Affected:
- (modified) clang/include/clang/Driver/Driver.h (+1-2)
- (modified) clang/lib/Driver/Driver.cpp (+7-7)
- (modified) clang/lib/Frontend/CompilerInvocation.cpp (+1-1)
- (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp (+1-1)
- (modified) lldb/unittests/Expression/ClangParserTest.cpp (+1-1)
``diff
diff --git a/clang/include/clang/Driver/Driver.h
b/clang/include/clang/Driver/Driver.h
index 04b46782467d6a..0382c8cb155e51 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -379,8 +379,7 @@ class Driver {
/// Takes the path to a binary that's either in bin/ or lib/ and returns
/// the path to clang's resource directory.
- static std::string GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir = "");
+ static std::string GetResourcesPath(StringRef BinaryPath);
Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine &Diags, std::string Title = "clang LLVM compiler",
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e44d5afa40e05..dac8185693cf58 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -171,18 +171,18 @@ getHIPOffloadTargetTriple(const Driver &D, const ArgList
&Args) {
}
// static
-std::string Driver::GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir) {
+std::string Driver::GetResourcesPath(StringRef BinaryPath) {
// Since the resource directory is embedded in the module hash, it's
important
// that all places that need it call this function, so that they get the
// exact same string ("a/../b/" and "b/" get different hashes, for example).
// Dir is bin/ or lib/, depending on where BinaryPath is.
- std::string Dir = std::string(llvm::sys::path::parent_path(BinaryPath));
-
+ StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
SmallString<128> P(Dir);
- if (CustomResourceDir != "") {
-llvm::sys::path::append(P, CustomResourceDir);
+
+ StringRef ConfiguredResourceDir(CLANG_RESOURCE_DIR);
+ if (!ConfiguredResourceDir.empty()) {
+llvm::sys::path::append(P, ConfiguredResourceDir);
} else {
// On Windows, libclang.dll is in bin/.
// On non-Windows, libclang.so/.dylib is in lib/.
@@ -239,7 +239,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef
TargetTriple,
#endif
// Compute the path to the resource directory.
- ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ ResourceDir = GetResourcesPath(ClangExecutable);
}
void Driver::setDriverMode(StringRef Value) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index f6b6c44a4cab6a..6962cb4f32b06a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3121,7 +3121,7 @@ std::string CompilerInvocation::GetResourcesPath(const
char *Argv0,
void *MainAddr) {
std::string ClangExecutable =
llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
- return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ return Driver::GetResourcesPath(ClangExecutable);
}
static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
index 6064c02c7fd67d..6de851081598fd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -53,7 +53,7 @@ static bool DefaultComputeClangResourceDirectory(FileSpec
&lldb_shlib_spec,
std::string raw_path = lldb_shlib_spec.GetPath();
llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
static const std::string clang_resource_path =
- clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
+ clang::driver::Driver::GetResourcesPath("bin/lldb");
static const llvm::StringRef kResourceDirSuffixes[] = {
// LLVM.org's build of LLDB uses the clang resource directory placed
diff
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Kim Gräsman (kimgr)
Changes
When Clang is consumed as a library, the CLANG_RESOURCE_DIR definition is not
exported from the CMake system, so external clients will be unable to compute
the same resource dir as Clang itself would, because they don't know what to
pass for the optional CustomResourceDir argument.
All call sites except one would pass CLANG_RESOURCE_DIR to
Driver::GetResourcesPath. It seems the one exception in libclang CIndexer was
an oversight.
Move the use of CLANG_RESOURCE_DIR into GetResourcesPath and remove the
optional argument to avoid this inconsistency between internal and external
clients.
---
Full diff: https://github.com/llvm/llvm-project/pull/103388.diff
5 Files Affected:
- (modified) clang/include/clang/Driver/Driver.h (+1-2)
- (modified) clang/lib/Driver/Driver.cpp (+7-7)
- (modified) clang/lib/Frontend/CompilerInvocation.cpp (+1-1)
- (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp (+1-1)
- (modified) lldb/unittests/Expression/ClangParserTest.cpp (+1-1)
``diff
diff --git a/clang/include/clang/Driver/Driver.h
b/clang/include/clang/Driver/Driver.h
index 04b46782467d6a..0382c8cb155e51 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -379,8 +379,7 @@ class Driver {
/// Takes the path to a binary that's either in bin/ or lib/ and returns
/// the path to clang's resource directory.
- static std::string GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir = "");
+ static std::string GetResourcesPath(StringRef BinaryPath);
Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine &Diags, std::string Title = "clang LLVM compiler",
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e44d5afa40e05..dac8185693cf58 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -171,18 +171,18 @@ getHIPOffloadTargetTriple(const Driver &D, const ArgList
&Args) {
}
// static
-std::string Driver::GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir) {
+std::string Driver::GetResourcesPath(StringRef BinaryPath) {
// Since the resource directory is embedded in the module hash, it's
important
// that all places that need it call this function, so that they get the
// exact same string ("a/../b/" and "b/" get different hashes, for example).
// Dir is bin/ or lib/, depending on where BinaryPath is.
- std::string Dir = std::string(llvm::sys::path::parent_path(BinaryPath));
-
+ StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
SmallString<128> P(Dir);
- if (CustomResourceDir != "") {
-llvm::sys::path::append(P, CustomResourceDir);
+
+ StringRef ConfiguredResourceDir(CLANG_RESOURCE_DIR);
+ if (!ConfiguredResourceDir.empty()) {
+llvm::sys::path::append(P, ConfiguredResourceDir);
} else {
// On Windows, libclang.dll is in bin/.
// On non-Windows, libclang.so/.dylib is in lib/.
@@ -239,7 +239,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef
TargetTriple,
#endif
// Compute the path to the resource directory.
- ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ ResourceDir = GetResourcesPath(ClangExecutable);
}
void Driver::setDriverMode(StringRef Value) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index f6b6c44a4cab6a..6962cb4f32b06a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3121,7 +3121,7 @@ std::string CompilerInvocation::GetResourcesPath(const
char *Argv0,
void *MainAddr) {
std::string ClangExecutable =
llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
- return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ return Driver::GetResourcesPath(ClangExecutable);
}
static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
index 6064c02c7fd67d..6de851081598fd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -53,7 +53,7 @@ static bool DefaultComputeClangResourceDirectory(FileSpec
&lldb_shlib_spec,
std::string raw_path = lldb_shlib_spec.GetPath();
llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
static const std::string clang_resource_path =
- clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
+ clang::driver::Driver::GetResourcesPath("bin/lldb");
static const llvm::StringRef kResourceDirSuffixes[] = {
// LLVM.org's build of LLDB uses the clang resource directory placed
diff --git a
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
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 4377656f2419a8eb18c01e86929b689dcf22b5d6
102517cd9d53695c9ae56135492bab09df9d90ee --extensions h,cpp --
clang/include/clang/Driver/Driver.h clang/lib/Driver/Driver.cpp
clang/lib/Frontend/CompilerInvocation.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
lldb/unittests/Expression/ClangParserTest.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/unittests/Expression/ClangParserTest.cpp
b/lldb/unittests/Expression/ClangParserTest.cpp
index 70fa494346..fab4487c73 100644
--- a/lldb/unittests/Expression/ClangParserTest.cpp
+++ b/lldb/unittests/Expression/ClangParserTest.cpp
@@ -42,8 +42,8 @@ TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
#else
std::string path_to_liblldb = "C:\\foo\\bar\\lib\\";
#endif
- std::string path_to_clang_dir = clang::driver::Driver::GetResourcesPath(
- path_to_liblldb + "liblldb");
+ std::string path_to_clang_dir =
+ clang::driver::Driver::GetResourcesPath(path_to_liblldb + "liblldb");
llvm::SmallString<256> path_to_clang_lib_dir_real;
llvm::sys::fs::real_path(path_to_clang_dir, path_to_clang_lib_dir_real);
``
https://github.com/llvm/llvm-project/pull/103388
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
https://github.com/kimgr updated
https://github.com/llvm/llvm-project/pull/103388
From 3722d673ee409c1320c9d66aa2f3f6568ffdfd77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kim=20Gr=C3=A4sman?=
Date: Mon, 5 Aug 2024 11:49:32 +0200
Subject: [PATCH] Use CLANG_RESOURCE_DIR more consistently
When Clang is consumed as a library, the CLANG_RESOURCE_DIR definition
is not exported from the CMake system, so external clients will be
unable to compute the same resource dir as Clang itself would, because
they don't know what to pass for the optional CustomResourceDir
argument.
All call sites except one would pass CLANG_RESOURCE_DIR to
Driver::GetResourcesPath. It seems the one exception in libclang
CIndexer was an oversight.
Move the use of CLANG_RESOURCE_DIR into GetResourcesPath and remove the
optional argument to avoid this inconsistency between internal and
external clients.
---
clang/include/clang/Driver/Driver.h| 3 +--
clang/lib/Driver/Driver.cpp| 14 +++---
clang/lib/Frontend/CompilerInvocation.cpp | 2 +-
.../Plugins/ExpressionParser/Clang/ClangHost.cpp | 2 +-
lldb/unittests/Expression/ClangParserTest.cpp | 4 ++--
5 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Driver/Driver.h
b/clang/include/clang/Driver/Driver.h
index 04b46782467d6a..0382c8cb155e51 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -379,8 +379,7 @@ class Driver {
/// Takes the path to a binary that's either in bin/ or lib/ and returns
/// the path to clang's resource directory.
- static std::string GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir = "");
+ static std::string GetResourcesPath(StringRef BinaryPath);
Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine &Diags, std::string Title = "clang LLVM compiler",
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e44d5afa40e05..dac8185693cf58 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -171,18 +171,18 @@ getHIPOffloadTargetTriple(const Driver &D, const ArgList
&Args) {
}
// static
-std::string Driver::GetResourcesPath(StringRef BinaryPath,
- StringRef CustomResourceDir) {
+std::string Driver::GetResourcesPath(StringRef BinaryPath) {
// Since the resource directory is embedded in the module hash, it's
important
// that all places that need it call this function, so that they get the
// exact same string ("a/../b/" and "b/" get different hashes, for example).
// Dir is bin/ or lib/, depending on where BinaryPath is.
- std::string Dir = std::string(llvm::sys::path::parent_path(BinaryPath));
-
+ StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
SmallString<128> P(Dir);
- if (CustomResourceDir != "") {
-llvm::sys::path::append(P, CustomResourceDir);
+
+ StringRef ConfiguredResourceDir(CLANG_RESOURCE_DIR);
+ if (!ConfiguredResourceDir.empty()) {
+llvm::sys::path::append(P, ConfiguredResourceDir);
} else {
// On Windows, libclang.dll is in bin/.
// On non-Windows, libclang.so/.dylib is in lib/.
@@ -239,7 +239,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef
TargetTriple,
#endif
// Compute the path to the resource directory.
- ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ ResourceDir = GetResourcesPath(ClangExecutable);
}
void Driver::setDriverMode(StringRef Value) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index f6b6c44a4cab6a..6962cb4f32b06a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3121,7 +3121,7 @@ std::string CompilerInvocation::GetResourcesPath(const
char *Argv0,
void *MainAddr) {
std::string ClangExecutable =
llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
- return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ return Driver::GetResourcesPath(ClangExecutable);
}
static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
index 6064c02c7fd67d..6de851081598fd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -53,7 +53,7 @@ static bool DefaultComputeClangResourceDirectory(FileSpec
&lldb_shlib_spec,
std::string raw_path = lldb_shlib_spec.GetPath();
llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
static const std::string clang_resource_path =
- clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
+ clang::driver::Driver::GetResourcesPath("bin/lldb");
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
kimgr wrote: @nico You're the original author of this thing back in 2019, maybe you have some additional information. Please take a look if you have a chance. https://github.com/llvm/llvm-project/pull/103388 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser] Don't pass CompilerType by non-const reference in the DWARFASTParserClang APIs (PR #103245)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/103245 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libc] [libcxx] [lldb] [llvm] [mlir] [InstCombine] Don't look at ConstantData users (PR #103302)
https://github.com/aengelke edited https://github.com/llvm/llvm-project/pull/103302 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libc] [libcxx] [lldb] [llvm] [mlir] [InstCombine] Don't look at ConstantData users (PR #103302)
https://github.com/aengelke closed https://github.com/llvm/llvm-project/pull/103302 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 28050e1 - [lldb][DWARFASTParser] Don't pass CompilerType by non-const reference in the DWARFASTParserClang APIs (#103245)
Author: Michael Buch
Date: 2024-08-13T20:04:18+01:00
New Revision: 28050e1b0b9da6d9c24ba20e8c70cf90b8135f49
URL:
https://github.com/llvm/llvm-project/commit/28050e1b0b9da6d9c24ba20e8c70cf90b8135f49
DIFF:
https://github.com/llvm/llvm-project/commit/28050e1b0b9da6d9c24ba20e8c70cf90b8135f49.diff
LOG: [lldb][DWARFASTParser] Don't pass CompilerType by non-const reference in
the DWARFASTParserClang APIs (#103245)
The `CompilerType` is just a wrapper around two pointers, and there is
no usage of the `CompilerType` where those are expected to change
underneath the caller.
To make the interface more straightforward to reason about, this patch
changes all instances of `CompilerType&` to `const CompilerType&` around
the `DWARFASTParserClang` APIs.
We could probably pass these by-value, but all other APIs don't, and
this patch just makes the parameter passing convention consistent with
the rest of the file.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index abaeb2502cbbd..636ff4b5cf11d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -45,7 +45,7 @@ class DWARFASTParser {
const AddressRange &range) = 0;
virtual bool CompleteTypeFromDWARF(const DWARFDIE &die, Type *type,
- CompilerType &compiler_type) = 0;
+ const CompilerType &compiler_type) = 0;
virtual CompilerDecl GetDeclForUIDFromDWARF(const DWARFDIE &die) = 0;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 86b3117f361cd..3c58be26c266b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2057,7 +2057,7 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
lldb_private::Type *type,
- CompilerType &clang_type) {
+ const CompilerType &clang_type) {
const dw_tag_t tag = die.Tag();
SymbolFileDWARF *dwarf = die.GetDWARF();
@@ -2152,7 +2152,7 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
- CompilerType &clang_type) {
+ const CompilerType &clang_type) {
if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
if (die.HasChildren()) {
bool is_signed = false;
@@ -2165,9 +2165,9 @@ bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE
&die,
return (bool)clang_type;
}
-bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die,
-lldb_private::Type *type,
-CompilerType &clang_type) {
+bool DWARFASTParserClang::CompleteTypeFromDWARF(
+const DWARFDIE &die, lldb_private::Type *type,
+const CompilerType &clang_type) {
SymbolFileDWARF *dwarf = die.GetDWARF();
std::lock_guard guard(
@@ -2244,7 +2244,7 @@
DWARFASTParserClang::GetDeclContextContainingUIDFromDWARF(const DWARFDIE &die) {
}
size_t DWARFASTParserClang::ParseChildEnumerators(
-lldb_private::CompilerType &clang_type, bool is_signed,
+const lldb_private::CompilerType &clang_type, bool is_signed,
uint32_t enumerator_byte_size, const DWARFDIE &parent_die) {
if (!parent_die)
return 0;
@@ -3032,7 +3032,7 @@ void DWARFASTParserClang::ParseSingleMember(
}
bool DWARFASTParserClang::ParseChildMembers(
-const DWARFDIE &parent_die, CompilerType &class_clang_type,
+const DWARFDIE &parent_die, const CompilerType &class_clang_type,
std::vector> &base_classes,
std::vector &member_function_dies,
std::vector &contained_type_dies,
@@ -3763,7 +3763,8 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
}
void DWARFASTParserClang::ParseRustVariantPart(
-DWARFDIE &die, const DWARFDIE &parent_die, CompilerType &class_clang_type,
+DWARFDIE &die, const DWARFDIE &parent_die,
+const CompilerType &class_clang_type,
const lldb::AccessType default_accesibility,
ClangASTImporter::LayoutInfo &layout_info) {
assert(die.Tag() == llvm::dwarf::DW_TAG_variant_part);
diff --git a/lldb/
[Lldb-commits] [lldb] [lldb][DWARFASTParser] Don't pass CompilerType by non-const reference in the DWARFASTParserClang APIs (PR #103245)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/103245 ___ 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" werat wrote: DIL doesn't use Clang for parsing and have its own parser, that was the whole point of project :D It does use Clang's lexer though, mostly for convenience, since it already has all the token types and provides source locations. If there's a strict rule of "not depending on clang libraries", then we can do it, but I'm not sure it's worth it. 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 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
@@ -0,0 +1,83 @@
+//===-- PostMortemProcess.cpp ---*- 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
+//
+//===--===//
+
+#include "lldb/Target/PostMortemProcess.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/lldb-forward.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+lldb::addr_t PostMortemProcess::FindInMemory(lldb::addr_t low,
+ lldb::addr_t high,
+ const uint8_t *buf, size_t size) {
+ const size_t region_size = high - low;
+ if (region_size < size)
+return LLDB_INVALID_ADDRESS;
+
+ llvm::ArrayRef data = PeekMemory(low, high);
+ if (data.empty()) {
+LLDB_LOG(GetLog(LLDBLog::Process),
+ "Failed to get contiguous memory region for search. low: 0x{}, "
+ "high: 0x{}. Failling back to Process::FindInMemory",
+ low, high);
+// In an edge case when the search has to happen across non-contiguous
+// memory, we will have to fall back on the Process::FindInMemory.
+return Process::FindInMemory(low, high, buf, size);
+ }
+
+ return Process::FindInMemoryGeneric(data, low, high, buf, size);
+}
+
+llvm::ArrayRef PostMortemProcess::PeekMemory(lldb::addr_t low,
+ lldb::addr_t high) {
+ return {};
+}
+
+llvm::ArrayRef
+PostMortemProcess::DoPeekMemory(lldb::ModuleSP &core_module_sp,
+VMRangeToFileOffset &core_aranges,
+lldb::addr_t low, lldb::addr_t high) {
+
+ ObjectFile *core_objfile = core_module_sp->GetObjectFile();
+
+ if (core_objfile == nullptr) {
+return {};
+ }
clayborg wrote:
remove {} from single statement if per llvm coding guidelines.
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)
https://github.com/clayborg requested changes to this pull request. 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)
@@ -0,0 +1,83 @@
+//===-- PostMortemProcess.cpp ---*- 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
+//
+//===--===//
+
+#include "lldb/Target/PostMortemProcess.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/lldb-forward.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+lldb::addr_t PostMortemProcess::FindInMemory(lldb::addr_t low,
+ lldb::addr_t high,
+ const uint8_t *buf, size_t size) {
+ const size_t region_size = high - low;
+ if (region_size < size)
+return LLDB_INVALID_ADDRESS;
+
+ llvm::ArrayRef data = PeekMemory(low, high);
+ if (data.empty()) {
+LLDB_LOG(GetLog(LLDBLog::Process),
+ "Failed to get contiguous memory region for search. low: 0x{}, "
+ "high: 0x{}. Failling back to Process::FindInMemory",
+ low, high);
+// In an edge case when the search has to happen across non-contiguous
+// memory, we will have to fall back on the Process::FindInMemory.
+return Process::FindInMemory(low, high, buf, size);
+ }
+
+ return Process::FindInMemoryGeneric(data, low, high, buf, size);
+}
+
+llvm::ArrayRef PostMortemProcess::PeekMemory(lldb::addr_t low,
+ lldb::addr_t high) {
+ return {};
+}
+
+llvm::ArrayRef
+PostMortemProcess::DoPeekMemory(lldb::ModuleSP &core_module_sp,
+VMRangeToFileOffset &core_aranges,
+lldb::addr_t low, lldb::addr_t high) {
+
+ ObjectFile *core_objfile = core_module_sp->GetObjectFile();
+
+ if (core_objfile == nullptr) {
+return {};
+ }
+
+ const VMRangeToFileOffset::Entry *core_memory_entry =
+ core_aranges.FindEntryThatContains(low);
+ if (core_memory_entry == nullptr || core_memory_entry->GetRangeEnd() < low) {
+return {};
+ }
+ const lldb::addr_t offset = low - core_memory_entry->GetRangeBase();
+ const lldb::addr_t file_start = core_memory_entry->data.GetRangeBase();
+ const lldb::addr_t file_end = core_memory_entry->data.GetRangeEnd();
+
+ if (file_start == file_end) {
+return {};
+ }
clayborg wrote:
remove {} from single statement if per llvm coding guidelines
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)
@@ -0,0 +1,83 @@
+//===-- PostMortemProcess.cpp ---*- 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
+//
+//===--===//
+
+#include "lldb/Target/PostMortemProcess.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/lldb-forward.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+lldb::addr_t PostMortemProcess::FindInMemory(lldb::addr_t low,
+ lldb::addr_t high,
+ const uint8_t *buf, size_t size) {
+ const size_t region_size = high - low;
+ if (region_size < size)
+return LLDB_INVALID_ADDRESS;
+
+ llvm::ArrayRef data = PeekMemory(low, high);
+ if (data.empty()) {
+LLDB_LOG(GetLog(LLDBLog::Process),
+ "Failed to get contiguous memory region for search. low: 0x{}, "
+ "high: 0x{}. Failling back to Process::FindInMemory",
+ low, high);
+// In an edge case when the search has to happen across non-contiguous
+// memory, we will have to fall back on the Process::FindInMemory.
+return Process::FindInMemory(low, high, buf, size);
+ }
+
+ return Process::FindInMemoryGeneric(data, low, high, buf, size);
+}
+
+llvm::ArrayRef PostMortemProcess::PeekMemory(lldb::addr_t low,
+ lldb::addr_t high) {
+ return {};
+}
+
+llvm::ArrayRef
+PostMortemProcess::DoPeekMemory(lldb::ModuleSP &core_module_sp,
+VMRangeToFileOffset &core_aranges,
+lldb::addr_t low, lldb::addr_t high) {
+
+ ObjectFile *core_objfile = core_module_sp->GetObjectFile();
+
+ if (core_objfile == nullptr) {
+return {};
+ }
+
+ const VMRangeToFileOffset::Entry *core_memory_entry =
+ core_aranges.FindEntryThatContains(low);
+ if (core_memory_entry == nullptr || core_memory_entry->GetRangeEnd() < low) {
+return {};
+ }
+ const lldb::addr_t offset = low - core_memory_entry->GetRangeBase();
+ const lldb::addr_t file_start = core_memory_entry->data.GetRangeBase();
+ const lldb::addr_t file_end = core_memory_entry->data.GetRangeEnd();
+
+ if (file_start == file_end) {
+return {};
+ }
+ size_t bytes_available = 0;
+ if (file_end > file_start + offset)
+bytes_available = file_end - (file_start + offset);
+
+ size_t bytes_to_read = high - low;
+ bytes_to_read = std::min(bytes_to_read, bytes_available);
+ if (bytes_to_read == 0) {
+return {};
+ }
+ DataExtractor extractor;
+ core_objfile->GetData(core_memory_entry->data.GetRangeBase() + offset,
+bytes_to_read, extractor);
+ if (extractor.GetByteSize() != bytes_to_read) {
+return {};
+ }
clayborg wrote:
remove {} from single statement if per llvm coding guidelines
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)
@@ -0,0 +1,83 @@
+//===-- PostMortemProcess.cpp ---*- 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
+//
+//===--===//
+
+#include "lldb/Target/PostMortemProcess.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/lldb-forward.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+lldb::addr_t PostMortemProcess::FindInMemory(lldb::addr_t low,
+ lldb::addr_t high,
+ const uint8_t *buf, size_t size) {
+ const size_t region_size = high - low;
+ if (region_size < size)
+return LLDB_INVALID_ADDRESS;
+
+ llvm::ArrayRef data = PeekMemory(low, high);
+ if (data.empty()) {
+LLDB_LOG(GetLog(LLDBLog::Process),
+ "Failed to get contiguous memory region for search. low: 0x{}, "
+ "high: 0x{}. Failling back to Process::FindInMemory",
+ low, high);
+// In an edge case when the search has to happen across non-contiguous
+// memory, we will have to fall back on the Process::FindInMemory.
+return Process::FindInMemory(low, high, buf, size);
+ }
+
+ return Process::FindInMemoryGeneric(data, low, high, buf, size);
+}
+
+llvm::ArrayRef PostMortemProcess::PeekMemory(lldb::addr_t low,
+ lldb::addr_t high) {
+ return {};
+}
+
+llvm::ArrayRef
+PostMortemProcess::DoPeekMemory(lldb::ModuleSP &core_module_sp,
+VMRangeToFileOffset &core_aranges,
+lldb::addr_t low, lldb::addr_t high) {
+
+ ObjectFile *core_objfile = core_module_sp->GetObjectFile();
+
+ if (core_objfile == nullptr) {
+return {};
+ }
+
+ const VMRangeToFileOffset::Entry *core_memory_entry =
+ core_aranges.FindEntryThatContains(low);
+ if (core_memory_entry == nullptr || core_memory_entry->GetRangeEnd() < low) {
+return {};
+ }
+ const lldb::addr_t offset = low - core_memory_entry->GetRangeBase();
+ const lldb::addr_t file_start = core_memory_entry->data.GetRangeBase();
+ const lldb::addr_t file_end = core_memory_entry->data.GetRangeEnd();
+
+ if (file_start == file_end) {
+return {};
+ }
+ size_t bytes_available = 0;
+ if (file_end > file_start + offset)
+bytes_available = file_end - (file_start + offset);
+
+ size_t bytes_to_read = high - low;
+ bytes_to_read = std::min(bytes_to_read, bytes_available);
+ if (bytes_to_read == 0) {
+return {};
+ }
clayborg wrote:
remove {} from single statement if per llvm coding guidelines
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)
@@ -0,0 +1,83 @@
+//===-- PostMortemProcess.cpp ---*- 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
+//
+//===--===//
+
+#include "lldb/Target/PostMortemProcess.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/lldb-forward.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+lldb::addr_t PostMortemProcess::FindInMemory(lldb::addr_t low,
+ lldb::addr_t high,
+ const uint8_t *buf, size_t size) {
+ const size_t region_size = high - low;
+ if (region_size < size)
+return LLDB_INVALID_ADDRESS;
+
+ llvm::ArrayRef data = PeekMemory(low, high);
+ if (data.empty()) {
+LLDB_LOG(GetLog(LLDBLog::Process),
+ "Failed to get contiguous memory region for search. low: 0x{}, "
+ "high: 0x{}. Failling back to Process::FindInMemory",
+ low, high);
+// In an edge case when the search has to happen across non-contiguous
+// memory, we will have to fall back on the Process::FindInMemory.
+return Process::FindInMemory(low, high, buf, size);
+ }
+
+ return Process::FindInMemoryGeneric(data, low, high, buf, size);
+}
+
+llvm::ArrayRef PostMortemProcess::PeekMemory(lldb::addr_t low,
+ lldb::addr_t high) {
+ return {};
+}
+
+llvm::ArrayRef
+PostMortemProcess::DoPeekMemory(lldb::ModuleSP &core_module_sp,
+VMRangeToFileOffset &core_aranges,
+lldb::addr_t low, lldb::addr_t high) {
+
+ ObjectFile *core_objfile = core_module_sp->GetObjectFile();
+
+ if (core_objfile == nullptr) {
+return {};
+ }
+
+ const VMRangeToFileOffset::Entry *core_memory_entry =
+ core_aranges.FindEntryThatContains(low);
+ if (core_memory_entry == nullptr || core_memory_entry->GetRangeEnd() < low) {
+return {};
+ }
clayborg wrote:
remove {} from single statement if per llvm coding guidelines
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] Realpath symlinks for breakpoints (PR #102223)
https://github.com/clayborg commented: I am good with this if Jim and Pavel's issues have been resolved. https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
https://github.com/walter-erquinigo created
https://github.com/llvm/llvm-project/pull/103458
LLDB on OSX is looking at a `bin` directory sibling to the `lib` one that
contains liblldb for its supporting executables. This works well for CMake,
however, for other build systems like bazel, it's not that easy to have that
build structure, for which it's much easier to also use the `lib` directory as
a fallback under the absence of `bin`.
This shouldn't break anything, but instead should make it a bit easier for LLDB
to work with different build systems and folder structures.
>From b011e77a937f4279295c55169c6d3049bfe2eaa2 Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Tue, 13 Aug 2024 17:09:38 -0400
Subject: [PATCH] [LLDB][OSX] Add a fallback support exe directory
LLDB on OSX is looking at a `bin` directory sibling to the `lib` one that
contains liblldb for its supporting executables. This works well for CMake,
however, for other build systems like bazel, it's not that easy to have that
build structure, for which it's much easier to also use the `lib` directory as
a fallback under the absence of `bin`.
This shouldn't break anything, but instead should make it a bit easier for LLDB
to work with different build systems and folder structures.
---
.../Host/macosx/objcxx/HostInfoMacOSX.mm | 24 ++-
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index f96e2cf80c5fac..02d02a21ad0955 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -124,6 +124,12 @@ static void ParseOSVersion(llvm::VersionTuple &version,
NSString *Key) {
return g_program_filespec;
}
+/// Resolve the given candidate support dir and return true if it's valid.
+static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
+ FileSystem::Instance().Resolve(path);
+ return FileSystem::Instance().IsDirectory(path);
+};
+
bool HostInfoMacOSX::ComputeSupportExeDirectory(FileSpec &file_spec) {
FileSpec lldb_file_spec = GetShlibDir();
if (!lldb_file_spec)
@@ -144,16 +150,22 @@ static void ParseOSVersion(llvm::VersionTuple &version,
NSString *Key) {
#endif
} else {
// Find the bin path relative to the lib path where the cmake-based
-// OS X .dylib lives. This is not going to work if the bin and lib
-// dir are not both in the same dir.
+// OS X .dylib lives. We try looking first at a possible sibling `bin`
+// directory, and then at the `lib` directory itself.
//
-// It is not going to work to do it by the executable path either,
+// It is not going to work to do it by the executable path,
// as in the case of a python script, the executable is python, not
// the lldb driver.
+FileSpec support_dir_spec_lib(raw_path);
raw_path.append("/../bin");
-FileSpec support_dir_spec(raw_path);
-FileSystem::Instance().Resolve(support_dir_spec);
-if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
+FileSpec support_dir_spec_bin(raw_path);
+FileSpec support_dir_spec;
+
+if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_bin)) {
+ support_dir_spec = support_dir_spec_bin;
+} else if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_lib)) {
+ support_dir_spec = support_dir_spec_lib;
+} else {
Log *log = GetLog(LLDBLog::Host);
LLDB_LOG(log, "failed to find support directory");
return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Walter Erquinigo (walter-erquinigo)
Changes
LLDB on OSX is looking at a `bin` directory sibling to the `lib` one that
contains liblldb for its supporting executables. This works well for CMake,
however, for other build systems like bazel, it's not that easy to have that
build structure, for which it's much easier to also use the `lib` directory as
a fallback under the absence of `bin`.
This shouldn't break anything, but instead should make it a bit easier for LLDB
to work with different build systems and folder structures.
---
Full diff: https://github.com/llvm/llvm-project/pull/103458.diff
1 Files Affected:
- (modified) lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm (+18-6)
``diff
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index f96e2cf80c5fac..02d02a21ad0955 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -124,6 +124,12 @@ static void ParseOSVersion(llvm::VersionTuple &version,
NSString *Key) {
return g_program_filespec;
}
+/// Resolve the given candidate support dir and return true if it's valid.
+static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
+ FileSystem::Instance().Resolve(path);
+ return FileSystem::Instance().IsDirectory(path);
+};
+
bool HostInfoMacOSX::ComputeSupportExeDirectory(FileSpec &file_spec) {
FileSpec lldb_file_spec = GetShlibDir();
if (!lldb_file_spec)
@@ -144,16 +150,22 @@ static void ParseOSVersion(llvm::VersionTuple &version,
NSString *Key) {
#endif
} else {
// Find the bin path relative to the lib path where the cmake-based
-// OS X .dylib lives. This is not going to work if the bin and lib
-// dir are not both in the same dir.
+// OS X .dylib lives. We try looking first at a possible sibling `bin`
+// directory, and then at the `lib` directory itself.
//
-// It is not going to work to do it by the executable path either,
+// It is not going to work to do it by the executable path,
// as in the case of a python script, the executable is python, not
// the lldb driver.
+FileSpec support_dir_spec_lib(raw_path);
raw_path.append("/../bin");
-FileSpec support_dir_spec(raw_path);
-FileSystem::Instance().Resolve(support_dir_spec);
-if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
+FileSpec support_dir_spec_bin(raw_path);
+FileSpec support_dir_spec;
+
+if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_bin)) {
+ support_dir_spec = support_dir_spec_bin;
+} else if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_lib)) {
+ support_dir_spec = support_dir_spec_lib;
+} else {
Log *log = GetLog(LLDBLog::Host);
LLDB_LOG(log, "failed to find support directory");
return false;
``
https://github.com/llvm/llvm-project/pull/103458
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
https://github.com/keith approved this pull request. lgtm, but not a LLDB expert https://github.com/llvm/llvm-project/pull/103458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
keith wrote: note that this is similar to lldb-server is discovered on linux as well https://github.com/llvm/llvm-project/pull/103458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/103458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
https://github.com/JDevlieghere approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/103458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
@@ -144,16 +150,22 @@ static void ParseOSVersion(llvm::VersionTuple &version,
NSString *Key) {
#endif
} else {
// Find the bin path relative to the lib path where the cmake-based
-// OS X .dylib lives. This is not going to work if the bin and lib
-// dir are not both in the same dir.
+// OS X .dylib lives. We try looking first at a possible sibling `bin`
+// directory, and then at the `lib` directory itself.
//
-// It is not going to work to do it by the executable path either,
+// It is not going to work to do it by the executable path,
// as in the case of a python script, the executable is python, not
// the lldb driver.
+FileSpec support_dir_spec_lib(raw_path);
raw_path.append("/../bin");
-FileSpec support_dir_spec(raw_path);
-FileSystem::Instance().Resolve(support_dir_spec);
-if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
+FileSpec support_dir_spec_bin(raw_path);
+FileSpec support_dir_spec;
JDevlieghere wrote:
You can simplify this slightly without modifying `raw_path`:
```
FileSpec support_dir_spec_lib(raw_path);
FileSpec support_dir_spec_bin =
support_dir_spec_lib.CopyByAppendingPathComponent("/../bin");
FileSpec support_dir_spec;
```
https://github.com/llvm/llvm-project/pull/103458
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/103458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
https://github.com/bulbazord approved this pull request. lgtm, one small suggestion https://github.com/llvm/llvm-project/pull/103458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)
@@ -144,16 +150,22 @@ static void ParseOSVersion(llvm::VersionTuple &version,
NSString *Key) {
#endif
} else {
// Find the bin path relative to the lib path where the cmake-based
-// OS X .dylib lives. This is not going to work if the bin and lib
-// dir are not both in the same dir.
+// OS X .dylib lives. We try looking first at a possible sibling `bin`
+// directory, and then at the `lib` directory itself.
bulbazord wrote:
If you're doing this to support non-CMake build systems (e.g. Bazel), could you
write some details explaining why we might want to look in `lib/`? I was
thinking something to the effect of "some build systems may put executables in
the same directory as the final liblldb so we look there too".
https://github.com/llvm/llvm-project/pull/103458
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
@@ -0,0 +1,77 @@
+//===-- RealpathPrefixes.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_REALPATHPREFIXES_H
+#define LLDB_CORE_REALPATHPREFIXES_H
+
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+#include
+#include
+
+namespace lldb_private {
+
+class RealpathPrefixes {
+public:
+ /// \param[in] file_spec_list
+ /// Prefixes are obtained from FileSpecList, through FileSpec::GetPath(),
+ /// which ensures that the paths are normalized. For example:
+ /// "./foo/.." -> ""
+ /// "./foo/../bar" -> "bar"
+ ///
+ /// \param[in] fs
+ /// An optional filesystem to use for realpath'ing. If not set, the real
+ /// filesystem will be used.
+ explicit RealpathPrefixes(const FileSpecList &file_spec_list,
+llvm::IntrusiveRefCntPtr fs
=
+llvm::vfs::getRealFileSystem());
+
+ std::optional ResolveSymlinks(const FileSpec &file_spec);
+
+ // If/when Statistics.h/cpp is moved into Utility, we can remove these
+ // methods, hold a (weak) pointer to `TargetStats` and directly increment
+ // on that object.
+ void IncreaseSourceRealpathAttemptCount() {
+++m_source_realpath_attempt_count;
+ }
+ uint32_t GetSourceRealpathAttemptCount() const {
+return m_source_realpath_attempt_count;
+ };
bulbazord wrote:
nit: stray semicolon, this will introduce a compiler warning.
https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/bulbazord commented: Interesting. I left a few minor comments, I think Jim and Greg got a lot of the edge cases and bulky feedback out of the way already. :) https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
@@ -0,0 +1,77 @@ +//===-- RealpathPrefixes.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_REALPATHPREFIXES_H bulbazord wrote: nit: `LLDB_UTILITY_REALPATHPREFIXES_H` https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
https://github.com/bulbazord approved this pull request. The LLDB changes look good to me. I can't speak for the clang portions but fwiw I think they look ok too. https://github.com/llvm/llvm-project/pull/103388 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Expose log path in extension settings (PR #103482)
https://github.com/vogelsgesang created
https://github.com/llvm/llvm-project/pull/103482
lldb-dap already supports a log file which can be enabled by setting the
`LLDBDAP_LOG` environment variable. With this commit, the log location can be
set directly through the VS-Code extension settings.
Also, this commit bumps the version number, such that the new VS Code extension
gets published to the Marketplace.
>From 6c9da388efecf0e33acd6678081bfb04aca4939f Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang
Date: Tue, 13 Aug 2024 00:34:42 +
Subject: [PATCH] [lldb-dap] Expose log path in extension settings
lldb-dap already supports a log file which can be enabled by setting the
`LLDBDAP_LOG` environment variable. With this commit, the log location
can be set directly through the VS-Code extension settings.
Also, this commit bumps the version number, such that the new VS Code
extension gets published to the Marketplace.
---
lldb/tools/lldb-dap/package-lock.json | 4 ++--
lldb/tools/lldb-dap/package.json| 7 ++-
lldb/tools/lldb-dap/src-ts/extension.ts | 26 -
3 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/lldb/tools/lldb-dap/package-lock.json
b/lldb/tools/lldb-dap/package-lock.json
index 8c70cc2d30e144..96570e42dbfdc4 100644
--- a/lldb/tools/lldb-dap/package-lock.json
+++ b/lldb/tools/lldb-dap/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "lldb-dap",
- "version": "0.2.0",
+ "version": "0.2.4",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "lldb-dap",
- "version": "0.2.0",
+ "version": "0.2.4",
"license": "Apache 2.0 License with LLVM exceptions",
"devDependencies": {
"@types/node": "^18.11.18",
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index 97e4efe7bac19d..4f4261d1718c01 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -1,7 +1,7 @@
{
"name": "lldb-dap",
"displayName": "LLDB DAP",
- "version": "0.2.3",
+ "version": "0.2.4",
"publisher": "llvm-vs-code-extensions",
"homepage": "https://lldb.llvm.org";,
"description": "LLDB debugging from VSCode",
@@ -73,6 +73,11 @@
"scope": "resource",
"type": "string",
"description": "The path to the lldb-dap binary."
+},
+"lldb-dap.log-path": {
+ "scope": "resource",
+ "type": "string",
+ "description": "The log path for lldb-dap (if any)"
}
}
},
diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts
b/lldb/tools/lldb-dap/src-ts/extension.ts
index 791175f7b46224..7df09f7a29dad7 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -14,13 +14,29 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
session: vscode.DebugSession,
packageJSONExecutable: vscode.DebugAdapterExecutable | undefined,
): Promise {
- const path = vscode.workspace
-.getConfiguration("lldb-dap", session.workspaceFolder)
-.get("executable-path");
+ const config = vscode.workspace
+.getConfiguration("lldb-dap", session.workspaceFolder);
+ const path = config.get("executable-path");
+ const log_path = config.get("log-path");
+
+ let env : { [key: string]: string } = {};
+ if (log_path) {
+env["LLDBDAP_LOG"] = log_path;
+ }
+
if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+return new vscode.DebugAdapterExecutable(path, [], {env});
+ } else if (packageJSONExecutable) {
+return new
vscode.DebugAdapterExecutable(packageJSONExecutable.command,
packageJSONExecutable.args, {
+ ...packageJSONExecutable.options,
+ env: {
+...packageJSONExecutable.options?.env,
+...env
+ }
+});
+ } else {
+return undefined;
}
- return packageJSONExecutable;
},
};
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Expose log path in extension settings (PR #103482)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Adrian Vogelsgesang (vogelsgesang)
Changes
lldb-dap already supports a log file which can be enabled by setting the
`LLDBDAP_LOG` environment variable. With this commit, the log location can be
set directly through the VS-Code extension settings.
Also, this commit bumps the version number, such that the new VS Code extension
gets published to the Marketplace.
---
Full diff: https://github.com/llvm/llvm-project/pull/103482.diff
3 Files Affected:
- (modified) lldb/tools/lldb-dap/package-lock.json (+2-2)
- (modified) lldb/tools/lldb-dap/package.json (+6-1)
- (modified) lldb/tools/lldb-dap/src-ts/extension.ts (+21-5)
``diff
diff --git a/lldb/tools/lldb-dap/package-lock.json
b/lldb/tools/lldb-dap/package-lock.json
index 8c70cc2d30e144..96570e42dbfdc4 100644
--- a/lldb/tools/lldb-dap/package-lock.json
+++ b/lldb/tools/lldb-dap/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "lldb-dap",
- "version": "0.2.0",
+ "version": "0.2.4",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "lldb-dap",
- "version": "0.2.0",
+ "version": "0.2.4",
"license": "Apache 2.0 License with LLVM exceptions",
"devDependencies": {
"@types/node": "^18.11.18",
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index 97e4efe7bac19d..4f4261d1718c01 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -1,7 +1,7 @@
{
"name": "lldb-dap",
"displayName": "LLDB DAP",
- "version": "0.2.3",
+ "version": "0.2.4",
"publisher": "llvm-vs-code-extensions",
"homepage": "https://lldb.llvm.org";,
"description": "LLDB debugging from VSCode",
@@ -73,6 +73,11 @@
"scope": "resource",
"type": "string",
"description": "The path to the lldb-dap binary."
+},
+"lldb-dap.log-path": {
+ "scope": "resource",
+ "type": "string",
+ "description": "The log path for lldb-dap (if any)"
}
}
},
diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts
b/lldb/tools/lldb-dap/src-ts/extension.ts
index 791175f7b46224..7df09f7a29dad7 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -14,13 +14,29 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
session: vscode.DebugSession,
packageJSONExecutable: vscode.DebugAdapterExecutable | undefined,
): Promise {
- const path = vscode.workspace
-.getConfiguration("lldb-dap", session.workspaceFolder)
-.get("executable-path");
+ const config = vscode.workspace
+.getConfiguration("lldb-dap", session.workspaceFolder);
+ const path = config.get("executable-path");
+ const log_path = config.get("log-path");
+
+ let env : { [key: string]: string } = {};
+ if (log_path) {
+env["LLDBDAP_LOG"] = log_path;
+ }
+
if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+return new vscode.DebugAdapterExecutable(path, [], {env});
+ } else if (packageJSONExecutable) {
+return new
vscode.DebugAdapterExecutable(packageJSONExecutable.command,
packageJSONExecutable.args, {
+ ...packageJSONExecutable.options,
+ env: {
+...packageJSONExecutable.options?.env,
+...env
+ }
+});
+ } else {
+return undefined;
}
- return packageJSONExecutable;
},
};
}
``
https://github.com/llvm/llvm-project/pull/103482
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Use CLANG_RESOURCE_DIR more consistently (PR #103388)
https://github.com/MaskRay approved this pull request. clangDriver changes look reasonable. https://github.com/llvm/llvm-project/pull/103388 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
@@ -0,0 +1,77 @@
+//===-- RealpathPrefixes.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_REALPATHPREFIXES_H
+#define LLDB_CORE_REALPATHPREFIXES_H
+
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+#include
+#include
+
+namespace lldb_private {
+
+class RealpathPrefixes {
+public:
+ /// \param[in] file_spec_list
+ /// Prefixes are obtained from FileSpecList, through FileSpec::GetPath(),
+ /// which ensures that the paths are normalized. For example:
+ /// "./foo/.." -> ""
+ /// "./foo/../bar" -> "bar"
+ ///
+ /// \param[in] fs
+ /// An optional filesystem to use for realpath'ing. If not set, the real
+ /// filesystem will be used.
+ explicit RealpathPrefixes(const FileSpecList &file_spec_list,
+llvm::IntrusiveRefCntPtr fs
=
+llvm::vfs::getRealFileSystem());
+
+ std::optional ResolveSymlinks(const FileSpec &file_spec);
+
+ // If/when Statistics.h/cpp is moved into Utility, we can remove these
+ // methods, hold a (weak) pointer to `TargetStats` and directly increment
+ // on that object.
+ void IncreaseSourceRealpathAttemptCount() {
+++m_source_realpath_attempt_count;
+ }
+ uint32_t GetSourceRealpathAttemptCount() const {
+return m_source_realpath_attempt_count;
+ };
royitaqi wrote:
Thanks for the catch~!
Fixed both.
Interestingly this one with the semicolon didn't give me compiler warning on a
fresh build. But yeah it should be removed anyways. Thanks again for your sharp
eyes.
https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jeffreytan81 wrote: Thanks for sharing. I have written a shell script to run `TestSingleThreadStepTimeout.py` in a loop infinitely until failing. Unfortunately, it has been running for 127 iterations (more than 7 hours) on my CentOS9 machine, haven't seen a single failure/timeout yet. I am not confident that I can reproduce with the linux machine I have. Assuming it is only linux platforms, If this is blocking stuff, I am thinking of disabling the tests for Linux platform until a solid reproduce can be found to investigate. Let me know if you have any other thoughts. 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][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/102708
>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/4] Initial attempt at new classes Summary statistics in
SummaryStatistics.h/cpp.
---
lldb/include/lldb/Target/SummaryStatistics.h | 37
lldb/include/lldb/Target/Target.h| 5 +++
lldb/source/Core/ValueObject.cpp | 5 +++
lldb/source/Target/CMakeLists.txt| 1 +
lldb/source/Target/SummaryStatistics.cpp | 26 ++
lldb/source/Target/Target.cpp| 9 +
6 files changed, 83 insertions(+)
create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
create mode 100644 lldb/source/Target/SummaryStatistics.cpp
diff --git a/lldb/include/lldb/Target/SummaryStatistics.h
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.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_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ lldb_private::StatsDuration &GetDurationReference();
+
+ lldb_private::ConstString GetName() const;
+
+ uint64_t GetSummaryCount() const;
+
+private:
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
#include "lldb/Target/PathMappingList.h"
#include "lldb/Target/SectionLoadHistory.h"
#include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
#include "lldb/Target/ThreadSpec.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
+
private:
std::optional
GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public
std::enable_shared_from_this,
void ClearAllLoadedSections();
+ lldb_private::StatsDuration&
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
/// Set the \a Trace object containing processor trace information of this
/// target.
///
@@ -1554,6 +1558,7 @@ class Target : public
std::enable_shared_from_this,
std::string m_label;
ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+ std::map
m_summary_stats_map;
SectionLoadHistory m_section_load_history;
BreakpointList m_breakpoint_list;
BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ 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%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
summary_ptr->FormatObject(this, destination, actual_options);
}
m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
Statistics.cpp
StopInfo.cpp
StructuredDataPlugin.cpp
+ SummaryStatistics.cpp
SystemRuntime.cpp
Target.cpp
TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp
b/lldb/sour
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/102708
>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/5] Initial attempt at new classes Summary statistics in
SummaryStatistics.h/cpp.
---
lldb/include/lldb/Target/SummaryStatistics.h | 37
lldb/include/lldb/Target/Target.h| 5 +++
lldb/source/Core/ValueObject.cpp | 5 +++
lldb/source/Target/CMakeLists.txt| 1 +
lldb/source/Target/SummaryStatistics.cpp | 26 ++
lldb/source/Target/Target.cpp| 9 +
6 files changed, 83 insertions(+)
create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
create mode 100644 lldb/source/Target/SummaryStatistics.cpp
diff --git a/lldb/include/lldb/Target/SummaryStatistics.h
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.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_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ lldb_private::StatsDuration &GetDurationReference();
+
+ lldb_private::ConstString GetName() const;
+
+ uint64_t GetSummaryCount() const;
+
+private:
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
#include "lldb/Target/PathMappingList.h"
#include "lldb/Target/SectionLoadHistory.h"
#include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
#include "lldb/Target/ThreadSpec.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
+
private:
std::optional
GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public
std::enable_shared_from_this,
void ClearAllLoadedSections();
+ lldb_private::StatsDuration&
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
/// Set the \a Trace object containing processor trace information of this
/// target.
///
@@ -1554,6 +1558,7 @@ class Target : public
std::enable_shared_from_this,
std::string m_label;
ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+ std::map
m_summary_stats_map;
SectionLoadHistory m_section_load_history;
BreakpointList m_breakpoint_list;
BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ 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%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
summary_ptr->FormatObject(this, destination, actual_options);
}
m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
Statistics.cpp
StopInfo.cpp
StructuredDataPlugin.cpp
+ SummaryStatistics.cpp
SystemRuntime.cpp
Target.cpp
TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp
b/lldb/sour
[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";
Jlalond wrote:
@clayborg
I created a basic template test, and made sure the formatter was invoked, and I
added a different test for the two vector cases.
I did not test nested summaries, and will add it if desired.
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;
+ }
+
+ llvm::json::Value ToJSON();
+
+private:
+ std::map
m_summary_stats_map;
Jlalond wrote:
I attempted to move us to `std::unordered_map`, but failed due to ConstString
not exposing a way to hash it. I could try implementing that but I think for v1
`std::map` should be acceptable.
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 updated
https://github.com/llvm/llvm-project/pull/102708
>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/6] Initial attempt at new classes Summary statistics in
SummaryStatistics.h/cpp.
---
lldb/include/lldb/Target/SummaryStatistics.h | 37
lldb/include/lldb/Target/Target.h| 5 +++
lldb/source/Core/ValueObject.cpp | 5 +++
lldb/source/Target/CMakeLists.txt| 1 +
lldb/source/Target/SummaryStatistics.cpp | 26 ++
lldb/source/Target/Target.cpp| 9 +
6 files changed, 83 insertions(+)
create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
create mode 100644 lldb/source/Target/SummaryStatistics.cpp
diff --git a/lldb/include/lldb/Target/SummaryStatistics.h
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.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_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+ SummaryStatistics(lldb_private::ConstString name) :
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+ lldb_private::StatsDuration &GetDurationReference();
+
+ lldb_private::ConstString GetName() const;
+
+ uint64_t GetSummaryCount() const;
+
+private:
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
#include "lldb/Target/PathMappingList.h"
#include "lldb/Target/SectionLoadHistory.h"
#include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
#include "lldb/Target/ThreadSpec.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
+
private:
std::optional
GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public
std::enable_shared_from_this,
void ClearAllLoadedSections();
+ lldb_private::StatsDuration&
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
/// Set the \a Trace object containing processor trace information of this
/// target.
///
@@ -1554,6 +1558,7 @@ class Target : public
std::enable_shared_from_this,
std::string m_label;
ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+ std::map
m_summary_stats_map;
SectionLoadHistory m_section_load_history;
BreakpointList m_breakpoint_list;
BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ 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%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
summary_ptr->FormatObject(this, destination, actual_options);
}
m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
Statistics.cpp
StopInfo.cpp
StructuredDataPlugin.cpp
+ SummaryStatistics.cpp
SystemRuntime.cpp
Target.cpp
TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp
b/lldb/sour
[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)
xusheng6 wrote: > Just to be clear, if I'm understanding the packet we'll be getting back, we > have no indication that we hit the breakpoint, we only show that we are > stopped at an address which has a breakpoint. Current lldb stepping behavior > will work -- because the rule is, when we stop at a breakpoint address, we > will say we hit the breakpoint. I am refining a patch #96260 which changes > this behavior -- we handle "instruction stepped / stopped at a breakpoint" > differently than "hit a breakpoint". I worry this difference will be lost > with a stub that reports `swbreak`/`hwbreak`, stepping won't work correctly, > and we won't capture it in the testsuite or on any of our CI bots. My work mainly concerns the case when lldb is connected to a gdbserver, and as far as I can tell, gdbserver does not send a `reason:breakpoint` packet at all. Maybe lldb-server sends it? I am not sure Also, I do not think lldb itself is adjusting the PC -- I think lldb-server does it, and that is also the reason why my PR did not need to alter the lldb behavior at all. lldb always expects the remote to have adjusted the PC if necessary, and it just uses whatever value that is reported 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)
https://github.com/clayborg 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)
@@ -258,6 +258,9 @@ class TypeSummaryImpl {
virtual std::string GetDescription() = 0;
+ virtual ConstString GetName() = 0;
+ virtual ConstString GetImplType() = 0;
clayborg wrote:
Add header doc for these explaining what they are.
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)
@@ -145,15 +148,34 @@ std::string CXXFunctionSummaryFormat::GetDescription() {
return std::string(sstr.GetString());
}
+ConstString CXXFunctionSummaryFormat::GetName() {
+ return ConstString(m_description);
+}
+
+ConstString CXXFunctionSummaryFormat::GetImplType() {
+ return ConstString("c++");
+}
+
clayborg wrote:
We should switch to `std::string` instead of `ConstString.. These objects
aren't starting out as ConstString objects. We only want to use ConstString
objects for things we are searching for by name where there will be many copies
of the string.
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)
@@ -602,7 +602,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr, actual_options.SetLanguage(GetPreferredDisplayLanguage()); // this is a hot path in code and we prefer to avoid setting this string all - // too often also clearing out other information that we might care to see in + // too often also clearing out other information that we might care` to see in clayborg wrote: remove this extra character that was added 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)
@@ -258,6 +258,9 @@ class TypeSummaryImpl {
virtual std::string GetDescription() = 0;
+ virtual ConstString GetName() = 0;
+ virtual ConstString GetImplType() = 0;
+
clayborg wrote:
We should switch to `std::string` instead of `ConstString.. These objects that
get returned here don't start out as `ConstString` objects. We only want to use
`ConstString` objects for things we are comparing many things by name where
there will be many instances of objects that have this 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;
clayborg wrote:
Yeah, we don't need `ConstString` objects here. See first inline comment.
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)
@@ -116,6 +116,9 @@ std::string StringSummaryFormat::GetDescription() {
return std::string(sstr.GetString());
}
+ConstString StringSummaryFormat::GetName() { return ConstString(m_format_str);
}
+ConstString StringSummaryFormat::GetImplType() { return ConstString("string");
}
clayborg wrote:
We should switch to `std::string` instead of `ConstString.. These objects
aren't starting out as ConstString objects. We only want to use ConstString
objects for things we are searching for by name where there will be many copies
of the string.
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 +177,84 @@ struct StatisticsOptions {
std::optional m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ explicit SummaryStatistics(lldb_private::ConstString name,
+ lldb_private::ConstString impl_type)
+ : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ 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; }
+
+ ConstString GetImplType() const { return m_impl_type; }
+
+ 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), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+ private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+ };
+
+ SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_impl_type;
+ 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:
+ /// 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::TypeSummaryImpl &provider)
{
+std::lock_guard guard(m_map_mutex);
+m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(),
+provider.GetImplType());
+
+SummaryStatistics &summary_stats =
+m_summary_stats_map.at(provider.GetName());
+return summary_stats;
+ }
+
+ llvm::json::Value ToJSON();
+
+private:
+ std::map
+ m_summary_stats_map;
clayborg wrote:
Since we don't need to use `ConstString` objects we should use a
`llvm::StringMap`
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] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
https://github.com/clayborg commented: I vote to leave this PR as is because `VMRangeToFileOffset` is a list of VMRanges that map to file offsets and this list combines entries with consecutive address ranges whose data is consecutive in the core file. And `VMRangeToPermissions` does not break things up. So not sure how we could use memory regions with this. It would also make the patch really large to have `Process::ReadMemory` return a `DataExtractor` and change all code over to using that. I do like this idea and think that we should work on a patch that can do this because `PeekMemory` could easily set the data pointers within a `DataExtractor` without copying the 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] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)
clayborg wrote: That being said, if there is something we can do to this PR to make it easy for you (Pavel) to implement this feature, we can make those changes. Like I think it would be fair to change all `PeekMemory/DoPeekMemory` over a virtual `ReadMemory/DoReadMemory` that returns a `DataExtractor` as this can easily take place of the `ArrayRef` values that are returned. This: ``` virtual llvm::ArrayRef PeekMemory(lldb::addr_t low, lldb::addr_t high); ``` Could become: ``` virtual DataExtractor ReadMemory(lldb::addr_t low, lldb::addr_t high); ``` And we can adjust all users of `PeekMemory` and `DoPeakMemory` to use the the above functions? 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
