[Lldb-commits] [PATCH] D142067: Remove the "help" subcommand - its undocumented, undiscoverable and doesn't work....

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

+1 to everything Jim said in the commit message. LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142067/new/

https://reviews.llvm.org/D142067

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

[Lldb-commits] [PATCH] D142140: [lldb] Remove legacy six module for py2->py3

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
Herald added a project: All.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

LLDB only supports Python3 now, so the `six` shim for Python2 is no longer 
necessary.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142140

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/third_party/Python/module/six/LICENSE
  lldb/third_party/Python/module/six/six.py

Index: lldb/third_party/Python/module/six/six.py
===
--- lldb/third_party/Python/module/six/six.py
+++ /dev/null
@@ -1,887 +0,0 @@
-"""Utilities for writing code that runs on Python 2 and 3"""
-
-# Copyright (c) 2010-2015 Benjamin Peterson
-#
-# Permission is hereby granted, free of charge, to any person obtaining a copy
-# of this software and associated documentation files (the "Software"), to deal
-# in the Software without restriction, including without limitation the rights
-# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-# copies of the Software, and to permit persons to whom the Software is
-# furnished to do so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice shall be included in all
-# copies or substantial portions of the Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-# SOFTWARE.
-
-from __future__ import absolute_import
-
-import functools
-import itertools
-import operator
-import sys
-import types
-
-__author__ = "Benjamin Peterson "
-__version__ = "1.10.0"
-
-
-# Useful for very coarse version differentiation.
-PY2 = sys.version_info[0] == 2
-PY3 = sys.version_info[0] == 3
-PY34 = sys.version_info[0:2] >= (3, 4)
-
-if PY3:
-string_types = str,
-integer_types = int,
-class_types = type,
-text_type = str
-binary_type = bytes
-
-MAXSIZE = sys.maxsize
-else:
-string_types = basestring,
-integer_types = (int, long)
-class_types = (type, types.ClassType)
-text_type = unicode
-binary_type = str
-
-if sys.platform.startswith("java"):
-# Jython always uses 32 bits.
-MAXSIZE = int((1 << 31) - 1)
-else:
-# It's possible to have sizeof(long) != sizeof(Py_ssize_t).
-class X(object):
-
-def __len__(self):
-return 1 << 31
-try:
-len(X())
-except OverflowError:
-# 32-bit
-MAXSIZE = int((1 << 31) - 1)
-else:
-# 64-bit
-MAXSIZE = int((1 << 63) - 1)
-del X
-
-
-def _add_doc(func, doc):
-"""Add documentation to a function."""
-func.__doc__ = doc
-
-
-def _import_module(name):
-"""Import module, returning the module after the last dot."""
-__import__(name)
-return sys.modules[name]
-
-
-class _LazyDescr(object):
-
-def __init__(self, name):
-self.name = name
-
-def __get__(self, obj, tp):
-result = self._resolve()
-setattr(obj, self.name, result)  # Invokes __set__.
-try:
-# This is a bit ugly, but it avoids running this again by
-# removing this descriptor.
-delattr(obj.__class__, self.name)
-except AttributeError:
-pass
-return result
-
-
-class MovedModule(_LazyDescr):
-
-def __init__(self, name, old, new=None):
-super(MovedModule, self).__init__(name)
-if PY3:
-if new is None:
-new = name
-self.mod = new
-else:
-self.mod = old
-
-def _resolve(self):
-return _import_module(self.mod)
-
-def __getattr__(self, attr):
-_module = self._resolve()
-value = getattr(_module, attr)
-setattr(self, attr, value)
-return value
-
-
-class _LazyModule(types.ModuleType):
-
-def __init__(self, name):
-super(_LazyModule, self).__init__(name)
-self.__doc__ = self.__class__.__doc__
-
-def __dir__(self):
-attrs = ["__doc__", "__name__"]
-attrs += [attr.name for attr in self._moved_attributes]
-return attrs
-
-# Subclasses should override this
-_moved_attributes = []
-
-
-class MovedAttribute(_LazyDescr):
-
-def __init__(self, name, old_mod, new_mod, old_attr=None, new_attr=None):
-super(MovedAttribute, self).__init__(name)
-if PY3:
-if new_mod is None:
-new_mod = name
-self.mod = new_mod
-   

[Lldb-commits] [PATCH] D142140: [lldb] Remove legacy six module for py2->py3

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142140/new/

https://reviews.llvm.org/D142140

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

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Update: There are some new failures after someone recently added new tests that 
involve simple template types. I need to fix my new lookup routine to do the 
right thing in light of these new tests. The main issue is currently if you 
have a "foo", the accelerator tables and DWARF contain "foo" only. This 
causes the expression parser to be able to lookup "foo" as it is parsing an 
expression that has "auto a = foo()" as it will actually return a 
specialized "foo" as the result since the accelerator table will point to 
the "foo" type. We need to outlaw these raw lookups from working. Worse 
yet, if there are multiple instantiations of "foo", it will return all of 
them and cause LLDB to crash later in some AST copying code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137900/new/

https://reviews.llvm.org/D137900

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

[Lldb-commits] [PATCH] D142141: [lldb][test] Skip TestRerunAndExprDylib on Ubuntu 18.04

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added a reviewer: Michael137.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Disable this test on Ubuntu 18.04, where it fails for yet to be determined 
reasons.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142141

Files:
  lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py


Index: 
lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
===
--- lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
+++ lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
@@ -9,7 +9,24 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
+
+def isUbuntu18_04():
+"""
+Check if the host OS is Ubuntu 18.04.
+Derived from `platform.freedesktop_os_release` in Python 3.10.
+"""
+for path in ("/etc/os-release", "/usr/lib/os-release"):
+if os.path.exists(path):
+with open(path) as f:
+contents = f.read()
+if "Ubuntu 18.04" in contents:
+return True
+
+return False
+
+
 class TestRerunExprDylib(TestBase):
+@skipTestIfFn(isUbuntu18_04, bugnumber="rdar://103831050")
 @skipIfWindows
 def test(self):
 """


Index: lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
===
--- lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
+++ lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
@@ -9,7 +9,24 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
+
+def isUbuntu18_04():
+"""
+Check if the host OS is Ubuntu 18.04.
+Derived from `platform.freedesktop_os_release` in Python 3.10.
+"""
+for path in ("/etc/os-release", "/usr/lib/os-release"):
+if os.path.exists(path):
+with open(path) as f:
+contents = f.read()
+if "Ubuntu 18.04" in contents:
+return True
+
+return False
+
+
 class TestRerunExprDylib(TestBase):
+@skipTestIfFn(isUbuntu18_04, bugnumber="rdar://103831050")
 @skipIfWindows
 def test(self):
 """
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142067: Remove the "help" subcommand - its undocumented, undiscoverable and doesn't work....

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Yeah this is a good cleanup, but fwiw maybe because using git, but I sometimes 
expect `break set --help` etc to work, forgetting which style lldb uses.  just 
throwing it out there as something I get tripped up by once in a while, not 
really related to this change, but I can appreciate where the original author 
thought they were going with it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142067/new/

https://reviews.llvm.org/D142067

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

[Lldb-commits] [PATCH] D142150: [lldb] Remove timer from SBModule copy ctor

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added a reviewer: aprantl.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The SBModule copy constructor has fast execution, and is high firing. Fast and
frequent functions are not good candidates for timers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142150

Files:
  lldb/source/API/SBModule.cpp


Index: lldb/source/API/SBModule.cpp
===
--- lldb/source/API/SBModule.cpp
+++ lldb/source/API/SBModule.cpp
@@ -43,9 +43,7 @@
 SetSP(module_sp);
 }
 
-SBModule::SBModule(const SBModule &rhs) : m_opaque_sp(rhs.m_opaque_sp) {
-  LLDB_INSTRUMENT_VA(this, rhs);
-}
+SBModule::SBModule(const SBModule &rhs) = default;
 
 SBModule::SBModule(lldb::SBProcess &process, lldb::addr_t header_addr) {
   LLDB_INSTRUMENT_VA(this, process, header_addr);


Index: lldb/source/API/SBModule.cpp
===
--- lldb/source/API/SBModule.cpp
+++ lldb/source/API/SBModule.cpp
@@ -43,9 +43,7 @@
 SetSP(module_sp);
 }
 
-SBModule::SBModule(const SBModule &rhs) : m_opaque_sp(rhs.m_opaque_sp) {
-  LLDB_INSTRUMENT_VA(this, rhs);
-}
+SBModule::SBModule(const SBModule &rhs) = default;
 
 SBModule::SBModule(lldb::SBProcess &process, lldb::addr_t header_addr) {
   LLDB_INSTRUMENT_VA(this, process, header_addr);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141605: [lldb] Detach the child process when stepping over a fork

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The part of handling the fork where we decide we're going to follow the child 
and so we need to switch the process PID & TID does have to happen on event 
receipt.  The point there is that until the client pulls an event from the 
public event queue, it doesn't know that the state change has happened yet.  We 
don't want the current state that you access with GetState, GetPID, 
GetSelectedThread, etc. to get out of sync with the event flow, so to the 
public world it should appear to it as if the process were in the state of the 
last event it actually received.  Having the PID or TID change before the fork 
event gets pulled off the queue would break this illusion.

However, the state of the process we aren't following isn't relevant to the 
state of the process we are  following, so the part of the DidFork dealing with 
the discarded side of the fork can happen when the fork event is received, in 
ProcessGDBRemote::SetThreadStopInfo or thereabouts.  That does seem like a 
better way to do this.

However, I don't think this patch is wrong in itself.  Formally, the step 
thread plans might want to know that a fork has happened.  But it's not 
terribly important as they can't really do anything about except maybe report 
an error.  And the same could be said of exec events which are already in this 
list.  What's actually going to happen is once we've stopped for the fork, all 
the step plans will be asked whether they are Stale, and given the thread they 
were on is no longer present, they should unship themselves.  So we're not 
losing anything by bypassing the plans for this event delivery.

Note, it might be worth checking that the stepping thread plan's ShouldStop 
check the  PID, not just the TID.  It's possible that the new TID was the same 
as the old TID and the plan might try to keep operating, which would be wrong.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141605/new/

https://reviews.llvm.org/D141605

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

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D141425#4044280 , @kastiglione 
wrote:

> @jingham The brief answer is that decisions have been based on compatibility 
> with the behavior of `p`.
>
> 1. `expression` (also `p`) and `frame variable`, only support the the format 
> part of gdb options. Neither support `--count` or `--size` and so don't 
> support the equivalent gdb options.
> 2. `p` takes no flags (other than the gdb-format), and `dwim-print` matches 
> that behavior.
>
> Taking the example of `dwim-print -fA variable`, if the user were to do the 
> same but with an expression, they'd have to write `dwim-print -fA -- 
> expression`. Will there be users who know and want the printing options you 
> mentioned (synthetic/raw, depth, etc) but who try to use those with 
> `dwim-print` instead of directly using `v` or `e`?
>
> My expectation has been that `dwim-print` would (1) generally not be used 
> directly, but be via an alias (either `p` or another choice) and that (2) for 
> compatibility, the alias would be `dwim-print --`.
>
> Thoughts?



1. I guess that sort of makes sense - since the results of `expr` & `frame var` 
are always typed, specifying sizes and counts doesn't make sense.  Those fields 
only make sense for `memory read`.  It makes the rationale for using gdb-format 
in the `/` accelerator a little less well motivated, but that's not on you...

2. To me the analogy between `p` and `dwim-print` is not the right one.  
`dwim-print` is parallel with `expr`: both are full featured commands from the 
standpoint of configuring the output printing.  `p` is the alias that we offer 
for user-convenience - it picks the defaults for the presentation.  This 
separation is nice because then if a user has a different set of default 
printing expectations (they always want to print pointers one level deep or 
something) they can re-alias `p` to fit their needs, or even make two variants 
that serve different purposes.

So I was expecting at first we'd add a `dp` or something alias, that was 
`dwim-print --` and in the fullness of time we would switch to it as the "first 
choice expression printer" by replacing the `p` -> `expr --` alias with 
`p`->`dwim-print --`, thus getting the same convenience AND configurability we 
got with `expr`...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141425/new/

https://reviews.llvm.org/D141425

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

[Lldb-commits] [PATCH] D142067: Remove the "help" subcommand - its undocumented, undiscoverable and doesn't work....

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I wouldn't be opposed to adding a --help option (with no short option) that 
every command would inherit.  Doing it as a command option alone can't suffice 
for lldb, since that offers no way to list all the commands.  git gets around 
this because you can say `git` on the command line to get all the available 
commands.  If we're doing the command help through an option, there isn't a 
really natural way to do "show me all commands".  For pedantic consistency, you 
could have:

(lldb) --help

do that, but that's pretty gross.  Or in analogy to git you could have

(lldb) lldb

do the job, but that's not particularly discoverable either.

Otherwise you really have to have a `help` command and once you have that it's 
a little weird to use `help` to get the list of commands but then not to get 
help on individual commands.  So --help as an option becomes a little 
redundant.  OTOH, you could do some fun stuff with it like:

(lldb) break set --help

would give help on break set, but

(lldb) break set --help -f

would just show the help string for the -f option...  So as an add-on I don't 
mind that, and being an option would keep if from messing with the parsing of 
everything.  Using the bare word `help` more widely is bound to cause 
collisions...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142067/new/

https://reviews.llvm.org/D142067

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

[Lldb-commits] [lldb] 887240f - Remove the undocumented `help` subcommand.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-01-19T12:03:06-08:00
New Revision: 887240faf769395511d1568b4e50aafc6fe3b696

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

LOG: Remove the undocumented `help` subcommand.

This is processed by hand in CommandObjectMultiword, and is undiscoverable,
it doesn't work in all cases.  Because it is a bare word, it can't really be
extended w/o introducing the possibility of collisions as well.  If we did
want to do something like this we should add a --help flag to CommandObject.  
That
way the feature would be consistent and documented.

Differential Revision: https://reviews.llvm.org/D142067

Added: 


Modified: 
lldb/source/Commands/CommandObjectMultiword.cpp
lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectMultiword.cpp 
b/lldb/source/Commands/CommandObjectMultiword.cpp
index 6bcf0dea03868..bae2717ffe68e 100644
--- a/lldb/source/Commands/CommandObjectMultiword.cpp
+++ b/lldb/source/Commands/CommandObjectMultiword.cpp
@@ -174,11 +174,6 @@ bool CommandObjectMultiword::Execute(const char 
*args_string,
 return result.Succeeded();
   }
 
-  if (sub_command.equals_insensitive("help")) {
-this->CommandObject::GenerateHelpText(result);
-return result.Succeeded();
-  }
-
   if (m_subcommand_dict.empty()) {
 result.AppendErrorWithFormat("'%s' does not have any subcommands.\n",
  GetCommandName().str().c_str());

diff  --git 
a/lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py 
b/lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
index e8b14d1629e1b..70a8b44b8d13f 100644
--- a/lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
+++ b/lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
@@ -19,13 +19,3 @@ def test_ambiguous_subcommand(self):
 @no_debug_info_test
 def test_empty_subcommand(self):
 self.expect("platform \"\"", error=True, substrs=["Need to specify a 
non-empty subcommand."])
-
-@no_debug_info_test
-def test_help(self):
-#  help brings up help.
-self.expect("platform help",
-substrs=["Commands to manage and create platforms.",
- "Syntax: platform [",
- "The following subcommands are supported:",
- "connect",
- "Select the current platform"])



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

[Lldb-commits] [PATCH] D142067: Remove the "help" subcommand - its undocumented, undiscoverable and doesn't work....

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG887240faf769: Remove the undocumented `help` subcommand. 
(authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D142067?vs=490340&id=490622#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142067/new/

https://reviews.llvm.org/D142067

Files:
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py


Index: lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
===
--- lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
+++ lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
@@ -19,13 +19,3 @@
 @no_debug_info_test
 def test_empty_subcommand(self):
 self.expect("platform \"\"", error=True, substrs=["Need to specify a 
non-empty subcommand."])
-
-@no_debug_info_test
-def test_help(self):
-#  help brings up help.
-self.expect("platform help",
-substrs=["Commands to manage and create platforms.",
- "Syntax: platform [",
- "The following subcommands are supported:",
- "connect",
- "Select the current platform"])
Index: lldb/source/Commands/CommandObjectMultiword.cpp
===
--- lldb/source/Commands/CommandObjectMultiword.cpp
+++ lldb/source/Commands/CommandObjectMultiword.cpp
@@ -174,11 +174,6 @@
 return result.Succeeded();
   }
 
-  if (sub_command.equals_insensitive("help")) {
-this->CommandObject::GenerateHelpText(result);
-return result.Succeeded();
-  }
-
   if (m_subcommand_dict.empty()) {
 result.AppendErrorWithFormat("'%s' does not have any subcommands.\n",
  GetCommandName().str().c_str());


Index: lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
===
--- lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
+++ lldb/test/API/functionalities/multiword-commands/TestMultiWordCommands.py
@@ -19,13 +19,3 @@
 @no_debug_info_test
 def test_empty_subcommand(self):
 self.expect("platform \"\"", error=True, substrs=["Need to specify a non-empty subcommand."])
-
-@no_debug_info_test
-def test_help(self):
-#  help brings up help.
-self.expect("platform help",
-substrs=["Commands to manage and create platforms.",
- "Syntax: platform [",
- "The following subcommands are supported:",
- "connect",
- "Select the current platform"])
Index: lldb/source/Commands/CommandObjectMultiword.cpp
===
--- lldb/source/Commands/CommandObjectMultiword.cpp
+++ lldb/source/Commands/CommandObjectMultiword.cpp
@@ -174,11 +174,6 @@
 return result.Succeeded();
   }
 
-  if (sub_command.equals_insensitive("help")) {
-this->CommandObject::GenerateHelpText(result);
-return result.Succeeded();
-  }
-
   if (m_subcommand_dict.empty()) {
 result.AppendErrorWithFormat("'%s' does not have any subcommands.\n",
  GetCommandName().str().c_str());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142141: [lldb][test] Skip TestRerunAndExprDylib on Ubuntu 18.04

[Michael Buch via Phabricator via lldb-commits]

Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

seems reasonable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142141/new/

https://reviews.llvm.org/D142141

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

[Lldb-commits] [PATCH] D141637: [lldb-vscode] Fix an issue where lldb-vscode tries to display a source file for generated code

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Change looks good. It would be nice to test this if possible.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141637/new/

https://reviews.llvm.org/D141637

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

[Lldb-commits] [PATCH] D142052: [lldb] Implement SymbolFile::CopyType

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We don't allow types to be copied into another SymbolFile. There is a strict 
rule in LLDB that SymbolFile objects only create types from their own data. 
There are many reasons we don't want this:

- Type objects have a  m_symbol_file which points to the SymbolFile that 
created it. This pointer can and will go bad if the original symbol file is 
freed.
- Type objects have a symbol context member variable ( "SymbolContextScope 
*m_context;"), same issue as above
- Type objects might have a valid encoding type member variable ("Type 
*m_encoding_type;") that is non NULL that points to a type that is owned by the 
original symbol file
- Type objects might have a encoding User ID and encoding type that point to 
the original symbol file (  lldb::user_id_t m_encoding_uid = LLDB_INVALID_UID; 
EncodingDataType m_encoding_uid_type = eEncodingInvalid;) and the user_id_t 
only makes sense in the original encoding type.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142052/new/

https://reviews.llvm.org/D142052

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

[Lldb-commits] [PATCH] D142052: [lldb] Implement SymbolFile::CopyType

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

@clayborg the intended usage here is to create a copy of the type in the same 
symbol file. I could add a sanity check that makes sure we're not creating a 
copy of something that isn't in the symbol file's type list in debug mode. 
Would that be enough?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142052/new/

https://reviews.llvm.org/D142052

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

[Lldb-commits] [PATCH] D142052: [lldb] Implement SymbolFile::CopyType

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D142052#4067177 , @augusto2112 
wrote:

> @clayborg the intended usage here is to create a copy of the type in the same 
> symbol file. I could add a sanity check that makes sure we're not creating a 
> copy of something that isn't in the symbol file's type list in debug mode. 
> Would that be enough?

When would you need to copy a type that is already in SymbolFile::m_type_list?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142052/new/

https://reviews.llvm.org/D142052

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

[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: 
lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py:73
+num_a = array_find(locals, lambda x: x['name'] == 'num_a')
+self.assertIsNotNone(num_a)
+

might be better as suggested?



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:15
+Watchpoint::Watchpoint(lldb::SBValue value, bool enabled, WatchpointType type)
+: value(value), enabled(enabled), type(type), watchpoint_active(false) {
+  this->sync();

We should either rename the argument variables so they don't match the member 
variable names or if we add the "m_" prefix this all just will work.



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:16
+: value(value), enabled(enabled), type(type), watchpoint_active(false) {
+  this->sync();
+}

remove "this->"



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:20-21
+void Watchpoint::set_enabled(bool enabled) {
+  this->enabled = enabled;
+  this->sync();
+}





Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:27
+void Watchpoint::sync() {
+  if (this->watchpoint_active) {
+g_vsc.target.DeleteWatchpoint(this->watchpoint.GetID());

Remove all "this->" from this and all changes in this patch.



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:36-41
+  this->value.Watch(true,
+this->type == WatchpointType::Read ||
+this->type == WatchpointType::ReadWrite,
+this->type == WatchpointType::Write ||
+this->type == WatchpointType::ReadWrite,
+this->error);

If we use two bools instead of using WatchPointType enum, this is much easier 
to code.



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:46-48
+std::string message = "Failed setting watchpoint: ";
+message.append(this->error.GetCString());
+g_vsc.SendOutput(OutputType::Console, message);

We should probably just leave the error inside of this object and then test if 
the watchpoint was set correctly and report it back in the response to 
"setDataBreakpoints" for each breakpoint instead of printing to console?



Comment at: lldb/tools/lldb-vscode/Watchpoint.h:20
+
+enum class WatchpointType { Read, Write, ReadWrite };
+

We could get rid of this enum and just store two bools and avoid the ReadWrite 
case. See comment below where WatchpointType is used for member variable



Comment at: lldb/tools/lldb-vscode/Watchpoint.h:29-34
+  lldb::SBValue value;
+  bool enabled;
+  WatchpointType type;
+  bool watchpoint_active;
+  lldb::SBWatchpoint watchpoint;
+  lldb::SBError error;

For classes we prefer things to have a "m_" prefix for all member variables. I 
know other parts of this code for breakpoints use "struct" instead. It helps 
keep code readable and avoids having variables and member variables with the 
same name. See next inline comment.



Comment at: lldb/tools/lldb-vscode/Watchpoint.h:31
+  bool enabled;
+  WatchpointType type;
+  bool watchpoint_active;

Maybe just use two bools and get rid of WatchPointType enum since we really 
want a bitfield for read and write?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2125-2138
+static WatchpointType get_watchpoint_type_from_request(std::string type) {
+  if (type == "write") {
+return WatchpointType::Write;
+  } else if (type == "read") {
+return WatchpointType::Read;
+  } else if (type == "readWrite") {
+return WatchpointType::ReadWrite;

We shouldn't be logging to the debug console if the setDataBreakpoint packet is 
not valid, but we should return an error for that packet. Might be better to 
have this return a bool to indicate success or failure and have the prototype 
be:
```
static bool get_watchpoint_type(const std::string &type, bool &read, bool 
&write);
```
If this caller gets false returned from this, then we return an error to the 
packet with an appropriate error message.




Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2188
+
+static std::optional set_data_breakpoint(const llvm::json::Object 
*breakpoint) {
+  std::string breakpoint_id = GetString(breakpoint, "id").str();

This function should probably be able to return an error for each watchpoint if 
anything fails so this can be reported back as a description or error response 
in the "setDataBreakpoints" response? We shouldn't be printing to the console 
for errors in the "breakpoint" object arguments or if the watchpoint actually 
fails to resolve (was in a register). An error message should be r

[Lldb-commits] [PATCH] D141629: Run address expression argument values through ABI::FixCodeAddress to strip TBI/pointer auth bytes on AArch64

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Someday I'd like to have a decorator that's a bit smarter and allows us to run 
these tests if the test compiler has ptrauth/PAC support and the host can run 
it. At least then these tests would automatically run for us (and maybe you) 
downstream. But with the current state of things this is as good as it gets.




Comment at: lldb/source/Interpreter/OptionArgParser.cpp:161-162
   error_ptr->Clear();
+Process *process = exe_ctx->GetProcessPtr();
+if (process)
+  if (ABISP abi_sp = process->GetABI())




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141629/new/

https://reviews.llvm.org/D141629

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

[Lldb-commits] [PATCH] D142052: [lldb] Implement SymbolFile::CopyType

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

@clayborg we have one instance downstream where we need to keep two types 
around. One for the original type and one for a slightly modified one (here 
).
 This patch is just a way to make sure any new type we copy is inserted in the 
type list as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142052/new/

https://reviews.llvm.org/D142052

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

[Lldb-commits] [lldb] 6083410 - [lldb] Re-enable xmm/ymm/zmm tests with the system debugserver

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-01-19T17:07:55-08:00
New Revision: 60834105d85cba27e1e1b2b4ecf4cd658019d867

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

LOG: [lldb] Re-enable xmm/ymm/zmm tests with the system debugserver

Re-enable the xmm/ymm/zmm tests now that the system debugserver used by
our CI is capable or writing xmm/ymm/zmm registers.

Added: 


Modified: 
lldb/test/Shell/Register/x86-64-ymm-write.test
lldb/test/Shell/Register/x86-64-zmm-write.test

Removed: 




diff  --git a/lldb/test/Shell/Register/x86-64-ymm-write.test 
b/lldb/test/Shell/Register/x86-64-ymm-write.test
index 9d9f3451b41a8..9df3ce73fe422 100644
--- a/lldb/test/Shell/Register/x86-64-ymm-write.test
+++ b/lldb/test/Shell/Register/x86-64-ymm-write.test
@@ -1,9 +1,3 @@
-# xfail with system debugserver until the fix for
-# https://reviews.llvm.org/D123269 in
-# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
-# has made it into released tools.
-# XFAIL: system-debugserver
-
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64 && native-cpu-avx
 # RUN: %clangxx_host %p/Inputs/x86-ymm-write.cpp -o %t

diff  --git a/lldb/test/Shell/Register/x86-64-zmm-write.test 
b/lldb/test/Shell/Register/x86-64-zmm-write.test
index 6304205779234..dbff54d7bce21 100644
--- a/lldb/test/Shell/Register/x86-64-zmm-write.test
+++ b/lldb/test/Shell/Register/x86-64-zmm-write.test
@@ -1,9 +1,3 @@
-# xfail with system debugserver until the fix for
-# https://reviews.llvm.org/D123269 in
-# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
-# has made it into released tools.
-# XFAIL: system-debugserver
-
 # XFAIL: system-freebsd
 # XFAIL: system-linux
 # XFAIL: system-netbsd



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

[Lldb-commits] [PATCH] D142052: [lldb] Implement SymbolFile::CopyType

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Gotcha, makes sense now. Thanks for the extra info. We do need to make sure the 
m_symbol_file values match and return an empty TypeSP if they don't. Once that 
is done, this is good to go.




Comment at: lldb/include/lldb/Symbol/SymbolFile.h:527
+  lldb::TypeSP CopyType(const lldb::TypeSP &other_type) override {
+ lldb::TypeSP type_sp (new Type(*other_type));
+ m_type_list.Insert(type_sp);

We have to check if other_type has the same symbol file and return nothing if 
it doesn't match.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142052/new/

https://reviews.llvm.org/D142052

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