date:20240226

[Lldb-commits] [lldb] 73f11f9 - [lldb][test] Correct results regex for Windows

2024-02-26 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-02-26T10:55:31Z
New Revision: 73f11f9579a3206608ad9a07b5793ba451676087

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

LOG: [lldb][test] Correct results regex for Windows

On Windows the line has \r\n at the end.

Added: 


Modified: 
lldb/test/API/lldbtest.py

Removed: 




diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
index bae73e71820f73..c888fb574f4b7f 100644
--- a/lldb/test/API/lldbtest.py
+++ b/lldb/test/API/lldbtest.py
@@ -101,7 +101,7 @@ def execute(self, test, litConfig):
 # Example: "OK (skipped=1, expected failures=1)"
 # Example: "FAILED (failures=3)"
 # Example: "OK"
-result_regex = r"^(?:OK|FAILED)(?: \((.*)\))?$"
+result_regex = r"^(?:OK|FAILED)(?: \((.*)\))?\r?$"
 results = re.search(result_regex, err, re.MULTILINE)
 
 # If parsing fails mark this test as unresolved.



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

[Lldb-commits] [lldb] d0b1fec - [lldb][test][Windows] Remove expected fail for a thread state test

2024-02-26 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-02-26T11:17:04Z
New Revision: d0b1fec9e1510d01dad2c9c429573eaa75f0963c

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

LOG: [lldb][test][Windows] Remove expected fail for a thread state test

No idea why but this is now passing (though if it randomly fails
I won't be surprised).

See https://github.com/llvm/llvm-project/issues/25034 for background
on the original expected fail.

Added: 


Modified: 
lldb/test/API/functionalities/thread/state/TestThreadStates.py

Removed: 




diff  --git a/lldb/test/API/functionalities/thread/state/TestThreadStates.py 
b/lldb/test/API/functionalities/thread/state/TestThreadStates.py
index 56954c9f34c7e4..97a6e250dcc39c 100644
--- a/lldb/test/API/functionalities/thread/state/TestThreadStates.py
+++ b/lldb/test/API/functionalities/thread/state/TestThreadStates.py
@@ -38,7 +38,6 @@ def test_state_after_continue(self):
 
 @skipIfDarwin  # 'llvm.org/pr23669', cause Python crash randomly
 @expectedFailureDarwin("llvm.org/pr23669")
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24660")
 @expectedFailureNetBSD
 # thread states not properly maintained
 @unittest.expectedFailure  # llvm.org/pr16712



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

[Lldb-commits] [lldb] 285bff3 - [lldb][test][Windows] Skip thread state test on Windows

2024-02-26 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-02-26T13:54:05Z
New Revision: 285bff39fd283b3a9a27c06525111d8d4f474e6e

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

LOG: [lldb][test][Windows] Skip thread state test on Windows

This actually passes on Windows but I don't know how to convey
that with an xfail without clashing with the xfail for all
platforms.

At least this avoids a UPASS.

Added: 


Modified: 
lldb/test/API/functionalities/thread/state/TestThreadStates.py

Removed: 




diff  --git a/lldb/test/API/functionalities/thread/state/TestThreadStates.py 
b/lldb/test/API/functionalities/thread/state/TestThreadStates.py
index 97a6e250dcc39c..f4c17df5233826 100644
--- a/lldb/test/API/functionalities/thread/state/TestThreadStates.py
+++ b/lldb/test/API/functionalities/thread/state/TestThreadStates.py
@@ -39,6 +39,9 @@ def test_state_after_continue(self):
 @skipIfDarwin  # 'llvm.org/pr23669', cause Python crash randomly
 @expectedFailureDarwin("llvm.org/pr23669")
 @expectedFailureNetBSD
+# This actually passes on Windows on Arm but it's hard to describe that
+# and xfail it everywhere else.
+@skipIfWindows
 # thread states not properly maintained
 @unittest.expectedFailure  # llvm.org/pr16712
 def test_state_after_expression(self):



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

[Lldb-commits] [lldb] 8ce81e5 - [lldb][test][Windows] Don't assert that module cache is empty

2024-02-26 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-02-26T14:01:54Z
New Revision: 8ce81e5924935436d49e0b4e835fa107531505b5

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

LOG: [lldb][test][Windows] Don't assert that module cache is empty

For whatever reason on Windows, it is not at this point.

The copy of unit test we used to use would ignore failures during
teardown but Python's does not.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 493152166094e8..c28a78a2c4a27a 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1060,7 +1060,9 @@ def tearDown(self):
 lldb.SBModule.GarbageCollectAllocatedModules()
 
 # Assert that the global module cache is empty.
-self.assertEqual(lldb.SBModule.GetNumberAllocatedModules(), 0)
+# FIXME: This assert fails on Windows.
+if self.getPlatform() != "windows":
+self.assertEqual(lldb.SBModule.GetNumberAllocatedModules(), 0)
 
 # =
 # Various callbacks to allow introspection of test progress



___
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] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)

2024-02-26 Thread Paul T Robinson via lldb-commits

pogo59 wrote:

> This seems akin to running clang-format on the entire project, which last 
> time we talked about still faced opposition

My impression (I admit I haven't reviewed the whole thread lately) is that the 
opposition has mostly to do with how clang-format mangles some constructs, not 
to the idea in general.

I'm juggling 4 projects at the moment, I have to finish some internal 
release-related work before I can get back to clang-format-all-the-things.

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

[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)

2024-02-26 Thread Jordan Rupprecht via lldb-commits


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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

> Overall this looks good but we should verify that this doesn't regress 
> running unmodified iOS binaries on macOS (https://reviews.llvm.org/D117340, 
> https://reviews.llvm.org/D121444). It was really tricky to get that right and 
> it's all based on the binary and host triples. We should at least hand-test 
> before & after this patch and possibly report an arm64e triple in 
> TestPlatformMacOSX.py.

I just tried it out and with the patch debugserver correctly reports a 
`cpusubtype:0` (change of behavior) and the Target gets set to 
`arm64-apple-ios-` which is identical to what my system LLDB reports (no change 
of behavior).

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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -1568,15 +1569,16 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
   if (m_arch.GetSpec().IsCompatibleMatch(other)) {
 compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
 
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
+// Workaround for for pre-2024 debugserver, which always
+// returns arm64e on arm64e-capable hardware regardless of
+// what the process is. This can be deleted at some point in
+// the future.

JDevlieghere wrote:

We face the exact same issue with the compiler and pretty much all other llvm 
tools that follow that versioning scheme. In other words I don't think we 
should go out of our way to fix that just for debugserver. A simple heuristic 
could be version < 1000 is llvm version and > 1000 is the Apple version and 
possibly have a table as those releases aren't necessary aligned. 

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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -,6 +,23 @@ static bool mach_header_validity_test(uint32_t magic, 
uint32_t cputype) {
   return FormatDynamicLibrariesIntoJSON(image_infos, report_load_commands);
 }
 
+std::optional>
+MachProcess::GetMainBinaryCPUTypes(nub_process_t pid) {
+  int pointer_size = GetInferiorAddrSize(pid);
+  std::vector image_infos;
+  GetAllLoadedBinariesViaDYLDSPI(image_infos);
+  uint32_t platform = GetPlatform();
+  for (auto &image_info : image_infos)
+if (GetMachOInformationFromMemory(platform, image_info.load_address,
+  pointer_size, image_info.macho_info))
+  if (image_info.macho_info.mach_header.filetype == MH_EXECUTE)
+return {
+
{static_cast(image_info.macho_info.mach_header.cputype),
+ static_cast(
+ image_info.macho_info.mach_header.cpusubtype)}};
+  return {};

JDevlieghere wrote:

Nit: I think this would benefit from extra braces. The [developer 
policy](https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements)
 says that you can add braces if it helps readability and there's an example 
with two levels of indentation that's a lot simpler than this one. 

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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/82938

>From 5c9e53d45ba948b8a5e8416aa9b3322c87fc46c8 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Fri, 23 Feb 2024 09:58:17 -0800
Subject: [PATCH 1/2] Replace ArchSpec::PiecewiseCompare() with
 Triple::operator==()

Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).

This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.

rdar://123338218
---
 lldb/include/lldb/Utility/ArchSpec.h  |  5 
 lldb/source/Target/Target.cpp |  8 +-
 lldb/source/Utility/ArchSpec.cpp  | 17 ---
 lldb/test/API/macosx/arm64e-attach/Makefile   |  2 ++
 .../macosx/arm64e-attach/TestArm64eAttach.py  | 28 +++
 lldb/test/API/macosx/arm64e-attach/main.c |  2 ++
 6 files changed, 33 insertions(+), 29 deletions(-)
 create mode 100644 lldb/test/API/macosx/arm64e-attach/Makefile
 create mode 100644 lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
 create mode 100644 lldb/test/API/macosx/arm64e-attach/main.c

diff --git a/lldb/include/lldb/Utility/ArchSpec.h 
b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
 
   bool IsFullySpecifiedTriple() const;
 
-  void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different,
-  bool &vendor_different, bool &os_different,
-  bool &os_version_different,
-  bool &env_different) const;
-
   /// Detect whether this architecture uses thumb code exclusively
   ///
   /// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
   if (m_arch.GetSpec().IsCompatibleMatch(other)) {
 compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
 
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
   }
 }
diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
   return true;
 }
 
-void ArchSpec::PiecewiseTripleCompare(
-const ArchSpec &other, bool &arch_different, bool &vendor_different,
-bool &os_different, bool &os_version_different, bool &env_different) const 
{
-  const llvm::Triple &me(GetTriple());
-  const llvm::Triple &them(other.GetTriple());
-
-  arch_different = (me.getArch() != them.getArch());
-
-  vendor_different = (me.getVendor() != them.getVendor());
-
-  os_different = (me.getOS() != them.getOS());
-
-  os_version_different = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
-  env_different = (me.getEnvironment() != them.getEnvironment());
-}
-
 bool ArchSpec::IsAlwaysThumbInstructions() const {
   std::string Status;
   if (GetTriple().getArch() == llvm::Triple::arm ||
diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile 
b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules
diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py 
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00..0dc8700ed02dd8
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestArm64eAttach(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
+@skipIf(archs=no_match(["arm64e"]))
+def test(self):
+# Skip this test if not run

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


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

LGTM with Jason's comment as it would help simplify the test.

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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Adrian Prantl via lldb-commits



@@ -26,3 +31,8 @@ def test(self):
  "target triple is updated correctly")
 error = process.Kill()
 self.assertSuccess(error)
+
+# Test debugserver behavior.
+self.filecheck('platform shell cat "%s"' % packets, __file__)
+# CHECK: 
cputype:10c;cpusubtype:2;ptrsize:8;ostype:macosx;vendor:apple;endian:little;

adrian-prantl wrote:

Thanks!

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

[Lldb-commits] [lldb] 01450dd - Change debugserver to report the cpu(sub)type of process, not the host.

2024-02-26 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2024-02-26T09:57:07-08:00
New Revision: 01450dd1c69d1edb0d01159352a56c99988839f4

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

LOG: Change debugserver to report the cpu(sub)type of process, not the host.

This way debugserver can correctly report qProcessInfo for arm64
processes on arm64e-capable hosts.

Patch implemented with help from Jason Molenda!

Added: 


Modified: 
lldb/source/Target/Target.cpp
lldb/test/API/macosx/arm64e-attach/Makefile
lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e982a30a3ae4ff..089915cab4915a 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -33,6 +33,7 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -1571,6 +1572,13 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
 if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
+// Workaround for for pre-2024 debugserver, which always
+// returns arm64e on arm64e-capable hardware regardless of
+// what the process is. This can be deleted at some point in
+// the future.
+if (!m_arch.GetSpec().GetMachOCPUSubType() &&
+other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E)
+  replace_local_arch = true;
   }
 }
   }

diff  --git a/lldb/test/API/macosx/arm64e-attach/Makefile 
b/lldb/test/API/macosx/arm64e-attach/Makefile
index c9319d6e6888a4..b24dccd7c02ad4 100644
--- a/lldb/test/API/macosx/arm64e-attach/Makefile
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -1,2 +1,7 @@
 C_SOURCES := main.c
+
+# Uncomment this for local debugging.
+#all:
+#  xcrun clang -g $(SRCDIR)/main.c -o a.out -target arm64e-apple-macosx
+
 include Makefile.rules

diff  --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py 
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
index 0dc8700ed02dd8..3ec977b507b890 100644
--- a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -13,10 +13,12 @@ def test(self):
 # Skip this test if not running on AArch64 target that supports PAC
 if not self.isAArch64PAuth():
 self.skipTest("Target must support pointer authentication.")
+
 self.build()
 popen = self.spawnSubprocess(self.getBuildArtifact(), [])
-error = lldb.SBError()
+
 # This simulates how Xcode attaches to a process by pid/name.
+error = lldb.SBError()
 target = self.dbg.CreateTarget("", "arm64", "", True, error)
 listener = lldb.SBListener("my.attach.listener")
 process = target.AttachToProcessWithID(listener, popen.pid, error)
@@ -24,5 +26,10 @@ def test(self):
 self.assertTrue(process, PROCESS_IS_VALID)
 self.assertEqual(target.GetTriple().split('-')[0], "arm64e",
  "target triple is updated correctly")
+
+self.expect('process plugin packet send qProcessInfo',
+"debugserver returns correct triple",
+substrs=['cputype:10c', 'cpusubtype:2', 'ptrsize:8'])
+
 error = process.Kill()
 self.assertSuccess(error)

diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index 0ec50df42d1fed..1ac9a8040c6163 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -1062,6 +1062,14 @@ DNBGetTSDAddressForThread(nub_process_t pid, 
nub_thread_t tid,
   return INVALID_NUB_ADDRESS;
 }
 
+std::optional>
+DNBGetMainBinaryCPUTypes(nub_process_t pid) {
+  MachProcessSP procSP;
+  if (GetProcessSP(pid, procSP))
+return procSP->GetMainBinaryCPUTypes(pid);
+  return {};
+}
+
 JSONGenerator::ObjectSP
 DNBGetAllLoadedLibrariesInfos(nub_process_t pid, bool report_load_commands) {
   MachProcessSP procSP;

diff  --git a/lldb/tools/debugserver/source/DNB.h 
b/lldb/tools/debugserver/source/DNB.h
index 97de83ef9ff80d..10d1f687943556 100644
--- a/lldb/tools/debugserver/source/DNB.h
+++ b/lldb/tools/debugserver/source/DNB.h
@@ -210,6 +210,8 @@ DNBGetTSDAddressForThread(nub_process_t pid, nub_thread_t 
tid,

[Lldb-commits] [lldb] f9f3316 - Replace ArchSpec::PiecewiseCompare() with Triple::operator==()

2024-02-26 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2024-02-26T09:57:07-08:00
New Revision: f9f331652d4f0aff9ece3570abe8c686cdfefff4

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

LOG: Replace ArchSpec::PiecewiseCompare() with Triple::operator==()

Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).

This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.

rdar://123338218

Added: 
lldb/test/API/macosx/arm64e-attach/Makefile
lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
lldb/test/API/macosx/arm64e-attach/main.c

Modified: 
lldb/include/lldb/Utility/ArchSpec.h
lldb/source/Target/Target.cpp
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ArchSpec.h 
b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
 
   bool IsFullySpecifiedTriple() const;
 
-  void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_
diff erent,
-  bool &vendor_
diff erent, bool &os_
diff erent,
-  bool &os_version_
diff erent,
-  bool &env_
diff erent) const;
-
   /// Detect whether this architecture uses thumb code exclusively
   ///
   /// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
   if (m_arch.GetSpec().IsCompatibleMatch(other)) {
 compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
 
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
   }
 }

diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
   return true;
 }
 
-void ArchSpec::PiecewiseTripleCompare(
-const ArchSpec &other, bool &arch_
diff erent, bool &vendor_
diff erent,
-bool &os_
diff erent, bool &os_version_
diff erent, bool &env_
diff erent) const {
-  const llvm::Triple &me(GetTriple());
-  const llvm::Triple &them(other.GetTriple());
-
-  arch_
diff erent = (me.getArch() != them.getArch());
-
-  vendor_
diff erent = (me.getVendor() != them.getVendor());
-
-  os_
diff erent = (me.getOS() != them.getOS());
-
-  os_version_
diff erent = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
-  env_
diff erent = (me.getEnvironment() != them.getEnvironment());
-}
-
 bool ArchSpec::IsAlwaysThumbInstructions() const {
   std::string Status;
   if (GetTriple().getArch() == llvm::Triple::arm ||

diff  --git a/lldb/test/API/macosx/arm64e-attach/Makefile 
b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules

diff  --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py 
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00..0dc8700ed02dd8
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestArm64eAttach(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
+@skipIf(archs=no_match(["arm64e"]))
+def test(self):
+# Skip this test if not running on AArch64 target that supports PAC
+if not self.isAArch64PAuth():
+self.skipTest("Target must support pointer

[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)

2024-02-26 Thread Felipe de Azevedo Piovezan via lldb-commits


felipepiovezan wrote:

> > This seems akin to running clang-format on the entire project, which last 
> > time we talked about still faced opposition
> 
> My impression (I admit I haven't reviewed the whole thread lately) is that 
> the opposition has mostly to do with how clang-format mangles some 
> constructs, not to the idea in general.
> 
> I'm juggling 4 projects at the moment, I have to finish some internal 
> release-related work before I can get back to clang-format-all-the-things.

Sorry, you might be right; my intent with that comment was not to a make broad 
summary of the thread, but rather to point out the thread's existence and that 
this PR should probably be folded into that discussion.

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

[Lldb-commits] [lldb] 3851558 - [lldb] Remove LLDB_DEBUGSERVER_CODESIGN_IDENTITY (NFC)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2024-02-26T11:11:30-08:00
New Revision: 38515580c4c5068e204ff69494ad11bbfacc89b4

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

LOG: [lldb] Remove LLDB_DEBUGSERVER_CODESIGN_IDENTITY (NFC)

This property was previously used by the test suite.

Added: 


Modified: 
lldb/tools/debugserver/source/CMakeLists.txt

Removed: 




diff  --git a/lldb/tools/debugserver/source/CMakeLists.txt 
b/lldb/tools/debugserver/source/CMakeLists.txt
index f0b9756becab6e..1a433898f6aa48 100644
--- a/lldb/tools/debugserver/source/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/CMakeLists.txt
@@ -117,10 +117,6 @@ 
get_debugserver_codesign_identity(debugserver_codesign_identity)
 # Override locally, so the identity is used for targets created in this scope.
 set(LLVM_CODESIGNING_IDENTITY ${debugserver_codesign_identity})
 
-# Use the same identity later in the test suite.
-set_property(GLOBAL PROPERTY
-  LLDB_DEBUGSERVER_CODESIGN_IDENTITY ${debugserver_codesign_identity})
-
 if(APPLE)
   set(LIBCOMPRESSION compression)
   if(APPLE_EMBEDDED)



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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

I also tested this with a rosetta process.

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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [lldb] Change debugserver to report the cpu(sub)type of process, not the host. (PR #82938)

2024-02-26 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

Landed in 01450dd1c69d1edb0d01159352a56c99988839f4 
f9f331652d4f0aff9ece3570abe8c686cdfefff4

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits


https://github.com/chelcassanova created 
https://github.com/llvm/llvm-project/pull/83069

This commit adds the functionality to broadcast events using the 
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these 
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is used in 
such a way that it will only broadcast the initial and final progress reports 
for specific progress categories that are managed by the progress manager.

This commit also adds a new test to the progress report unit test that checks 
that only the initial/final reports are broadcasted when using the new bit.

>From c27cab4a6fe067aeac6864e7262b2c57a53f2d0c Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova 
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
 manager

This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.

This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
 lldb/include/lldb/Core/Debugger.h  |  4 +-
 lldb/include/lldb/Core/Progress.h  | 41 ++--
 lldb/source/Core/Debugger.cpp  | 38 ---
 lldb/source/Core/Progress.cpp  | 47 +-
 lldb/unittests/Core/ProgressReportTest.cpp | 57 ++
 5 files changed, 164 insertions(+), 23 deletions(-)

diff --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..714ca83b890d87 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public 
std::enable_shared_from_this,
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
+  friend class ProgressManager;
 
   /// Report progress events.
   ///
@@ -626,7 +627,8 @@ class Debugger : public 
std::enable_shared_from_this,
   static void ReportProgress(uint64_t progress_id, std::string title,
  std::string details, uint64_t completed,
  uint64_t total,
- std::optional debugger_id);
+ std::optional debugger_id,
+ uint32_t progress_category_bit);
 
   static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h 
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..434a155ca7590e 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
 #ifndef LLDB_CORE_PROGRESS_H
 #define LLDB_CORE_PROGRESS_H
 
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringMap.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -97,12 +98,32 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Use a struct to send data from a Progress object to
+  /// ProgressManager::ReportProgress. In addition to the Progress member 
fields
+  /// this also passes the debugger progress broadcast bit. Using the progress
+  /// category bit indicates that the given progress report is the initial or
+  /// final report for its category.
+  struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
+  };
+
 private:
+  friend class Debugger;
   void ReportProgress();
   static std::atomic g_id;
-  /// The title of the progress activity.
+  /// The title of the progress activity, also used as a category to group
+  /// reports.
   std::string m_title;
+  /// More specific information about the current file being displayed in the
+  /// report.
   std::string m_details;
+  /// Mutex for synchronization.
   std::mutex m_mutex;
   /// A unique integer identifier for progress reporting.
   const uint64_t m_id;
@@ -118,6 +139,8 @@ class Progress {
   /// to ensure that we don't send progress updates after progress has
   /// completed.
   bool m_complete = false;
+  /// Data needed by the debugger to broadcast a progress event.
+  ProgressData m_progress_data;
 };
 
 /// A class used to group progress reports by category. This is done by using a
@@ -130,13 +153,21 @@ class Prog

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Chelsea Cassanova (chelcassanova)


Changes

This commit adds the functionality to broadcast events using the 
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these 
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is used in 
such a way that it will only broadcast the initial and final progress reports 
for specific progress categories that are managed by the progress manager.

This commit also adds a new test to the progress report unit test that checks 
that only the initial/final reports are broadcasted when using the new bit.

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


5 Files Affected:

- (modified) lldb/include/lldb/Core/Debugger.h (+3-1) 
- (modified) lldb/include/lldb/Core/Progress.h (+36-5) 
- (modified) lldb/source/Core/Debugger.cpp (+32-6) 
- (modified) lldb/source/Core/Progress.cpp (+36-11) 
- (modified) lldb/unittests/Core/ProgressReportTest.cpp (+57) 


``diff
diff --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..714ca83b890d87 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public 
std::enable_shared_from_this,
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
+  friend class ProgressManager;
 
   /// Report progress events.
   ///
@@ -626,7 +627,8 @@ class Debugger : public 
std::enable_shared_from_this,
   static void ReportProgress(uint64_t progress_id, std::string title,
  std::string details, uint64_t completed,
  uint64_t total,
- std::optional debugger_id);
+ std::optional debugger_id,
+ uint32_t progress_category_bit);
 
   static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h 
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..434a155ca7590e 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
 #ifndef LLDB_CORE_PROGRESS_H
 #define LLDB_CORE_PROGRESS_H
 
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringMap.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -97,12 +98,32 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Use a struct to send data from a Progress object to
+  /// ProgressManager::ReportProgress. In addition to the Progress member 
fields
+  /// this also passes the debugger progress broadcast bit. Using the progress
+  /// category bit indicates that the given progress report is the initial or
+  /// final report for its category.
+  struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
+  };
+
 private:
+  friend class Debugger;
   void ReportProgress();
   static std::atomic g_id;
-  /// The title of the progress activity.
+  /// The title of the progress activity, also used as a category to group
+  /// reports.
   std::string m_title;
+  /// More specific information about the current file being displayed in the
+  /// report.
   std::string m_details;
+  /// Mutex for synchronization.
   std::mutex m_mutex;
   /// A unique integer identifier for progress reporting.
   const uint64_t m_id;
@@ -118,6 +139,8 @@ class Progress {
   /// to ensure that we don't send progress updates after progress has
   /// completed.
   bool m_complete = false;
+  /// Data needed by the debugger to broadcast a progress event.
+  ProgressData m_progress_data;
 };
 
 /// A class used to group progress reports by category. This is done by using a
@@ -130,13 +153,21 @@ class ProgressManager {
   ~ProgressManager();
 
   /// Control the refcount of the progress report category as needed.
-  void Increment(std::string category);
-  void Decrement(std::string category);
+  void Increment(Progress::ProgressData);
+  void Decrement(Progress::ProgressData);
 
   static ProgressManager &Instance();
 
+  llvm::StringMap>
+  GetProgressCategoryMap() {
+return m_progress_category_map;
+  }
+
+  void ReportProgress(Progress::ProgressData);
+
 private:
-  llvm::StringMap m_progress_category_map;
+  llvm::StringMap>
+  m_progress_category_map;
   std::mutex m_progress_map_mutex;
 };
 
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..cd0ce3a9558ce6 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -15,6

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -593,6 +593,7 @@ class Debugger : public 
std::enable_shared_from_this,
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
+  friend class ProgressManager;

JDevlieghere wrote:

I don't think you need this (anymore)?

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -626,7 +627,8 @@ class Debugger : public 
std::enable_shared_from_this,
   static void ReportProgress(uint64_t progress_id, std::string title,
  std::string details, uint64_t completed,
  uint64_t total,
- std::optional debugger_id);
+ std::optional debugger_id,
+ uint32_t progress_category_bit);

JDevlieghere wrote:

should this default to `eBroadcastBitProgress`? 

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -97,12 +98,32 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Use a struct to send data from a Progress object to
+  /// ProgressManager::ReportProgress. In addition to the Progress member 
fields
+  /// this also passes the debugger progress broadcast bit. Using the progress
+  /// category bit indicates that the given progress report is the initial or
+  /// final report for its category.
+  struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;

JDevlieghere wrote:

I don't think the `progress_broadcast_bit` should be stored in the struct. The 
bit is a property of the progress manager, which will set the bit for the first 
and last progress event. 

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere requested changes to this pull request.

I think this is going in the right direction but I think the interaction 
between the ProgressManager and the Debugger needs to change:

At a high level I'm expecting:

1. Progress object is created
2. Progress is added to the ProgressManager
3. Debugger::ReportProgress is called with (`eBroadcastBitProgress | 
eBroadcastBitProgressCategory`)
4. Incremental updates call directly into Debugger::ReportProgress without 
going through the manager, with just `Debugger::ReportProgress`.
5. When progress is complete, we go through the progress manager again and 
report the final progress to both broadcast bits: `eBroadcastBitProgress | 
eBroadcastBitProgressCategory`


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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   std::string title, std::string details,
   uint64_t completed, uint64_t total,
-  bool is_debugger_specific) {
+  bool is_debugger_specific,
+  uint32_t progress_broadcast_bit) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
-  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+  const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+   category_event_type))
 return;
+
+  if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount = 
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+  EventSP event_sp(new Event(
+  category_event_type,
+  new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+  debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+  }

JDevlieghere wrote:

I think it would be better from a layering perspective to keep 
`PrivateReportProgress` "dumb" and pass it the right arguments from the 
ProgressManager. The map held by the progress manager should be a private 
implementation detail. By letting the debugger mess with it you lose the 
ability to synchronize access to it. When we do the timeouts, this isn't going 
to work.

I expect this function to simply broadcast to whichever broadcast bit is set. 

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -97,12 +98,32 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Use a struct to send data from a Progress object to
+  /// ProgressManager::ReportProgress. In addition to the Progress member 
fields

JDevlieghere wrote:

I think the first sentence isn't entirely accurate. The struct is used to store 
information about the progress event in the progress manager? 

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -97,12 +98,32 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Use a struct to send data from a Progress object to
+  /// ProgressManager::ReportProgress. In addition to the Progress member 
fields
+  /// this also passes the debugger progress broadcast bit. Using the progress
+  /// category bit indicates that the given progress report is the initial or
+  /// final report for its category.
+  struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
+  };
+
 private:
+  friend class Debugger;
   void ReportProgress();
   static std::atomic g_id;
-  /// The title of the progress activity.
+  /// The title of the progress activity, also used as a category to group
+  /// reports.
   std::string m_title;
+  /// More specific information about the current file being displayed in the
+  /// report.
   std::string m_details;
+  /// Mutex for synchronization.
   std::mutex m_mutex;
   /// A unique integer identifier for progress reporting.

JDevlieghere wrote:

Given that you store the ProgressData, did you mean to document the fields in 
the new Struct and remove these? 

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits



@@ -97,12 +98,32 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Use a struct to send data from a Progress object to
+  /// ProgressManager::ReportProgress. In addition to the Progress member 
fields

chelcassanova wrote:

Yes, this can be generalized more since we use the struct for more than just 
the call to `ReportProgress`.

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits



@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   std::string title, std::string details,
   uint64_t completed, uint64_t total,
-  bool is_debugger_specific) {
+  bool is_debugger_specific,
+  uint32_t progress_broadcast_bit) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
-  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+  const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+   category_event_type))
 return;
+
+  if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount = 
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+  EventSP event_sp(new Event(
+  category_event_type,
+  new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+  debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+  }

chelcassanova wrote:

So IIUC, instead of passing the broadcast bit as an argument we just set it in 
the progress manager and access it from there in the Debugger?

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Jonas Devlieghere via lldb-commits



@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   std::string title, std::string details,
   uint64_t completed, uint64_t total,
-  bool is_debugger_specific) {
+  bool is_debugger_specific,
+  uint32_t progress_broadcast_bit) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
-  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+  const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+   category_event_type))
 return;
+
+  if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount = 
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+  EventSP event_sp(new Event(
+  category_event_type,
+  new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+  debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+  }

JDevlieghere wrote:

Not really. I'm saying that I think we don't need to store it at all, because 
its implied in when we call `DebuggerReportProgress`.

Can ProgressManager call `ReportProgress(bit = eBroadcastBitProgress | 
eBroadcastBitProgressCategory)` for the first and last update, and every other 
time call `ReportProgress(bit = eBroadcastBitProgress) every other time?

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

[Lldb-commits] [lldb] [lldb] python-bindings: fix `SBTarget.get_target_watchpoints()` (PR #82295)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


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

LGTM. Let me know if you need someone to land this for you (and if you want to 
[set your e-mail address to public](https://github.com/settings/emails) before 
that). 

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits



@@ -593,6 +593,7 @@ class Debugger : public 
std::enable_shared_from_this,
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
+  friend class ProgressManager;

chelcassanova wrote:

`Debugger::ReportProgress` is protected, so to my understanding if we call that 
function from `ProgressManager::ReportProgress` without having it as a friend 
it leads to a compile error.

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

[Lldb-commits] [lldb] [lldb] python-bindings: fix `SBTarget.get_target_watchpoints()` (PR #82295)

2024-02-26 Thread via lldb-commits


nikitalita wrote:




> LGTM. Let me know if you need someone to land this for you

I do need someone to land this for me. People keep asking me that; Is there a 
way for me to apply for write access or something?

> (and if you want to [set your e-mail address to 
> public](https://github.com/settings/emails) before that).
I don’t want to get an inbox full of spam, so I’d rather not.



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

[Lldb-commits] [lldb] [lldb] python-bindings: fix `SBTarget.get_target_watchpoints()` (PR #82295)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


JDevlieghere wrote:

> > LGTM. Let me know if you need someone to land this for you
> 
> I do need someone to land this for me. People keep asking me that; Is there a 
> way for me to apply for write access or something?

https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

> > (and if you want to [set your e-mail address to 
> > public](https://github.com/settings/emails) before that).
> I don’t want to get an inbox full of spam, so I’d rather not.

That's fair, though that means that you will not get notifications when bots 
fail so you'll need to actively monitor them yourself.

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

[Lldb-commits] [lldb] dc5dfc1 - [lldb] python-bindings: fix `SBTarget.get_target_watchpoints()` (#82295)

2024-02-26 Thread via lldb-commits


Author: nikitalita
Date: 2024-02-26T15:05:02-08:00
New Revision: dc5dfc102ffc3b870f7565fb4a90d53b31ec92f8

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

LOG: [lldb] python-bindings: fix `SBTarget.get_target_watchpoints()` (#82295)

Fixes erroneous usage of `bkpts` instead of `watchpoints` (probably
introduced from copying and pasting `get_target_bkpts()`).

Added: 


Modified: 
lldb/bindings/interface/SBTargetExtensions.i

Removed: 




diff  --git a/lldb/bindings/interface/SBTargetExtensions.i 
b/lldb/bindings/interface/SBTargetExtensions.i
index c80dadfc0c5ca5..d756a351a810ab 100644
--- a/lldb/bindings/interface/SBTargetExtensions.i
+++ b/lldb/bindings/interface/SBTargetExtensions.i
@@ -172,7 +172,7 @@ STRING_EXTENSION_LEVEL_OUTSIDE(SBTarget, 
lldb::eDescriptionLevelBrief)
 '''An accessor function that returns a list() that contains all 
watchpoints in a lldb.SBtarget object.'''
 watchpoints = []
 for idx in range(self.GetNumWatchpoints()):
-bkpts.append(self.GetWatchpointAtIndex(idx))
+watchpoints.append(self.GetWatchpointAtIndex(idx))
 return watchpoints
 
 modules = property(get_modules_array, None, doc='''A read only 
property that returns a list() of lldb.SBModule objects contained in this 
target. This list is a list all modules that the target currently is tracking 
(the main executable and all dependent shared libraries).''')



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

[Lldb-commits] [lldb] [lldb] python-bindings: fix `SBTarget.get_target_watchpoints()` (PR #82295)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits



@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   std::string title, std::string details,
   uint64_t completed, uint64_t total,
-  bool is_debugger_specific) {
+  bool is_debugger_specific,
+  uint32_t progress_broadcast_bit) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
-  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+  const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+   category_event_type))
 return;
+
+  if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount = 
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+  EventSP event_sp(new Event(
+  category_event_type,
+  new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+  debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+  }

chelcassanova wrote:

So that instead of having it be a member of the struct or the class we just 
call `Debugger::ReportProgress(, eBroadcastBitProgress | 
eBroadcastBitProgressCategory)` and set the bit as needed from the progress 
manager itself?

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits



@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   std::string title, std::string details,
   uint64_t completed, uint64_t total,
-  bool is_debugger_specific) {
+  bool is_debugger_specific,
+  uint32_t progress_broadcast_bit) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
-  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+  const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+   category_event_type))
 return;
+
+  if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount = 
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+  EventSP event_sp(new Event(
+  category_event_type,
+  new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+  debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+  }

chelcassanova wrote:

Such that we set `progress_category_bit` from the progress manager and 
broadcast to that as such:
```  EventSP event_sp(new Event(
  category_event_type,
  new ProgressEventData(progress_id, std::move(title), "", completed,
total, is_debugger_specific)));
  debugger.GetBroadcaster().BroadcastEvent(event_sp);
```

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)

2024-02-26 Thread Alex Langford via lldb-commits


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

This updates the remaining SetOptionValue methods in CommandObjectBreakpoint to 
use CreateOptionParsingError.

I found a few minor bugs that were fixed during this refactor (e.g. using the 
wrong flag in an error message). That is one of the benefits of centralizing 
error message creation.

I also found some option parsing code that is written incorrectly. I do not 
make an attempt to update those here because this PR is primarily about 
changing existing error handling code, not adding new error handling code.

>From feb4588cd12a5f513061dffa99f6370067f91463 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Mon, 26 Feb 2024 16:01:24 -0800
Subject: [PATCH] [lldb] Use CreateOptionParsingError in
 CommandObjectBreakpoint

This updates the remaining SetOptionValue methods in
CommandObjectBreakpoint to use CreateOptionParsingError.

I found a few minor bugs that were fixed during this refactor (e.g.
using the wrong flag in an error message). That is one of the benefits
of centralizing error message creation.

I also found some option parsing code that is written incorrectly. I do
not make an attempt to update those here because this PR is primarily
about changing existing error handling code, not adding new error
handling code.
---
 lldb/include/lldb/Interpreter/Options.h   |   2 +
 .../Commands/CommandObjectBreakpoint.cpp  | 101 +-
 2 files changed, 54 insertions(+), 49 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index 18a87e49deee5c..9a6a17c2793fa4 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -368,6 +368,8 @@ static constexpr llvm::StringLiteral 
g_bool_parsing_error_message =
 "Failed to parse as boolean";
 static constexpr llvm::StringLiteral g_int_parsing_error_message =
 "Failed to parse as integer";
+static constexpr llvm::StringLiteral g_language_parsing_error_message =
+"Unknown language";
 
 } // namespace lldb_private
 
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index fc2217608a0bb9..cfb0b87a59e640 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -266,6 +266,8 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
   Status error;
   const int short_option =
   g_breakpoint_set_options[option_idx].short_option;
+  const char *long_option =
+  g_breakpoint_set_options[option_idx].long_option;
 
   switch (short_option) {
   case 'a': {
@@ -284,13 +286,15 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
 
   case 'u':
 if (option_arg.getAsInteger(0, m_column))
-  error.SetErrorStringWithFormat("invalid column number: %s",
- option_arg.str().c_str());
+  error =
+  CreateOptionParsingError(option_arg, short_option, long_option,
+   g_int_parsing_error_message);
 break;
 
   case 'E': {
 LanguageType language = 
Language::GetLanguageTypeFromString(option_arg);
 
+llvm::StringRef error_context;
 switch (language) {
 case eLanguageTypeC89:
 case eLanguageTypeC:
@@ -308,19 +312,18 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
   m_exception_language = eLanguageTypeObjC;
   break;
 case eLanguageTypeObjC_plus_plus:
-  error.SetErrorStringWithFormat(
-  "Set exception breakpoints separately for c++ and objective-c");
+  error_context =
+  "Set exception breakpoints separately for c++ and objective-c";
   break;
 case eLanguageTypeUnknown:
-  error.SetErrorStringWithFormat(
-  "Unknown language type: '%s' for exception breakpoint",
-  option_arg.str().c_str());
+  error_context = "Unknown language type for exception breakpoint";
   break;
 default:
-  error.SetErrorStringWithFormat(
-  "Unsupported language type: '%s' for exception breakpoint",
-  option_arg.str().c_str());
+  error_context = "Unsupported language type for exception breakpoint";
 }
+if (!error_context.empty())
+  error = CreateOptionParsingError(option_arg, short_option,
+   long_option, error_context);
   } break;
 
   case 'f':
@@ -336,9 +339,9 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
 bool success;
 m_catch_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
 if (!success)
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for on-catch option: '%s'",
-  optio

[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)

2024-02-26 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)


Changes

This updates the remaining SetOptionValue methods in CommandObjectBreakpoint to 
use CreateOptionParsingError.

I found a few minor bugs that were fixed during this refactor (e.g. using the 
wrong flag in an error message). That is one of the benefits of centralizing 
error message creation.

I also found some option parsing code that is written incorrectly. I do not 
make an attempt to update those here because this PR is primarily about 
changing existing error handling code, not adding new error handling code.

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


2 Files Affected:

- (modified) lldb/include/lldb/Interpreter/Options.h (+2) 
- (modified) lldb/source/Commands/CommandObjectBreakpoint.cpp (+52-49) 


``diff
diff --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index 18a87e49deee5c..9a6a17c2793fa4 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -368,6 +368,8 @@ static constexpr llvm::StringLiteral 
g_bool_parsing_error_message =
 "Failed to parse as boolean";
 static constexpr llvm::StringLiteral g_int_parsing_error_message =
 "Failed to parse as integer";
+static constexpr llvm::StringLiteral g_language_parsing_error_message =
+"Unknown language";
 
 } // namespace lldb_private
 
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index fc2217608a0bb9..cfb0b87a59e640 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -266,6 +266,8 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
   Status error;
   const int short_option =
   g_breakpoint_set_options[option_idx].short_option;
+  const char *long_option =
+  g_breakpoint_set_options[option_idx].long_option;
 
   switch (short_option) {
   case 'a': {
@@ -284,13 +286,15 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
 
   case 'u':
 if (option_arg.getAsInteger(0, m_column))
-  error.SetErrorStringWithFormat("invalid column number: %s",
- option_arg.str().c_str());
+  error =
+  CreateOptionParsingError(option_arg, short_option, long_option,
+   g_int_parsing_error_message);
 break;
 
   case 'E': {
 LanguageType language = 
Language::GetLanguageTypeFromString(option_arg);
 
+llvm::StringRef error_context;
 switch (language) {
 case eLanguageTypeC89:
 case eLanguageTypeC:
@@ -308,19 +312,18 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
   m_exception_language = eLanguageTypeObjC;
   break;
 case eLanguageTypeObjC_plus_plus:
-  error.SetErrorStringWithFormat(
-  "Set exception breakpoints separately for c++ and objective-c");
+  error_context =
+  "Set exception breakpoints separately for c++ and objective-c";
   break;
 case eLanguageTypeUnknown:
-  error.SetErrorStringWithFormat(
-  "Unknown language type: '%s' for exception breakpoint",
-  option_arg.str().c_str());
+  error_context = "Unknown language type for exception breakpoint";
   break;
 default:
-  error.SetErrorStringWithFormat(
-  "Unsupported language type: '%s' for exception breakpoint",
-  option_arg.str().c_str());
+  error_context = "Unsupported language type for exception breakpoint";
 }
+if (!error_context.empty())
+  error = CreateOptionParsingError(option_arg, short_option,
+   long_option, error_context);
   } break;
 
   case 'f':
@@ -336,9 +339,9 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
 bool success;
 m_catch_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
 if (!success)
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for on-catch option: '%s'",
-  option_arg.str().c_str());
+  error =
+  CreateOptionParsingError(option_arg, short_option, long_option,
+   g_bool_parsing_error_message);
   } break;
 
   case 'H':
@@ -355,23 +358,24 @@ class CommandObjectBreakpointSet : public 
CommandObjectParsed {
   m_skip_prologue = eLazyBoolNo;
 
 if (!success)
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for skip prologue option: '%s'",
-  option_arg.str().c_str());
+  error =
+  CreateOptionParsingError(option_arg, short_option, long_option,
+

[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)

2024-02-26 Thread via lldb-commits


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

LGTM

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

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-02-26 Thread Chelsea Cassanova via lldb-commits



@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   std::string title, std::string details,
   uint64_t completed, uint64_t total,
-  bool is_debugger_specific) {
+  bool is_debugger_specific,
+  uint32_t progress_broadcast_bit) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
-  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+  const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+  if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+   category_event_type))
 return;
+
+  if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount = 
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+  EventSP event_sp(new Event(
+  category_event_type,
+  new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+  debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+  }

chelcassanova wrote:

Also looking back at `PrivateReportProgress`, the variables for the instance 
and refcount are leftover from a previous implementation that I had, they 
shouldn't be here anymore.

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

[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)

2024-02-26 Thread Felipe de Azevedo Piovezan via lldb-commits


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

LGTM!

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

[Lldb-commits] [lldb] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)

2024-02-26 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda created 
https://github.com/llvm/llvm-project/pull/83095

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905 which 
was approved but I wasn't thrilled with all the API I was adding to SBProcess 
for all of the address mask types / memory regions. In this update, I added 
enums to control type address mask type (code, data, any) and address space 
specifiers (low, high, all) with defaulted arguments for the most common case.

This patch is also fixing a bug in the "addressable bits to address mask" 
calculation I added in AddressableBits::SetProcessMasks. If lldb were told that 
64 bits are valid for addressing, this method would overflow the calculation 
and set an invalid mask.  Added tests to check this specific bug while I was 
adding these APIs.

rdar://123530562

>From dae16776e8c97158e8965e4d0e950cd2ce836f75 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Mon, 26 Feb 2024 18:05:27 -0800
Subject: [PATCH] [lldb] Add SBProcess methods for get/set/use address masks

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was
adding to SBProcess for all of the address mask types / memory
regions. In this update, I added enums to control type address mask
type (code, data, any) and address space specifiers (low, high,
all) with defaulted arguments for the most common case.

This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks.
If lldb were told that 64 bits are valid for addressing, this method
would overflow the calculation and set an invalid mask.  Added tests
to check this specific bug while I was adding these APIs.

rdar://123530562
---
 lldb/include/lldb/API/SBProcess.h | 123 ++
 lldb/include/lldb/Utility/AddressableBits.h   |   3 +
 lldb/include/lldb/lldb-enumerations.h |  14 ++
 lldb/source/API/SBProcess.cpp |  89 +
 lldb/source/Utility/AddressableBits.cpp   |  12 +-
 .../python_api/process/address-masks/Makefile |   3 +
 .../process/address-masks/TestAddressMasks.py |  64 +
 .../python_api/process/address-masks/main.c   |   5 +
 8 files changed, 311 insertions(+), 2 deletions(-)
 create mode 100644 lldb/test/API/python_api/process/address-masks/Makefile
 create mode 100644 
lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
 create mode 100644 lldb/test/API/python_api/process/address-masks/main.c

diff --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7e9ad7d9a274f2 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,129 @@ class LLDB_API SBProcess {
   /// the process isn't loaded from a core file.
   lldb::SBFileSpec GetCoreFile();
 
+  /// Get the current address mask that can be applied to addresses
+  /// before reading from memory.
+  ///
+  /// \param[in] type
+  /// lldb may have different address masks for code and data
+  /// addresses.  Either can be requested, or most commonly,
+  /// eAddressMaskTypeAny can be requested and the least specific
+  /// mask will be fetched.  e.g. on a target where instructions
+  /// are word aligned, the Code mask might clear the low 2 bits.
+  ///
+  /// \param[in] addr_range
+  /// Specify whether the address mask for high or low address spaces
+  /// is requested.
+  /// It is highly unusual to have different address masks in high
+  /// or low memory, and by default the eAddressMaskRangeLow is the
+  /// only one used for both types of addresses, the default value for
+  /// this argument is the correct one.
+  ///
+  /// On some architectures like AArch64, it is possible to have
+  /// different page table setups for low and high memory, so different
+  /// numbers of bits relevant to addressing, and it is possible to have
+  /// a program running in one half of memory and accessing the other
+  /// as heap, etc.  In that case the eAddressMaskRangeLow and
+  /// eAddressMaskRangeHigh will have different masks that must be handled.
+  ///
+  /// \return
+  /// The address mask currently in use.  Bits which are not used
+  /// for addressing will be set to 1 in the mask.
+  lldb::addr_t GetAddressMask(
+  lldb::AddressMaskType type,
+  lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow);
+
+  /// Set the current address mask that can be applied to addresses
+  /// before reading from memory.
+  ///
+  /// \param[in] type
+  /// lldb may have different address masks for code and data
+  /// addresses.  Either can be set, or most commonly,
+  /// eAddressMaskTypeAll can be set for both types of addresses.
+  /// An example where they could be different is a target where
+  /// instructions are word aligned, so the low 2 bits are always
+

[Lldb-commits] [lldb] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)

2024-02-26 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905 which 
was approved but I wasn't thrilled with all the API I was adding to SBProcess 
for all of the address mask types / memory regions. In this update, I added 
enums to control type address mask type (code, data, any) and address space 
specifiers (low, high, all) with defaulted arguments for the most common case.

This patch is also fixing a bug in the "addressable bits to address mask" 
calculation I added in AddressableBits::SetProcessMasks. If lldb were told that 
64 bits are valid for addressing, this method would overflow the calculation 
and set an invalid mask.  Added tests to check this specific bug while I was 
adding these APIs.

rdar://123530562

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


8 Files Affected:

- (modified) lldb/include/lldb/API/SBProcess.h (+123) 
- (modified) lldb/include/lldb/Utility/AddressableBits.h (+3) 
- (modified) lldb/include/lldb/lldb-enumerations.h (+14) 
- (modified) lldb/source/API/SBProcess.cpp (+89) 
- (modified) lldb/source/Utility/AddressableBits.cpp (+10-2) 
- (added) lldb/test/API/python_api/process/address-masks/Makefile (+3) 
- (added) lldb/test/API/python_api/process/address-masks/TestAddressMasks.py 
(+64) 
- (added) lldb/test/API/python_api/process/address-masks/main.c (+5) 


``diff
diff --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7e9ad7d9a274f2 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,129 @@ class LLDB_API SBProcess {
   /// the process isn't loaded from a core file.
   lldb::SBFileSpec GetCoreFile();
 
+  /// Get the current address mask that can be applied to addresses
+  /// before reading from memory.
+  ///
+  /// \param[in] type
+  /// lldb may have different address masks for code and data
+  /// addresses.  Either can be requested, or most commonly,
+  /// eAddressMaskTypeAny can be requested and the least specific
+  /// mask will be fetched.  e.g. on a target where instructions
+  /// are word aligned, the Code mask might clear the low 2 bits.
+  ///
+  /// \param[in] addr_range
+  /// Specify whether the address mask for high or low address spaces
+  /// is requested.
+  /// It is highly unusual to have different address masks in high
+  /// or low memory, and by default the eAddressMaskRangeLow is the
+  /// only one used for both types of addresses, the default value for
+  /// this argument is the correct one.
+  ///
+  /// On some architectures like AArch64, it is possible to have
+  /// different page table setups for low and high memory, so different
+  /// numbers of bits relevant to addressing, and it is possible to have
+  /// a program running in one half of memory and accessing the other
+  /// as heap, etc.  In that case the eAddressMaskRangeLow and
+  /// eAddressMaskRangeHigh will have different masks that must be handled.
+  ///
+  /// \return
+  /// The address mask currently in use.  Bits which are not used
+  /// for addressing will be set to 1 in the mask.
+  lldb::addr_t GetAddressMask(
+  lldb::AddressMaskType type,
+  lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow);
+
+  /// Set the current address mask that can be applied to addresses
+  /// before reading from memory.
+  ///
+  /// \param[in] type
+  /// lldb may have different address masks for code and data
+  /// addresses.  Either can be set, or most commonly,
+  /// eAddressMaskTypeAll can be set for both types of addresses.
+  /// An example where they could be different is a target where
+  /// instructions are word aligned, so the low 2 bits are always
+  /// zero.
+  ///
+  /// \param[in] mask
+  /// The address mask to set.  Bits which are not used for addressing
+  /// should be set to 1 in the mask.
+  ///
+  /// \param[in] addr_range
+  /// Specify whether the address mask for high or low address spaces
+  /// is being set.
+  /// It is highly unusual to have different address masks in high
+  /// or low memory, and by default the eAddressMaskRangeLow is the
+  /// only one used for both types of addresses, the default value for
+  /// this argument is the correct one.
+  ///
+  /// On some architectures like AArch64, it is possible to have
+  /// different page table setups for low and high memory, so different
+  /// numbers of bits relevant to addressing, and it is possible to have
+  /// a program running in one half of memory and accessing the other
+  /// as heap, etc.  In that case the eAddressMaskRangeLow and
+  /// eAddressMaskRangeHigh will have different masks that must be
+  /// specified.
+  void SetAddressMask(
+  lldb::AddressMaskType typ

[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)

2024-02-26 Thread via lldb-commits


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

Partly, there's just a lot of unnecessary boiler plate.  It's also possible to 
define combinations of arguments that make no sense (e.g. eArgRepeatPlus 
followed by eArgRepeatPlain...) but these are never checked since we just 
push_back directly into the argument definitions.

This commit is step 1 of this cleanup - do the obvious stuff.  In it, all the 
simple homogenous argument lists and the breakpoint/watchpoint ID/Range types, 
are set with common functions.  This is an NFC change, it just centralizes 
boiler plate.  There's no checking yet because you can't get a single argument 
wrong.

The end goal is that all argument definition goes through functions and 
m_arguments is hidden so that you can't define inconsistent argument sets.

>From 2ed71038d9d16a09b3a04cf667dd272c911fef23 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Mon, 26 Feb 2024 18:09:18 -0800
Subject: [PATCH] Start to clean up the process of defining command arguments.

Partly, there's just a lot of unnecessary boiler plate.  It's also
possible to define combinations of arguments that make no sense (e.g.
eArgRepeatPlus followed by eArgRepeatPlain...) but these are never
checked since we just push_back directly into the argument definitions.

This commit is step 1.  In it, all the simple homogenous argument lists are
set with a common function.  This is an NFC change, it just centralizes boiler
plate.  There's no checking yet because you can't get a single argument wrong.
---
 lldb/include/lldb/Interpreter/CommandObject.h |  21 +++-
 lldb/source/Commands/CommandObjectApropos.cpp |  14 +--
 .../Commands/CommandObjectBreakpoint.cpp  |  72 ++-
 .../CommandObjectBreakpointCommand.cpp|  42 +--
 .../source/Commands/CommandObjectCommands.cpp | 119 ++
 .../Commands/CommandObjectDWIMPrint.cpp   |   3 +-
 .../Commands/CommandObjectExpression.cpp  |  14 +--
 lldb/source/Commands/CommandObjectFrame.cpp   |  59 +
 lldb/source/Commands/CommandObjectHelp.cpp|  13 +-
 lldb/source/Commands/CommandObjectLog.cpp |  56 +
 .../source/Commands/CommandObjectPlatform.cpp |  80 ++--
 lldb/source/Commands/CommandObjectPlugin.cpp  |  14 +--
 lldb/source/Commands/CommandObjectProcess.cpp |  50 ++--
 lldb/source/Commands/CommandObjectQuit.cpp|   3 +-
 .../source/Commands/CommandObjectRegister.cpp |  22 +---
 lldb/source/Commands/CommandObjectSession.cpp |   4 +-
 .../source/Commands/CommandObjectSettings.cpp |  42 +--
 lldb/source/Commands/CommandObjectTarget.cpp  | 107 +++-
 lldb/source/Commands/CommandObjectThread.cpp  |  90 ++---
 .../Commands/CommandObjectThreadUtil.cpp  |   6 +-
 lldb/source/Commands/CommandObjectTrace.cpp   |   9 +-
 lldb/source/Commands/CommandObjectType.cpp| 113 ++---
 .../Commands/CommandObjectWatchpoint.cpp  |  68 ++
 .../CommandObjectWatchpointCommand.cpp|  42 +--
 lldb/source/Interpreter/CommandObject.cpp |  39 --
 .../ItaniumABI/ItaniumABILanguageRuntime.cpp  |  14 +--
 .../AppleObjCRuntime/AppleObjCRuntimeV2.cpp   |  28 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   6 +-
 28 files changed, 164 insertions(+), 986 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/CommandObject.h 
b/lldb/include/lldb/Interpreter/CommandObject.h
index a326c6dc38a37a..49cf4a21e25d73 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -206,6 +206,22 @@ class CommandObject : public 
std::enable_shared_from_this {
 
   static const ArgumentTableEntry *
   FindArgumentDataByType(lldb::CommandArgumentType arg_type);
+  
+  // Sets the argument list for this command to one homogenous argument type,
+  // with the repeat specified.
+  void AddSimpleArgumentList(lldb::CommandArgumentType arg_type, 
+  ArgumentRepetitionType repetition_type = eArgRepeatPlain);
+
+  // Helper function to set BP IDs or ID ranges as the command argument data
+  // for this command.
+  // This used to just populate an entry you could add to, but that was never
+  // used.  If we ever need that we can take optional extra args here.
+  // Use this to define a simple argument list:
+  enum IDType {
+eBreakpointArgs = 0,
+eWatchpointArgs = 1
+  };
+  void AddIDsArgumentData(IDType type);
 
   int GetNumArgumentEntries();
 
@@ -392,11 +408,6 @@ class CommandObject : public 
std::enable_shared_from_this {
   void *m_command_override_baton;
   bool m_is_user_command = false;
 
-  // Helper function to populate IDs or ID ranges as the command argument data
-  // to the specified command argument entry.
-  static void AddIDsArgumentData(CommandArgumentEntry &arg,
- lldb::CommandArgumentType ID,
- lldb::CommandArgumentType IDRange);
 };
 
 class CommandObjectParsed : public CommandObject {
di

[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)

2024-02-26 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes

Partly, there's just a lot of unnecessary boiler plate.  It's also possible to 
define combinations of arguments that make no sense (e.g. eArgRepeatPlus 
followed by eArgRepeatPlain...) but these are never checked since we just 
push_back directly into the argument definitions.

This commit is step 1 of this cleanup - do the obvious stuff.  In it, all the 
simple homogenous argument lists and the breakpoint/watchpoint ID/Range types, 
are set with common functions.  This is an NFC change, it just centralizes 
boiler plate.  There's no checking yet because you can't get a single argument 
wrong.

The end goal is that all argument definition goes through functions and 
m_arguments is hidden so that you can't define inconsistent argument sets.

---

Patch is 91.31 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/83097.diff


28 Files Affected:

- (modified) lldb/include/lldb/Interpreter/CommandObject.h (+16-5) 
- (modified) lldb/source/Commands/CommandObjectApropos.cpp (+1-13) 
- (modified) lldb/source/Commands/CommandObjectBreakpoint.cpp (+9-63) 
- (modified) lldb/source/Commands/CommandObjectBreakpointCommand.cpp (+3-39) 
- (modified) lldb/source/Commands/CommandObjectCommands.cpp (+9-110) 
- (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+1-2) 
- (modified) lldb/source/Commands/CommandObjectExpression.cpp (+1-13) 
- (modified) lldb/source/Commands/CommandObjectFrame.cpp (+5-54) 
- (modified) lldb/source/Commands/CommandObjectHelp.cpp (+1-12) 
- (modified) lldb/source/Commands/CommandObjectLog.cpp (+4-52) 
- (modified) lldb/source/Commands/CommandObjectPlatform.cpp (+13-67) 
- (modified) lldb/source/Commands/CommandObjectPlugin.cpp (+1-13) 
- (modified) lldb/source/Commands/CommandObjectProcess.cpp (+7-43) 
- (modified) lldb/source/Commands/CommandObjectQuit.cpp (+1-2) 
- (modified) lldb/source/Commands/CommandObjectRegister.cpp (+2-20) 
- (modified) lldb/source/Commands/CommandObjectSession.cpp (+1-3) 
- (modified) lldb/source/Commands/CommandObjectSettings.cpp (+3-39) 
- (modified) lldb/source/Commands/CommandObjectTarget.cpp (+14-93) 
- (modified) lldb/source/Commands/CommandObjectThread.cpp (+8-82) 
- (modified) lldb/source/Commands/CommandObjectThreadUtil.cpp (+2-4) 
- (modified) lldb/source/Commands/CommandObjectTrace.cpp (+3-6) 
- (modified) lldb/source/Commands/CommandObjectType.cpp (+12-101) 
- (modified) lldb/source/Commands/CommandObjectWatchpoint.cpp (+8-60) 
- (modified) lldb/source/Commands/CommandObjectWatchpointCommand.cpp (+3-39) 
- (modified) lldb/source/Interpreter/CommandObject.cpp (+31-8) 
- (modified) 
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
 (+1-13) 
- (modified) 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 (+2-26) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+2-4) 


``diff
diff --git a/lldb/include/lldb/Interpreter/CommandObject.h 
b/lldb/include/lldb/Interpreter/CommandObject.h
index a326c6dc38a37a..49cf4a21e25d73 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -206,6 +206,22 @@ class CommandObject : public 
std::enable_shared_from_this {
 
   static const ArgumentTableEntry *
   FindArgumentDataByType(lldb::CommandArgumentType arg_type);
+  
+  // Sets the argument list for this command to one homogenous argument type,
+  // with the repeat specified.
+  void AddSimpleArgumentList(lldb::CommandArgumentType arg_type, 
+  ArgumentRepetitionType repetition_type = eArgRepeatPlain);
+
+  // Helper function to set BP IDs or ID ranges as the command argument data
+  // for this command.
+  // This used to just populate an entry you could add to, but that was never
+  // used.  If we ever need that we can take optional extra args here.
+  // Use this to define a simple argument list:
+  enum IDType {
+eBreakpointArgs = 0,
+eWatchpointArgs = 1
+  };
+  void AddIDsArgumentData(IDType type);
 
   int GetNumArgumentEntries();
 
@@ -392,11 +408,6 @@ class CommandObject : public 
std::enable_shared_from_this {
   void *m_command_override_baton;
   bool m_is_user_command = false;
 
-  // Helper function to populate IDs or ID ranges as the command argument data
-  // to the specified command argument entry.
-  static void AddIDsArgumentData(CommandArgumentEntry &arg,
- lldb::CommandArgumentType ID,
- lldb::CommandArgumentType IDRange);
 };
 
 class CommandObjectParsed : public CommandObject {
diff --git a/lldb/source/Commands/CommandObjectApropos.cpp 
b/lldb/source/Commands/CommandObjectApropos.cpp
index 88c214d4fc56ab..d663f2bd923fe2 100644
--- a/lldb/source/Commands/CommandObjectApropos.cpp
+++ b/lldb/source/Commands/CommandObjectApropos.cpp
@@ -21,19 +21,7 @@ 
CommandObjectApropos::CommandObjectAp

[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)

2024-02-26 Thread via lldb-commits


github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff dc06d75ab27b4dcae2940fc386fadd06f70faffe 
2ed71038d9d16a09b3a04cf667dd272c911fef23 -- 
lldb/include/lldb/Interpreter/CommandObject.h 
lldb/source/Commands/CommandObjectApropos.cpp 
lldb/source/Commands/CommandObjectBreakpoint.cpp 
lldb/source/Commands/CommandObjectBreakpointCommand.cpp 
lldb/source/Commands/CommandObjectCommands.cpp 
lldb/source/Commands/CommandObjectDWIMPrint.cpp 
lldb/source/Commands/CommandObjectExpression.cpp 
lldb/source/Commands/CommandObjectFrame.cpp 
lldb/source/Commands/CommandObjectHelp.cpp 
lldb/source/Commands/CommandObjectLog.cpp 
lldb/source/Commands/CommandObjectPlatform.cpp 
lldb/source/Commands/CommandObjectPlugin.cpp 
lldb/source/Commands/CommandObjectProcess.cpp 
lldb/source/Commands/CommandObjectQuit.cpp 
lldb/source/Commands/CommandObjectRegister.cpp 
lldb/source/Commands/CommandObjectSession.cpp 
lldb/source/Commands/CommandObjectSettings.cpp 
lldb/source/Commands/CommandObjectTarget.cpp 
lldb/source/Commands/CommandObjectThread.cpp 
lldb/source/Commands/CommandObjectThreadUtil.cpp 
lldb/source/Commands/CommandObjectTrace.cpp 
lldb/source/Commands/CommandObjectType.cpp 
lldb/source/Commands/CommandObjectWatchpoint.cpp 
lldb/source/Commands/CommandObjectWatchpointCommand.cpp 
lldb/source/Interpreter/CommandObject.cpp 
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/include/lldb/Interpreter/CommandObject.h 
b/lldb/include/lldb/Interpreter/CommandObject.h
index 49cf4a21e2..a641a468b4 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -206,10 +206,11 @@ public:
 
   static const ArgumentTableEntry *
   FindArgumentDataByType(lldb::CommandArgumentType arg_type);
-  
+
   // Sets the argument list for this command to one homogenous argument type,
   // with the repeat specified.
-  void AddSimpleArgumentList(lldb::CommandArgumentType arg_type, 
+  void AddSimpleArgumentList(
+  lldb::CommandArgumentType arg_type,
   ArgumentRepetitionType repetition_type = eArgRepeatPlain);
 
   // Helper function to set BP IDs or ID ranges as the command argument data
@@ -217,10 +218,7 @@ public:
   // This used to just populate an entry you could add to, but that was never
   // used.  If we ever need that we can take optional extra args here.
   // Use this to define a simple argument list:
-  enum IDType {
-eBreakpointArgs = 0,
-eWatchpointArgs = 1
-  };
+  enum IDType { eBreakpointArgs = 0, eWatchpointArgs = 1 };
   void AddIDsArgumentData(IDType type);
 
   int GetNumArgumentEntries();
@@ -407,7 +405,6 @@ protected:
   lldb_private::CommandOverrideCallbackWithResult m_command_override_callback;
   void *m_command_override_baton;
   bool m_is_user_command = false;
-
 };
 
 class CommandObjectParsed : public CommandObject {
diff --git a/lldb/source/Interpreter/CommandObject.cpp 
b/lldb/source/Interpreter/CommandObject.cpp
index 2c6e6e1d44..4634b75c6a 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/lldb/source/Interpreter/CommandObject.cpp
@@ -392,9 +392,9 @@ bool CommandObject::ParseOptionsAndNotify(Args &args,
   return true;
 }
 
-void CommandObject::AddSimpleArgumentList(CommandArgumentType arg_type, 
- ArgumentRepetitionType repetition_type) {
-
+void CommandObject::AddSimpleArgumentList(
+CommandArgumentType arg_type, ArgumentRepetitionType repetition_type) {
+
   CommandArgumentEntry arg_entry;
   CommandArgumentData simple_arg;
 
@@ -720,14 +720,14 @@ void 
CommandObject::AddIDsArgumentData(CommandObject::IDType type) {
   // Create the first variant for the first (and only) argument for this
   // command.
   switch (type) {
-case eBreakpointArgs:
-  id_arg.arg_type = eArgTypeBreakpointID;
-  id_range_arg.arg_type = eArgTypeBreakpointIDRange;
-  break;
-case eWatchpointArgs:
-  id_arg.arg_type = eArgTypeWatchpointID;
-  id_range_arg.arg_type = eArgTypeWatchpointIDRange;
-  break;
+  case eBreakpointArgs:
+id_arg.arg_type = eArgTypeBreakpointID;
+id_range_arg.arg_type = eArgTypeBreakpointIDRange;
+break;
+  case eWatchpointArgs:
+id_arg.arg_type = eArgTypeWatchpointID;
+id_range_arg.arg_type = eArgTypeWatchpointIDRange;
+break;
   }
   id_arg.arg_repetition = eArgRepeatOptional;
   id_range_arg.arg_repetition = eArgRepeatOptional;

``




https://github.com/llvm/llvm-project/pull/83097
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
ht

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl created 
https://github.com/llvm/llvm-project/pull/83099

None

>From 1b020afcb50cd6059bbe2ab26779ed6d3e7c2f7a Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Mon, 26 Feb 2024 18:43:57 -0800
Subject: [PATCH] Aim debugserver workaround more precisely.

---
 .../gdb-remote/GDBRemoteCommunicationClient.cpp   | 15 ++-
 lldb/source/Target/Target.cpp |  4 +++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6f8aa262289946..c38d74ca8e6c95 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -17,6 +17,7 @@
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -2147,8 +2148,20 @@ bool 
GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
   if (!value.getAsInteger(16, cpu))
 ++num_keys_decoded;
 } else if (name.equals("cpusubtype")) {
-  if (!value.getAsInteger(16, sub))
+  if (!value.getAsInteger(16, sub)) {
 ++num_keys_decoded;
+// Workaround for for pre-2024 Apple debugserver, which always
+// returns arm64e on arm64e-capable hardware regardless of
+// what the process is. This can be deleted at some point in
+// the future.
+if (cpu == llvm::MachO::CPU_TYPE_ARM64 &&
+sub == llvm::MachO::CPU_SUBTYPE_ARM64E) {
+  if (GetGDBServerVersion())
+if (m_gdb_server_version >= 1000 &&
+m_gdb_server_version <= 1504)
+  sub = 0;
+}
+  }
 } else if (name.equals("triple")) {
   StringExtractor extractor(value);
   extractor.GetHexByteString(triple);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 089915cab4915a..b592016e7a9824 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -33,7 +33,6 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
-#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -1572,6 +1571,7 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
 if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
+<<< HEAD
 // Workaround for for pre-2024 debugserver, which always
 // returns arm64e on arm64e-capable hardware regardless of
 // what the process is. This can be deleted at some point in
@@ -1579,6 +1579,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 if (!m_arch.GetSpec().GetMachOCPUSubType() &&
 other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E)
   replace_local_arch = true;
+===
+>>> e491dde5dab4 (Aim debugserver workaround more precisely.)
   }
 }
   }

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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)


Changes



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


2 Files Affected:

- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+14-1) 
- (modified) lldb/source/Target/Target.cpp (+3-1) 


``diff
diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6f8aa262289946..c38d74ca8e6c95 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -17,6 +17,7 @@
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -2147,8 +2148,20 @@ bool 
GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
   if (!value.getAsInteger(16, cpu))
 ++num_keys_decoded;
 } else if (name.equals("cpusubtype")) {
-  if (!value.getAsInteger(16, sub))
+  if (!value.getAsInteger(16, sub)) {
 ++num_keys_decoded;
+// Workaround for for pre-2024 Apple debugserver, which always
+// returns arm64e on arm64e-capable hardware regardless of
+// what the process is. This can be deleted at some point in
+// the future.
+if (cpu == llvm::MachO::CPU_TYPE_ARM64 &&
+sub == llvm::MachO::CPU_SUBTYPE_ARM64E) {
+  if (GetGDBServerVersion())
+if (m_gdb_server_version >= 1000 &&
+m_gdb_server_version <= 1504)
+  sub = 0;
+}
+  }
 } else if (name.equals("triple")) {
   StringExtractor extractor(value);
   extractor.GetHexByteString(triple);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 089915cab4915a..b592016e7a9824 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -33,7 +33,6 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
-#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -1572,6 +1571,7 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
 if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
+<<< HEAD
 // Workaround for for pre-2024 debugserver, which always
 // returns arm64e on arm64e-capable hardware regardless of
 // what the process is. This can be deleted at some point in
@@ -1579,6 +1579,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 if (!m_arch.GetSpec().GetMachOCPUSubType() &&
 other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E)
   replace_local_arch = true;
+===
+>>> e491dde5dab4 (Aim debugserver workaround more precisely.)
   }
 }
   }

``




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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/83099

>From f26292e7db372380fab4fd83f9df79a5b8929772 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Mon, 26 Feb 2024 18:43:57 -0800
Subject: [PATCH] Aim debugserver workaround more precisely.

---
 .../gdb-remote/GDBRemoteCommunicationClient.cpp   | 15 ++-
 lldb/source/Target/Target.cpp |  8 
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6f8aa262289946..c38d74ca8e6c95 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -17,6 +17,7 @@
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -2147,8 +2148,20 @@ bool 
GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
   if (!value.getAsInteger(16, cpu))
 ++num_keys_decoded;
 } else if (name.equals("cpusubtype")) {
-  if (!value.getAsInteger(16, sub))
+  if (!value.getAsInteger(16, sub)) {
 ++num_keys_decoded;
+// Workaround for for pre-2024 Apple debugserver, which always
+// returns arm64e on arm64e-capable hardware regardless of
+// what the process is. This can be deleted at some point in
+// the future.
+if (cpu == llvm::MachO::CPU_TYPE_ARM64 &&
+sub == llvm::MachO::CPU_SUBTYPE_ARM64E) {
+  if (GetGDBServerVersion())
+if (m_gdb_server_version >= 1000 &&
+m_gdb_server_version <= 1504)
+  sub = 0;
+}
+  }
 } else if (name.equals("triple")) {
   StringExtractor extractor(value);
   extractor.GetHexByteString(triple);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 089915cab4915a..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -33,7 +33,6 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
-#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -1572,13 +1571,6 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
 if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
-// Workaround for for pre-2024 debugserver, which always
-// returns arm64e on arm64e-capable hardware regardless of
-// what the process is. This can be deleted at some point in
-// the future.
-if (!m_arch.GetSpec().GetMachOCPUSubType() &&
-other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E)
-  replace_local_arch = true;
   }
 }
   }

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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/83099

>From 6f74b0bd4cfb3aacf42b9b65a019dba425c7d18e Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Mon, 26 Feb 2024 16:50:27 -0800
Subject: [PATCH] Make the workaround for older debugserver versions more
 narrow.

---
 .../gdb-remote/GDBRemoteCommunicationClient.cpp   | 15 ++-
 lldb/source/Target/Target.cpp |  8 
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6f8aa262289946..1f6116967a4f64 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -17,6 +17,7 @@
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -2147,8 +2148,20 @@ bool 
GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
   if (!value.getAsInteger(16, cpu))
 ++num_keys_decoded;
 } else if (name.equals("cpusubtype")) {
-  if (!value.getAsInteger(16, sub))
+  if (!value.getAsInteger(16, sub)) {
 ++num_keys_decoded;
+// Workaround for pre-2024 Apple debugserver, which always
+// returns arm64e on arm64e-capable hardware regardless of
+// what the process is. This can be deleted at some point
+// in the future.
+if (cpu == llvm::MachO::CPU_TYPE_ARM64 &&
+sub == llvm::MachO::CPU_SUBTYPE_ARM64E) {
+  if (GetGDBServerVersion())
+if (m_gdb_server_version >= 1000 &&
+m_gdb_server_version <= 1504)
+  sub = 0;
+}
+  }
 } else if (name.equals("triple")) {
   StringExtractor extractor(value);
   extractor.GetHexByteString(triple);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 089915cab4915a..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -33,7 +33,6 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
-#include "lldb/Host/SafeMachO.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -1572,13 +1571,6 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, 
bool set_platform,
 
 if (m_arch.GetSpec().GetTriple() == other.GetTriple())
   replace_local_arch = false;
-// Workaround for for pre-2024 debugserver, which always
-// returns arm64e on arm64e-capable hardware regardless of
-// what the process is. This can be deleted at some point in
-// the future.
-if (!m_arch.GetSpec().GetMachOCPUSubType() &&
-other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E)
-  replace_local_arch = true;
   }
 }
   }

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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Jason Molenda via lldb-commits


jasonmolenda wrote:

I'm a little split because the current packet is not super useful as it's 
constructed today - I'd really like to see a domain or vendor along with a 
version number  And instead of a simple number, maybe a major/minor version 
number and/or a version string?  I'm also fine with landing this now, detecting 
an "old" apple debugserver as 1000 < version number < 1504, and I can do the 
version packet fix separately and update the check.

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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Jason Molenda via lldb-commits


jasonmolenda wrote:

I'm not fishing for Adrian to dig in to that, I'm happy to make the change.  
but I'm curious what @JDevlieghere and @adrian-prantl think, if it's worth 
changing this packet.  It's not used anywhere yet, so overhauling it now 
before/shortly after we actually start using it, is the right way to go.

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

[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)

2024-02-26 Thread Med Ismail Bennani via lldb-commits


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

This looks great! Left a comment.

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

[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)

2024-02-26 Thread Med Ismail Bennani via lldb-commits



@@ -694,27 +712,32 @@ void CommandObject::GenerateHelpText(Stream &output_strm) 
{
   }
 }
 
-void CommandObject::AddIDsArgumentData(CommandArgumentEntry &arg,
-   CommandArgumentType ID,
-   CommandArgumentType IDRange) {
+void CommandObject::AddIDsArgumentData(CommandObject::IDType type) {
+  CommandArgumentEntry arg;
   CommandArgumentData id_arg;
   CommandArgumentData id_range_arg;
 
   // Create the first variant for the first (and only) argument for this
   // command.
-  id_arg.arg_type = ID;
+  switch (type) {
+case eBreakpointArgs:
+  id_arg.arg_type = eArgTypeBreakpointID;
+  id_range_arg.arg_type = eArgTypeBreakpointIDRange;
+  break;
+case eWatchpointArgs:
+  id_arg.arg_type = eArgTypeWatchpointID;
+  id_range_arg.arg_type = eArgTypeWatchpointIDRange;
+  break;
+  }

medismailben wrote:

This looks repetitive ... I wonder if every case of this switch could be a 
macro that uses string substitution, to simplify it even more.

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

[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)

2024-02-26 Thread Med Ismail Bennani via lldb-commits


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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Jason Molenda via lldb-commits


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

LGTM, let's land this patch.  I will extend the stub version packet with 
additional fields (Jonas points out that other stubs implement this packet 
already as-is) and update this once that PR is approved and merged.  But we 
don't need to block this PR on getting that done.

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

[Lldb-commits] [lldb] Aim debugserver workaround more precisely. (PR #83099)

2024-02-26 Thread Jonas Devlieghere via lldb-commits


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

I'm happy if Jason is happy!

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

62 matches

Mail list logo