[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 172488.
kastiglione added a comment.

Added IOHandlerActivatedInteractively


https://reviews.llvm.org/D48752

Files:
  include/lldb/Core/IOHandler.h
  
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
  source/Commands/CommandObjectCommands.cpp
  source/Core/IOHandler.cpp


Index: source/Core/IOHandler.cpp
===
--- source/Core/IOHandler.cpp
+++ source/Core/IOHandler.cpp
@@ -333,6 +333,8 @@
 void IOHandlerEditline::Activate() {
   IOHandler::Activate();
   m_delegate.IOHandlerActivated(*this);
+  if (GetIsInteractive())
+m_delegate.IOHandlerActivatedInteractively(*this);
 }
 
 void IOHandlerEditline::Deactivate() {
Index: source/Commands/CommandObjectCommands.cpp
===
--- source/Commands/CommandObjectCommands.cpp
+++ source/Commands/CommandObjectCommands.cpp
@@ -980,10 +980,10 @@
   ~CommandObjectCommandsAddRegex() override = default;
 
 protected:
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivatedInteractively(IOHandler &io_handler) override {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
 if (output_sp) {
-  output_sp->PutCString("Enter one of more sed substitution commands in "
+  output_sp->PutCString("Enter one or more sed substitution commands in "
 "the form: 's///'.\nTerminate the "
 "substitution list with an empty line.\n");
   output_sp->Flush();
Index: 
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
===
--- 
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
+++ 
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
@@ -24,7 +24,7 @@
 """Test a simple scenario of 'command regex' invocation and subsequent 
use."""
 import pexpect
 prompt = "(lldb) "
-regex_prompt = "Enter one of more sed substitution commands in the 
form: 's///'.\r\nTerminate the substitution list with an empty 
line.\r\n"
+regex_prompt = "Enter one or more sed substitution commands in the 
form: 's///'.\r\nTerminate the substitution list with an empty 
line.\r\n"
 regex_prompt1 = "\r\n"
 
 child = pexpect.spawn('%s %s' %
Index: include/lldb/Core/IOHandler.h
===
--- include/lldb/Core/IOHandler.h
+++ include/lldb/Core/IOHandler.h
@@ -200,6 +200,8 @@
 
   virtual void IOHandlerActivated(IOHandler &io_handler) {}
 
+  virtual void IOHandlerActivatedInteractively(IOHandler &io_handler) {}
+
   virtual void IOHandlerDeactivated(IOHandler &io_handler) {}
 
   virtual int IOHandlerComplete(IOHandler &io_handler, const char 
*current_line,


Index: source/Core/IOHandler.cpp
===
--- source/Core/IOHandler.cpp
+++ source/Core/IOHandler.cpp
@@ -333,6 +333,8 @@
 void IOHandlerEditline::Activate() {
   IOHandler::Activate();
   m_delegate.IOHandlerActivated(*this);
+  if (GetIsInteractive())
+m_delegate.IOHandlerActivatedInteractively(*this);
 }
 
 void IOHandlerEditline::Deactivate() {
Index: source/Commands/CommandObjectCommands.cpp
===
--- source/Commands/CommandObjectCommands.cpp
+++ source/Commands/CommandObjectCommands.cpp
@@ -980,10 +980,10 @@
   ~CommandObjectCommandsAddRegex() override = default;
 
 protected:
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivatedInteractively(IOHandler &io_handler) override {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
 if (output_sp) {
-  output_sp->PutCString("Enter one of more sed substitution commands in "
+  output_sp->PutCString("Enter one or more sed substitution commands in "
 "the form: 's///'.\nTerminate the "
 "substitution list with an empty line.\n");
   output_sp->Flush();
Index: packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
===
--- packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
+++ packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
@@ -24,7 +24,7 @@
 """Test a simple scenario of 'command regex' invocation and subsequent use."""
 import pexpect
 prompt = "(lldb) "
-regex_prompt = "Enter one of more sed substitution commands in the form: 's///'.\r\nTerminate the substitution list with an empty line.\r\n"
+regex_prompt = "Enter one or more sed substitution commands in the form: 's///'.\r\nTerminate the substitution list with an empty line.\r\n

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

There are other commands that print instructions in the same way that `command 
regex` does, should they be updated in this change too?


https://reviews.llvm.org/D48752



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

[Lldb-commits] [PATCH] D49776: Update framework-header-fix to force system sed

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346099: Update framework-header-fix to force system sed 
(authored by kastiglione, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49776?vs=157194&id=172523#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49776

Files:
  lldb/trunk/scripts/framework-header-fix.sh


Index: lldb/trunk/scripts/framework-header-fix.sh
===
--- lldb/trunk/scripts/framework-header-fix.sh
+++ lldb/trunk/scripts/framework-header-fix.sh
@@ -1,14 +1,17 @@
 #!/bin/sh
 # Usage: framework-header-fix.sh  
+
+set -e
+
 for file in `find $1 -name "*.h"`
 do
-  sed -i.bak 's/\(#include\)[ ]*"lldb\/\(API\/\)\{0,1\}\(.*\)"/\1 
/1' "$file"
-  sed -i.bak 's|/1' "$file"
+  /usr/bin/sed -i.bak 's|Index: lldb/trunk/scripts/framework-header-fix.sh
===
--- lldb/trunk/scripts/framework-header-fix.sh
+++ lldb/trunk/scripts/framework-header-fix.sh
@@ -1,14 +1,17 @@
 #!/bin/sh
 # Usage: framework-header-fix.sh  
+
+set -e
+
 for file in `find $1 -name "*.h"`
 do
-  sed -i.bak 's/\(#include\)[ ]*"lldb\/\(API\/\)\{0,1\}\(.*\)"/\1 /1' "$file"
-  sed -i.bak 's|/1' "$file"
+  /usr/bin/sed -i.bak 's|___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49776: Update framework-header-fix to force system sed

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@Keith I screwed up and committed without amending the commit message to 
attribute you as the author :(


Repository:
  rL LLVM

https://reviews.llvm.org/D49776



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

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@jingham If you're still willing to review this change to `command regex`, it 
is updated per our previous discussion, thanks.


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

https://reviews.llvm.org/D48752



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

[Lldb-commits] [PATCH] D48620: Fix a single typo in SBSymbolContext

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added a reviewer: xiaobai.

Fix a "Manay" in SBSymbolContext.i


https://reviews.llvm.org/D48620

Files:
  scripts/interface/SBSymbolContext.i


Index: scripts/interface/SBSymbolContext.i
===
--- scripts/interface/SBSymbolContext.i
+++ scripts/interface/SBSymbolContext.i
@@ -12,7 +12,7 @@
 %feature("docstring",
 "A context object that provides access to core debugger entities.
 
-Manay debugger functions require a context when doing lookups. This class
+Many debugger functions require a context when doing lookups. This class
 provides a common structure that can be used as the result of a query that
 can contain a single result.
 


Index: scripts/interface/SBSymbolContext.i
===
--- scripts/interface/SBSymbolContext.i
+++ scripts/interface/SBSymbolContext.i
@@ -12,7 +12,7 @@
 %feature("docstring",
 "A context object that provides access to core debugger entities.
 
-Manay debugger functions require a context when doing lookups. This class
+Many debugger functions require a context when doing lookups. This class
 provides a common structure that can be used as the result of a query that
 can contain a single result.
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48620: Fix a single typo in SBSymbolContext

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL335709: Fix a single typo in SBSymbolContext (authored by 
kastiglione, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48620

Files:
  lldb/trunk/scripts/interface/SBSymbolContext.i


Index: lldb/trunk/scripts/interface/SBSymbolContext.i
===
--- lldb/trunk/scripts/interface/SBSymbolContext.i
+++ lldb/trunk/scripts/interface/SBSymbolContext.i
@@ -12,7 +12,7 @@
 %feature("docstring",
 "A context object that provides access to core debugger entities.
 
-Manay debugger functions require a context when doing lookups. This class
+Many debugger functions require a context when doing lookups. This class
 provides a common structure that can be used as the result of a query that
 can contain a single result.
 


Index: lldb/trunk/scripts/interface/SBSymbolContext.i
===
--- lldb/trunk/scripts/interface/SBSymbolContext.i
+++ lldb/trunk/scripts/interface/SBSymbolContext.i
@@ -12,7 +12,7 @@
 %feature("docstring",
 "A context object that provides access to core debugger entities.
 
-Manay debugger functions require a context when doing lookups. This class
+Many debugger functions require a context when doing lookups. This class
 provides a common structure that can be used as the result of a query that
 can contain a single result.
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: clayborg, granata.enrico.

This change fixes one issue with `lldb.command`, and also reduces the 
implementation.

The fix: a command function's docstring was not shown when running `help 
`. This is because the docstring attached the source function is 
not propagated to the decorated function (`f.__call__`). By returning the 
original function, the docstring will be properly displayed by `help`.

Additionally, the implementation was updated to:

- Remove inner class
- Remove use of `inspect` module
- Remove `*args` and `**kwargs`


https://reviews.llvm.org/D48658

Files:
  scripts/Python/python-extensions.swig


Index: scripts/Python/python-extensions.swig
===
--- scripts/Python/python-extensions.swig
+++ scripts/Python/python-extensions.swig
@@ -839,29 +839,18 @@
 
 %pythoncode %{
 
-def command(*args, **kwargs):
+def command(command_name, doc=None):
 import lldb
-import inspect
 """A decorator function that registers an LLDB command line
 command that is bound to the function it is attached to."""
-class obj(object):
-"""The object that tracks adding the command to LLDB one time and 
handles
-calling the function on subsequent calls."""
-def __init__(self, function, command_name, doc = None):
-if doc:
-function.__doc__ = doc
-command = "command script add -f %s.%s %s" % (function.__module__, 
function.__name__, command_name)
-lldb.debugger.HandleCommand(command)
-self.function = function
-def __call__(self, debugger, command, exe_ctx, result, dict):
-if len(inspect.getargspec(self.function).args) == 5:
-self.function(debugger, command, exe_ctx, result, dict)
-else:
-self.function(debugger, command, result, dict)
 def callable(function):
 """Creates a callable object that gets used."""
-f = obj(function, *args, **kwargs)
-return f.__call__
+command = "command script add -f %s.%s %s" % (function.__module__, 
function.__name__, command_name)
+lldb.debugger.HandleCommand(command)
+if doc:
+function.__doc__ = doc
+return function
+
 return callable
 
 class declaration(object):


Index: scripts/Python/python-extensions.swig
===
--- scripts/Python/python-extensions.swig
+++ scripts/Python/python-extensions.swig
@@ -839,29 +839,18 @@
 
 %pythoncode %{
 
-def command(*args, **kwargs):
+def command(command_name, doc=None):
 import lldb
-import inspect
 """A decorator function that registers an LLDB command line
 command that is bound to the function it is attached to."""
-class obj(object):
-"""The object that tracks adding the command to LLDB one time and handles
-calling the function on subsequent calls."""
-def __init__(self, function, command_name, doc = None):
-if doc:
-function.__doc__ = doc
-command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name)
-lldb.debugger.HandleCommand(command)
-self.function = function
-def __call__(self, debugger, command, exe_ctx, result, dict):
-if len(inspect.getargspec(self.function).args) == 5:
-self.function(debugger, command, exe_ctx, result, dict)
-else:
-self.function(debugger, command, result, dict)
 def callable(function):
 """Creates a callable object that gets used."""
-f = obj(function, *args, **kwargs)
-return f.__call__
+command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name)
+lldb.debugger.HandleCommand(command)
+if doc:
+function.__doc__ = doc
+return function
+
 return callable
 
 class declaration(object):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 153114.
kastiglione added a comment.

Update a comment


https://reviews.llvm.org/D48658

Files:
  scripts/Python/python-extensions.swig


Index: scripts/Python/python-extensions.swig
===
--- scripts/Python/python-extensions.swig
+++ scripts/Python/python-extensions.swig
@@ -839,29 +839,18 @@
 
 %pythoncode %{
 
-def command(*args, **kwargs):
+def command(command_name, doc=None):
 import lldb
-import inspect
 """A decorator function that registers an LLDB command line
 command that is bound to the function it is attached to."""
-class obj(object):
-"""The object that tracks adding the command to LLDB one time and 
handles
-calling the function on subsequent calls."""
-def __init__(self, function, command_name, doc = None):
-if doc:
-function.__doc__ = doc
-command = "command script add -f %s.%s %s" % (function.__module__, 
function.__name__, command_name)
-lldb.debugger.HandleCommand(command)
-self.function = function
-def __call__(self, debugger, command, exe_ctx, result, dict):
-if len(inspect.getargspec(self.function).args) == 5:
-self.function(debugger, command, exe_ctx, result, dict)
-else:
-self.function(debugger, command, result, dict)
 def callable(function):
-"""Creates a callable object that gets used."""
-f = obj(function, *args, **kwargs)
-return f.__call__
+"""Registers an lldb command for the decorated function."""
+command = "command script add -f %s.%s %s" % (function.__module__, 
function.__name__, command_name)
+lldb.debugger.HandleCommand(command)
+if doc:
+function.__doc__ = doc
+return function
+
 return callable
 
 class declaration(object):


Index: scripts/Python/python-extensions.swig
===
--- scripts/Python/python-extensions.swig
+++ scripts/Python/python-extensions.swig
@@ -839,29 +839,18 @@
 
 %pythoncode %{
 
-def command(*args, **kwargs):
+def command(command_name, doc=None):
 import lldb
-import inspect
 """A decorator function that registers an LLDB command line
 command that is bound to the function it is attached to."""
-class obj(object):
-"""The object that tracks adding the command to LLDB one time and handles
-calling the function on subsequent calls."""
-def __init__(self, function, command_name, doc = None):
-if doc:
-function.__doc__ = doc
-command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name)
-lldb.debugger.HandleCommand(command)
-self.function = function
-def __call__(self, debugger, command, exe_ctx, result, dict):
-if len(inspect.getargspec(self.function).args) == 5:
-self.function(debugger, command, exe_ctx, result, dict)
-else:
-self.function(debugger, command, result, dict)
 def callable(function):
-"""Creates a callable object that gets used."""
-f = obj(function, *args, **kwargs)
-return f.__call__
+"""Registers an lldb command for the decorated function."""
+command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name)
+lldb.debugger.HandleCommand(command)
+if doc:
+function.__doc__ = doc
+return function
+
 return callable
 
 class declaration(object):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a subscriber: granata.enrico.
kastiglione added a comment.

Thanks @granata.enrico, sorry for the disruption, I noticed previous comments 
after I had submitted this.


https://reviews.llvm.org/D48658



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

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@clayborg I'll dig into how `python-extensions.swig` gets tested, and add 
something accordingly.

I have only briefly looked at `cmdtemplate.py`. I can look into adding similar 
registration behavior to it. For what it's worth, I prefer functions to classes 
for defining commands.


https://reviews.llvm.org/D48658



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

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: clayborg, jingham.

Within .lldbinit, regex commands can be structured as a list of substitutions 
over
multiple lines. It's possible that this is uninentional, but it works and has
benefits.

For example:

  command regex 
  s/pat1/repl1/
  s/pat2/repl2/
  ...


I use this form of `command regex` in my `~/.lldbinit`, because it makes it
clearer to write and read compared to a single line definition, because
multiline substitutions don't need to be quoted, and are broken up one per line.

However, multiline definitions result in usage instructions being printed for
each use. The result is that every time I run `lldb`, I get a dozen or more
lines of noise. With this change, the instructions are only printed when
`command regex` is invoked interactively, or from a terminal, neither of which
are true when lldb is sourcing `~/.lldbinit`.


https://reviews.llvm.org/D48752

Files:
  
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
  source/Commands/CommandObjectCommands.cpp


Index: source/Commands/CommandObjectCommands.cpp
===
--- source/Commands/CommandObjectCommands.cpp
+++ source/Commands/CommandObjectCommands.cpp
@@ -1009,9 +1009,12 @@
 
 protected:
   void IOHandlerActivated(IOHandler &io_handler) override {
+if (!io_handler.GetIsInteractive() && !io_handler.GetIsRealTerminal())
+  // Don't print instructions during batch execution, such as .lldbinit.
+  return;
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
 if (output_sp) {
-  output_sp->PutCString("Enter one of more sed substitution commands in "
+  output_sp->PutCString("Enter one or more sed substitution commands in "
 "the form: 's///'.\nTerminate the "
 "substitution list with an empty line.\n");
   output_sp->Flush();
Index: 
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
===
--- 
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
+++ 
packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
@@ -24,7 +24,7 @@
 """Test a simple scenario of 'command regex' invocation and subsequent 
use."""
 import pexpect
 prompt = "(lldb) "
-regex_prompt = "Enter one of more sed substitution commands in the 
form: 's///'.\r\nTerminate the substitution list with an empty 
line.\r\n"
+regex_prompt = "Enter one or more sed substitution commands in the 
form: 's///'.\r\nTerminate the substitution list with an empty 
line.\r\n"
 regex_prompt1 = "\r\n"
 
 child = pexpect.spawn('%s %s' %


Index: source/Commands/CommandObjectCommands.cpp
===
--- source/Commands/CommandObjectCommands.cpp
+++ source/Commands/CommandObjectCommands.cpp
@@ -1009,9 +1009,12 @@
 
 protected:
   void IOHandlerActivated(IOHandler &io_handler) override {
+if (!io_handler.GetIsInteractive() && !io_handler.GetIsRealTerminal())
+  // Don't print instructions during batch execution, such as .lldbinit.
+  return;
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
 if (output_sp) {
-  output_sp->PutCString("Enter one of more sed substitution commands in "
+  output_sp->PutCString("Enter one or more sed substitution commands in "
 "the form: 's///'.\nTerminate the "
 "substitution list with an empty line.\n");
   output_sp->Flush();
Index: packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
===
--- packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
+++ packages/Python/lldbsuite/test/functionalities/command_regex/TestCommandRegex.py
@@ -24,7 +24,7 @@
 """Test a simple scenario of 'command regex' invocation and subsequent use."""
 import pexpect
 prompt = "(lldb) "
-regex_prompt = "Enter one of more sed substitution commands in the form: 's///'.\r\nTerminate the substitution list with an empty line.\r\n"
+regex_prompt = "Enter one or more sed substitution commands in the form: 's///'.\r\nTerminate the substitution list with an empty line.\r\n"
 regex_prompt1 = "\r\n"
 
 child = pexpect.spawn('%s %s' %
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Thanks @jingham.

> if the `IOHandlerDelegate` could say whether it wants to be notified for 
> `IOHandlerActivated` when non-interactive (like have a 
> "`ShouldNotifyWhenNonInteractive`" method), and then the `IOHandler` could 
> dispatch it or not based on the response to that method

So the "`ShouldNotifyWhenNonInteractive`" would only control whether 
`IOHandlerActivated` is called, not any of the other `IOHandlerDelegate` 
methods?

What do you think about adding a `IOHandlerActivatedInteractively`delegate 
method? This could be called in addition to `IOHandlerActivatedInteractively`, 
but only for interactive IO?


https://reviews.llvm.org/D48752



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

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 153654.
kastiglione added a comment.

Add tests and make command_name optional


https://reviews.llvm.org/D48658

Files:
  
packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
  packages/Python/lldbsuite/test/functionalities/command_script/decorated.py
  packages/Python/lldbsuite/test/functionalities/command_script/py_import
  scripts/Python/python-extensions.swig

Index: scripts/Python/python-extensions.swig
===
--- scripts/Python/python-extensions.swig
+++ scripts/Python/python-extensions.swig
@@ -839,29 +839,18 @@
 
 %pythoncode %{
 
-def command(*args, **kwargs):
+def command(command_name=None, doc=None):
 import lldb
-import inspect
 """A decorator function that registers an LLDB command line
 command that is bound to the function it is attached to."""
-class obj(object):
-"""The object that tracks adding the command to LLDB one time and handles
-calling the function on subsequent calls."""
-def __init__(self, function, command_name, doc = None):
-if doc:
-function.__doc__ = doc
-command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name)
-lldb.debugger.HandleCommand(command)
-self.function = function
-def __call__(self, debugger, command, exe_ctx, result, dict):
-if len(inspect.getargspec(self.function).args) == 5:
-self.function(debugger, command, exe_ctx, result, dict)
-else:
-self.function(debugger, command, result, dict)
 def callable(function):
-"""Creates a callable object that gets used."""
-f = obj(function, *args, **kwargs)
-return f.__call__
+"""Registers an lldb command for the decorated function."""
+command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name or function.__name__)
+lldb.debugger.HandleCommand(command)
+if doc:
+function.__doc__ = doc
+return function
+
 return callable
 
 class declaration(object):
Index: packages/Python/lldbsuite/test/functionalities/command_script/py_import
===
--- packages/Python/lldbsuite/test/functionalities/command_script/py_import
+++ packages/Python/lldbsuite/test/functionalities/command_script/py_import
@@ -10,3 +10,4 @@
 command script add tell_async --function welcome.check_for_synchro --synchronicity async
 command script add tell_curr --function welcome.check_for_synchro --synchronicity curr
 command script add takes_exe_ctx --function welcome.takes_exe_ctx
+command script import decorated.py
Index: packages/Python/lldbsuite/test/functionalities/command_script/decorated.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/command_script/decorated.py
@@ -0,0 +1,35 @@
+from __future__ import print_function
+
+import lldb
+
+
+@lldb.command()
+def decorated1(debugger, args, exe_ctx, result, dict):
+"""
+Python command defined by @lldb.command
+"""
+print("hello from decorated1", file=result)
+
+
+@lldb.command(doc="Python command defined by @lldb.command")
+def decorated2(debugger, args, exe_ctx, result, dict):
+"""
+This docstring is overridden.
+"""
+print("hello from decorated2", file=result)
+
+
+@lldb.command()
+def decorated3(debugger, args, result, dict):
+"""
+Python command defined by @lldb.command
+"""
+print("hello from decorated3", file=result)
+
+
+@lldb.command("decorated4")
+def _decorated4(debugger, args, exe_ctx, result, dict):
+"""
+Python command defined by @lldb.command
+"""
+print("hello from decorated4", file=result)
Index: packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
===
--- packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
+++ packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
@@ -49,6 +49,7 @@
 self.runCmd('command script delete tell_curr', check=False)
 self.runCmd('command script delete bug11569', check=False)
 self.runCmd('command script delete takes_exe_ctx', check=False)
+self.runCmd('command script delete decorated', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -67,13 +68,19 @@
 substrs=['Just a docstring for welcome_impl',
  'A command that says hello to LLDB users'])
 
+decorated_commands = ["decorated" + str(n) for n in range(1, 5)]
+for name in decorated_commands:
+self

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336287: Fix and simplify lldb.command decorator (authored by 
kastiglione, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48658

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/decorated.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/py_import
  lldb/trunk/scripts/Python/python-extensions.swig

Index: lldb/trunk/scripts/Python/python-extensions.swig
===
--- lldb/trunk/scripts/Python/python-extensions.swig
+++ lldb/trunk/scripts/Python/python-extensions.swig
@@ -839,29 +839,18 @@
 
 %pythoncode %{
 
-def command(*args, **kwargs):
+def command(command_name=None, doc=None):
 import lldb
-import inspect
 """A decorator function that registers an LLDB command line
 command that is bound to the function it is attached to."""
-class obj(object):
-"""The object that tracks adding the command to LLDB one time and handles
-calling the function on subsequent calls."""
-def __init__(self, function, command_name, doc = None):
-if doc:
-function.__doc__ = doc
-command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name)
-lldb.debugger.HandleCommand(command)
-self.function = function
-def __call__(self, debugger, command, exe_ctx, result, dict):
-if len(inspect.getargspec(self.function).args) == 5:
-self.function(debugger, command, exe_ctx, result, dict)
-else:
-self.function(debugger, command, result, dict)
 def callable(function):
-"""Creates a callable object that gets used."""
-f = obj(function, *args, **kwargs)
-return f.__call__
+"""Registers an lldb command for the decorated function."""
+command = "command script add -f %s.%s %s" % (function.__module__, function.__name__, command_name or function.__name__)
+lldb.debugger.HandleCommand(command)
+if doc:
+function.__doc__ = doc
+return function
+
 return callable
 
 class declaration(object):
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/TestCommandScript.py
@@ -49,6 +49,7 @@
 self.runCmd('command script delete tell_curr', check=False)
 self.runCmd('command script delete bug11569', check=False)
 self.runCmd('command script delete takes_exe_ctx', check=False)
+self.runCmd('command script delete decorated', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -67,13 +68,19 @@
 substrs=['Just a docstring for welcome_impl',
  'A command that says hello to LLDB users'])
 
+decorated_commands = ["decorated" + str(n) for n in range(1, 5)]
+for name in decorated_commands:
+self.expect(name, substrs=["hello from " + name])
+self.expect("help " + name,
+substrs=["Python command defined by @lldb.command"])
+
 self.expect("help",
 substrs=['For more information run',
- 'welcome'])
+ 'welcome'] + decorated_commands)
 
 self.expect("help -a",
 substrs=['For more information run',
- 'welcome'])
+ 'welcome'] + decorated_commands)
 
 self.expect("help -u", matching=False,
 substrs=['For more information run'])
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/py_import
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/py_import
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/py_import
@@ -10,3 +10,4 @@
 command script add tell_async --function welcome.check_for_synchro --synchronicity async
 command script add tell_curr --function welcome.check_for_synchro --synchronicity curr
 command script add takes_exe_ctx --function welcome.takes_exe_ctx
+command script import decorated.py
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script/decorated.py
===
--- lldb/tru

[Lldb-commits] [PATCH] D49776: Update framework-header-fix to force system sed

[Dave Lee via Phabricator via lldb-commits]

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

LGTM


https://reviews.llvm.org/D49776



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

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 187135.
kastiglione added a comment.
Herald added a subscriber: jdoerfert.

Add interactive parameter


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

https://reviews.llvm.org/D48752

Files:
  include/lldb/Core/IOHandler.h
  include/lldb/Expression/REPL.h
  lit/Commands/command-regex-delete.test
  lit/Commands/command-regex-unalias.test
  source/Commands/CommandObjectBreakpointCommand.cpp
  source/Commands/CommandObjectCommands.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Commands/CommandObjectType.cpp
  source/Commands/CommandObjectWatchpointCommand.cpp
  source/Core/IOHandler.cpp
  source/Expression/REPL.cpp
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -459,7 +459,7 @@
   //--
   // IOHandlerDelegate
   //--
-  void IOHandlerActivated(IOHandler &io_handler) override;
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override;
 
   void IOHandlerInputComplete(IOHandler &io_handler,
   std::string &data) override;
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -430,7 +430,7 @@
 
 uint32_t ScriptInterpreterPython::GetPluginVersion() { return 1; }
 
-void ScriptInterpreterPython::IOHandlerActivated(IOHandler &io_handler) {
+void ScriptInterpreterPython::IOHandlerActivated(IOHandler &io_handler, bool interactive) {
   const char *instructions = nullptr;
 
   switch (m_active_io_handler) {
@@ -451,7 +451,7 @@
 
   if (instructions) {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(instructions);
   output_sp->Flush();
 }
Index: source/Expression/REPL.cpp
===
--- source/Expression/REPL.cpp
+++ source/Expression/REPL.cpp
@@ -99,7 +99,7 @@
   return m_io_handler_sp;
 }
 
-void REPL::IOHandlerActivated(IOHandler &io_handler) {
+void REPL::IOHandlerActivated(IOHandler &io_handler, bool interactive) {
   lldb::ProcessSP process_sp = m_target.GetProcessSP();
   if (process_sp && process_sp->IsAlive())
 return;
Index: source/Core/IOHandler.cpp
===
--- source/Core/IOHandler.cpp
+++ source/Core/IOHandler.cpp
@@ -327,7 +327,7 @@
 
 void IOHandlerEditline::Activate() {
   IOHandler::Activate();
-  m_delegate.IOHandlerActivated(*this);
+  m_delegate.IOHandlerActivated(*this, GetIsInteractive());
 }
 
 void IOHandlerEditline::Deactivate() {
Index: source/Commands/CommandObjectWatchpointCommand.cpp
===
--- source/Commands/CommandObjectWatchpointCommand.cpp
+++ source/Commands/CommandObjectWatchpointCommand.cpp
@@ -207,9 +207,9 @@
 
   Options *GetOptions() override { return &m_options; }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(
   "Enter your debugger command(s).  Type 'DONE' to end.\n");
   output_sp->Flush();
Index: source/Commands/CommandObjectType.cpp
===
--- source/Commands/CommandObjectType.cpp
+++ source/Commands/CommandObjectType.cpp
@@ -160,7 +160,7 @@
 
   ~CommandObjectTypeSummaryAdd() override = default;
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
 static const char *g_summary_addreader_instructions =
 "Enter your Python command(s). Type 'DONE' to end.\n"
 "def function (valobj,internal_dict):\n"
@@ -169,7 +169,7 @@
 "internal_dict: an LLDB support object not to be used\"\"\"\n";
 
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(g_summary_addreader_instructions);
   output_sp->Flush();
 }
@@ -412,9 +412,9 @@
 }
   }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IO

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@clayborg this has been updated the approach you suggested, adding an 
`interactive` parameter.


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

https://reviews.llvm.org/D48752



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

[Lldb-commits] [PATCH] D59101: [SBAPI] Log from record macro

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

I'm getting build failures that appear to be caused by this patch. I'm 
compiling on macOS with Xcode 10.1. The failures are uses of 
`LLDB_RECORD_DUMMY()`, passing a callback function pointer as one of the args. 
This results in errors like:

  ../../tools/lldb/include/lldb/Utility/ReproducerInstrumentation.h:37:6: 
error: invalid operands to binary expression ('llvm::raw_string_ostream' and 
'bool (*)(void *, lldb::SBProcess &, lldb::SBThread &, 
lldb::SBBreakpointLocation &)')
ss << t;
~~ ^  ~

and

  ../../include/llvm/Support/raw_ostream.h:202:16: note: candidate function not 
viable: no known conversion from 'bool (*)(void *, lldb::SBProcess &, 
lldb::SBThread &, lldb::SBBreakpointLocation &)' to 'const void *' for 1st 
argument; take the address of the argument with &
raw_ostream &operator<<(const void *P);


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59101



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

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB355793: Quiet command regex instructions during batch 
execution (authored by kastiglione, committed by ).
Herald added a subscriber: abidh.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D48752?vs=187135&id=190030#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D48752

Files:
  include/lldb/Core/IOHandler.h
  include/lldb/Expression/REPL.h
  lit/Commands/command-regex-delete.test
  lit/Commands/command-regex-unalias.test
  source/Commands/CommandObjectBreakpointCommand.cpp
  source/Commands/CommandObjectCommands.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Commands/CommandObjectType.cpp
  source/Commands/CommandObjectWatchpointCommand.cpp
  source/Core/IOHandler.cpp
  source/Expression/REPL.cpp
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -459,7 +459,7 @@
   //--
   // IOHandlerDelegate
   //--
-  void IOHandlerActivated(IOHandler &io_handler) override;
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override;
 
   void IOHandlerInputComplete(IOHandler &io_handler,
   std::string &data) override;
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -442,7 +442,7 @@
 
 uint32_t ScriptInterpreterPython::GetPluginVersion() { return 1; }
 
-void ScriptInterpreterPython::IOHandlerActivated(IOHandler &io_handler) {
+void ScriptInterpreterPython::IOHandlerActivated(IOHandler &io_handler, bool interactive) {
   const char *instructions = nullptr;
 
   switch (m_active_io_handler) {
@@ -463,7 +463,7 @@
 
   if (instructions) {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(instructions);
   output_sp->Flush();
 }
Index: source/Commands/CommandObjectBreakpointCommand.cpp
===
--- source/Commands/CommandObjectBreakpointCommand.cpp
+++ source/Commands/CommandObjectBreakpointCommand.cpp
@@ -222,9 +222,9 @@
 
   Options *GetOptions() override { return &m_options; }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(g_reader_instructions);
   output_sp->Flush();
 }
Index: source/Commands/CommandObjectType.cpp
===
--- source/Commands/CommandObjectType.cpp
+++ source/Commands/CommandObjectType.cpp
@@ -160,7 +160,7 @@
 
   ~CommandObjectTypeSummaryAdd() override = default;
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
 static const char *g_summary_addreader_instructions =
 "Enter your Python command(s). Type 'DONE' to end.\n"
 "def function (valobj,internal_dict):\n"
@@ -169,7 +169,7 @@
 "internal_dict: an LLDB support object not to be used\"\"\"\n";
 
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(g_summary_addreader_instructions);
   output_sp->Flush();
 }
@@ -412,9 +412,9 @@
 }
   }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
 StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-if (output_sp) {
+if (output_sp && interactive) {
   output_sp->PutCString(g_synth_addreader_instructions);
   output_sp->Flush();
 }
Index: source/Commands/CommandObjectWatchpointCommand.cpp
===
--- source/Commands/CommandObjectWatchpointCommand.cpp
+++ source/Commands/CommandObjectWatchpointCommand.cpp
@@ -207,9 +207,9 @@
 
   Options *GetOptions() override { return &m_options; }
 
-  void IOHandlerActivated(IOHandler &io_handler) overri

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Thanks for reviewing @clayborg.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D48752



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

[Lldb-commits] [PATCH] D153489: [lldb] Print hint if object description is requested but not implemented

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/include/lldb/Target/Target.h:496
 
+  std::once_flag target_once_flag;
   // These two functions fill out the Broadcaster interface:

did you mean to use this, or should it be deleted?



Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:129-130
+  static llvm::SmallPtrSet targets_warned;
+  if (targets_warned.contains(target_ptr))
+return;
+

question: would people want to be warned once per target, or once per type?

If I do like the warning, I might want to know every time I `po` a type that 
doesn't support it.



Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:158-162
+StreamString temp_result_stream;
+valobj_sp->Dump(temp_result_stream, dump_options);
+llvm::StringRef output = temp_result_stream.GetString();
+maybe_add_hint(output);
+result.GetOutputStream() << output;

what do you think of passing in the `result`'s stream into `maybe_add_hint`? 
Perhaps I am overlooking something, but I wonder if it would simplify the code 
to reuse the one stream, instead of separating and then combining two streams.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153489

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

[Lldb-commits] [PATCH] D153489: [lldb] Print hint if object description is requested but not implemented

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:133-135
+  << "note: object description requested, but type doesn't implement "
+ "a custom object description. Consider using \"p\" instead of "
+ "\"po\"\n";

ok, I have a new suggestion. Since lldb will warn only once per target, and not 
per type, I think this note should be reworded to focus the guidance on the 
format of the output, not the type.

My concern is lldb emits basically "this type doesn't need a `po`", but then 
the diagnostic is printed for only one type, and never tells you about other 
types. How will people know that other types should use `p` not `po`?

If the message were on the format, and not the type, then I think it makes more 
sense as a once per target message.

A possible rewording:
> note: this `po` used the default object description, which shows none of the 
> objects properties. When you output like this, consider using `p` instead of 
> `po` when you see such output.



Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:158-162
+StreamString temp_result_stream;
+valobj_sp->Dump(temp_result_stream, dump_options);
+llvm::StringRef output = temp_result_stream.GetString();
+maybe_add_hint(output);
+result.GetOutputStream() << output;

augusto2112 wrote:
> kastiglione wrote:
> > what do you think of passing in the `result`'s stream into 
> > `maybe_add_hint`? Perhaps I am overlooking something, but I wonder if it 
> > would simplify the code to reuse the one stream, instead of separating and 
> > then combining two streams.
> I need the two streams to print it in the correct order (hint first, result 
> later)
do we have a precedent for before vs after? Maybe I need to see some examples, 
but I think it should be after. My logic is "here's the output you requested, 
and then here's a note about it". Also the note would be next to the next 
prompt, so maybe closer to the eyes? Just figured it was worth hashing out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153489

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

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 539268.
kastiglione added a comment.

Refactor for better API layering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

Files:
  lldb/include/lldb/Target/LanguageRuntime.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Core/ValueObjectDynamicValue.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -22,6 +22,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
@@ -1553,6 +1554,13 @@
   return runtime;
 }
 
+LanguageRuntime *Process::GetLanguageRuntime(ValueObject &in_value) {
+  auto language = in_value.GetObjectRuntimeLanguage();
+  if (auto *runtime = GetLanguageRuntime(language))
+return runtime->GetPreferredLanguageRuntime(in_value);
+  return nullptr;
+}
+
 bool Process::IsPossibleDynamicValue(ValueObject &in_value) {
   if (m_finalizing)
 return false;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -86,6 +86,9 @@
   return (m_is_cf == eLazyBoolYes);
 }
 
+/// Determine whether this class is implemented in Swift.
+virtual bool IsSwift() const { return false; }
+
 virtual bool IsValid() = 0;
 
 /// There are two routines in the ObjC runtime that tagged pointer clients
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -38,6 +38,8 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "apple-objc-v2"; }
 
+  LanguageRuntime *GetPreferredLanguageRuntime(ValueObject &in_value) override;
+
   static char ID;
 
   bool isA(const void *ClassID) const override {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -712,6 +712,20 @@
   RegisterObjCExceptionRecognizer(process);
 }
 
+LanguageRuntime *
+AppleObjCRuntimeV2::GetPreferredLanguageRuntime(ValueObject &in_value) {
+  if (auto process_sp = in_value.GetProcessSP()) {
+assert(process_sp.get() == m_process);
+if (auto descriptor_sp = GetNonKVOClassDescriptor(in_value))
+  if (descriptor_sp->IsSwift())
+// This class is implemented in Swift, making it the preferred runtime.
+if (auto *swift_runtime =
+process_sp->GetLanguageRuntime(lldb::eLanguageTypeSwift))
+  return swift_runtime;
+  }
+  return this;
+}
+
 bool AppleObjCRuntimeV2::GetDynamicTypeAndAddress(
 ValueObject &in_value, lldb::DynamicValueType use_dynamic,
 TypeAndOrName &class_type_or_name, Address &address,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -34,6 +34,8 @@
 return true; // any Objective-C v2 runtime class descriptor we vend is valid
   }
 
+  bool IsSwift() const override;
+
   // a custom descriptor is used for tagged pointers
   bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
 uint64_t *value_bits = nullptr,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@bulbazord  @augusto2112 hopefully this is a quick review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

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

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 539359.
kastiglione added a comment.

Change `bool IsSwift()` to `LanguageType GetImplementationLanguage()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

Files:
  lldb/include/lldb/Target/LanguageRuntime.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Core/ValueObjectDynamicValue.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -22,6 +22,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
@@ -1553,6 +1554,13 @@
   return runtime;
 }
 
+LanguageRuntime *Process::GetLanguageRuntime(ValueObject &in_value) {
+  auto language = in_value.GetObjectRuntimeLanguage();
+  if (auto *runtime = GetLanguageRuntime(language))
+return runtime->GetPreferredLanguageRuntime(in_value);
+  return nullptr;
+}
+
 bool Process::IsPossibleDynamicValue(ValueObject &in_value) {
   if (m_finalizing)
 return false;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -24,6 +24,7 @@
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/ThreadSafeDenseMap.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
@@ -86,6 +87,11 @@
   return (m_is_cf == eLazyBoolYes);
 }
 
+/// Determine whether this class is implemented in Swift.
+virtual lldb::LanguageType GetImplementationLanguage() const {
+  return lldb::eLanguageTypeObjC;
+}
+
 virtual bool IsValid() = 0;
 
 /// There are two routines in the ObjC runtime that tagged pointer clients
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -38,6 +38,8 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "apple-objc-v2"; }
 
+  LanguageRuntime *GetPreferredLanguageRuntime(ValueObject &in_value) override;
+
   static char ID;
 
   bool isA(const void *ClassID) const override {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -712,6 +712,20 @@
   RegisterObjCExceptionRecognizer(process);
 }
 
+LanguageRuntime *
+AppleObjCRuntimeV2::GetPreferredLanguageRuntime(ValueObject &in_value) {
+  if (auto process_sp = in_value.GetProcessSP()) {
+assert(process_sp.get() == m_process);
+if (auto descriptor_sp = GetNonKVOClassDescriptor(in_value)) {
+  LanguageType impl_lang = descriptor_sp->GetImplementationLanguage();
+  if (impl_lang != eLanguageTypeUnknown)
+if (auto *impl_runtime = process_sp->GetLanguageRuntime(impl_lang))
+  return impl_runtime;
+}
+  }
+  return this;
+}
+
 bool AppleObjCRuntimeV2::GetDynamicTypeAndAddress(
 ValueObject &in_value, lldb::DynamicValueType use_dynamic,
 TypeAndOrName &class_type_or_name, Address &address,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -12,6 +12,7 @@
 #include 
 
 #include "AppleObjCRuntimeV2.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
@@ -34,6 +35,8 @@
 return true; // any Objective-C v2 runtime class descriptor we vend is valid
   }
 
+  lldb::LanguageTyp

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 542169.
kastiglione added a comment.

Redo ValueObjectDynamicValue::UpdateValue changes based on upstream testing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

Files:
  lldb/include/lldb/Target/LanguageRuntime.h
  lldb/source/Core/ValueObjectDynamicValue.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -24,6 +24,7 @@
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/ThreadSafeDenseMap.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
@@ -86,6 +87,11 @@
   return (m_is_cf == eLazyBoolYes);
 }
 
+/// Determine whether this class is implemented in Swift.
+virtual lldb::LanguageType GetImplementationLanguage() const {
+  return lldb::eLanguageTypeObjC;
+}
+
 virtual bool IsValid() = 0;
 
 /// There are two routines in the ObjC runtime that tagged pointer clients
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -38,6 +38,8 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "apple-objc-v2"; }
 
+  LanguageRuntime *GetPreferredLanguageRuntime(ValueObject &in_value) override;
+
   static char ID;
 
   bool isA(const void *ClassID) const override {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -33,6 +33,7 @@
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
@@ -712,6 +713,19 @@
   RegisterObjCExceptionRecognizer(process);
 }
 
+LanguageRuntime *
+AppleObjCRuntimeV2::GetPreferredLanguageRuntime(ValueObject &in_value) {
+  if (auto process_sp = in_value.GetProcessSP()) {
+assert(process_sp.get() == m_process);
+if (auto descriptor_sp = GetNonKVOClassDescriptor(in_value)) {
+  LanguageType impl_lang = descriptor_sp->GetImplementationLanguage();
+  if (impl_lang != eLanguageTypeUnknown)
+return process_sp->GetLanguageRuntime(impl_lang);
+}
+  }
+  return nullptr;
+}
+
 bool AppleObjCRuntimeV2::GetDynamicTypeAndAddress(
 ValueObject &in_value, lldb::DynamicValueType use_dynamic,
 TypeAndOrName &class_type_or_name, Address &address,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -12,6 +12,7 @@
 #include 
 
 #include "AppleObjCRuntimeV2.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
@@ -34,6 +35,8 @@
 return true; // any Objective-C v2 runtime class descriptor we vend is valid
   }
 
+  lldb::LanguageType GetImplementationLanguage() const override;
+
   // a custom descriptor is used for tagged pointers
   bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
 uint64_t *value_bits = nullptr,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -10,8 +10,10 @@
 
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utilit

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

[Dave Lee via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe19339f5f8c1: [lldb] Identify Swift-implemented ObjC classes 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152837

Files:
  lldb/include/lldb/Target/LanguageRuntime.h
  lldb/source/Core/ValueObjectDynamicValue.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -24,6 +24,7 @@
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/ThreadSafeDenseMap.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
@@ -86,6 +87,11 @@
   return (m_is_cf == eLazyBoolYes);
 }
 
+/// Determine whether this class is implemented in Swift.
+virtual lldb::LanguageType GetImplementationLanguage() const {
+  return lldb::eLanguageTypeObjC;
+}
+
 virtual bool IsValid() = 0;
 
 /// There are two routines in the ObjC runtime that tagged pointer clients
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -38,6 +38,8 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "apple-objc-v2"; }
 
+  LanguageRuntime *GetPreferredLanguageRuntime(ValueObject &in_value) override;
+
   static char ID;
 
   bool isA(const void *ClassID) const override {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -33,6 +33,7 @@
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
@@ -753,6 +754,19 @@
   RegisterObjCExceptionRecognizer(process);
 }
 
+LanguageRuntime *
+AppleObjCRuntimeV2::GetPreferredLanguageRuntime(ValueObject &in_value) {
+  if (auto process_sp = in_value.GetProcessSP()) {
+assert(process_sp.get() == m_process);
+if (auto descriptor_sp = GetNonKVOClassDescriptor(in_value)) {
+  LanguageType impl_lang = descriptor_sp->GetImplementationLanguage();
+  if (impl_lang != eLanguageTypeUnknown)
+return process_sp->GetLanguageRuntime(impl_lang);
+}
+  }
+  return nullptr;
+}
+
 bool AppleObjCRuntimeV2::GetDynamicTypeAndAddress(
 ValueObject &in_value, lldb::DynamicValueType use_dynamic,
 TypeAndOrName &class_type_or_name, Address &address,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -12,6 +12,7 @@
 #include 
 
 #include "AppleObjCRuntimeV2.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
@@ -34,6 +35,8 @@
 return true; // any Objective-C v2 runtime class descriptor we vend is valid
   }
 
+  lldb::LanguageType GetImplementationLanguage() const override;
+
   // a custom descriptor is used for tagged pointers
   bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
 uint64_t *value_bits = nullptr,
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -10,8 +10,10 @@
 
 #include "lldb/Expression/FunctionCaller.h"
 #incl

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Interpreter/OptionValue.cpp:41
 llvm::StringRef value) {
+  std::lock_guard lock(m_mutex);
   Status error;

this one doesn't need a lock



Comment at: lldb/source/Interpreter/OptionValue.cpp:393
 
 std::optional OptionValue::GetFormatValue() const {
   if (const OptionValueFormat *option_value = GetAsFormat())

this one doesn't have a lock, should it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157041

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

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Interpreter/OptionValue.cpp:49
+  std::lock_guard lock(m_mutex);
   if (GetType() == OptionValue::eTypeBoolean)
 return static_cast(this);

do these GetAsX functions need to lock?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157041

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

[Lldb-commits] [PATCH] D158663: [lldb] Fix stdcpp type summary mistakenly marked as regex (NFC)

[Dave Lee via Phabricator via lldb-commits]

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

`std::basic_string` is not a regex, and treating it as such could 
unintentionally
cause a formatter to substring match a template type parameter, for example:
`std::vector>`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158663

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1009,7 +1009,7 @@
   cpp_category_sp->AddTypeSummary("std::string", eFormatterMatchExact,
   std_string_summary_sp);
   cpp_category_sp->AddTypeSummary("std::basic_string",
-  eFormatterMatchRegex, std_string_summary_sp);
+  eFormatterMatchExact, std_string_summary_sp);
   cpp_category_sp->AddTypeSummary(
   "std::basic_string,std::allocator >",
   eFormatterMatchExact, std_string_summary_sp);


Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1009,7 +1009,7 @@
   cpp_category_sp->AddTypeSummary("std::string", eFormatterMatchExact,
   std_string_summary_sp);
   cpp_category_sp->AddTypeSummary("std::basic_string",
-  eFormatterMatchRegex, std_string_summary_sp);
+  eFormatterMatchExact, std_string_summary_sp);
   cpp_category_sp->AddTypeSummary(
   "std::basic_string,std::allocator >",
   eFormatterMatchExact, std_string_summary_sp);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Thanks for doing this!

Question to all: Should the summary string include the unit? lldb doesn't 
always show the type, so it could help comprehension if the unit is included. 
For example `60s` instead of `60`.




Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:986-993
+  "^std::__[[:alnum:]]+::chrono::"
+  "(((nano)|(micro)|(milli)|())seconds)|"
+  "(minutes)|"
+  "(hours)|"
+  "(days)|"
+  "(weeks)|"
+  "(months)|"

there's some regex grouping that doesn't seem to be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159127

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

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Core/Section.cpp:153
+  case eSectionTypeSwiftModules:
+return "swift-modules";
   }

I wonder if this should be "swiftmodules". I have never seen it spelled with a 
hyphen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159142

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

[Lldb-commits] [PATCH] D158663: [lldb] Fix stdcpp type summary mistakenly marked as regex (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Merged in https://github.com/llvm/llvm-project/pull/66949


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158663

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

[Lldb-commits] [PATCH] D115182: [lldb] Remove some trivial scoped timers

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, aprantl.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While profiling lldb (from swift/llvm-project), these timers were noticed to be 
short lived and high firing, and so they add noise more than value.

The data points I recorded are:

`FindTypes_Impl`: 49,646 calls, 812ns avg, 40.33ms total
`AppendSymbolIndexesWithName`: 36,229 calls, 913ns avg, 33.09ms total
`FindAllSymbolsWithNameAndType`: 36,229 calls, 1.93µs avg, 70.05ms total
`FindSymbolsWithNameAndType`: 23,263 calls, 3.09µs avg, 71.88ms total


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115182

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D115182: [lldb] Remove some trivial scoped timers

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG13278efd0c95: [lldb] Remove some trivial scoped timers 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115182

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, aprantl.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

`memory region` displays the top-level section name, which for MachO is the 
segment name. The segment name alone is not much use, it's very course grained. 
This changes `memory region` to display the segment and section names for MachO 
images. For example: `__DATA,__const`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116419

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -62,3 +62,21 @@
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
 self.assertRegexpMatches(result.GetError(), "Usage: memory region 
ADDR")
+
+@skipUnlessDarwin
+def test_macho(self):
+self.build()
+
+# Set breakpoint in main and run
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+lldbutil.run_break_set_by_symbol(
+self, "main", sym_exact=True, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+
+interp.HandleCommand("memory region $pc", result)
+self.assertRegexpMatches(result.GetOutput(), r"\b__TEXT,__text\b")
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -34,6 +34,7 @@
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataBufferLLVM.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include 
 #include 
@@ -1673,13 +1674,26 @@
   lldb_private::Address addr;
   ConstString name = range_info.GetName();
   ConstString section_name;
-  if (process_sp->GetTarget().ResolveLoadAddress(load_addr, addr)) {
+  auto &target = process_sp->GetTarget();
+  if (target.ResolveLoadAddress(load_addr, addr)) {
 SectionSP section_sp(addr.GetSection());
 if (section_sp) {
-  // Got the top most section, not the deepest section
-  while (section_sp->GetParent())
-section_sp = section_sp->GetParent();
-  section_name = section_sp->GetName();
+  if (target.GetArchitecture().GetTriple().isOSBinFormatMachO()) {
+// Display the conventional Mach-O format: __SEG,__SECT
+if (auto segment_sp = section_sp->GetParent()) {
+  auto segment_name = segment_sp->GetName();
+  auto section_name_ = section_sp->GetName();
+  section_name = ConstString(
+  llvm::formatv("{0},{1}", segment_name, section_name_).str());
+}
+  }
+
+  if (section_name.IsEmpty()) {
+// Got the top most section, not the deepest section
+while (section_sp->GetParent())
+  section_sp = section_sp->GetParent();
+section_name = section_sp->GetName();
+  }
 }
   }
 


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -62,3 +62,21 @@
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
 self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+
+@skipUnlessDarwin
+def test_macho(self):
+self.build()
+
+# Set breakpoint in main and run
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+lldbutil.run_break_set_by_symbol(
+self, "main", sym_exact=True, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+
+interp.HandleCommand("memory region $pc", result)
+self.assertRegexpMatches(result.GetOutput(), r"\b__TEXT,__text\b")
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -34,6 +34,7 @@
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataBuf

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: aprantl, JDevlieghere, augusto2112.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Simplify getting the length of `NSPathStore2` strings.

`NSStringSummaryProvider` uses a single field from `NSPathStore2` instances, 
its first ivar: `_lengthAndRefCount`. This change uses 
`GetSyntheticChildAtOffset` to replace the use of `ProcessStructReader`, and 
removes the hard coded `CompilerType` definition of `NSPathStore2`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116461

Files:
  lldb/source/Plugins/Language/ObjC/NSString.cpp


Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -8,13 +8,11 @@
 
 #include "NSString.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
@@ -31,24 +29,6 @@
   return g_map;
 }
 
-static CompilerType GetNSPathStore2Type(Target &target) {
-  static ConstString g_type_name("__lldb_autogen_nspathstore2");
-
-  TypeSystemClang *ast_ctx = ScratchTypeSystemClang::GetForTarget(target);
-
-  if (!ast_ctx)
-return CompilerType();
-
-  CompilerType voidstar =
-  ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
-  CompilerType uint32 =
-  ast_ctx->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 32);
-
-  return ast_ctx->GetOrCreateStructForIdentifier(
-  g_type_name,
-  {{"isa", voidstar}, {"lengthAndRef", uint32}, {"buffer", voidstar}});
-}
-
 bool lldb_private::formatters::NSStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &summary_options) {
@@ -229,11 +209,16 @@
 return StringPrinter::ReadStringAndDumpToStream<
 StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
-ProcessStructReader reader(valobj.GetProcessSP().get(),
-   valobj.GetValueAsUnsigned(0),
-   GetNSPathStore2Type(*valobj.GetTargetSP()));
-explicit_length =
-reader.GetField(ConstString("lengthAndRef")) >> 20;
+// _lengthAndRefCount is the first ivar of NSPathStore2 (after the isa).
+uint64_t length_ivar_offset = 1 * ptr_size;
+CompilerType length_type = valobj.GetCompilerType().GetBasicTypeFromAST(
+lldb::eBasicTypeUnsignedInt);
+ValueObjectSP length_valobj_sp =
+valobj.GetSyntheticChildAtOffset(length_ivar_offset, length_type, 
true);
+if (!length_valobj_sp)
+  return false;
+// Get the length out of _lengthAndRefCount.
+explicit_length = length_valobj_sp->GetValueAsUnsigned(0) >> 20;
 lldb::addr_t location = valobj.GetValueAsUnsigned(0) + ptr_size + 4;
 
 options.SetLocation(location);


Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -8,13 +8,11 @@
 
 #include "NSString.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
@@ -31,24 +29,6 @@
   return g_map;
 }
 
-static CompilerType GetNSPathStore2Type(Target &target) {
-  static ConstString g_type_name("__lldb_autogen_nspathstore2");
-
-  TypeSystemClang *ast_ctx = ScratchTypeSystemClang::GetForTarget(target);
-
-  if (!ast_ctx)
-return CompilerType();
-
-  CompilerType voidstar =
-  ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
-  CompilerType uint32 =
-  ast_ctx->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 32);
-
-  return ast_ctx->GetOrCreateStructForIdentifier(
-  g_type_name,
-  {{"isa", voidstar}, {"lengthAndRef", uint32}, {"buffer", voidstar}});
-}
-
 bool lldb_private::formatters::NSStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &summary_options) {
@@ -229,11 +209,16 @@
 return StringPrinter::ReadStringAndDumpToStream<
 StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
-ProcessStructReader reader(valobj.GetProcessSP().get(),
- 

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: jingham, aprantl, augusto2112.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Fixes incomplete command names in `apropos` results.

The full command names given by `apropos` have come from command name string
literals given to `CommandObject` constructors. For most commands, this has
been accurate, but some commands have incorrect strings. This results in
`apropos` output that doesn't tell the user the full command name they might
want learn more about. These strings can be fixed.

There's a seperate issue that can't be fixed as easily: plugin commands. With
the way they're implemented, plugin commands have to exclude the root command
from their command name string. To illustrate, the `language objc` subcommand
has to set its command name string to "objc", which results in apropos printing
results as `objc ...` instead of `language objc ...`.

To fix both of these issues, this commit changes `FindCommandsForApropos` to
derive the fully qualified command name using the keys of subcommand maps.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116491

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,12 +2839,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2854,14 +2852,18 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcommandDictionary());
+  for (const auto &subcommand_name : subcommands_found) {
+auto qualified_name = (command_name + " " + subcommand_name).str();
+commands_found.AppendString(qualified_name);
+  }
 }
   }
 }
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  CommandObject::CommandMap &command_map);
+  const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,12 +2839,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool se

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 396955.
kastiglione added a comment.

Use std::move


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116491

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,12 +2839,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2854,14 +2852,18 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcommandDictionary());
+  for (const auto &subcommand_name : subcommands_found) {
+auto qualified_name = (command_name + " " + subcommand_name).str();
+commands_found.AppendString(std::move(qualified_name));
+  }
 }
   }
 }
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  CommandObject::CommandMap &command_map);
+  const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,12 +2839,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2854,14 +2852,18 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcom

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

I see now this isn't valid as written. The address range and rwx permissions 
printed by `memory region` are for the segment not the section. I think it 
would be helpful to print the section too, but as this change is written now, 
it could appear that all the info is for the section, when it's really for the 
segment. Maybe a better change would be to print a second line with an address 
range for the section, and the section's name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116419

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

[Lldb-commits] [PATCH] D116682: [lldb] Add StringList::AppendString(const Twine&) (NFC)

[Dave Lee via Phabricator via lldb-commits]

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

Add a convenience for appending constructed string values.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116682

Files:
  lldb/include/lldb/Utility/StringList.h
  lldb/source/Utility/StringList.cpp


Index: lldb/source/Utility/StringList.cpp
===
--- lldb/source/Utility/StringList.cpp
+++ lldb/source/Utility/StringList.cpp
@@ -55,6 +55,10 @@
   m_strings.push_back(str.str());
 }
 
+void StringList::AppendString(const llvm::Twine &str) {
+  m_strings.push_back(str.str());
+}
+
 void StringList::AppendList(const char **strv, int strc) {
   for (int i = 0; i < strc; ++i) {
 if (strv[i])
Index: lldb/include/lldb/Utility/StringList.h
===
--- lldb/include/lldb/Utility/StringList.h
+++ lldb/include/lldb/Utility/StringList.h
@@ -10,6 +10,7 @@
 #define LLDB_UTILITY_STRINGLIST_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 
 #include 
 #include 
@@ -44,6 +45,8 @@
 
   void AppendString(llvm::StringRef str);
 
+  void AppendString(const llvm::Twine &str);
+
   void AppendList(const char **strv, int strc);
 
   void AppendList(StringList strings);


Index: lldb/source/Utility/StringList.cpp
===
--- lldb/source/Utility/StringList.cpp
+++ lldb/source/Utility/StringList.cpp
@@ -55,6 +55,10 @@
   m_strings.push_back(str.str());
 }
 
+void StringList::AppendString(const llvm::Twine &str) {
+  m_strings.push_back(str.str());
+}
+
 void StringList::AppendList(const char **strv, int strc) {
   for (int i = 0; i < strc; ++i) {
 if (strv[i])
Index: lldb/include/lldb/Utility/StringList.h
===
--- lldb/include/lldb/Utility/StringList.h
+++ lldb/include/lldb/Utility/StringList.h
@@ -10,6 +10,7 @@
 #define LLDB_UTILITY_STRINGLIST_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 
 #include 
 #include 
@@ -44,6 +45,8 @@
 
   void AppendString(llvm::StringRef str);
 
+  void AppendString(const llvm::Twine &str);
+
   void AppendList(const char **strv, int strc);
 
   void AppendList(StringList strings);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2864
+  for (const auto &subcommand_name : subcommands_found) {
+auto qualified_name = (command_name + " " + subcommand_name).str();
+commands_found.AppendString(std::move(qualified_name));

JDevlieghere wrote:
> It's not obvious what type `qualified_name` is. I would either do:
> 
> ```
> std::string qualified_name = (command_name + " " + 
> subcommand_name).str();
> ```
> 
> or 
> 
> ```
> auto qualified_name = std::string(command_name + " " + 
> subcommand_name);
> ```
As you've given feedback on use of `auto` before, should we document some 
standards around `auto`? It seems to vary in the code, and from person to 
person. 

I'm cool with explicit `std::string`, but I think a few bits of context here do 
indicate it's a string type (ex use of `.str()`, use of `+` and a string 
literal, being passed to `AppendString()`). I'm thinking that some of these 
could be documented as either sufficient or insufficient for `auto`.

Somewhat separately, what do you think about adding `AppendString(const 
Twine&)` (D116682) and then making this:

```
commands_found.AppendString(command_name + " " + subcommand_name);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116491

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

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2864
+  for (const auto &subcommand_name : subcommands_found) {
+auto qualified_name = (command_name + " " + subcommand_name).str();
+commands_found.AppendString(std::move(qualified_name));

JDevlieghere wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > It's not obvious what type `qualified_name` is. I would either do:
> > > 
> > > ```
> > > std::string qualified_name = (command_name + " " + 
> > > subcommand_name).str();
> > > ```
> > > 
> > > or 
> > > 
> > > ```
> > > auto qualified_name = std::string(command_name + " " + 
> > > subcommand_name);
> > > ```
> > As you've given feedback on use of `auto` before, should we document some 
> > standards around `auto`? It seems to vary in the code, and from person to 
> > person. 
> > 
> > I'm cool with explicit `std::string`, but I think a few bits of context 
> > here do indicate it's a string type (ex use of `.str()`, use of `+` and a 
> > string literal, being passed to `AppendString()`). I'm thinking that some 
> > of these could be documented as either sufficient or insufficient for 
> > `auto`.
> > 
> > Somewhat separately, what do you think about adding `AppendString(const 
> > Twine&)` (D116682) and then making this:
> > 
> > ```
> > commands_found.AppendString(command_name + " " + subcommand_name);
> > ```
> The LLVM Coding Standards have a section on `auto`: 
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable.
>  
> 
> If it were any other type, I might agree with your argument, but we have a 
> bunch of "string" types (`std::string`, `llvm::StringRef` and 
> `lldb_private::ConstString` being the usual suspects) making it non-obvious. 
from the doc:

> or other places where the type is already obvious from the context.

I find this leaves a lot of room for interpretation, ideally it would be more 
precise. In the mean time I will switch to string but I bet this isn't our last 
conversation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116491

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

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 397681.
kastiglione added a comment.

explicit std::string type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116491

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,12 +2839,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2854,14 +2852,19 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcommandDictionary());
+  for (const auto &subcommand_name : subcommands_found) {
+std::string qualified_name =
+(command_name + " " + subcommand_name).str();
+commands_found.AppendString(std::move(qualified_name));
+  }
 }
   }
 }
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  CommandObject::CommandMap &command_map);
+  const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,12 +2839,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2854,14 +2852,19 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ 

[Lldb-commits] [PATCH] D116708: [lldb] Break out long help for 'frame var'

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: augusto2112, JDevlieghere, jingham, rastogishubham.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The current help for `frame variable` is somewhat long. Its length, combined 
with the few aliases (`var`, `v`, and `vo`) can make the output of `apropos` 
redundant and noisy.

This separates out the details into a separate long help.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116708

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -394,19 +394,7 @@
 interpreter, "frame variable",
 "Show variables for the current stack frame. Defaults to all "
 "arguments and local variables in scope. Names of argument, "
-"local, file static and file global variables can be specified. "
-"Children of aggregate variables can be specified such as "
-"'var->child.x'.  The -> and [] operators in 'frame variable' do "
-"not invoke operator overloads if they exist, but directly access "
-"the specified element.  If you want to trigger operator overloads 
"
-"use the expression command to print the variable instead."
-"\nIt is worth noting that except for overloaded "
-"operators, when printing local variables 'expr local_var' and "
-"'frame var local_var' produce the same "
-"results.  However, 'frame variable' is more efficient, since it "
-"uses debug information and memory reads directly, rather than "
-"parsing and evaluating an expression, which may even involve "
-"JITing and running code in the target program.",
+"local, file static and file global variables can be specified.",
 nullptr,
 eCommandRequiresFrame | eCommandTryTargetAPILock |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused |
@@ -415,6 +403,18 @@
 m_option_variable(
 true), // Include the frame specific options by passing "true"
 m_option_format(eFormatDefault), m_varobj_options() {
+SetHelpLong(R"(
+Children of aggregate variables can be specified such as 'var->child.x'.  In
+'frame variable', the operators -> and [] do not invoke operator overloads if
+they exist, but directly access the specified element.  If you want to trigger
+operator overloads use the expression command to print the variable instead.
+
+It is worth noting that except for overloaded operators, when printing local
+variables 'expr local_var' and 'frame var local_var' produce the same results.
+However, 'frame variable' is more efficient, since it uses debug information 
and
+memory reads directly, rather than parsing and evaluating an expression, which
+may even involve JITing and running code in the target program.)");
+
 CommandArgumentEntry arg;
 CommandArgumentData var_name_arg;
 


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -394,19 +394,7 @@
 interpreter, "frame variable",
 "Show variables for the current stack frame. Defaults to all "
 "arguments and local variables in scope. Names of argument, "
-"local, file static and file global variables can be specified. "
-"Children of aggregate variables can be specified such as "
-"'var->child.x'.  The -> and [] operators in 'frame variable' do "
-"not invoke operator overloads if they exist, but directly access "
-"the specified element.  If you want to trigger operator overloads "
-"use the expression command to print the variable instead."
-"\nIt is worth noting that except for overloaded "
-"operators, when printing local variables 'expr local_var' and "
-"'frame var local_var' produce the same "
-"results.  However, 'frame variable' is more efficient, since it "
-"uses debug information and memory reads directly, rather than "
-"parsing and evaluating an expression, which may even involve "
-"JITing and running code in the target program.",
+"local, file static and file global variables can be specified.",
 nullptr,
 eCommandRequiresFrame | eCommandTryTargetAPILock |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused |
@@ -415,6 +403,18 @@
 m_option_variable(
 true), // Include the frame specific options by passing "true"

[Lldb-commits] [PATCH] D116708: [lldb] Break out long help for 'frame var'

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGef6817f9329c: [lldb] Break out long help for 'frame 
var' (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116708

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -394,19 +394,7 @@
 interpreter, "frame variable",
 "Show variables for the current stack frame. Defaults to all "
 "arguments and local variables in scope. Names of argument, "
-"local, file static and file global variables can be specified. "
-"Children of aggregate variables can be specified such as "
-"'var->child.x'.  The -> and [] operators in 'frame variable' do "
-"not invoke operator overloads if they exist, but directly access "
-"the specified element.  If you want to trigger operator overloads 
"
-"use the expression command to print the variable instead."
-"\nIt is worth noting that except for overloaded "
-"operators, when printing local variables 'expr local_var' and "
-"'frame var local_var' produce the same "
-"results.  However, 'frame variable' is more efficient, since it "
-"uses debug information and memory reads directly, rather than "
-"parsing and evaluating an expression, which may even involve "
-"JITing and running code in the target program.",
+"local, file static and file global variables can be specified.",
 nullptr,
 eCommandRequiresFrame | eCommandTryTargetAPILock |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused |
@@ -415,6 +403,18 @@
 m_option_variable(
 true), // Include the frame specific options by passing "true"
 m_option_format(eFormatDefault), m_varobj_options() {
+SetHelpLong(R"(
+Children of aggregate variables can be specified such as 'var->child.x'.  In
+'frame variable', the operators -> and [] do not invoke operator overloads if
+they exist, but directly access the specified element.  If you want to trigger
+operator overloads use the expression command to print the variable instead.
+
+It is worth noting that except for overloaded operators, when printing local
+variables 'expr local_var' and 'frame var local_var' produce the same results.
+However, 'frame variable' is more efficient, since it uses debug information 
and
+memory reads directly, rather than parsing and evaluating an expression, which
+may even involve JITing and running code in the target program.)");
+
 CommandArgumentEntry arg;
 CommandArgumentData var_name_arg;
 


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -394,19 +394,7 @@
 interpreter, "frame variable",
 "Show variables for the current stack frame. Defaults to all "
 "arguments and local variables in scope. Names of argument, "
-"local, file static and file global variables can be specified. "
-"Children of aggregate variables can be specified such as "
-"'var->child.x'.  The -> and [] operators in 'frame variable' do "
-"not invoke operator overloads if they exist, but directly access "
-"the specified element.  If you want to trigger operator overloads "
-"use the expression command to print the variable instead."
-"\nIt is worth noting that except for overloaded "
-"operators, when printing local variables 'expr local_var' and "
-"'frame var local_var' produce the same "
-"results.  However, 'frame variable' is more efficient, since it "
-"uses debug information and memory reads directly, rather than "
-"parsing and evaluating an expression, which may even involve "
-"JITing and running code in the target program.",
+"local, file static and file global variables can be specified.",
 nullptr,
 eCommandRequiresFrame | eCommandTryTargetAPILock |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused |
@@ -415,6 +403,18 @@
 m_option_variable(
 true), // Include the frame specific options by passing "true"
 m_option_format(eFormatDefault), m_varobj_options() {
+SetHelpLong(R"(
+Children of aggregate variables can be specified such as 'var->child.x'.  In
+'frame variable', the operators -> and [] do not invoke operator ove

[Lldb-commits] [PATCH] D116682: [lldb] Add StringList::AppendString(const Twine&) (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a1a0d4957ec: [lldb] Add StringList::AppendString(const 
Twine&) (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116682

Files:
  lldb/include/lldb/Utility/StringList.h
  lldb/source/Utility/StringList.cpp


Index: lldb/source/Utility/StringList.cpp
===
--- lldb/source/Utility/StringList.cpp
+++ lldb/source/Utility/StringList.cpp
@@ -55,6 +55,10 @@
   m_strings.push_back(str.str());
 }
 
+void StringList::AppendString(const llvm::Twine &str) {
+  m_strings.push_back(str.str());
+}
+
 void StringList::AppendList(const char **strv, int strc) {
   for (int i = 0; i < strc; ++i) {
 if (strv[i])
Index: lldb/include/lldb/Utility/StringList.h
===
--- lldb/include/lldb/Utility/StringList.h
+++ lldb/include/lldb/Utility/StringList.h
@@ -10,6 +10,7 @@
 #define LLDB_UTILITY_STRINGLIST_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 
 #include 
 #include 
@@ -44,6 +45,8 @@
 
   void AppendString(llvm::StringRef str);
 
+  void AppendString(const llvm::Twine &str);
+
   void AppendList(const char **strv, int strc);
 
   void AppendList(StringList strings);


Index: lldb/source/Utility/StringList.cpp
===
--- lldb/source/Utility/StringList.cpp
+++ lldb/source/Utility/StringList.cpp
@@ -55,6 +55,10 @@
   m_strings.push_back(str.str());
 }
 
+void StringList::AppendString(const llvm::Twine &str) {
+  m_strings.push_back(str.str());
+}
+
 void StringList::AppendList(const char **strv, int strc) {
   for (int i = 0; i < strc; ++i) {
 if (strv[i])
Index: lldb/include/lldb/Utility/StringList.h
===
--- lldb/include/lldb/Utility/StringList.h
+++ lldb/include/lldb/Utility/StringList.h
@@ -10,6 +10,7 @@
 #define LLDB_UTILITY_STRINGLIST_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 
 #include 
 #include 
@@ -44,6 +45,8 @@
 
   void AppendString(llvm::StringRef str);
 
+  void AppendString(const llvm::Twine &str);
+
   void AppendList(const char **strv, int strc);
 
   void AppendList(StringList strings);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3bfd595a548: [lldb] Compute fully qualified command names 
in FindCommandsForApropos (authored by kastiglione).

Changed prior to commit:
  https://reviews.llvm.org/D116491?vs=397681&id=398034#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116491

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2841,12 +2841,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2856,14 +2854,19 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcommandDictionary());
+  for (const auto &subcommand_name : subcommands_found) {
+std::string qualified_name =
+(command_name + " " + subcommand_name).str();
+commands_found.AppendString(qualified_name);
+  }
 }
   }
 }
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  CommandObject::CommandMap &command_map);
+  const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2841,12 +2841,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2856,14 +2854,19 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd =

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, clayborg.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Ensure that errors in `frame variable` are reflected in result object.

The statistics for `frame variable` show invocations as being successful, even
when executing one of the error paths.

This change replaces `result.GetErrorStream()` with `result.AppendError()`,
which also sets the status to `eReturnStatusFailed`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116788

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendWarningWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-   "variable expression path that "
-   "matches '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"unable to find any variable expression path that matches "
+"'%s'.",
+entry.c_str());
 }
   }
 }
@@ -680,7 +677,8 @@
   }
 }
   }
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (result.GetStatus() != eReturnStatusFailed)
+result.SetStatus(eReturnStatusSuccessFinishResult);
 }
 
 if (m_option_variable.show_recognized_args) {


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression '%s'.\n",
-   entry.c_str());
+result.AppendWarningWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr 

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 398040.
kastiglione added a comment.

s/warning/error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-   "variable expression path that "
-   "matches '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"unable to find any variable expression path that matches "
+"'%s'.",
+entry.c_str());
 }
   }
 }
@@ -680,7 +677,8 @@
   }
 }
   }
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (result.GetStatus() != eReturnStatusFailed)
+result.SetStatus(eReturnStatusSuccessFinishResult);
 }
 
 if (m_option_variable.show_recognized_args) {


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-   

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectFrame.cpp:560-563
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());

this regex error is a weird edge case. For example, considering running:

```
frame var --regex matchesSomeVars doesntMatchAnyVars
```

if the `doesntMatchAnyVars` pattern has no matches, then the command prints an 
error, and the result would be marked as an error. But if the `matchesSomeVars` 
does have matches, then we have a partial success / partial failure. In such a 
case, should the result be marked success, or failure? I don't know, but I 
would lean to success since it does entirely fail. Maybe a user could expect 
some patterns to match and some to not match. For example: a user alias that 
prints any variables based on a set of patterns they're interested in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a subscriber: jingham.
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectFrame.cpp:560-563
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());

kastiglione wrote:
> this regex error is a weird edge case. For example, considering running:
> 
> ```
> frame var --regex matchesSomeVars doesntMatchAnyVars
> ```
> 
> if the `doesntMatchAnyVars` pattern has no matches, then the command prints 
> an error, and the result would be marked as an error. But if the 
> `matchesSomeVars` does have matches, then we have a partial success / partial 
> failure. In such a case, should the result be marked success, or failure? I 
> don't know, but I would lean to success since it does entirely fail. Maybe a 
> user could expect some patterns to match and some to not match. For example: 
> a user alias that prints any variables based on a set of patterns they're 
> interested in.
I think this could be changed to a warning. @jingham what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c7d10c41278: [lldb] Set result error state in 'frame 
variable' (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-   "variable expression path that "
-   "matches '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"unable to find any variable expression path that matches "
+"'%s'.",
+entry.c_str());
 }
   }
 }
@@ -680,7 +677,8 @@
   }
 }
   }
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (result.GetStatus() != eReturnStatusFailed)
+result.SetStatus(eReturnStatusSuccessFinishResult);
 }
 
 if (m_option_variable.show_recognized_args) {


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

To add more context, `ProcessStructReader` is used in only a few places, and 
none seem like they should truly need it. This was a low hanging fruit.

Something I don't understand about this code is why it the objc metadata is 
incomplete. The class is known, but not its ivars, why? In theory this code 
should be able to request the `_lengthAndRefCount` child member directly by 
name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116461

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

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 398341.
kastiglione added a comment.

give name to synthetic child


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116461

Files:
  lldb/source/Plugins/Language/ObjC/NSString.cpp


Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -8,14 +8,13 @@
 
 #include "NSString.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
@@ -31,24 +30,6 @@
   return g_map;
 }
 
-static CompilerType GetNSPathStore2Type(Target &target) {
-  static ConstString g_type_name("__lldb_autogen_nspathstore2");
-
-  TypeSystemClang *ast_ctx = ScratchTypeSystemClang::GetForTarget(target);
-
-  if (!ast_ctx)
-return CompilerType();
-
-  CompilerType voidstar =
-  ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
-  CompilerType uint32 =
-  ast_ctx->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 32);
-
-  return ast_ctx->GetOrCreateStructForIdentifier(
-  g_type_name,
-  {{"isa", voidstar}, {"lengthAndRef", uint32}, {"buffer", voidstar}});
-}
-
 bool lldb_private::formatters::NSStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &summary_options) {
@@ -229,11 +210,17 @@
 return StringPrinter::ReadStringAndDumpToStream<
 StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
-ProcessStructReader reader(valobj.GetProcessSP().get(),
-   valobj.GetValueAsUnsigned(0),
-   GetNSPathStore2Type(*valobj.GetTargetSP()));
-explicit_length =
-reader.GetField(ConstString("lengthAndRef")) >> 20;
+// _lengthAndRefCount is the first ivar of NSPathStore2 (after the isa).
+uint64_t length_ivar_offset = 1 * ptr_size;
+CompilerType length_type = valobj.GetCompilerType().GetBasicTypeFromAST(
+lldb::eBasicTypeUnsignedInt);
+ValueObjectSP length_valobj_sp =
+valobj.GetSyntheticChildAtOffset(length_ivar_offset, length_type, true,
+ ConstString("_lengthAndRefCount"));
+if (!length_valobj_sp)
+  return false;
+// Get the length out of _lengthAndRefCount.
+explicit_length = length_valobj_sp->GetValueAsUnsigned(0) >> 20;
 lldb::addr_t location = valobj.GetValueAsUnsigned(0) + ptr_size + 4;
 
 options.SetLocation(location);


Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -8,14 +8,13 @@
 
 #include "NSString.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
@@ -31,24 +30,6 @@
   return g_map;
 }
 
-static CompilerType GetNSPathStore2Type(Target &target) {
-  static ConstString g_type_name("__lldb_autogen_nspathstore2");
-
-  TypeSystemClang *ast_ctx = ScratchTypeSystemClang::GetForTarget(target);
-
-  if (!ast_ctx)
-return CompilerType();
-
-  CompilerType voidstar =
-  ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
-  CompilerType uint32 =
-  ast_ctx->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 32);
-
-  return ast_ctx->GetOrCreateStructForIdentifier(
-  g_type_name,
-  {{"isa", voidstar}, {"lengthAndRef", uint32}, {"buffer", voidstar}});
-}
-
 bool lldb_private::formatters::NSStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &summary_options) {
@@ -229,11 +210,17 @@
 return StringPrinter::ReadStringAndDumpToStream<
 StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
-ProcessStructReader reader(valobj.GetProcessSP().get(),
-   valobj.GetValueAsUnsigned(0),
-   GetNSPathStore2Type(*valobj.GetTargetSP()));
-explicit_length =
-

[Lldb-commits] [PATCH] D116863: [lldb] Remove anon struct from frame-var-anon-unions test

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, jingham, aprantl.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This test for anonymous unions seems off. It tests the following:

  union {
// fields
  };
  
  struct {
// fields
  } var{...};

Both are anonymous types, but the first does not declare a variable and the
second one does. The test then checks that `frame var` can access the fields of
the anonymous union, but can't access the fields of the anonymous struct.

The second test, to access the members of the struct variable, seems pointless
as similar code would not compile. A demonstration:

  struct {
int a;
int z;
  } a_z{23, 45};
  printf("%d\n", a_z.a); // fine
  printf("%d\n", a); // this does not compile

Since we can't directly access the fields in code, I'm not sure there's a
reason to test that lldb also can't access them (other than perhaps as a
regression test).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116863

Files:
  
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
  lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116863: [lldb] Remove anon struct from frame-var-anon-unions test

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 398348.
kastiglione added a comment.

clarify commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116863

Files:
  
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
  lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116863: [lldb] Remove anon struct from frame-var-anon-unions test

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 398349.
kastiglione added a comment.

once more


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116863

Files:
  
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
  lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d9f3e0ec773: [lldb] Remove ProcessStructReader from 
NSStringSummaryProvider (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116461

Files:
  lldb/source/Plugins/Language/ObjC/NSString.cpp


Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -8,14 +8,13 @@
 
 #include "NSString.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
@@ -31,24 +30,6 @@
   return g_map;
 }
 
-static CompilerType GetNSPathStore2Type(Target &target) {
-  static ConstString g_type_name("__lldb_autogen_nspathstore2");
-
-  TypeSystemClang *ast_ctx = ScratchTypeSystemClang::GetForTarget(target);
-
-  if (!ast_ctx)
-return CompilerType();
-
-  CompilerType voidstar =
-  ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
-  CompilerType uint32 =
-  ast_ctx->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 32);
-
-  return ast_ctx->GetOrCreateStructForIdentifier(
-  g_type_name,
-  {{"isa", voidstar}, {"lengthAndRef", uint32}, {"buffer", voidstar}});
-}
-
 bool lldb_private::formatters::NSStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &summary_options) {
@@ -229,11 +210,17 @@
 return StringPrinter::ReadStringAndDumpToStream<
 StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
-ProcessStructReader reader(valobj.GetProcessSP().get(),
-   valobj.GetValueAsUnsigned(0),
-   GetNSPathStore2Type(*valobj.GetTargetSP()));
-explicit_length =
-reader.GetField(ConstString("lengthAndRef")) >> 20;
+// _lengthAndRefCount is the first ivar of NSPathStore2 (after the isa).
+uint64_t length_ivar_offset = 1 * ptr_size;
+CompilerType length_type = valobj.GetCompilerType().GetBasicTypeFromAST(
+lldb::eBasicTypeUnsignedInt);
+ValueObjectSP length_valobj_sp =
+valobj.GetSyntheticChildAtOffset(length_ivar_offset, length_type, true,
+ ConstString("_lengthAndRefCount"));
+if (!length_valobj_sp)
+  return false;
+// Get the length out of _lengthAndRefCount.
+explicit_length = length_valobj_sp->GetValueAsUnsigned(0) >> 20;
 lldb::addr_t location = valobj.GetValueAsUnsigned(0) + ptr_size + 4;
 
 options.SetLocation(location);


Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -8,14 +8,13 @@
 
 #include "NSString.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
@@ -31,24 +30,6 @@
   return g_map;
 }
 
-static CompilerType GetNSPathStore2Type(Target &target) {
-  static ConstString g_type_name("__lldb_autogen_nspathstore2");
-
-  TypeSystemClang *ast_ctx = ScratchTypeSystemClang::GetForTarget(target);
-
-  if (!ast_ctx)
-return CompilerType();
-
-  CompilerType voidstar =
-  ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
-  CompilerType uint32 =
-  ast_ctx->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 32);
-
-  return ast_ctx->GetOrCreateStructForIdentifier(
-  g_type_name,
-  {{"isa", voidstar}, {"lengthAndRef", uint32}, {"buffer", voidstar}});
-}
-
 bool lldb_private::formatters::NSStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &summary_options) {
@@ -229,11 +210,17 @@
 return StringPrinter::ReadStringAndDumpToStream<
 StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
-ProcessStructReader reader(valobj.GetProcessSP().get(),
-   valobj.GetValueAsUnsigned(0),
-  

[Lldb-commits] [PATCH] D116901: [lldb] Guard libstdc++ specific 'frame var' test

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: aprantl, JDevlieghere.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While working on D116788  (properly error out 
of `frame var`), this libstdc++
specific `frame var` invocation was found in the tests. This test is in the
generic directory, but has this one case that requires libstdc++. The fix here
is to put the one `expect()` inside of a condition that checks for libstdc++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116901

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
@@ -62,12 +62,14 @@
 self.expect("frame variable numbers_list --raw", matching=False,
 substrs=['size=0',
  '{}'])
-self.expect(
-"frame variable &numbers_list._M_impl._M_node --raw",
-matching=False,
-substrs=[
-'size=0',
-'{}'])
+
+if stdlib_type == USE_LIBSTDCPP:
+self.expect(
+"frame variable &numbers_list._M_impl._M_node --raw",
+matching=False,
+substrs=[
+'size=0',
+'{}'])
 
 self.expect("frame variable numbers_list",
 substrs=['size=0',


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
@@ -62,12 +62,14 @@
 self.expect("frame variable numbers_list --raw", matching=False,
 substrs=['size=0',
  '{}'])
-self.expect(
-"frame variable &numbers_list._M_impl._M_node --raw",
-matching=False,
-substrs=[
-'size=0',
-'{}'])
+
+if stdlib_type == USE_LIBSTDCPP:
+self.expect(
+"frame variable &numbers_list._M_impl._M_node --raw",
+matching=False,
+substrs=[
+'size=0',
+'{}'])
 
 self.expect("frame variable numbers_list",
 substrs=['size=0',
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116912: [lldb] Check for arm64 in TestDisassembleRawData

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: jasonmolenda, JDevlieghere.
Herald added subscribers: omjavaid, kristof.beyls.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This test checks for `aarch64` but the lit config could also contain `arm64`.
This change adds `arm64` to make the test pass in all cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116912

Files:
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py


Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -31,7 +31,7 @@
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"aarch64")
 raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
 elif arch == "arm":
@@ -57,7 +57,7 @@
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
 elif arch == "arm":


Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -31,7 +31,7 @@
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "aarch64")
 raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
 elif arch == "arm":
@@ -57,7 +57,7 @@
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
 elif arch == "arm":
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116912: [lldb] Check for arm64 in TestDisassembleRawData

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaad27a890754: [lldb] Check for arm64 in 
TestDisassembleRawData (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116912

Files:
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py


Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -31,7 +31,7 @@
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"aarch64")
 raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
 elif arch == "arm":
@@ -57,7 +57,7 @@
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
 elif arch == "arm":


Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -31,7 +31,7 @@
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "aarch64")
 raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
 elif arch == "arm":
@@ -57,7 +57,7 @@
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
-elif arch == "aarch64":
+elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
 elif arch == "arm":
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116901: [lldb] Guard libstdc++ specific 'frame var' test

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a8549354c1b: [lldb] Guard libstdc++ specific 'frame 
var' test (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116901

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
@@ -62,12 +62,14 @@
 self.expect("frame variable numbers_list --raw", matching=False,
 substrs=['size=0',
  '{}'])
-self.expect(
-"frame variable &numbers_list._M_impl._M_node --raw",
-matching=False,
-substrs=[
-'size=0',
-'{}'])
+
+if stdlib_type == USE_LIBSTDCPP:
+self.expect(
+"frame variable &numbers_list._M_impl._M_node --raw",
+matching=False,
+substrs=[
+'size=0',
+'{}'])
 
 self.expect("frame variable numbers_list",
 substrs=['size=0',


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
@@ -62,12 +62,14 @@
 self.expect("frame variable numbers_list --raw", matching=False,
 substrs=['size=0',
  '{}'])
-self.expect(
-"frame variable &numbers_list._M_impl._M_node --raw",
-matching=False,
-substrs=[
-'size=0',
-'{}'])
+
+if stdlib_type == USE_LIBSTDCPP:
+self.expect(
+"frame variable &numbers_list._M_impl._M_node --raw",
+matching=False,
+substrs=[
+'size=0',
+'{}'])
 
 self.expect("frame variable numbers_list",
 substrs=['size=0',
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectMemory.cpp:1695
+  section_sp = section_sp->GetParent();
+section_name = section_sp->GetName();
+  }

aprantl wrote:
> Why did `section_sp->GetName();` return the Segment name in the original 
> code? Is that a bug?
The original code is traversing the parent chain of sections, which for mach-o 
ends up on the segment. Now that I have learned the address range printed is 
for the segment, then it makes sense why the segment name alone is printed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116419

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

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:76
+
+self.runCmd("run", RUN_SUCCEEDED)
+

aprantl wrote:
> Doesn't `lldbutil.run_break_set_by_symbol()` do the "file" and "run" already 
> and this run command runs it a second time?
I will learn more to find out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116419

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

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

In D116788#3231612 , @clayborg wrote:

> I would vote that if any argument or option is specified, and results in 
> something not being found, that an error be returned.

Is this in response to my question about regexes? It seems to be. Specifically, 
you're of the opinion that any failing regex results in an error, even when 
other regexes do match? Your point about it not affecting the command line is a 
good point, thanks.

All your other points sound good to me too.

> Any tests that are failing now when they didn't before can be modified.

Yes there were a couple, D116901  and D116863 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

> Let me know your thoughts as mine are just my initial thoughts after thinking 
> about things a bit more.

For most of what you said, I was thinking the same. The only place where I 
wasn't as sure about (regex), I was on the fence. Your cases and arguments all 
seem good to me. I think the consistency is also good.

thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

> if I run “frame var” from HandleCommand in a script, with this change I can’t 
> rely on the command status to tell whether the command output had anything 
> interesting in it. That seems awkward.

This could be resolved by introducing a new enum value to indicate partial 
success, partial failure. I think that would be reasonable. I don't think that 
should block this change though, do you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

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

Convert the `crashlog` command to be implemented as a class. The `Symbolicate`
function is switched to a class, to implement `get_long_help`. The text for the
long help comes from the help output generated by `OptionParser`. That is, the
output of `help crashlog` is the same as `crashlog --help`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117165

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide 
source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the 
location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses 
found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in 
the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the 
crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def SymbolicateCrashLogs(debugger, command_args):
+option_parser = CrashLogOptionParser()
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
@@ -1219,13 +1226,15 @@
 for crash_log_file in args:
 crash_log = CrashLogParser().parse(debugger, crash_log_file, 
options.verbose)
 SymbolicateCrashLog(crash_log, options)
+
 if __name__ == '__main__':
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 SymbolicateCrashLogs(debugger, sys.argv[1:])
 lldb.SBDebugger.Destroy(debugger)
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 lldb.debugger.HandleCommand(
-'command script add -f lldb.macosx.crashlog.Symbolicate crashlog')
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
 lldb.debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in the crash log and functions can
 be disassembled and lookups can be

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.
Herald added a subscriber: JDevlieghere.

The output of `help crashlog` is:

  (lldb) help crashlog 
  For more information run 'help crashlog'  Expects 'raw' input (see 'help 
raw-input'.)
  
  Syntax: crashlog
  Usage: crashlog [options]  [FILE ...]
  
  Symbolicate one or more darwin crash log files to provide source file and line
  information, inlined stack frames back to the concrete functions, and
  disassemble the location of the crash for the first frame of the crashed
  thread. If this script is imported into the LLDB command interpreter, a
  "crashlog" command will be added to the interpreter for use at the LLDB
  command line. After a crash log has been parsed and symbolicated, a target
  will have been created that has all of the shared libraries loaded at the load
  addresses found in the crash log file. This allows you to explore the program
  as if it were stopped at the locations described in the crash log and
  functions can be disassembled and lookups can be performed using the addresses
  found in the crash log.
  
  Options:
-h, --helpshow this help message and exit
-v, --verbose display verbose debug info
-g, --debug   display verbose debug logging
-a, --load-allload all executable images, not just the images found
  in the crashed stack frames
--images  show image list
--debug-delay=NSECpause for NSEC seconds for debugger
-c, --crashed-onlyonly symbolicate the crashed thread
-d DISASSEMBLE_DEPTH, --disasm-depth=DISASSEMBLE_DEPTH
  set the depth in stack frames that should be
  disassembled (default is 1)
-D, --disasm-all  enabled disassembly of frames on all threads (not just
  the crashed thread)
-B DISASSEMBLE_BEFORE, --disasm-before=DISASSEMBLE_BEFORE
  the number of instructions to disassemble before the
  frame PC
-A DISASSEMBLE_AFTER, --disasm-after=DISASSEMBLE_AFTER
  the number of instructions to disassemble after the
  frame PC
-C NLINES, --source-context=NLINES
  show NLINES source lines of source context (default =
  4)
--source-frames=NFRAMES
  show source for NFRAMES (default = 4)
--source-all  show source for all threads, not just the crashed
  thread
-i, --interactive parse all crash logs and enter interactive mode


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399517.
kastiglione added a comment.

refer to the debugger parameter, not lldb.debugger


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide 
source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the 
location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses 
found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in 
the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the 
crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def SymbolicateCrashLogs(debugger, command_args):
+option_parser = CrashLogOptionParser()
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
@@ -1219,13 +1226,15 @@
 for crash_log_file in args:
 crash_log = CrashLogParser().parse(debugger, crash_log_file, 
options.verbose)
 SymbolicateCrashLog(crash_log, options)
+
 if __name__ == '__main__':
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 SymbolicateCrashLogs(debugger, sys.argv[1:])
 lldb.SBDebugger.Destroy(debugger)
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
-'command script add -f lldb.macosx.crashlog.Symbolicate crashlog')
-lldb.debugger.HandleCommand(
+
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,16 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.get_usage()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1191,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1200,10 @@
 created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/examples/python/crashlog.py:1236-1240
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')

Note that I introduced `__lldb_init_module` here because there's some bug when 
registering a command class using `lldb.debugger`. In other words, this failed:

```
lldb.debugger.HandleCommand('command script add -c mod.Command cmd')
```

and this succeeds:

```
def __lldb_init_module(debugger, _):
debugger.HandleCommand('command script add -c mod.Command cmd')
```

When using the first form, an error would happen when running the command. The 
error message is "no function to execute".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399701.
kastiglione added a comment.

Add get_short_help, and add missing changes that should have been part of 
original diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,22 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __init__(self, debugger, internal_dict):
+pass
+
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_short_help(self):
+return "Symbolicate one or more darwin crash log files."
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.format_help()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1197,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide 
source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the 
location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1206,10 @@
 created that has all of the shared libraries loaded at the load addresses 
found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in 
the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the 
crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def SymbolicateCrashLogs(debugger, command_args):
+option_parser = CrashLogOptionParser()
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
@@ -1219,13 +1232,15 @@
 for crash_log_file in args:
 crash_log = CrashLogParser().parse(debugger, crash_log_file, 
options.verbose)
 SymbolicateCrashLog(crash_log, options)
+
 if __name__ == '__main__':
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 SymbolicateCrashLogs(debugger, sys.argv[1:])
 lldb.SBDebugger.Destroy(debugger)
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
-'command script add -f lldb.macosx.crashlog.Symbolicate crashlog')
-lldb.debugger.HandleCommand(
+
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,22 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __init__(self, debugger, internal_dict):
+pass
+
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_short_help(self):
+return "Symbolicate one or more darwin crash log files."
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.format_help()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1197,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1206,10 @@
 created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
 you to e

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

> Can you see if also adding a "get_short_help" gets rid of that dopey:

Good call, it worked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/examples/python/crashlog.py:1236-1240
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')

jingham wrote:
> kastiglione wrote:
> > Note that I introduced `__lldb_init_module` here because there's some bug 
> > when registering a command class using `lldb.debugger`. In other words, 
> > this failed:
> > 
> > ```
> > lldb.debugger.HandleCommand('command script add -c mod.Command cmd')
> > ```
> > 
> > and this succeeds:
> > 
> > ```
> > def __lldb_init_module(debugger, _):
> > debugger.HandleCommand('command script add -c mod.Command cmd')
> > ```
> > 
> > When using the first form, an error would happen when running the command. 
> > The error message is "no function to execute".
> That makes sense.  All of the "lldb.{debugger, target, process, thread, etc}" 
> variables are only set when entering the interactive script interpreter.  
> They don't get set when processing "command script import".  So lldb.debugger 
> would have been None here.
> 
> In general, since pretty much no python we write in the lldb project is going 
> to run in the interactive interpreter, you should never use those lldb.etc 
> variables.
> 
> I actually think we could make the lldb.debugger work, since that really is a 
> singleton for the command interpreter, but it seems much better form to pass 
> debugger directly to whoever needs it, so for the sake of clarity they all 
> get unset when we leave the interactive script interpreter.
Yes I've come to learn to avoid "lldb.{debugger, target, process, thread, etc}" 
at all costs, which is how I was able to get past the strange error message.

> So `lldb.debugger` would have been None here.

It wasn't, which made things trickier to understand.

Also note that the docs recommend this idiom used here in crashlog.py. Look for 
`if __name__ == '__main__'` in 
https://lldb.llvm.org/use/python-reference.html#create-a-new-lldb-command-using-a-python-function.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

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

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: jingham, JDevlieghere, clayborg.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Update examples and docs to demonstrate using `__lldb_init_module` instead of
the idiom that checks for `lldb.debugger` at the top-level.

  if __name__ == '__main__':
  ...
  elif lldb.debugger:
  ...

Is replaced with:

  if __name__ == '__main__':
  ...
  
  def __lldb_init_module(debugger, internal_dict):
  ...

This change is for two reasons. First, it's generally encouraged not to only
use the convenience singletons (`lldb.{debugger,process,target,etc}`)
interactively from the `script` command. Second, there's a bug where
registering a python class as a command (using `command script add -c ...`),
result in the command not being runnable. Note that registering function-backed
commands does not have this bug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117237

Files:
  lldb/docs/use/python-reference.rst
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/delta.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/memory.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,7 +351,7 @@
 continue
 verify_types(target, options)
 
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
 'command script add -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -270,8 +270,9 @@
 
 if __name__ == '__main__':
 print('error: this script is designed to be used within the embedded script interpreter in LLDB')
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -1619,13 +1619,11 @@
 else:
 parse_gdb_log(sys.stdin, options)
 
-else:
-import lldb
-if lldb.debugger:
-# This initializer is being run from LLDB in the embedded command interpreter
-# Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.start_gdb_log start_gdb_log')
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
-print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
+def __lldb_init_module(debugger, internal_dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f gdbremote.start_gdb_log start_gdb_log')
+debugger.HandleCommand(
+'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
+print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
Index: lldb/examples/python/delta.py
===
--- lldb/examples/python/delta.py
+++ lldb/examples/python/delta.py
@@ -125,11 +125,10 @@
 import sys
 parse_time_log_args(sys.argv[1:])
 
-else:
-import lldb
-if lldb.debugger:
+
+def __lldb_init_module(debugger, internal_dict):
 # This initializer is being run from LLDB in the embedded command interpreter
 # Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f delta.parse_time_log parse_time_log')
 print('The "parse_time_log" command is now installed and ready for use, type "parse_time_log --help" for more information')
Index: lldb/examples/darwin/heap_find/heap.py
===
--- lldb/examples/darwin/heap_find/heap.py
+++ lldb/examples/darwin/heap_find/heap.py
@@ -1494,30 +1494,31 @@
 if __name__ == '__main__':
 lldb.debugg

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399724.
kastiglione added a comment.

fix a few more


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

Files:
  lldb/docs/use/python-reference.rst
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/delta.py
  lldb/examples/python/diagnose_unwind.py
  lldb/examples/python/gdb_disassemble.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/jump.py
  lldb/examples/python/memory.py
  lldb/examples/python/stacks.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,7 +351,7 @@
 continue
 verify_types(target, options)
 
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
 'command script add -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -63,6 +63,7 @@
 print(frame_info)
 
 
-lldb.debugger.HandleCommand(
-"command script add -f stacks.stack_frames stack_frames")
-print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+"command script add -f stacks.stack_frames stack_frames")
+print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -270,8 +270,9 @@
 
 if __name__ == '__main__':
 print('error: this script is designed to be used within the embedded script interpreter in LLDB')
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -191,8 +191,8 @@
 
 frame.SetPC(desired_address.GetLoadAddress(target))
 
-if lldb.debugger:
+def __lldb_init_module(debugger, internal_dict):
 # Module is being run inside the LLDB interpreter
 jump.__doc__ = usage_string()
-lldb.debugger.HandleCommand('command script add -f jump.jump jump')
+debugger.HandleCommand('command script add -f jump.jump jump')
 print('The "jump" command has been installed, type "help jump" or "jump " for detailed help.')
Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -1619,13 +1619,11 @@
 else:
 parse_gdb_log(sys.stdin, options)
 
-else:
-import lldb
-if lldb.debugger:
-# This initializer is being run from LLDB in the embedded command interpreter
-# Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.start_gdb_log start_gdb_log')
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
-print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
+def __lldb_init_module(debugger, internal_dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f gdbremote.start_gdb_log start_gdb_log')
+debugger.HandleCommand(
+'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
+print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
Index: lldb/examples/python/gdb_disassemble.py
===
--- lldb/examples/python/gdb_disassemble.py
+++ lldb/examples/python/gdb_disassemble.py
@@ -21,6 +21,7 @@
 print("<%s + %-4u> 0x%x %8s  %s" % (name, inst_offset, inst_addr, inst.mnemonic, inst.operands))
 
 # Install the command when the

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/docs/use/python-reference.rst:586
 the module is loaded allowing you to add whatever commands you want into the
-current debugger. Note that this function will only be run when using the LLDB
-command command script import, it will not get run if anyone imports your
-module from another module. If you want to always run code when your module is
-loaded from LLDB or when loaded via an import statement in python code you can
-test the lldb.debugger object, since you imported the module at the top of the
-python ls.py module. This test must be in code that isn't contained inside of
-any function or class, just like the standard test for __main__ like all python
-modules usually do. Sample code would look like:
-
-::
-
-  if __name__ == '__main__':
-  # Create a new debugger instance in your module if your module
-  # can be run from the command line. When we run a script from
-  # the command line, we won't have any debugger object in
-  # lldb.debugger, so we can just create it if it will be needed
-  lldb.debugger = lldb.SBDebugger.Create()
-  elif lldb.debugger:
-  # Module is being run inside the LLDB interpreter
-  lldb.debugger.HandleCommand('command script add -f ls.ls ls')
-  print 'The "ls" python command has been installed and is ready for use.'
+current debugger.
 

jingham wrote:
> I think it's okay to show the "if __name__  == '__main__': part of this 
> example, but it should be:
> 
> 
> ```
> if __name__ == '__main__':
>   # Create a new debugger instance in your module if your module
>   # can be run from the command line. When we run a script from
>   # the command line, we won't have any debugger object in
>   # lldb.debugger, so we can just create it if it will be needed
>   lldb.debugger = lldb.SBDebugger.Create()
>   # Now do whatever work this module would do when run as a command
>   # Now dispose of the debugger you just made.
>   lldb.SBDebugger.Destroy(debugger)
> 
> 
> ```That seems useful trick.
good point



Comment at: lldb/examples/darwin/heap_find/heap.py:1518
+__name__)
+# debugger.HandleCommand('command script add -f %s.heap heap' % 
package_name)
+# debugger.HandleCommand('command script add -f %s.section_ptr_refs 
section_ptr_refs' % package_name)

jingham wrote:
> Do you know what these commented-out lines are for?
I don't, no. Of the three, only the `section_ptr_refs` exists in this file. The 
other two, `heap` and `stack_ptr_refs` do not exist. I can remove the latter 
two, and leave the commented out line for the existing `section_ptr_refs` 
function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

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

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399729.
kastiglione added a comment.

restore `if __name__ == '__main__'` example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

Files:
  lldb/docs/use/python-reference.rst
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/delta.py
  lldb/examples/python/diagnose_unwind.py
  lldb/examples/python/gdb_disassemble.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/jump.py
  lldb/examples/python/memory.py
  lldb/examples/python/stacks.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,7 +351,7 @@
 continue
 verify_types(target, options)
 
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
 'command script add -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -63,6 +63,7 @@
 print(frame_info)
 
 
-lldb.debugger.HandleCommand(
-"command script add -f stacks.stack_frames stack_frames")
-print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+"command script add -f stacks.stack_frames stack_frames")
+print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -270,8 +270,9 @@
 
 if __name__ == '__main__':
 print('error: this script is designed to be used within the embedded script interpreter in LLDB')
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -191,8 +191,8 @@
 
 frame.SetPC(desired_address.GetLoadAddress(target))
 
-if lldb.debugger:
+def __lldb_init_module(debugger, internal_dict):
 # Module is being run inside the LLDB interpreter
 jump.__doc__ = usage_string()
-lldb.debugger.HandleCommand('command script add -f jump.jump jump')
+debugger.HandleCommand('command script add -f jump.jump jump')
 print('The "jump" command has been installed, type "help jump" or "jump " for detailed help.')
Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -1619,13 +1619,11 @@
 else:
 parse_gdb_log(sys.stdin, options)
 
-else:
-import lldb
-if lldb.debugger:
-# This initializer is being run from LLDB in the embedded command interpreter
-# Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.start_gdb_log start_gdb_log')
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
-print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
+def __lldb_init_module(debugger, internal_dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f gdbremote.start_gdb_log start_gdb_log')
+debugger.HandleCommand(
+'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
+print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
Index: lldb/examples/python/gdb_disassemble.py
===
--- lldb/examples/python/gdb_disassemble.py
+++ lldb/examples/python/gdb_disassemble.py
@@ -21,6 +21,7 @@
 print("<%s + %-4u> 0x%x %8s  %s" % (name, inst_offset, inst_addr, inst.mnemonic, inst.operands))
 
 #

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399731.
kastiglione added a comment.

updates from reviewers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

Files:
  lldb/docs/use/python-reference.rst
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/delta.py
  lldb/examples/python/diagnose_unwind.py
  lldb/examples/python/gdb_disassemble.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/jump.py
  lldb/examples/python/memory.py
  lldb/examples/python/stacks.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,7 +351,7 @@
 continue
 verify_types(target, options)
 
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
 'command script add -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -63,6 +63,7 @@
 print(frame_info)
 
 
-lldb.debugger.HandleCommand(
-"command script add -f stacks.stack_frames stack_frames")
-print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+"command script add -f stacks.stack_frames stack_frames")
+print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -270,8 +270,9 @@
 
 if __name__ == '__main__':
 print('error: this script is designed to be used within the embedded script interpreter in LLDB')
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -191,8 +191,8 @@
 
 frame.SetPC(desired_address.GetLoadAddress(target))
 
-if lldb.debugger:
+def __lldb_init_module(debugger, internal_dict):
 # Module is being run inside the LLDB interpreter
 jump.__doc__ = usage_string()
-lldb.debugger.HandleCommand('command script add -f jump.jump jump')
+debugger.HandleCommand('command script add -f jump.jump jump')
 print('The "jump" command has been installed, type "help jump" or "jump " for detailed help.')
Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -1619,13 +1619,11 @@
 else:
 parse_gdb_log(sys.stdin, options)
 
-else:
-import lldb
-if lldb.debugger:
-# This initializer is being run from LLDB in the embedded command interpreter
-# Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.start_gdb_log start_gdb_log')
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
-print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
+def __lldb_init_module(debugger, internal_dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f gdbremote.start_gdb_log start_gdb_log')
+debugger.HandleCommand(
+'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
+print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
Index: lldb/examples/python/gdb_disassemble.py
===
--- lldb/examples/python/gdb_disassemble.py
+++ lldb/examples/python/gdb_disassemble.py
@@ -21,6 +21,7 @@
 print("<%s + %-4u> 0x%x %8s  %s" % (name, inst_offset, inst_addr, inst.mnemonic, inst.operands))
 
 # Install the command 

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/docs/use/python-reference.rst:586
 the module is loaded allowing you to add whatever commands you want into the
-current debugger. Note that this function will only be run when using the LLDB
-command command script import, it will not get run if anyone imports your
-module from another module. If you want to always run code when your module is
-loaded from LLDB or when loaded via an import statement in python code you can
-test the lldb.debugger object, since you imported the module at the top of the
-python ls.py module. This test must be in code that isn't contained inside of
-any function or class, just like the standard test for __main__ like all python
-modules usually do. Sample code would look like:
-
-::
-
-  if __name__ == '__main__':
-  # Create a new debugger instance in your module if your module
-  # can be run from the command line. When we run a script from
-  # the command line, we won't have any debugger object in
-  # lldb.debugger, so we can just create it if it will be needed
-  lldb.debugger = lldb.SBDebugger.Create()
-  elif lldb.debugger:
-  # Module is being run inside the LLDB interpreter
-  lldb.debugger.HandleCommand('command script add -f ls.ls ls')
-  print 'The "ls" python command has been installed and is ready for use.'
+current debugger.
 

JDevlieghere wrote:
> kastiglione wrote:
> > jingham wrote:
> > > I think it's okay to show the "if __name__  == '__main__': part of this 
> > > example, but it should be:
> > > 
> > > 
> > > ```
> > > if __name__ == '__main__':
> > >   # Create a new debugger instance in your module if your module
> > >   # can be run from the command line. When we run a script from
> > >   # the command line, we won't have any debugger object in
> > >   # lldb.debugger, so we can just create it if it will be needed
> > >   lldb.debugger = lldb.SBDebugger.Create()
> > >   # Now do whatever work this module would do when run as a command
> > >   # Now dispose of the debugger you just made.
> > >   lldb.SBDebugger.Destroy(debugger)
> > > 
> > > 
> > > ```That seems useful trick.
> > good point
> Let's also discourage people from setting `lldb.debugger` and instead do what 
> crashlog does:
> 
> ```
>   debugger = lldb.SBDebugger.Create()
>   # Now do whatever work this module would do when run as a command
>   # Now dispose of the debugger you just made.
>   lldb.SBDebugger.Destroy(debugger)
> ```
I just noticed that myself. Thanks, I will add the part about Destroy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

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

[Lldb-commits] [PATCH] D117165: [lldb] Add long help to `crashlog`

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb5ea132d2ee: [lldb] Add long help to `crashlog` (authored 
by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117165

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,22 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __init__(self, debugger, internal_dict):
+pass
+
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_short_help(self):
+return "Symbolicate one or more darwin crash log files."
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.format_help()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1197,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide 
source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the 
location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1206,10 @@
 created that has all of the shared libraries loaded at the load addresses 
found in the crash log file. This allows
 you to explore the program as if it were stopped at the locations described in 
the crash log and functions can
 be disassembled and lookups can be performed using the addresses found in the 
crash log.'''
-option_parser = CreateSymbolicateCrashLogOptions(
-'crashlog', description, True)
+return CreateSymbolicateCrashLogOptions('crashlog', description, True)
+
+def SymbolicateCrashLogs(debugger, command_args):
+option_parser = CrashLogOptionParser()
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
@@ -1219,13 +1232,15 @@
 for crash_log_file in args:
 crash_log = CrashLogParser().parse(debugger, crash_log_file, 
options.verbose)
 SymbolicateCrashLog(crash_log, options)
+
 if __name__ == '__main__':
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 SymbolicateCrashLogs(debugger, sys.argv[1:])
 lldb.SBDebugger.Destroy(debugger)
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
-'command script add -f lldb.macosx.crashlog.Symbolicate crashlog')
-lldb.debugger.HandleCommand(
+
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
+debugger.HandleCommand(
 'command script add -f lldb.macosx.crashlog.save_crashlog 
save_crashlog')


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1003,11 +1003,22 @@
 result.PutCString("error: invalid target")
 
 
-def Symbolicate(debugger, command, result, dict):
-try:
-SymbolicateCrashLogs(debugger, shlex.split(command))
-except Exception as e:
-result.PutCString("error: python exception: %s" % e)
+class Symbolicate:
+def __init__(self, debugger, internal_dict):
+pass
+
+def __call__(self, debugger, command, exe_ctx, result):
+try:
+SymbolicateCrashLogs(debugger, shlex.split(command))
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
+
+def get_short_help(self):
+return "Symbolicate one or more darwin crash log files."
+
+def get_long_help(self):
+option_parser = CrashLogOptionParser()
+return option_parser.format_help()
 
 
 def SymbolicateCrashLog(crash_log, options):
@@ -1186,7 +1197,7 @@
 return option_parser
 
 
-def SymbolicateCrashLogs(debugger, command_args):
+def CrashLogOptionParser():
 description = '''Symbolicate one or more darwin crash log files to provide source file and line information,
 inlined stack frames back to the concrete functions, and disassemble the location of the crash
 for the first frame of the crashed thread.
@@ -1195,8 +1206,10 @@
 created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
 you to ex

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases to command-regex

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added a reviewer: jingham.
Herald added a subscriber: jeroen.dobbelaere.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Allow users to create aliases which invoke aliases to command-regex. That
probably sounds convoluted, so here's an example:

  command alias some-setup env SOMEVAR=SOMEVALUE

This an alias that invokes `env`, which is an alias for `_regex-env`, and
`_regex-env` is a `command regex`.

The above command fails with:

  error: Unable to create requested alias.

This change allows such aliases to be created.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 399835.
kastiglione added a comment.

add @clayborg's improvements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

Files:
  lldb/docs/use/python-reference.rst
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/delta.py
  lldb/examples/python/diagnose_unwind.py
  lldb/examples/python/gdb_disassemble.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/jump.py
  lldb/examples/python/memory.py
  lldb/examples/python/stacks.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,7 +351,7 @@
 continue
 verify_types(target, options)
 
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
 'command script add -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -63,6 +63,7 @@
 print(frame_info)
 
 
-lldb.debugger.HandleCommand(
-"command script add -f stacks.stack_frames stack_frames")
-print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+"command script add -f stacks.stack_frames stack_frames")
+print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -270,8 +270,9 @@
 
 if __name__ == '__main__':
 print('error: this script is designed to be used within the embedded script interpreter in LLDB')
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -191,8 +191,8 @@
 
 frame.SetPC(desired_address.GetLoadAddress(target))
 
-if lldb.debugger:
+def __lldb_init_module(debugger, internal_dict):
 # Module is being run inside the LLDB interpreter
 jump.__doc__ = usage_string()
-lldb.debugger.HandleCommand('command script add -f jump.jump jump')
+debugger.HandleCommand('command script add -f jump.jump jump')
 print('The "jump" command has been installed, type "help jump" or "jump " for detailed help.')
Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -1619,13 +1619,11 @@
 else:
 parse_gdb_log(sys.stdin, options)
 
-else:
-import lldb
-if lldb.debugger:
-# This initializer is being run from LLDB in the embedded command interpreter
-# Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.start_gdb_log start_gdb_log')
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
-print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
+def __lldb_init_module(debugger, internal_dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f gdbremote.start_gdb_log start_gdb_log')
+debugger.HandleCommand(
+'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
+print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
Index: lldb/examples/python/gdb_disassemble.py
===
--- lldb/examples/python/gdb_disassemble.py
+++ lldb/examples/python/gdb_disassemble.py
@@ -21,6 +21,7 @@
 print("<%s + %-4u> 0x%x %8s  %s" % (name, inst_offset, inst_addr, inst.mnemonic, inst.operands))
 
 # Install the co

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

kastiglione marked 2 inline comments as done.
kastiglione added a comment.

thanks @clayborg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

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

[Lldb-commits] [PATCH] D117237: [lldb] Use __lldb_init_module instead of "if lldb.debugger" idiom

[Dave Lee via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1441ffe6a6da: [lldb] Use __lldb_init_module instead of 
"if lldb.debugger" idiom (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117237

Files:
  lldb/docs/use/python-reference.rst
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/delta.py
  lldb/examples/python/diagnose_unwind.py
  lldb/examples/python/gdb_disassemble.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/jump.py
  lldb/examples/python/memory.py
  lldb/examples/python/stacks.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,7 +351,7 @@
 continue
 verify_types(target, options)
 
-elif getattr(lldb, 'debugger', None):
-lldb.debugger.HandleCommand(
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
 'command script add -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -63,6 +63,7 @@
 print(frame_info)
 
 
-lldb.debugger.HandleCommand(
-"command script add -f stacks.stack_frames stack_frames")
-print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
+def __lldb_init_module(debugger, internal_dict):
+debugger.HandleCommand(
+"command script add -f stacks.stack_frames stack_frames")
+print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -270,8 +270,9 @@
 
 if __name__ == '__main__':
 print('error: this script is designed to be used within the embedded script interpreter in LLDB')
-elif getattr(lldb, 'debugger', None):
+
+def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
-lldb.debugger.HandleCommand(
+debugger.HandleCommand(
 'command script add -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -191,8 +191,8 @@
 
 frame.SetPC(desired_address.GetLoadAddress(target))
 
-if lldb.debugger:
+def __lldb_init_module(debugger, internal_dict):
 # Module is being run inside the LLDB interpreter
 jump.__doc__ = usage_string()
-lldb.debugger.HandleCommand('command script add -f jump.jump jump')
+debugger.HandleCommand('command script add -f jump.jump jump')
 print('The "jump" command has been installed, type "help jump" or "jump " for detailed help.')
Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -1619,13 +1619,11 @@
 else:
 parse_gdb_log(sys.stdin, options)
 
-else:
-import lldb
-if lldb.debugger:
-# This initializer is being run from LLDB in the embedded command interpreter
-# Add any commands contained in this module to LLDB
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.start_gdb_log start_gdb_log')
-lldb.debugger.HandleCommand(
-'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
-print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
+def __lldb_init_module(debugger, internal_dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Add any commands contained in this module to LLDB
+debugger.HandleCommand(
+'command script add -f gdbremote.start_gdb_log start_gdb_log')
+debugger.HandleCommand(
+'command script add -f gdbremote.stop_gdb_log stop_gdb_log')
+print('The "start_gdb_log" and "stop_gdb_log" commands are now installed and ready for use, type "start_gdb_log --help" or "stop_gdb_log --help" for more information')
Index: lldb/examples/python/gdb_disassemble.py
===
--- lldb/examples/python/gdb_disassemble.py
+++ lldb/examples/python/gdb_disassemble.py

[Lldb-commits] [PATCH] D117383: [lldb] From unordered_map synthetic provider, return std::pair children

[Dave Lee via Phabricator via lldb-commits]

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

Change the behavior of the libc++ `unordered_map` synthetic provider to present
children as `std::pair` values, just like `std::map` does.

The synthetic provider for libc++ `std::unordered_map` has returned children
that expose a level of internal structure (over top of the key/value pair). For
example, given an unordered map initialized with `{{1,2}, {3, 4}}`, the output
is:

  (std::unordered_map, std::equal_to, 
std::allocator > >) map = size=2 {
[0] = {
  __cc = (first = 3, second = 4)
}
[1] = {
  __cc = (first = 1, second = 2)
}
  }

It's not ideal/necessary to have the numbered children embdedded in the `__cc`
field.

Note: the numbered children have type
`std::__hash_node, void *>::__node_value_type`,
and the `__cc` fields have type `std::__hash_value_type::value_type`.

Compare this output to `std::map`:

  (std::map, std::allocator 
> >) map = size=2 {
[0] = (first = 1, second = 2)
[1] = (first = 3, second = 4)

Where the numbered children have type `std::pair`.

This changes the behavior of the synthetic provider for `unordered_map` to also
present children as `pairs`, just like `std::map`.

It appears the synthetic provider implementation for `unordered_map` was meant
to provide this behavior, but was maybe incomplete (see
d22a94377f7554a7e9df050f6dfc3ee42384e3fe). It has both an `m_node_type` and an
`m_element_type`, but uses only the former. The latter is exactly the type
needed for the children pairs. With this existing code, it's not much of a
change to make this work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117383

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
@@ -48,12 +48,17 @@
 "corrupt_map", ['%s::unordered_map' %
 ns, 'size=0 {}'])
 
+must_not_contain__cc = r'(?s)^(?!.*\b__cc = )'
+
 self.look_for_content_and_continue(
-"map", ['%s::unordered_map' %
-ns, 'size=5 {', 'hello', 'world', 'this', 'is', 'me'])
+"map", ['%s::unordered_map' % ns,
+must_not_contain__cc,
+'size=5 {', 'hello', 'world', 'this', 'is', 'me'])
 
 self.look_for_content_and_continue(
-"mmap", ['%s::unordered_multimap' % ns, 'size=6 {', 'first = 3', 
'second = "this"',
+"mmap", ['%s::unordered_multimap' % ns,
+ must_not_contain__cc,
+ 'size=6 {', 'first = 3', 'second = "this"',
  'first = 2', 'second = "hello"'])
 
 self.look_for_content_and_continue(
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -118,10 +118,16 @@
 m_element_type = m_element_type.GetPointeeType();
 m_node_type = m_element_type;
 m_element_type = m_element_type.GetTypeTemplateArgument(0);
-std::string name;
-m_element_type =
-m_element_type.GetFieldAtIndex(0, name, nullptr, nullptr, nullptr);
-m_element_type = m_element_type.GetTypedefedType();
+if (m_element_type.GetNumFields() > 0) {
+ // Check for - and use - the underlying key/value container type used
+ // within std::unordered_map.
+  std::string name;
+  auto key_value_type =
+  m_element_type.GetFieldAtIndex(0, name, nullptr, nullptr, 
nullptr);
+  // __cc is a field of __hash_value_type, which is a typedef.
+  if (name == "__cc")
+m_element_type = key_value_type.GetTypedefedType();
+}
   }
   if (!m_node_type)
 return nullptr;
@@ -153,7 +159,7 @@
   ExecutionContext exe_ctx = val_hash.first->GetExecutionContextRef().Lock(
   thread_and_frame_only_if_stopped);
   return CreateValueObjectFromData(stream.GetString(), data, exe_ctx,
-   val_hash.first->GetCompilerType());
+   m_element_type);
 }
 
 bool lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::


Index: lldb/test/API/f

[Lldb-commits] [PATCH] D117383: [lldb] From unordered_map synthetic provider, return std::pair children

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 400234.
kastiglione added a comment.
Herald added a subscriber: JDevlieghere.

some tabs snuck in


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117383

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
@@ -48,12 +48,17 @@
 "corrupt_map", ['%s::unordered_map' %
 ns, 'size=0 {}'])
 
+must_not_contain__cc = r'(?s)^(?!.*\b__cc = )'
+
 self.look_for_content_and_continue(
-"map", ['%s::unordered_map' %
-ns, 'size=5 {', 'hello', 'world', 'this', 'is', 'me'])
+"map", ['%s::unordered_map' % ns,
+must_not_contain__cc,
+'size=5 {', 'hello', 'world', 'this', 'is', 'me'])
 
 self.look_for_content_and_continue(
-"mmap", ['%s::unordered_multimap' % ns, 'size=6 {', 'first = 3', 
'second = "this"',
+"mmap", ['%s::unordered_multimap' % ns,
+ must_not_contain__cc,
+ 'size=6 {', 'first = 3', 'second = "this"',
  'first = 2', 'second = "hello"'])
 
 self.look_for_content_and_continue(
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -118,10 +118,16 @@
 m_element_type = m_element_type.GetPointeeType();
 m_node_type = m_element_type;
 m_element_type = m_element_type.GetTypeTemplateArgument(0);
-std::string name;
-m_element_type =
-m_element_type.GetFieldAtIndex(0, name, nullptr, nullptr, nullptr);
-m_element_type = m_element_type.GetTypedefedType();
+if (m_element_type.GetNumFields() > 0) {
+  // Check for - and use - the underlying key/value container type used
+  // within std::unordered_map.
+  std::string name;
+  auto key_value_type =
+  m_element_type.GetFieldAtIndex(0, name, nullptr, nullptr, 
nullptr);
+  // __cc is a field of __hash_value_type, which is a typedef.
+  if (name == "__cc")
+m_element_type = key_value_type.GetTypedefedType();
+}
   }
   if (!m_node_type)
 return nullptr;
@@ -153,7 +159,7 @@
   ExecutionContext exe_ctx = val_hash.first->GetExecutionContextRef().Lock(
   thread_and_frame_only_if_stopped);
   return CreateValueObjectFromData(stream.GetString(), data, exe_ctx,
-   val_hash.first->GetCompilerType());
+   m_element_type);
 }
 
 bool lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
@@ -48,12 +48,17 @@
 "corrupt_map", ['%s::unordered_map' %
 ns, 'size=0 {}'])
 
+must_not_contain__cc = r'(?s)^(?!.*\b__cc = )'
+
 self.look_for_content_and_continue(
-"map", ['%s::unordered_map' %
-ns, 'size=5 {', 'hello', 'world', 'this', 'is', 'me'])
+"map", ['%s::unordered_map' % ns,
+must_not_contain__cc,
+'size=5 {', 'hello', 'world', 'this', 'is', 'me'])
 
 self.look_for_content_and_continue(
-"mmap", ['%s::unordered_multimap' % ns, 'size=6 {', 'first = 3', 'second = "this"',
+"mmap", ['%s::unordered_multimap' % ns,
+ must_not_contain__cc,
+ 'size=6 {', 'first = 3', 'second = "this"',
  'first = 2', 'second = "hello"'])
 
 self.look_for_content_and_continue(
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp

[Lldb-commits] [PATCH] D117383: [lldb] From unordered_map synthetic provider, return std::pair children

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp:128
+  // __cc is a field of __hash_value_type, which is a typedef.
+  if (name == "__cc")
+m_element_type = key_value_type.GetTypedefedType();

This block of changes is needed to support `unordered_set`, because this 
synthetic provider is shared by `unordered_set` and `unordered_map`.

For `unordered_map`, the element type is an internal hash node. For 
`unoredered_set`, it's the type `T` from `unordered_set`. This part of the 
code needs to know whether there's a hash node here, and to step down through 
the `__cc` field.

I am not sure what the best way to do this is. I could do a string prefix check 
against `std::__hash_value_type`. I could do a smarter full string equality 
check, but could that possibly have false negatives if the wrong string type 
name is constructed. As it currently is, I assume if there's a one or more 
fields, and if the first one is named `__cc`, then it's assumed to be an 
internal hash node.

Suggestions welcome.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp:162
   return CreateValueObjectFromData(stream.GetString(), data, exe_ctx,
-   val_hash.first->GetCompilerType());
+   m_element_type);
 }

This change is the fix needed to get `unordered_map` working.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py:51
 
+must_not_contain__cc = r'(?s)^(?!.*\b__cc = )'
+

This regex checks that the string `__cc = ` is not in the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117383

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

[Lldb-commits] [PATCH] D116863: [lldb] Remove anon struct from frame-var-anon-unions test

[Dave Lee via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7e7f541c081: [lldb] Remove anon struct from 
frame-var-anon-unions test (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116863

Files:
  
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
  lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ 
lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])


Index: lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/main.cpp
@@ -3,11 +3,6 @@
 int i;
 char c;
   };
-  struct {
-int x;
-char y;
-short z;
-  } s{3,'B',14};
   i = 0xFF00;
   c = 'A';
   return c; // break here
Index: lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
===
--- lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
+++ lldb/test/API/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py
@@ -29,7 +29,3 @@
 self.expect('frame variable -f x i', substrs=['4100'])
 
 self.expect('frame variable c', substrs=["'A"])
-
-self.expect('frame variable x', matching=False, substrs=['3'])
-self.expect('frame variable y', matching=False, substrs=["'B'"])
-self.expect('frame variable z', matching=False, substrs=['14'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117837: [lldb] Fix timer logging inverted quiet condition

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, labath.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The logic of `g_quiet` was inverted in D26243 
. This corrects the issue.

Without this, running `log timers enable` produces a high volume of incremental
timer output.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117837

Files:
  lldb/source/Utility/Timer.cpp


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Dave Lee via Phabricator via lldb-commits]

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

nice


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

I find "Unallocated" ambiguous. To the Allocator, maybe it's not, but to VM it 
is. What about Total/UsedUnused? Note that the docs for `BytesAllocated` say it 
can be used to calculated "wasted" space.

  /// How many bytes we've allocated.
  ///
  /// Used so that we can compute how much space was wasted.
  size_t BytesAllocated = 0;


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117837: [lldb] Fix timer logging inverted quiet condition

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58ee14e29e98: [lldb] Fix timer logging inverted quiet 
condition (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117837

Files:
  lldb/source/Utility/Timer.cpp


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases to command-regex

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 402132.
kastiglione added a comment.

add a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias ls', check=False)
+self.runCmd('command unalias sh', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,7 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias sh platform shell -h --')
+self.expect('command alias ls sh ls')
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias ls', check=False)
+self.runCmd('command unalias sh', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,7 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias sh platform shell -h --')
+self.expect('command alias ls sh ls')
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases of raw input commands

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 402143.
kastiglione edited the summary of this revision.
kastiglione added a comment.

use Jim's testing improvement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases of raw input commands

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb95150418fb6: [lldb] Allow aliases to aliases of raw input 
commands (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D118494: [lldb] Observe SG_READ_ONLY flag in MachO binaries

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: aprantl, augusto2112.
kastiglione requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

Change `Section::GetPermissions()` to indicate non-writability when the Mach-O
segment has the `SG_READ_ONLY` flag.

MachO has a segment flag named `SG_READ_ONLY` which indicates that the segment
is semantically read-only, even if its permissions are marked writable.
Segments such as `__DATA_CONST` contain data that is semantically "const", but
needs to be writable for dyld to perform fixups (bindings or rebases).

The header documentation for `SG_READ_ONLY` states:

> This segment is made read-only after fixups.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118494

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  llvm/include/llvm/BinaryFormat/MachO.h


Index: llvm/include/llvm/BinaryFormat/MachO.h
===
--- llvm/include/llvm/BinaryFormat/MachO.h
+++ llvm/include/llvm/BinaryFormat/MachO.h
@@ -106,6 +106,7 @@
   SG_FVMLIB = 0x2u,
   SG_NORELOC = 0x4u,
   SG_PROTECTED_VERSION_1 = 0x8u,
+  SG_READ_ONLY = 0x10u,
 
   // Constant masks for the "flags" field in llvm::MachO::section and
   // llvm::MachO::section_64
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1433,7 +1433,7 @@
   uint32_t result = 0;
   if (seg_cmd.initprot & VM_PROT_READ)
 result |= ePermissionsReadable;
-  if (seg_cmd.initprot & VM_PROT_WRITE)
+  if ((seg_cmd.initprot & VM_PROT_WRITE) && !(seg_cmd.flags & SG_READ_ONLY))
 result |= ePermissionsWritable;
   if (seg_cmd.initprot & VM_PROT_EXECUTE)
 result |= ePermissionsExecutable;


Index: llvm/include/llvm/BinaryFormat/MachO.h
===
--- llvm/include/llvm/BinaryFormat/MachO.h
+++ llvm/include/llvm/BinaryFormat/MachO.h
@@ -106,6 +106,7 @@
   SG_FVMLIB = 0x2u,
   SG_NORELOC = 0x4u,
   SG_PROTECTED_VERSION_1 = 0x8u,
+  SG_READ_ONLY = 0x10u,
 
   // Constant masks for the "flags" field in llvm::MachO::section and
   // llvm::MachO::section_64
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1433,7 +1433,7 @@
   uint32_t result = 0;
   if (seg_cmd.initprot & VM_PROT_READ)
 result |= ePermissionsReadable;
-  if (seg_cmd.initprot & VM_PROT_WRITE)
+  if ((seg_cmd.initprot & VM_PROT_WRITE) && !(seg_cmd.flags & SG_READ_ONLY))
 result |= ePermissionsWritable;
   if (seg_cmd.initprot & VM_PROT_EXECUTE)
 result |= ePermissionsExecutable;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D118494: [lldb] Observe SG_READ_ONLY flag in MachO binaries

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1436
 result |= ePermissionsReadable;
-  if (seg_cmd.initprot & VM_PROT_WRITE)
+  if ((seg_cmd.initprot & VM_PROT_WRITE) && !(seg_cmd.flags & SG_READ_ONLY))
 result |= ePermissionsWritable;

augusto2112 wrote:
> Could we add a new value in the enumeration? Something like 
> ePermissionsLinkerWritable? As it is right now this would be dangerous for 
> the existing file-cache optimization as we'd happily read pointers that are 
> supposed to be fixed by the linker from the file-cache.
That works for me. I think we'd want `ePermissionsLoaderWritable`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118494

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