[Lldb-commits] [lldb] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (PR #84998)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/84998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Test] Fix the test case of listing verbose break info on Windows (PR #85200)
https://github.com/bvlgah created
https://github.com/llvm/llvm-project/pull/85200
I noticed a failure of [running LLDB test suites on Windows
AArch64](https://lab.llvm.org/buildbot/#/builders/219/builds/9849). The failed
test case is about
checking output of command `breakpoint list -v -L c++`, and an mismatch on the
demangled
name of a function occurred. The test case expects `ns::func(void)`, but on
Windows it is `int ns::func(void)`.
It results from the different mangling scheme used by MSVC, and the comparison
is as follows:
| Scheme | Mangled | Demangled (fully) | Note |
| --- | --- | --- | --- |
| MSVC | `?func@ns@@YAHXZ` | `int __cdecl ns::func(void)` |
[Godbolt](https://godbolt.org/z/5ns8c7xW3) (I have no available Windows device)
|
| Itanium | `_ZN2ns4funcEv` | `ns::func()` | |
According to the current use of MSVC demangling,
https://github.com/llvm/llvm-project/blob/8f68022f8e6e54d1aeae4ed301f5a015963089b7/lldb/source/Core/Mangled.cpp#L128-L143
the `__cdecl` specifier is not part of the name. However, the function's
parameter types should be present
as ` llvm::MSDF_NoVariableType` [does not affect a symbol for
functions](https://github.com/llvm/llvm-project/blob/8f68022f8e6e54d1aeae4ed301f5a015963089b7/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp#L417-L453).
Therefore, it is inappropriate to assume the demangled name are the same on all
platforms. Instead of tweaking the
existing code of demangling to get the same (demangled) name, I think it is
more reasonable to modify the test case.
>From e7d29ef84b67aa877a4055aa0e0551209efa1136 Mon Sep 17 00:00:00 2001
From: bvlgah
Date: Thu, 14 Mar 2024 15:22:02 +0800
Subject: [PATCH] [LLDB][Test] Fix the test case of listing verbose break info
on Windows
---
.../breakpoint/breakpoint_options/TestBreakpointOptions.py| 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git
a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
index 5179ffe730b9a0..d262b627195bc8 100644
---
a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
+++
b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
@@ -95,8 +95,10 @@ def breakpoint_options_language_test(self):
self.expect(
"breakpoint list -v",
"Verbose breakpoint list contains mangled names",
+# The demangled function name is system-dependent, e.g.
+# 'int ns::func(void)' on Windows and 'ns::func()' on Linux.
substrs=[
-"function = ns::func",
+f"function = {function.GetName()}",
f"mangled function = {function.GetMangledName()}",
],
)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Test] Fix the test case of listing verbose break info on Windows (PR #85200)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: None (bvlgah)
Changes
I noticed a failure of [running LLDB test suites on Windows
AArch64](https://lab.llvm.org/buildbot/#/builders/219/builds/9849). The failed
test case is about
checking output of command `breakpoint list -v -L c++`, and an mismatch on the
demangled
name of a function occurred. The test case expects `ns::func(void)`, but on
Windows it is `int ns::func(void)`.
It results from the different mangling scheme used by MSVC, and the comparison
is as follows:
| Scheme | Mangled | Demangled (fully) | Note |
| --- | --- | --- | --- |
| MSVC | `?func@ns@@YAHXZ` | `int __cdecl ns::func(void)` |
[Godbolt](https://godbolt.org/z/5ns8c7xW3) (I have no available Windows device)
|
| Itanium | `_ZN2ns4funcEv` | `ns::func()` | |
According to the current use of MSVC demangling,
https://github.com/llvm/llvm-project/blob/8f68022f8e6e54d1aeae4ed301f5a015963089b7/lldb/source/Core/Mangled.cpp#L128-L143
the `__cdecl` specifier is not part of the name. However, the function's
parameter types should be present
as ` llvm::MSDF_NoVariableType` [does not affect a symbol for
functions](https://github.com/llvm/llvm-project/blob/8f68022f8e6e54d1aeae4ed301f5a015963089b7/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp#L417-L453).
Therefore, it is inappropriate to assume the demangled name are the same on all
platforms. Instead of tweaking the
existing code of demangling to get the same (demangled) name, I think it is
more reasonable to modify the test case.
---
Full diff: https://github.com/llvm/llvm-project/pull/85200.diff
1 Files Affected:
- (modified)
lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
(+3-1)
``diff
diff --git
a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
index 5179ffe730b9a0..d262b627195bc8 100644
---
a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
+++
b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
@@ -95,8 +95,10 @@ def breakpoint_options_language_test(self):
self.expect(
"breakpoint list -v",
"Verbose breakpoint list contains mangled names",
+# The demangled function name is system-dependent, e.g.
+# 'int ns::func(void)' on Windows and 'ns::func()' on Linux.
substrs=[
-"function = ns::func",
+f"function = {function.GetName()}",
f"mangled function = {function.GetMangledName()}",
],
)
``
https://github.com/llvm/llvm-project/pull/85200
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Test] Fix the test case of listing verbose break info on Windows (PR #85200)
DavidSpickett wrote: Thanks for the fix! We (Linaro) were about to look into this ourselves. I'm going to merge this now so we can see if it works ASAP. https://github.com/llvm/llvm-project/pull/85200 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Test] Fix the test case of listing verbose break info on Windows (PR #85200)
https://github.com/DavidSpickett approved this pull request. https://github.com/llvm/llvm-project/pull/85200 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] ece2c25 - [LLDB][Test] Fix the test case of listing verbose break info on Windows (#85200)
Author: bvlgah
Date: 2024-03-14T09:11:37Z
New Revision: ece2c25ab9f572f719fd18f1ced4fa80f3e5ed1c
URL:
https://github.com/llvm/llvm-project/commit/ece2c25ab9f572f719fd18f1ced4fa80f3e5ed1c
DIFF:
https://github.com/llvm/llvm-project/commit/ece2c25ab9f572f719fd18f1ced4fa80f3e5ed1c.diff
LOG: [LLDB][Test] Fix the test case of listing verbose break info on Windows
(#85200)
I noticed a failure of [running LLDB test suites on Windows
AArch64](https://lab.llvm.org/buildbot/#/builders/219/builds/9849). The
failed test case is about
checking output of command `breakpoint list -v -L c++`, and an mismatch
on the demangled
name of a function occurred. The test case expects `ns::func(void)`, but
on Windows it is `int ns::func(void)`.
It results from the different mangling scheme used by MSVC, and the
comparison is as follows:
| Scheme | Mangled | Demangled (fully) | Note |
| --- | --- | --- | --- |
| MSVC | `?func@ns@@YAHXZ` | `int __cdecl ns::func(void)` |
[Godbolt](https://godbolt.org/z/5ns8c7xW3) (I have no available Windows
device) |
| Itanium | `_ZN2ns4funcEv` | `ns::func()` | |
According to the current use of MSVC demangling,
https://github.com/llvm/llvm-project/blob/8f68022f8e6e54d1aeae4ed301f5a015963089b7/lldb/source/Core/Mangled.cpp#L128-L143
the `__cdecl` specifier is not part of the name. However, the function's
parameter types should be present
as ` llvm::MSDF_NoVariableType` [does not affect a symbol for
functions](https://github.com/llvm/llvm-project/blob/8f68022f8e6e54d1aeae4ed301f5a015963089b7/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp#L417-L453).
Therefore, it is inappropriate to assume the demangled name are the same
on all platforms. Instead of tweaking the
existing code of demangling to get the same (demangled) name, I think it
is more reasonable to modify the test case.
Added:
Modified:
lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
Removed:
diff --git
a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
index 5179ffe730b9a0..d262b627195bc8 100644
---
a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
+++
b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
@@ -95,8 +95,10 @@ def breakpoint_options_language_test(self):
self.expect(
"breakpoint list -v",
"Verbose breakpoint list contains mangled names",
+# The demangled function name is system-dependent, e.g.
+# 'int ns::func(void)' on Windows and 'ns::func()' on Linux.
substrs=[
-"function = ns::func",
+f"function = {function.GetName()}",
f"mangled function = {function.GetMangledName()}",
],
)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Test] Fix the test case of listing verbose break info on Windows (PR #85200)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/85200 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [flang] [libc] [libcxx] [lldb] [llvm] [mlir] [X86] Fast AVX-512-VNNI vpdpwssd tuning (PR #85033)
https://github.com/RKSimon requested changes to this pull request. This patch needs to be cleanly rebased on trunk (force push is OK in PR branchs) https://github.com/llvm/llvm-project/pull/85033 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [flang] [libc] [libcxx] [lldb] [llvm] [mlir] [X86] Fast AVX-512-VNNI vpdpwssd tuning (PR #85033)
ganeshgit wrote: > This patch needs to be cleanly rebased on trunk (force push is OK in PR > branchs) I think my overnight rebase scripts screwed the fixups. I will close this and I will raise a new pull request. https://github.com/llvm/llvm-project/pull/85033 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
kovdan01 wrote: @jasonmolenda Just in case you've missed - I've provided the use case description leading to the issue (as you requested) above https://github.com/llvm/llvm-project/pull/82603#issuecomment-1970019956. Would be glad if you let me know if it gives you enough info & if this particular PR is OK or the issue should be fixed in another way. https://github.com/llvm/llvm-project/pull/82603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
DavidSpickett wrote: I will be away for a while so @jasonmolenda please merge it if it looks fine to you. https://github.com/llvm/llvm-project/pull/82603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add lldb-python-scripts to the things installed on Linux. (PR #85080)
al45tair wrote: @JDevlieghere I don't believe I have merge rights here, so you'll have to do the honours. https://github.com/llvm/llvm-project/pull/85080 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a75d0a3 - [LLDB] Add lldb-python-scripts to the things installed on Linux. (#85080)
Author: Alastair Houghton Date: 2024-03-14T08:22:29-07:00 New Revision: a75d0a3dbab6215db228cedf5f2458e251c30637 URL: https://github.com/llvm/llvm-project/commit/a75d0a3dbab6215db228cedf5f2458e251c30637 DIFF: https://github.com/llvm/llvm-project/commit/a75d0a3dbab6215db228cedf5f2458e251c30637.diff LOG: [LLDB] Add lldb-python-scripts to the things installed on Linux. (#85080) Added: Modified: lldb/cmake/caches/Apple-lldb-Linux.cmake Removed: diff --git a/lldb/cmake/caches/Apple-lldb-Linux.cmake b/lldb/cmake/caches/Apple-lldb-Linux.cmake index 13d3839f852f61..b2d3cf595fe18d 100644 --- a/lldb/cmake/caches/Apple-lldb-Linux.cmake +++ b/lldb/cmake/caches/Apple-lldb-Linux.cmake @@ -5,4 +5,5 @@ set(LLVM_DISTRIBUTION_COMPONENTS liblldb lldb-argdumper lldb-server + lldb-python-scripts CACHE STRING "") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add lldb-python-scripts to the things installed on Linux. (PR #85080)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/85080 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 52557bc - [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (#84998)
Author: Jason Molenda
Date: 2024-03-14T08:58:27-07:00
New Revision: 52557bce73f64df5da13d42dd97b57fbd4ab1b12
URL:
https://github.com/llvm/llvm-project/commit/52557bce73f64df5da13d42dd97b57fbd4ab1b12
DIFF:
https://github.com/llvm/llvm-project/commit/52557bce73f64df5da13d42dd97b57fbd4ab1b12.diff
LOG: [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (#84998)
Darwin AArch64 application processors are run with Top Byte Ignore mode
enabled so metadata may be stored in the top byte, it needs to be
ignored when reading/writing memory. David Spickett handled this already
in the base class Process::ReadMemory but ProcessMachCore overrides that
method (to avoid the memory cache) and did not pick up the same change.
I add a test case that creates a pointer with metadata in the top byte
and dereferences it with a live process and with a corefile.
rdar://123784501
Added:
lldb/test/API/macosx/tbi-honored/Makefile
lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
lldb/test/API/macosx/tbi-honored/main.c
Modified:
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
Removed:
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 3961dcf0fbcc0e..7b9938d4f02020 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -652,7 +652,7 @@ size_t ProcessMachCore::ReadMemory(addr_t addr, void *buf,
size_t size,
Status &error) {
// Don't allow the caching that lldb_private::Process::ReadMemory does since
// in core files we have it all cached our our core file anyway.
- return DoReadMemory(addr, buf, size, error);
+ return DoReadMemory(FixAnyAddress(addr), buf, size, error);
}
size_t ProcessMachCore::DoReadMemory(addr_t addr, void *buf, size_t size,
diff --git a/lldb/test/API/macosx/tbi-honored/Makefile
b/lldb/test/API/macosx/tbi-honored/Makefile
new file mode 100644
index 00..10495940055b63
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
new file mode 100644
index 00..a5c0abd70e5a90
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
@@ -0,0 +1,57 @@
+"""Test that lldb on Darwin ignores metadata in the top byte of addresses,
both corefile and live."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTBIHonored(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def do_variable_access_tests(self, frame):
+self.assertEqual(
+frame.variables["pb"][0]
+.GetChildMemberWithName("p")
+.Dereference()
+.GetValueAsUnsigned(),
+15,
+)
+addr =
frame.variables["pb"][0].GetChildMemberWithName("p").GetValueAsUnsigned()
+# Confirm that there is metadata in the top byte of our pointer
+self.assertEqual((addr >> 56) & 0xFF, 0xFE)
+self.expect("expr -- *pb.p", substrs=["15"])
+self.expect("frame variable *pb.p", substrs=["15"])
+self.expect("expr -- *(int*)0x%x" % addr, substrs=["15"])
+
+# This test is valid on AArch64 systems with TBI mode enabled,
+# and an address mask that clears the top byte before reading
+# from memory.
+@skipUnlessDarwin
+@skipIf(archs=no_match(["arm64", "arm64e"]))
+@skipIfRemote
+def test(self):
+corefile = self.getBuildArtifact("process.core")
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.c")
+)
+
+# Test that we can dereference a pointer with TBI data
+# in a live process.
+self.do_variable_access_tests(thread.GetFrameAtIndex(0))
+
+# Create a corefile, delete this process
+self.runCmd("process save-core -s stack " + corefile)
+process.Destroy()
+self.dbg.DeleteTarget(target)
+
+# Now load the corefile
+target = self.dbg.CreateTarget("")
+process = target.LoadCore(corefile)
+thread = process.GetSelectedThread()
+self.assertTrue(process.GetSelectedThread().IsValid())
+
+# Test that we can dereference a pointer with TBI data
+# in a corefile process.
+self.do_variable_access_tests(thread.GetFrameAtIndex(0))
diff --git a/lldb/test/API/macosx/tbi-honored/main.c
b/lldb/test/API/macosx/tbi-honored/main.c
new file mode 100644
index 00..3d7ad0b04cd664
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/main.c
@@ -0,0 +1,13 @@
+#include
[Lldb-commits] [lldb] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (PR #84998)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/84998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Single thread stepping timeout prototype (PR #85260)
https://github.com/jeffreytan81 created
https://github.com/llvm/llvm-project/pull/85260
None
>From 6cccde22723157260e7c0b19bf8372aae8d1afaa Mon Sep 17 00:00:00 2001
From: jeffreytan81
Date: Wed, 6 Mar 2024 12:07:03 -0800
Subject: [PATCH 1/2] Fix strcmp build error on buildbot
---
lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
index b0a4446ba01581..40cb63755ee8a5 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
@@ -1,6 +1,7 @@
#include
#include
#include
+#include
#include
#include
#include
>From 6982a47579b372b0e34c751c9952b4aa03531aeb Mon Sep 17 00:00:00 2001
From: jeffreytan81
Date: Thu, 14 Mar 2024 09:24:58 -0700
Subject: [PATCH 2/2] Single thread timeout prototype
---
lldb/include/lldb/Target/ThreadPlan.h | 3 +-
.../Target/ThreadPlanSingleThreadTimeout.h| 134 ++
.../lldb/Target/ThreadPlanStepOverRange.h | 2 +
.../source/Target/ThreadPlanStepOverRange.cpp | 14 +-
4 files changed, 151 insertions(+), 2 deletions(-)
create mode 100644 lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
diff --git a/lldb/include/lldb/Target/ThreadPlan.h
b/lldb/include/lldb/Target/ThreadPlan.h
index bf68a42e54d18f..3b488841bd9010 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -302,7 +302,8 @@ class ThreadPlan : public
std::enable_shared_from_this,
eKindStepInRange,
eKindRunToAddress,
eKindStepThrough,
-eKindStepUntil
+eKindStepUntil,
+eKindSingleThreadTimeout
};
virtual ~ThreadPlan();
diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
new file mode 100644
index 00..a7b8b4ffaa8c40
--- /dev/null
+++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
@@ -0,0 +1,134 @@
+//===-- ThreadPlanSingleThreadTimeout.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
+//
+//===--===//
+
+// TODO
+#ifndef LLDB_TARGET_THREADPLANSINGLETHREADTIMEOUT_H
+#define LLDB_TARGET_THREADPLANSINGLETHREADTIMEOUT_H
+
+#include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Utility/Event.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/State.h"
+
+#include
+
+namespace lldb_private {
+
+enum class SingleThreadPlanTimeoutState {
+ InitialResume,
+ TimeoutHalt,
+ ResumingAllThreads,
+ AfterThreadResumed,
+};
+
+class ThreadPlanSingleThreadTimeout : public ThreadPlan {
+public:
+ ThreadPlanSingleThreadTimeout(Thread &thread)
+ : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout,
+ "Single thread timeout", thread, eVoteNo, eVoteNoOpinion),
+m_state(SingleThreadPlanTimeoutState::InitialResume) {
+std::thread t(thread_function, this);
+t.detach();
+ }
+
+ ~ThreadPlanSingleThreadTimeout() override = default;
+
+ void GetDescription(Stream *s, lldb::DescriptionLevel level) override {
+s->Printf("SingleThreadPlanTimeout, state(%d)", (int)m_state);
+ }
+ bool ValidatePlan(Stream *error) override { return true; }
+ bool WillStop() override { return true; }
+ bool DoPlanExplainsStop(Event *event_ptr) override { return true; }
+ lldb::StateType GetPlanRunState() override { return lldb::eStateRunning; }
+ static void thread_function(ThreadPlanSingleThreadTimeout *self) {
+int timeout_ms = 5000; // 5 seconds timeout
+std::this_thread::sleep_for(
+std::chrono::milliseconds(timeout_ms));
+self->HandleTimeout();
+ }
+
+ bool MischiefManaged() override {
+// return m_state == SingleThreadPlanTimeoutState::AfterThreadResumed;
+return GetPreviousPlan()->MischiefManaged();
+ }
+
+ bool ShouldStop(Event *event_ptr) override {
+if (m_state == SingleThreadPlanTimeoutState::InitialResume) {
+ return GetPreviousPlan()->ShouldStop(event_ptr);
+}
+return HandleEvent(event_ptr);
+ }
+
+ void SetStopOthers(bool new_value) override {
+GetPreviousPlan()->SetStopOthers(new_value);
+ }
+
+ bool StopOthers() override {
+if (m_state == SingleThreadPlanTimeoutState::ResumingAllThreads ||
+m_state == SingleThreadPlanTimeoutState::AfterThreadResumed)
+ return false;
+else
+ return GetPreviousPlan()->StopOthers();
+ }
+
+protected:
+ bool DoWillResume(lldb::StateType resume_state, bool current_plan) override {
+if (m_state == SingleThreadPlanTimeoutState::ResumingAllThreads) {
+ m_state = SingleTh
[Lldb-commits] [lldb] Single thread stepping timeout prototype (PR #85260)
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 10edabbcf331fdd53d27c5195de1b692a0063721
6982a47579b372b0e34c751c9952b4aa03531aeb --
lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanStepOverRange.h
lldb/source/Target/ThreadPlanStepOverRange.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
index a7b8b4ffaa..51a3abfcb7 100644
--- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
+++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
@@ -49,8 +49,7 @@ public:
lldb::StateType GetPlanRunState() override { return lldb::eStateRunning; }
static void thread_function(ThreadPlanSingleThreadTimeout *self) {
int timeout_ms = 5000; // 5 seconds timeout
-std::this_thread::sleep_for(
-std::chrono::milliseconds(timeout_ms));
+std::this_thread::sleep_for(std::chrono::milliseconds(timeout_ms));
self->HandleTimeout();
}
``
https://github.com/llvm/llvm-project/pull/85260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)
https://github.com/felipepiovezan updated
https://github.com/llvm/llvm-project/pull/83908
>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/7] [lldb] Allow languages to filter breakpoints set by line
Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.
It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
lldb/include/lldb/Target/Language.h | 4
lldb/source/Breakpoint/BreakpointResolver.cpp | 12
2 files changed, 16 insertions(+)
diff --git a/lldb/include/lldb/Target/Language.h
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
virtual llvm::StringRef GetInstanceVariableName() { return {}; }
+ virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+ }
+
protected:
// Classes that inherit from Language can see and modify these
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
#include "lldb/Symbol/CompileUnit.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
}
} // namespace
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl &sc_list) {
+ llvm::erase_if(sc_list, [](SymbolContext &sc) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+ return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+ });
+}
+
void BreakpointResolver::SetSCMatchesByLine(
SearchFilter &filter, SymbolContextList &sc_list, bool skip_prologue,
llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
all_scs.push_back(sc_list[i]);
+ ApplyLanguageFilters(all_scs);
+
while (all_scs.size()) {
uint32_t closest_line = UINT32_MAX;
>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/7] fixup! add target option
---
lldb/include/lldb/Target/Language.h | 9 ++--
lldb/include/lldb/Target/Target.h | 2 ++
lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
lldb/source/Target/Target.cpp | 8 +++
lldb/source/Target/TargetProperties.td| 3 +++
5 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/lldb/include/lldb/Target/Language.h
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
virtual llvm::StringRef GetInstanceVariableName() { return {}; }
- virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+ // Returns true if this SymbolContext should be used when setting breakpoints
+ // by line (number or regex). This is useful for languages that create
+ // artificial functions without any meaningful user code associated with them
+ // (e.g. code that gets expanded in late compilation stages, like by
+ // CoroSplitter).
+ virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
}
protected:
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
+ bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
private:
// Callbacks for m_launch_info.
void Arg0ValueChangedCallback();
diff --git a/lldb/source/Breakpoint/
[Lldb-commits] [lldb] 0adccd1 - [lldb] Allow languages to filter breakpoints set by line (#83908)
Author: Felipe de Azevedo Piovezan
Date: 2024-03-14T09:40:00-07:00
New Revision: 0adccd1a7fd8e30650f76bd33471de28b7939455
URL:
https://github.com/llvm/llvm-project/commit/0adccd1a7fd8e30650f76bd33471de28b7939455
DIFF:
https://github.com/llvm/llvm-project/commit/0adccd1a7fd8e30650f76bd33471de28b7939455.diff
LOG: [lldb] Allow languages to filter breakpoints set by line (#83908)
Some languages may create artificial functions that have no real user
code, even though there is line table information for them. One such
case is with coroutine code that receives the CoroSplitter
transformation in LLVM IR. That code transformation creates many
different Functions, cloning one Instruction into many Instructions in
many different Functions and copying the associated debug locations.
It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB
can ignore certain locations by querying its Language APIs and having it
decide based on, for example, mangling information.
Added:
Modified:
lldb/include/lldb/Target/Language.h
lldb/source/Breakpoint/BreakpointResolver.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Target/Language.cpp
lldb/source/Target/TargetProperties.td
Removed:
diff --git a/lldb/include/lldb/Target/Language.h
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..67714e6fdf942e 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -26,6 +26,15 @@
namespace lldb_private {
+class LanguageProperties : public Properties {
+public:
+ LanguageProperties();
+
+ static llvm::StringRef GetSettingName();
+
+ bool GetEnableFilterForLineBreakpoints() const;
+};
+
class Language : public PluginInterface {
public:
class TypeScavenger {
@@ -324,6 +333,8 @@ class Language : public PluginInterface {
static LanguageSet GetLanguagesSupportingTypeSystemsForExpressions();
static LanguageSet GetLanguagesSupportingREPLs();
+ static LanguageProperties &GetGlobalLanguageProperties();
+
// Given a mangled function name, calculates some alternative manglings since
// the compiler mangling may not line up with the symbol we are expecting.
virtual std::vector
@@ -339,6 +350,15 @@ class Language : public PluginInterface {
virtual llvm::StringRef GetInstanceVariableName() { return {}; }
+ /// Returns true if this SymbolContext should be ignored when setting
+ /// breakpoints by line (number or regex). Helpful for languages that create
+ /// artificial functions without meaningful user code associated with them
+ /// (e.g. code that gets expanded in late compilation stages, like by
+ /// CoroSplitter).
+ virtual bool IgnoreForLineBreakpoints(const SymbolContext &) const {
+return false;
+ }
+
protected:
// Classes that inherit from Language can see and modify these
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..1861a0fe7c4fe4 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
#include "lldb/Symbol/CompileUnit.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
@@ -203,8 +204,15 @@ void BreakpointResolver::SetSCMatchesByLine(
SearchFilter &filter, SymbolContextList &sc_list, bool skip_prologue,
llvm::StringRef log_ident, uint32_t line, std::optional column) {
llvm::SmallVector all_scs;
- for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
-all_scs.push_back(sc_list[i]);
+
+ for (const auto &sc : sc_list) {
+if (Language::GetGlobalLanguageProperties()
+.GetEnableFilterForLineBreakpoints())
+ if (Language *lang = Language::FindPlugin(sc.GetLanguage());
+ lang && lang->IgnoreForLineBreakpoints(sc))
+continue;
+all_scs.push_back(sc);
+ }
while (all_scs.size()) {
uint32_t closest_line = UINT32_MAX;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 90aabde2b764f1..ebd112110e5f2d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -860,6 +860,9 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback,
void *baton)
m_collection_sp->AppendProperty(
"symbols", "Symbol lookup and cache settings.", true,
ModuleList::GetGlobalModuleListProperties().GetValueProperties());
+ m_collection_sp->AppendProperty(
+ LanguageProperties::GetSettingName(), "Language settings.", true,
+ Language::GetGlobalLanguageProperties().GetValueProperties());
if (m_command_interpreter_up) {
m_collection_sp->AppendProperty(
"interpreter",
diff --git a/lldb/source/Target/Langu
[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/83908 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)
@@ -170,6 +170,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, ExpressionResults expr_result = target.EvaluateExpression( expr, exe_scope, valobj_sp, eval_options, &fixed_expression); +auto persistent_name = valobj_sp->GetName(); +// EvaluateExpression doesn't generate a new persistent result (`$0`) when +// the expression is already just a persistent variable (`$var`). Instead, +// the same persistent variable is reused. Take note of when a persistent +// result is created, to prevent unintentional deletion of a user's +// persistent variable. +bool did_persist_result = persistent_name != expr; kastiglione wrote: I agree that it's not ideal, but I felt it was sufficient. > what you want to test, which is whether the result of the expression was a > new expression result variable exactly. However I think peeking at the result variables is also not ideal. Both my initial stab at this, and your suggestion are deducing whether a new expression result variable was created. If I'm to change the API, I think it'd be ideal to make the API communicate have means to communicate this explicitly. There's another approach I considered taking. Before calling `EvaluateExpression`, try the expression as a persistent variable and check in the target to see if it exists, and if it does exist, use it (and avoid `EvaluateExpression` altogether). This is similar to how expressions are first treated as frame variables (and as @felipepiovezan has requested, we should try `target variable` too). https://github.com/llvm/llvm-project/pull/85152 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)
@@ -170,6 +170,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, ExpressionResults expr_result = target.EvaluateExpression( expr, exe_scope, valobj_sp, eval_options, &fixed_expression); +auto persistent_name = valobj_sp->GetName(); +// EvaluateExpression doesn't generate a new persistent result (`$0`) when +// the expression is already just a persistent variable (`$var`). Instead, +// the same persistent variable is reused. Take note of when a persistent +// result is created, to prevent unintentional deletion of a user's +// persistent variable. +bool did_persist_result = persistent_name != expr; jimingham wrote: It would be good to have EvaluateExpression tell you whether it created a result variable or not. For instance, calling a void function produces no result. It would be nice to know that happened directly rather than attempting to guess from the returned ValueObject. We definitely know this... That would definitely be a better way to do this. https://github.com/llvm/llvm-project/pull/85152 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)
https://github.com/chelcassanova approved this pull request. https://github.com/llvm/llvm-project/pull/84854 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)
https://github.com/kastiglione edited https://github.com/llvm/llvm-project/pull/85152 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)
JDevlieghere wrote: Seems like the alarm part of this patch is pretty uncontroversial so I'm going to go ahead and split that off into its own PR so we can settle on the ProgressManager part here. https://github.com/llvm/llvm-project/pull/84854 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/85329
The commit introduces a new, generic, Alarm class. The class lets you to
schedule functions (callbacks) that will execute after a predefined timeout.
Once scheduled, you can cancel and reset a callback, given the timeout hasn't
expired yet.
The alarm class worker thread that sleeps until the next timeout expires. When
the thread wakes up, it checks for all the callbacks that have expired and
calls them in order. Because the callback is called from the worker thread, the
only guarantee is that a callback is called no sooner than the timeout. A long
running callback could potentially block the worker threads and delay other
callbacks from getting called.
I intentionally kept the implementation as simple as possible while addressing
the needs for the use case of coalescing progress events as discussed in [1].
If we want to rely on this somewhere else, we can reassess whether we need to
address this class' limitations.
[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
>From 39972360d762f6185d59d54217870945482ecff7 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Tue, 5 Mar 2024 22:57:43 -0800
Subject: [PATCH] [lldb] Add an Alarm class for coalescing progress reports
The commit introduces a new, generic, Alarm class. The class lets you to
schedule functions (callbacks) that will execute after a predefined
timeout. Once scheduled, you can cancel and reset a callback, given the
timeout hasn't expired yet.
The alarm class worker thread that sleeps until the next timeout
expires. When the thread wakes up, it checks for all the callbacks that
have expired and calls them in order. Because the callback is called
from the worker thread, the only guarantee is that a callback is called
no sooner than the timeout. A long running callback could potentially
block the worker threads and delay other callbacks from getting called.
I intentionally kept the implementation as simple as possible while
addressing the needs for the use case of coalescing progress events as
discussed in [1]. If we want to rely on this somewhere else, we can
reassess whether we need to address this class' limitations.
[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
---
lldb/include/lldb/Host/Alarm.h | 112 +
lldb/source/Host/CMakeLists.txt| 1 +
lldb/source/Host/common/Alarm.cpp | 193 +
lldb/unittests/Host/AlarmTest.cpp | 164
lldb/unittests/Host/CMakeLists.txt | 1 +
5 files changed, 471 insertions(+)
create mode 100644 lldb/include/lldb/Host/Alarm.h
create mode 100644 lldb/source/Host/common/Alarm.cpp
create mode 100644 lldb/unittests/Host/AlarmTest.cpp
diff --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
new file mode 100644
index 00..7bdbc8f2b0ed74
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,112 @@
+//===-- Alarm.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_HOST_ALARM_H
+#define LLDB_HOST_ALARM_H
+
+#include "lldb/Host/HostThread.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Chrono.h"
+
+namespace lldb_private {
+
+/// \class Alarm abstraction that enables scheduling a callback function after
a
+/// specified timeout. Creating an alarm for a callback returns a Handle that
+/// can be used to restart or cancel the alarm.
+class Alarm {
+public:
+ using Handle = uint64_t;
+ using Callback = std::function;
+ using TimePoint = llvm::sys::TimePoint<>;
+ using Duration = std::chrono::milliseconds;
+
+ Alarm(Duration timeout, bool run_callback_on_exit = false);
+ ~Alarm();
+
+ /// Create an alarm for the given callback. The alarm will expire and the
+ /// callback will be called after the timeout.
+ ///
+ /// \returns
+ /// Handle which can be used to restart or cancel the alarm.
+ Handle Create(Callback callback);
+
+ /// Restart the alarm for the given Handle. The alarm will expire and the
+ /// callback will be called after the timeout.
+ ///
+ /// \returns
+ /// True if the alarm was successfully restarted. False if there is no
alarm
+ /// for the given Handle or the alarm already expired.
+ bool Restart(Handle handle);
+
+ /// Cancel the alarm for the given Handle. The alarm and its handle will be
+ /// removed.
+ ///
+ /// \returns
+ /// True if the alarm was successfully canceled and the Handle removed.
+ /// False if there is no alarm for the given Handle or the alarm already
+ /// expired.
+ bool Cancel(Handle handle);
+
+ static constexpr Handle INVALID_HAN
[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
The commit introduces a new, generic, Alarm class. The class lets you to
schedule functions (callbacks) that will execute after a predefined timeout.
Once scheduled, you can cancel and reset a callback, given the timeout hasn't
expired yet.
The alarm class worker thread that sleeps until the next timeout expires. When
the thread wakes up, it checks for all the callbacks that have expired and
calls them in order. Because the callback is called from the worker thread, the
only guarantee is that a callback is called no sooner than the timeout. A long
running callback could potentially block the worker threads and delay other
callbacks from getting called.
I intentionally kept the implementation as simple as possible while addressing
the needs for the use case of coalescing progress events as discussed in [1].
If we want to rely on this somewhere else, we can reassess whether we need to
address this class' limitations.
[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
---
Full diff: https://github.com/llvm/llvm-project/pull/85329.diff
5 Files Affected:
- (added) lldb/include/lldb/Host/Alarm.h (+112)
- (modified) lldb/source/Host/CMakeLists.txt (+1)
- (added) lldb/source/Host/common/Alarm.cpp (+193)
- (added) lldb/unittests/Host/AlarmTest.cpp (+164)
- (modified) lldb/unittests/Host/CMakeLists.txt (+1)
``diff
diff --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
new file mode 100644
index 00..7bdbc8f2b0ed74
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,112 @@
+//===-- Alarm.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_HOST_ALARM_H
+#define LLDB_HOST_ALARM_H
+
+#include "lldb/Host/HostThread.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Chrono.h"
+
+namespace lldb_private {
+
+/// \class Alarm abstraction that enables scheduling a callback function after
a
+/// specified timeout. Creating an alarm for a callback returns a Handle that
+/// can be used to restart or cancel the alarm.
+class Alarm {
+public:
+ using Handle = uint64_t;
+ using Callback = std::function;
+ using TimePoint = llvm::sys::TimePoint<>;
+ using Duration = std::chrono::milliseconds;
+
+ Alarm(Duration timeout, bool run_callback_on_exit = false);
+ ~Alarm();
+
+ /// Create an alarm for the given callback. The alarm will expire and the
+ /// callback will be called after the timeout.
+ ///
+ /// \returns
+ /// Handle which can be used to restart or cancel the alarm.
+ Handle Create(Callback callback);
+
+ /// Restart the alarm for the given Handle. The alarm will expire and the
+ /// callback will be called after the timeout.
+ ///
+ /// \returns
+ /// True if the alarm was successfully restarted. False if there is no
alarm
+ /// for the given Handle or the alarm already expired.
+ bool Restart(Handle handle);
+
+ /// Cancel the alarm for the given Handle. The alarm and its handle will be
+ /// removed.
+ ///
+ /// \returns
+ /// True if the alarm was successfully canceled and the Handle removed.
+ /// False if there is no alarm for the given Handle or the alarm already
+ /// expired.
+ bool Cancel(Handle handle);
+
+ static constexpr Handle INVALID_HANDLE = 0;
+
+private:
+ /// Helper functions to start, stop and check the status of the alarm thread.
+ /// @{
+ void StartAlarmThread();
+ void StopAlarmThread();
+ bool AlarmThreadRunning();
+ /// @}
+
+ /// Return an unique, monotonically increasing handle.
+ static Handle GetNextUniqueHandle();
+
+ /// Helper to compute the next time the alarm thread needs to wake up.
+ TimePoint GetNextExpiration() const;
+
+ /// Alarm entry.
+ struct Entry {
+Handle handle;
+Callback callback;
+TimePoint expiration;
+
+Entry(Callback callback, TimePoint expiration);
+bool operator==(const Entry &rhs) { return handle == rhs.handle; }
+ };
+
+ /// List of alarm entries.
+ std::vector m_entries;
+
+ /// Timeout between when an alarm is created and when it fires.
+ Duration m_timeout;
+
+ /// The alarm thread.
+ /// @{
+ HostThread m_alarm_thread;
+ lldb::thread_result_t AlarmThread();
+ /// @}
+
+ /// Synchronize access between the alarm thread and the main thread.
+ std::mutex m_alarm_mutex;
+
+ /// Condition variable used to wake up the alarm thread.
+ std::condition_variable m_alarm_cv;
+
+ /// Flag to signal the alarm thread that something changed and we need to
+ /// recompute the next alarm.
+ bool m_recompute_next_alarm = false;
+
+ /// Flag to signal the alarm thread to
[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)
JDevlieghere wrote: This contains the Alarm changes from #84854. @chelcassanova and @adrian-prantl please have another look as you had comments in the other PR (that have since been addressed). https://github.com/llvm/llvm-project/pull/85329 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/84854 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/85342
Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it can also
be an absolute path. The former is pretty uninformative and the latter
needlessly noisy.
This changes the progress update to print the UUID and the module name, if both
are available. Otherwise, we print the UUID or the module name depending on
which one is available.
We now also unconditionally pass the module file spec and architecture to
DownloadObjectAndSymbolFile, while previously this was conditional on the file
existing on-disk. This should be harmless:
- We already check that the file exists in DownloadObjectAndSymbolFile.
- It doesn't make sense to check the filesystem for the architecutre.
rdar://124643548
>From b50eb7442ab8f37659596efad19f8b30b145 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 14 Mar 2024 17:43:02 -0700
Subject: [PATCH] [lldb] Show module name in progress update for downloading
symbols
Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it
can also be an absolute path. The former is pretty uninformative and the
latter needlessly noisy.
This changes the progress update to print the UUID and the module name,
if both are available. Otherwise, we print the UUID or the module name
depending on which one is available.
We now also unconditionally pass the module file spec and architecture
to DownloadObjectAndSymbolFile, while previously this was conditional on
the file existing on-disk. This should be harmless:
- We already check that the file exists in DownloadObjectAndSymbolFile.
- It doesn't make sense to check the filesystem for the architecutre.
rdar://124643548
---
lldb/source/Commands/CommandObjectTarget.cpp| 17 +
.../DebugSymbols/SymbolLocatorDebugSymbols.cpp | 14 --
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp
b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402a81d..ae6c6d5479a198 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ class CommandObjectTargetModulesList : public
CommandObjectParsed {
case 'r': {
size_t ref_count = 0;
char in_shared_cache = 'Y';
-
+
ModuleSP module_sp(module->shared_from_this());
if (!ModuleList::ModuleIsInCache(module))
in_shared_cache = 'N';
@@ -4508,11 +4508,8 @@ class CommandObjectTargetSymbolsAdd : public
CommandObjectParsed {
ModuleSpec module_spec;
module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec()))
{
- module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
- module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-}
+module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
result.AppendError("unable to find debug symbols for the current frame");
@@ -4557,12 +4554,8 @@ class CommandObjectTargetSymbolsAdd : public
CommandObjectParsed {
ModuleSpec module_spec;
module_spec.GetUUID() = frame_module_sp->GetUUID();
-
- if (FileSystem::Instance().Exists(
- frame_module_sp->GetPlatformFileSpec())) {
-module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
- }
+ module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+ module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
bool current_frame_flush = false;
if (DownloadObjectAndSymbolFile(module_spec, result,
current_frame_flush))
diff --git
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index f7df4650941a80..acdf962a447266 100644
---
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1066,11 +1066,21 @@ bool
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
command << lookup_arg;
// Log and report progress.
+ std::string lookup_desc;
+ if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+ uuid_ptr->GetAsString());
+ else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+ else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it can also
be an absolute path. The former is pretty uninformative and the latter
needlessly noisy.
This changes the progress update to print the UUID and the module name, if both
are available. Otherwise, we print the UUID or the module name depending on
which one is available.
We now also unconditionally pass the module file spec and architecture to
DownloadObjectAndSymbolFile, while previously this was conditional on the file
existing on-disk. This should be harmless:
- We already check that the file exists in DownloadObjectAndSymbolFile.
- It doesn't make sense to check the filesystem for the architecutre.
rdar://124643548
---
Full diff: https://github.com/llvm/llvm-project/pull/85342.diff
2 Files Affected:
- (modified) lldb/source/Commands/CommandObjectTarget.cpp (+5-12)
- (modified)
lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
(+12-2)
``diff
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp
b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402a81d..ae6c6d5479a198 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ class CommandObjectTargetModulesList : public
CommandObjectParsed {
case 'r': {
size_t ref_count = 0;
char in_shared_cache = 'Y';
-
+
ModuleSP module_sp(module->shared_from_this());
if (!ModuleList::ModuleIsInCache(module))
in_shared_cache = 'N';
@@ -4508,11 +4508,8 @@ class CommandObjectTargetSymbolsAdd : public
CommandObjectParsed {
ModuleSpec module_spec;
module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec()))
{
- module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
- module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-}
+module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
result.AppendError("unable to find debug symbols for the current frame");
@@ -4557,12 +4554,8 @@ class CommandObjectTargetSymbolsAdd : public
CommandObjectParsed {
ModuleSpec module_spec;
module_spec.GetUUID() = frame_module_sp->GetUUID();
-
- if (FileSystem::Instance().Exists(
- frame_module_sp->GetPlatformFileSpec())) {
-module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
- }
+ module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+ module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
bool current_frame_flush = false;
if (DownloadObjectAndSymbolFile(module_spec, result,
current_frame_flush))
diff --git
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index f7df4650941a80..acdf962a447266 100644
---
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1066,11 +1066,21 @@ bool
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
command << lookup_arg;
// Log and report progress.
+ std::string lookup_desc;
+ if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+ uuid_ptr->GetAsString());
+ else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+ else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
Log *log = GetLog(LLDBLog::Host);
LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
- lookup_arg, command.GetString());
+ lookup_desc, command.GetString());
- Progress progress("Downloading symbol file", lookup_arg);
+ Progress progress("Downloading symbol file", lookup_desc);
// Invoke dsymForUUID.
int exit_status = -1;
``
https://github.com/llvm/llvm-project/pull/85342
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)
https://github.com/chelcassanova approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/85342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
