[Lldb-commits] [lldb] 6eb40bf - [test] Fix a test that wasn't running

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2022-10-12T20:43:46-07:00
New Revision: 6eb40bf51b768672218539935f60ce55ae6ad750

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

LOG: [test] Fix a test that wasn't running

The functionality is fine (we don't run the breakpoint command twice), but this 
test forgot to call `FileCheck` and isn't checking the same value isn't there 
twice..

Added: 


Modified: 
lldb/test/Shell/Breakpoint/breakpoint-command.test

Removed: 




diff  --git a/lldb/test/Shell/Breakpoint/breakpoint-command.test 
b/lldb/test/Shell/Breakpoint/breakpoint-command.test
index 6104713cde5ae..1236c557a0232 100644
--- a/lldb/test/Shell/Breakpoint/breakpoint-command.test
+++ b/lldb/test/Shell/Breakpoint/breakpoint-command.test
@@ -1,5 +1,5 @@
 # RUN: %build %p/Inputs/dummy-target.c -o %t.out
-# RUN: %lldb %t.out -o 'b main' -o 'break command add 1 -o "script print(95000 
+ 126)"' -o 'r'
+# RUN: %lldb %t.out -o 'b main' -o 'break command add 1 -o "script print(95000 
+ 126)"' -o 'r' | FileCheck %s
 
-# CHECK: 95125
+# CHECK: 95126
 # CHECK-NOT: 95126



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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

[J. Ryan Stinnett via Phabricator via lldb-commits]

jryans added a comment.

Exciting, welcome to the LLVM community! 

Overall your commit looks okay, but here are a few tips for the future... The 
"Summary:" prefix in the commit title is not needed. It's good to start the 
commit title with an active verb e.g. "Add foo to bar". You can optionally use 
prefix tags like "[Docs]" to suggest the area. I recommend looking over other 
commit messages (https://github.com/llvm/llvm-project/commits/main) to get a 
feel for the typical style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

[Henrique Bucher via Phabricator via lldb-commits]

HenriqueBucher added a comment.

Got a very nice email from Chris Lattner with the approval. Very nice.
Dude I took the plunge and did it. I hope it went through right. Please let me 
know if I screwed it up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

[Henrique Bucher via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a52c5c42669: Summary: This documentation patch adds 
information to allow remote users to… (authored by HenriqueBucher).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

Files:
  lldb/tools/lldb-vscode/README.md


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable 
when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch 
configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations).
 This file


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for users logged in locally to the machine. If you are remoting into the box using Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for extensions on `~/.vscode-server/extensions` only and you will not see your just installed lldb-vscode plug-in. If you want this plugin to be visible to remoting users, you will need to either repeat the process above for the `~/.vscode-server` folder or create a symbolic link from it to `~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5a52c5c - Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

[Henrique Bucher via lldb-commits]

Author: Henrique Bucher
Date: 2022-10-12T19:29:36-05:00
New Revision: 5a52c5c42669b3e547c9ca69b59b560d0268b85c

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

LOG: Summary: This documentation patch adds information to allow remote users 
to also use the plugin as it will be invisible to them using the current 
instructions. It solves issue #58252.

This documentation patch adds information to allow remote users to also use the 
plugin as it will be invisible to them using the current instructions. It 
solves issue #58252.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/tools/lldb-vscode/README.md

Removed: 




diff  --git a/lldb/tools/lldb-vscode/README.md 
b/lldb/tools/lldb-vscode/README.md
index 70ca245f85b92..f82293daa5128 100644
--- a/lldb/tools/lldb-vscode/README.md
+++ b/lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@ $ cp /path/to/a/built/lldb-vscode .
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 
@@ -61,6 +67,8 @@ $ ln -s /path/to/a/built/lldb-vscode
 
 This is handy if you want to debug and develope the `lldb-vscode` executable 
when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch 
configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations).
 This file



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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

[Henrique Bucher via Phabricator via lldb-commits]

HenriqueBucher updated this revision to Diff 467313.
HenriqueBucher retitled this revision from "Summary:" to "Summary: This 
documentation patch adds information to allow remote users to also use the 
plugin as it will be invisible to them using the current instructions. It 
solves issue #58252.".
HenriqueBucher added a comment.

The current documentation shows how to install the lldb-vscode plugin as a 
local user. 
When they remote, the plugin will be invisible.
This documentation patch adds information on how to extend the plugin to remote 
users as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

Files:
  lldb/tools/lldb-vscode/README.md


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable 
when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch 
configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations).
 This file


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for users logged in locally to the machine. If you are remoting into the box using Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for extensions on `~/.vscode-server/extensions` only and you will not see your just installed lldb-vscode plug-in. If you want this plugin to be visible to remoting users, you will need to either repeat the process above for the `~/.vscode-server` folder or create a symbolic link from it to `~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135577: Summary:

[Henrique Bucher via Phabricator via lldb-commits]

HenriqueBucher updated this revision to Diff 467310.
HenriqueBucher added a comment.

Visual Studio code has different folders for plugins when used by local users 
and users remoting into the box. 
The current instructions show how to install the lldb-vscode plugin only for 
the local users and it will be invisible to them if they remote. 
This patch adds instructions on how to install the plugin for remote users too. 
It solves issue #58252.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

Files:
  lldb/tools/lldb-vscode/README.md


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable 
when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch 
configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations).
 This file


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for users logged in locally to the machine. If you are remoting into the box using Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for extensions on `~/.vscode-server/extensions` only and you will not see your just installed lldb-vscode plug-in. If you want this plugin to be visible to remoting users, you will need to either repeat the process above for the `~/.vscode-server` folder or create a symbolic link from it to `~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135827: [lldb] Print newline between found types

[Arthur Eubanks via Phabricator via lldb-commits]

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

(depends on https://reviews.llvm.org/D135826 for the test to be meaningful)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135827

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

[Lldb-commits] [PATCH] D135827: [lldb] Print newline between found types

[Arthur Eubanks via Phabricator via lldb-commits]

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

Or else multiple entries end up overlapping on the same line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135827

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/API/lang/cpp/type_lookup_duplicate/Makefile
  lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py
  lldb/test/API/lang/cpp/type_lookup_duplicate/main.cpp


Index: lldb/test/API/lang/cpp/type_lookup_duplicate/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/type_lookup_duplicate/main.cpp
@@ -0,0 +1,13 @@
+namespace a {
+struct Foo {};
+} // namespace a
+
+namespace b {
+struct Foo {};
+} // namespace b
+
+int main() {
+  a::Foo a;
+  b::Foo b;
+  return 0; // Set breakpoint here
+}
Index: 
lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py
@@ -0,0 +1,16 @@
+"""
+Test that we properly print multiple types.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import decorators
+
+class TestTypeLookupDuplicate(TestBase):
+
+def test_namespace_only(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", 
lldb.SBFileSpec("main.cpp"))
+
+self.expect("image lookup -A -t Foo", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["2 matches found", "\nid =", "\nid ="])
Index: lldb/test/API/lang/cpp/type_lookup_duplicate/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/type_lookup_duplicate/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1663,8 +1663,8 @@
 typedef_type_sp = typedefed_type_sp;
 typedefed_type_sp = typedef_type_sp->GetTypedefType();
   }
+  strm.EOL();
 }
-strm.EOL();
   }
   return type_list.GetSize();
 }


Index: lldb/test/API/lang/cpp/type_lookup_duplicate/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/type_lookup_duplicate/main.cpp
@@ -0,0 +1,13 @@
+namespace a {
+struct Foo {};
+} // namespace a
+
+namespace b {
+struct Foo {};
+} // namespace b
+
+int main() {
+  a::Foo a;
+  b::Foo b;
+  return 0; // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py
@@ -0,0 +1,16 @@
+"""
+Test that we properly print multiple types.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import decorators
+
+class TestTypeLookupDuplicate(TestBase):
+
+def test_namespace_only(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+self.expect("image lookup -A -t Foo", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["2 matches found", "\nid =", "\nid ="])
Index: lldb/test/API/lang/cpp/type_lookup_duplicate/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/type_lookup_duplicate/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1663,8 +1663,8 @@
 typedef_type_sp = typedefed_type_sp;
 typedefed_type_sp = typedef_type_sp->GetTypedefType();
   }
+  strm.EOL();
 }
-strm.EOL();
   }
   return type_list.GetSize();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135826: [lldb] Start from end of previous substr when checking ordered substrs

[Arthur Eubanks via Phabricator via lldb-commits]

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

I'm trying to add a test which tests that the same substr occurs twice in a 
row, but it matches even if only one of the substr occurs.

This found a bug in concurrent_base.py.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135826

Files:
  lldb/packages/Python/lldbsuite/test/concurrent_base.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -116,7 +116,7 @@
 substrs=[
 '(char *) $',
 ' = ptr = ',
-' 
"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
+
'"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
 
 self.runCmd("type summary add -c TestPoint")
 
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2358,7 +2358,7 @@
 start = 0
 for substr in substrs:
 index = output[start:].find(substr)
-start = start + index if ordered and matching else 0
+start = start + index + len(substr) if ordered and matching 
else 0
 matched = index != -1
 log_lines.append("{} sub string: \"{}\" ({})".format(
 expecting_str, substr, found_str(matched)))
Index: lldb/packages/Python/lldbsuite/test/concurrent_base.py
===
--- lldb/packages/Python/lldbsuite/test/concurrent_base.py
+++ lldb/packages/Python/lldbsuite/test/concurrent_base.py
@@ -74,9 +74,7 @@
 bpno = lldbutil.run_break_set_by_file_and_line(
 self, self.filename, line, num_expected_locations=-1)
 bp = self.inferior_target.FindBreakpointByID(bpno)
-descriptions.append(
-": file = 'main.cpp', line = %d" %
-self.finish_breakpoint_line)
+descriptions.append(": file = 'main.cpp', line = %d" % line)
 return bp
 
 def inferior_done(self):


Index: lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -116,7 +116,7 @@
 substrs=[
 '(char *) $',
 ' = ptr = ',
-' "1234567890123456789012345678901234567890123456789012345678901234ABC"'])
+'"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
 
 self.runCmd("type summary add -c TestPoint")
 
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2358,7 +2358,7 @@
 start = 0
 for substr in substrs:
 index = output[start:].find(substr)
-start = start + index if ordered and matching else 0
+start = start + index + len(substr) if ordered and matching else 0
 matched = index != -1
 log_lines.append("{} sub string: \"{}\" ({})".format(
 expecting_str, substr, found_str(matched)))
Index: lldb/packages/Python/lldbsuite/test/concurrent_base.py
===
--- lldb/packages/Python/lldbsuite/test/concurrent_base.py
+++ lldb/packages/Python/lldbsuite/test/concurrent_base.py
@@ -74,9 +74,7 @@
 bpno = lldbutil.run_break_set_by_file_and_line(
 self, self.filename, line, num_expected_locations=-1)
 bp = self.inferior_target.FindBreakpointByID(bpno)
-descriptions.append(
-": file = 'main.cpp', line = %d" %
-self.finish_breakpoint_line)
+descriptions.append(": file = 'main.cpp', line = %d" % line)
 return bp
 
 def inferior_done(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

[Alex Langford via Phabricator via lldb-commits]

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

It's easy enough to disable the lldb-server build. The lldb-server unit tests 
already have logic to disable them if we don't build, so this just makes it 
even.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135825

Files:
  lldb/test/CMakeLists.txt
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test


Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,5 @@
 # Windows does not build lldb-server.
+# REQUIRES: lldb-server
 # UNSUPPORTED: system-windows
 # RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 
--max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than 
--max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,3 +1,5 @@
+REQUIRES: lldb-server
+
 RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
 FD1: error: --fd: missing argument
 
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", 
"lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -167,6 +167,7 @@
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_TOOL_LLDB_SERVER_BUILD
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 


Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,5 @@
 # Windows does not build lldb-server.
+# REQUIRES: lldb-server
 # UNSUPPORTED: system-windows
 # RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,3 +1,5 @@
+REQUIRES: lldb-server
+
 RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
 FD1: error: --fd: missing argument
 
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits 

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 467266.
zequanwu added a comment.

- Handle the case when anonymous struct inside an union.
- Append labath's changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
  lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt
  lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -0,0 +1,244 @@
+//===-- UdtRecordCompleterTests.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private::npdb;
+using namespace llvm;
+
+namespace {
+using Member = UdtRecordCompleter::Member;
+using MemberUP = std::unique_ptr;
+using Record = UdtRecordCompleter::Record;
+
+class WrappedMember {
+public:
+  WrappedMember(const Member ) : m_obj(obj) {}
+
+private:
+  const Member _obj;
+
+  friend bool operator==(const WrappedMember , const WrappedMember ) {
+return lhs.m_obj.kind == rhs.m_obj.kind &&
+   lhs.m_obj.name == rhs.m_obj.name &&
+   lhs.m_obj.bit_offset == rhs.m_obj.bit_offset &&
+   lhs.m_obj.bit_size == rhs.m_obj.bit_size &&
+   lhs.m_obj.base_offset == rhs.m_obj.base_offset &&
+   std::equal(lhs.m_obj.fields.begin(), lhs.m_obj.fields.end(),
+  rhs.m_obj.fields.begin(), rhs.m_obj.fields.end(),
+  [](const MemberUP , const MemberUP ) {
+return WrappedMember(*lhs) == WrappedMember(*rhs);
+  });
+  }
+
+  friend llvm::raw_ostream <<(llvm::raw_ostream ,
+   const WrappedMember ) {
+os << llvm::formatv("Member{.kind={0}, .name=\"{1}\", .bit_offset={2}, "
+".bit_size={3}, .base_offset={4}, .fields=[",
+w.m_obj.kind, w.m_obj.name, w.m_obj.bit_offset,
+w.m_obj.bit_size, w.m_obj.base_offset);
+llvm::ListSeparator sep;
+for (auto  : w.m_obj.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class WrappedRecord {
+public:
+  WrappedRecord(const Record ) : m_obj(obj) {}
+
+private:
+  const Record _obj;
+
+  friend bool operator==(const WrappedRecord , const WrappedRecord ) {
+return lhs.m_obj.start_offset == rhs.m_obj.start_offset &&
+   std::equal(
+   lhs.m_obj.record.fields.begin(), lhs.m_obj.record.fields.end(),
+   rhs.m_obj.record.fields.begin(), rhs.m_obj.record.fields.end(),
+   [](const MemberUP , const MemberUP ) {
+ return WrappedMember(*lhs) == WrappedMember(*rhs);
+   });
+  }
+
+  friend llvm::raw_ostream <<(llvm::raw_ostream ,
+   const WrappedRecord ) {
+os << llvm::formatv("Record{.start_offset={0}, .record.fields=[",
+w.m_obj.start_offset);
+llvm::ListSeparator sep;
+for (const MemberUP  : w.m_obj.record.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class UdtRecordCompleterRecordTests : public testing::Test {
+protected:
+  Record record;
+
+public:
+  void SetKind(Member::Kind kind) { record.record.kind = kind; }
+  void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) {
+record.CollectMember(name, byte_offset * 8, byte_size * 8,
+ clang::QualType(), lldb::eAccessPublic, 0);
+  }
+  void ConstructRecord() { record.ConstructRecord(); }
+};
+Member *AddField(Member *member, StringRef name, uint64_t byte_offset,
+ uint64_t byte_size, Member::Kind kind,
+ uint64_t base_offset = 0) {
+  auto field =
+  std::make_unique(name, byte_offset * 8, byte_size * 8,
+  

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459
+  uint64_t end_offset = offset + fields.back()->bit_size;
+  parent->fields.push_back(fields.back());
+  end_offset_map[end_offset].push_back(parent);

labath wrote:
> zequanwu wrote:
> > labath wrote:
> > > Can `parent` be a union here? Would the algorithm still be correct?
> > `parent` could only be union when the top level record is a union (at this 
> > line `Member *parent = `). That's the only case when we add an 
> > union into `end_offset_map`. In that case, all the fields would start at 
> > the same offset and we only iterate the loop `for (auto  : fields) {` 
> > once and then we are done. 
> > Otherwise, we only insert {end offset, struct/field} into `end_offset_map` 
> > where field must be within an union.
> Are you sure about that? I've created what I think is a [[ 
> https://reviews.llvm.org/D135768 | counterexample ]] to these statements. 
> Here a top-level union contains a bunch of sequential fields, which is 
> perfectly possible if the only member of that union is an anonymous struct 
> which contains those fields.
> 
> I don't think that's what supposed to happen in this case, but I'm open to 
> being convinced otherwise.
> 
> (I've also rewritten the test logic so it produces better error messages than 
> "false is not true".)
Thanks for writing the counter example and making better error message. I 
appended your changes in this file.
I updated to handle the case when parent is an union here by creating an 
anonymous struct to hold it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Target/Statistics.cpp:305
+std::error_code ec;
+raw_fd_ostream stats_stream(stat_file.GetPath(), ec, sys::fs::OF_None);
+if (ec)

clayborg wrote:
> What happens if the file already exists here? Will this return an error? I am 
> guessing we are passing in a temp directory for the logs
The file will get overwritten. My reasoning was that maybe you generated the 
statistics and then you were like oh wait an error showed up after I did a 
step, let me regenerate them. Alternatives are:

 - Erroring out if the file exists
 - Erroring out if any file exists in the diagnostic dir

I prefer that because we can check it once and give an actionable error to the 
user. Let me know what you think. 



Comment at: lldb/source/Target/Statistics.cpp:309
+
+stats_stream << ReportStatistics(*debugger_sp, nullptr);
+  }

clayborg wrote:
> There are many syscalls and OS APIs that aren't supposed to be called during 
> a interrupt handler, and this function might call many of them. Deadlocks are 
> a serious concern here as well since any thread might have a mutex locked 
> that could stop the statistics reporting in its tracks since the program was 
> asynchronously interrupted. If this does happen this process will deadlock 
> forever and not be able to respond. We might need a bool parameter do 
> "ReportStatistics" like "bool during_interrupt" and if this is true, we need 
> to try and lock any module mutexes and if they fail, don't report that 
> module. So the entire ReportStatistics function will need to be checked for 
> potential deadlocks, mostly the Module mutex.
I thought almost everything in the statistics was computed upfront, but if 
that's not the case then we need to be careful. I picked the statistics as an 
example of some data that can be emitted here. I don't want to block the 
broader infrastructure on that so I'll hoist it into a separate patch, similar 
to the ones for the logs. 



Comment at: lldb/source/Utility/Diagnostics.cpp:43
+  std::lock_guard guard(m_callbacks_mutex);
+  m_callbacks.push_back(callback);
+}

DavidSpickett wrote:
> DavidSpickett wrote:
> > Is it worth adding an assert that the callback is not already in the list?
> > 
> > (and below, that the callback is in fact in the list)
> Is this still relevant?
Not anymore, you cannot compare std::functions. 


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

https://reviews.llvm.org/D134991

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

[Lldb-commits] [PATCH] D135798: Add runToBinaryEntry option for lldb-vscode

[Greg Clayton via Phabricator via lldb-commits]

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

Woops, forgot, you will need to modify the package.json to document the new 
launch config options


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135798

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

[Lldb-commits] [PATCH] D135798: Add runToBinaryEntry option for lldb-vscode

[Greg Clayton via Phabricator via lldb-commits]

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

Fix the comment as suggested and good to go. Also please test on both macOS and 
linux to ensure this works on both prior to pushing the diff, I often have 
issues on either side with lldb-vscode modifications.




Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1647
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry failed: can't place the breakpoint");
+return error;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135798

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

[Lldb-commits] [PATCH] D135798: Add runToBinaryEntry option for lldb-vscode

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere, 
aadsm, kusmour, fixathon.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a new runToBinaryEntry option which sets a one-shot breakpoint
at program entry. This option is useful for synchronizing module loading with
dynamic loader to measure debugger startup performance:  when program entry 
one-short breakpoint hits most of the dependency modules should have been
loaded so this provides a good sample point for debugger startup time.

More explicitly for lldb-vscode, when this option is enabled, "Initialized" DAP
event is synchronously sent after most dependency modules are loaded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135798

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1610,6 +1610,81 @@
  error.GetCString());
 }
 
+lldb::SBError RunToBinaryEntry() {
+  lldb::SBError error;
+  if (!g_vsc.run_to_binary_entry)
+return error;
+
+  if (g_vsc.stop_at_entry) {
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry is ignored due to StopOnEntry");
+return error;
+  }
+
+  lldb::SBTarget target = g_vsc.debugger.GetSelectedTarget();
+  if (!target.IsValid())
+return error;
+  lldb::SBFileSpec exe_file = target.GetExecutable();
+  if (!exe_file.IsValid())
+return error;
+  lldb::SBModule exe_module = target.FindModule(exe_file);
+  if (!exe_module.IsValid()) {
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry failed: invalid executable module");
+return error;
+  }
+
+  lldb::SBAddress entry_point = exe_module.GetObjectFileEntryPointAddress();
+  if (!entry_point.IsValid()) {
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry failed: can't find entry point");
+return error;
+  }
+  lldb::SBBreakpoint entry_breakpoint =
+  target.BreakpointCreateBySBAddress(entry_point);
+  if (!entry_breakpoint.IsValid() || entry_breakpoint.GetNumLocations() == 0) {
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry failed: can't place the breakpoint");
+return error;
+  }
+  entry_breakpoint.SetOneShot(true);
+
+  // Holder class to ensure async flag is restored during unwinding.
+  class DebuggerAsyncFlagHolder {
+bool m_old_async_flag;
+  public:
+DebuggerAsyncFlagHolder() {
+  m_old_async_flag = g_vsc.debugger.GetAsync();
+}
+
+~DebuggerAsyncFlagHolder() {
+  g_vsc.debugger.SetAsync(m_old_async_flag);
+}
+  };
+
+  // Synchronously run to binary entry one time breakpoint and scoped to restore
+  // the async flag.
+  {
+DebuggerAsyncFlagHolder _asyncFlagHolder;
+g_vsc.debugger.SetAsync(false);
+error = target.GetProcess().Continue();
+if (error.Fail())
+  return error;
+  }
+
+  // Successfully got a process stop; we still need to check if the stop is what
+  // we expected.
+  if (entry_breakpoint.GetHitCount() == 0)
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry failed: process stopped not at the "
+ "binary's entry point");
+  else
+g_vsc.SendOutput(OutputType::Console,
+ "RunToBinaryEntry success: Process stopped successfully "
+ "at the binary's entry point");
+  return error;
+}
+
 // "LaunchRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 // "type": "object",
@@ -1659,6 +1734,7 @@
   std::vector postRunCommands =
   GetStrings(arguments, "postRunCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
+  g_vsc.run_to_binary_entry = GetBoolean(arguments, "runToBinaryEntry", false);
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
   const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
 
@@ -1741,6 +1817,9 @@
 error = g_vsc.WaitForProcessToStop(timeout_seconds);
   }
 
+  if (error.Success())
+error = RunToBinaryEntry();
+
   if (error.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", std::string(error.GetCString()));
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -144,6 +144,7 @@
   lldb::tid_t 

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

In D134878#3836840 , @aaron.ballman 
wrote:

> Pinging reviewers from projects other than libcxx (I'm hoping to get buy-in 
> from someone on the LLVM side of things; lldb would be nice as well).

I'll be landing this later today on the assumption that folks are comfortable 
with the changes here, so if you have concerns, bringing them up now would be 
appreciated.


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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D133530: [lldb] Add zstd support

[Denis Revunov via Phabricator via lldb-commits]

treapster added a comment.

It seems like the new code doesn't check zstd version, thus breaking the build 
if you have something below 1.4.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133530

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

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459
+  uint64_t end_offset = offset + fields.back()->bit_size;
+  parent->fields.push_back(fields.back());
+  end_offset_map[end_offset].push_back(parent);

zequanwu wrote:
> labath wrote:
> > Can `parent` be a union here? Would the algorithm still be correct?
> `parent` could only be union when the top level record is a union (at this 
> line `Member *parent = `). That's the only case when we add an union 
> into `end_offset_map`. In that case, all the fields would start at the same 
> offset and we only iterate the loop `for (auto  : fields) {` once and 
> then we are done. 
> Otherwise, we only insert {end offset, struct/field} into `end_offset_map` 
> where field must be within an union.
Are you sure about that? I've created what I think is a [[ 
https://reviews.llvm.org/D135768 | counterexample ]] to these statements. Here 
a top-level union contains a bunch of sequential fields, which is perfectly 
possible if the only member of that union is an anonymous struct which contains 
those fields.

I don't think that's what supposed to happen in this case, but I'm open to 
being convinced otherwise.

(I've also rewritten the test logic so it produces better error messages than 
"false is not true".)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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

[Lldb-commits] [PATCH] D135768: Counterexample for D134849

[Pavel Labath via Phabricator via lldb-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135768

Files:
  lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
===
--- lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
+++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -15,38 +15,80 @@
 
 namespace {
 using Member = UdtRecordCompleter::Member;
-using Record = UdtRecordCompleter::Record;
 using MemberUP = std::unique_ptr;
+using Record = UdtRecordCompleter::Record;
+
+class WrappedMember {
+public:
+  WrappedMember(const Member ) : m_obj(obj) {}
+
+private:
+  const Member _obj;
+
+  friend bool operator==(const WrappedMember , const WrappedMember ) {
+return lhs.m_obj.kind == rhs.m_obj.kind &&
+   lhs.m_obj.name == rhs.m_obj.name &&
+   lhs.m_obj.bit_offset == rhs.m_obj.bit_offset &&
+   lhs.m_obj.bit_size == rhs.m_obj.bit_size &&
+   lhs.m_obj.base_offset == rhs.m_obj.base_offset &&
+   std::equal(lhs.m_obj.fields.begin(), lhs.m_obj.fields.end(),
+  rhs.m_obj.fields.begin(), rhs.m_obj.fields.end(),
+  [](const MemberUP , const MemberUP ) {
+return WrappedMember(*lhs) == WrappedMember(*rhs);
+  });
+  }
+
+  friend llvm::raw_ostream <<(llvm::raw_ostream ,
+   const WrappedMember ) {
+os << llvm::formatv("Member{.kind={0}, .name=\"{1}\", .bit_offset={2}, "
+".bit_size={3}, .base_offset={4}, .fields=[",
+w.m_obj.kind, w.m_obj.name, w.m_obj.bit_offset,
+w.m_obj.bit_size, w.m_obj.base_offset);
+llvm::ListSeparator sep;
+for (auto  : w.m_obj.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class WrappedRecord {
+public:
+  WrappedRecord(const Record ) : m_obj(obj) {}
+
+private:
+  const Record _obj;
+
+  friend bool operator==(const WrappedRecord , const WrappedRecord ) {
+return lhs.m_obj.start_offset == rhs.m_obj.start_offset &&
+   std::equal(
+   lhs.m_obj.record.fields.begin(), lhs.m_obj.record.fields.end(),
+   rhs.m_obj.record.fields.begin(), rhs.m_obj.record.fields.end(),
+   [](const MemberUP , const MemberUP ) {
+ return WrappedMember(*lhs) == WrappedMember(*rhs);
+   });
+  }
+
+  friend llvm::raw_ostream <<(llvm::raw_ostream ,
+   const WrappedRecord ) {
+os << llvm::formatv("Record{.start_offset={0}, .record.fields=[",
+w.m_obj.start_offset);
+llvm::ListSeparator sep;
+for (const MemberUP  : w.m_obj.record.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
 
 class UdtRecordCompleterRecordTests : public testing::Test {
+protected:
   Record record;
 
-public:
   void SetKind(Member::Kind kind) { record.record.kind = kind; }
   void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) {
 record.CollectMember(name, byte_offset * 8, byte_size * 8,
  clang::QualType(), lldb::eAccessPublic, 0);
   }
   void ConstructRecord() { record.ConstructRecord(); }
-
-  static bool VerifyMembers(const llvm::SmallVector ,
-const llvm::SmallVector ) {
-if (lhs.size() != rhs.size())
-  return false;
-for (size_t i = 0; i < lhs.size(); ++i) {
-  if (lhs[i]->kind != rhs[i]->kind || lhs[i]->name != rhs[i]->name ||
-  lhs[i]->bit_offset != rhs[i]->bit_offset ||
-  lhs[i]->bit_size != rhs[i]->bit_size ||
-  lhs[i]->base_offset != rhs[i]->base_offset ||
-  !VerifyMembers(lhs[i]->fields, rhs[i]->fields))
-return false;
-}
-return true;
-  }
-  bool VerifyRecord(Record ) {
-return record.start_offset == testRecord.start_offset &&
-   VerifyMembers(record.record.fields, testRecord.record.fields);
-  }
 };
 Member *AddField(Member *member, StringRef name, uint64_t byte_offset,
  uint64_t byte_size, Member::Kind kind,
@@ -84,7 +126,7 @@
   AddField(u, "m2", 0, 4, Member::Field);
   AddField(u, "m3", 0, 1, Member::Field);
   AddField(u, "m4", 0, 8, Member::Field);
-  EXPECT_TRUE(VerifyRecord(record));
+  EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record));
 }
 
 TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInUnion) {
@@ -107,7 +149,30 @@
   AddField(, "m2", 0, 4, Member::Field);
   AddField(, "m3", 0, 1, Member::Field);
   AddField(, "m4", 0, 8, Member::Field);
-  EXPECT_TRUE(VerifyRecord(record));
+  

[Lldb-commits] [lldb] 0205aa4 - [lldb] Fix member access in GetExpressionPath

[Andy Yankovsky via lldb-commits]

Author: Tonko Sabolčec
Date: 2022-10-12T12:08:57Z
New Revision: 0205aa4a02570dfeda5807f66756ebdbb102744b

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

LOG: [lldb] Fix member access in GetExpressionPath

This change fixes two issues in ValueObject::GetExpressionPath method:

 1. Accessing members of struct references used to produce expression
paths such as "str." (instead of the expected
"str.member"). This is fixed by assigning the flag tha the child
value is a dereference when calling Dereference() on references
and adjusting logic in expression path creation.

 2. If the parent of member access is dereference, the produced
expression path was "*(ptr).member". This is incorrect, since it
dereferences the member instead of the pointer. This is fixed by
wrapping dereference expression into parenthesis, resulting with
"(*ptr).member".

Reviewed By: werat, clayborg

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

Added: 
lldb/test/API/python_api/expression_path/Makefile
lldb/test/API/python_api/expression_path/TestExpressionPath.py
lldb/test/API/python_api/expression_path/main.cpp

Modified: 
lldb/source/Core/ValueObject.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-synth/TestDataFormatterSynth.py

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 19d86bee40e1f..226a2c3f690f2 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -273,8 +273,6 @@ CompilerType ValueObject::MaybeCalculateCompleteType() {
   return compiler_type;
 }
 
-
-
 DataExtractor ::GetDataExtractor() {
   UpdateValueIfNeeded(false);
   return m_data;
@@ -409,8 +407,9 @@ ValueObject::GetChildAtIndexPath(llvm::ArrayRef 
idxs,
   return root;
 }
 
-lldb::ValueObjectSP ValueObject::GetChildAtIndexPath(
-  llvm::ArrayRef> idxs, size_t *index_of_error) {
+lldb::ValueObjectSP
+ValueObject::GetChildAtIndexPath(llvm::ArrayRef> idxs,
+ size_t *index_of_error) {
   if (idxs.size() == 0)
 return GetSP();
   ValueObjectSP root(GetSP());
@@ -1185,9 +1184,10 @@ bool ValueObject::DumpPrintableRepresentation(
   {
 Status error;
 lldb::WritableDataBufferSP buffer_sp;
-std::pair read_string = ReadPointedString(
-buffer_sp, error, 0, (custom_format == eFormatVectorOfChar) ||
- (custom_format == eFormatCharArray));
+std::pair read_string =
+ReadPointedString(buffer_sp, error, 0,
+  (custom_format == eFormatVectorOfChar) ||
+  (custom_format == eFormatCharArray));
 lldb_private::formatters::StringPrinter::
 ReadBufferAndDumpToStreamOptions options(*this);
 options.SetData(DataExtractor(
@@ -1552,8 +1552,7 @@ bool ValueObject::GetDeclaration(Declaration ) {
   return false;
 }
 
-void ValueObject::AddSyntheticChild(ConstString key,
-ValueObject *valobj) {
+void ValueObject::AddSyntheticChild(ConstString key, ValueObject *valobj) {
   m_synthetic_children[key] = valobj;
 }
 
@@ -1924,64 +1923,96 @@ void ValueObject::GetExpressionPath(Stream ,
 return;
   }
 
-  const bool is_deref_of_parent = IsDereferenceOfParent();
+  // Checks whether a value is dereference of a non-reference parent.
+  // This is used to determine whether to print a dereference operation (*).
+  auto is_deref_of_non_reference = [](ValueObject *value) {
+if (value == nullptr)
+  return false;
+ValueObject *value_parent = value->GetParent();
+if (value_parent) {
+  CompilerType parent_compiler_type = value_parent->GetCompilerType();
+  if (parent_compiler_type) {
+const uint32_t parent_type_info = parent_compiler_type.GetTypeInfo();
+if (parent_type_info & eTypeIsReference)
+  return false;
+  }
+}
+return value->IsDereferenceOfParent();
+  };
+
+  ValueObject *parent = GetParent();
 
-  if (is_deref_of_parent &&
+  if (is_deref_of_non_reference(this) &&
   epformat == eGetExpressionPathFormatDereferencePointers) {
 // this is the original format of GetExpressionPath() producing code like
 // *(a_ptr).memberName, which is entirely fine, until you put this into
 // StackFrame::GetValueForVariableExpressionPath() which prefers to see
-// a_ptr->memberName. the eHonorPointers mode is meant to produce strings
-// in this latter format
-s.PutCString("*(");
+// a_ptr->memberName. The eHonorPointers mode is meant to produce strings
+// in this latter format.
+s.PutChar('*');
+if (parent)
+ 

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

[Andy Yankovsky via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0205aa4a0257: [lldb] Fix member access in GetExpressionPath 
(authored by tonkosi, committed by werat).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132734

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-synth/TestDataFormatterSynth.py
  lldb/test/API/python_api/expression_path/Makefile
  lldb/test/API/python_api/expression_path/TestExpressionPath.py
  lldb/test/API/python_api/expression_path/main.cpp

Index: lldb/test/API/python_api/expression_path/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/expression_path/main.cpp
@@ -0,0 +1,34 @@
+struct StructA {
+  int x;
+  int y;
+};
+
+struct StructB {
+  int x;
+  StructA _ref;
+  StructA *_ptr_ref;
+};
+
+struct StructC : public StructB {
+  int y;
+
+  StructC(int x, StructA _ref, StructA *_ref_ptr, int y)
+  : StructB{x, a_ref, a_ref_ptr}, y(y) {}
+};
+
+int main() {
+  StructA a{1, 2};
+  StructA *a_ptr = 
+
+  StructB b{3, a, a_ptr};
+  StructB *b_ptr = 
+  StructB _ref = b;
+  StructB *_ptr_ref = b_ptr;
+
+  StructC c(4, a, a_ptr, 5);
+  StructC *c_ptr = 
+  StructC _ref = c;
+  StructC *_ptr_ref = c_ptr;
+
+  return 0; // Set breakpoint here
+}
\ No newline at end of file
Index: lldb/test/API/python_api/expression_path/TestExpressionPath.py
===
--- /dev/null
+++ lldb/test/API/python_api/expression_path/TestExpressionPath.py
@@ -0,0 +1,119 @@
+"""Test that SBFrame::GetExpressionPath construct valid expressions"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class SBValueGetExpressionPathTest(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def path(self, value):
+"""Constructs the expression path given the SBValue"""
+if not value:
+return None
+stream = lldb.SBStream()
+if not value.GetExpressionPath(stream):
+return None
+return stream.GetData()
+
+def test_expression_path(self):
+"""Test that SBFrame::GetExpressionPath construct valid expressions"""
+self.build()
+self.setTearDownCleanup()
+
+exe = self.getBuildArtifact("a.out")
+
+# Create the target
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set the breakpoints
+breakpoint = target.BreakpointCreateBySourceRegex(
+'Set breakpoint here', lldb.SBFileSpec("main.cpp"))
+self.assertTrue(breakpoint.GetNumLocations() > 0, VALID_BREAKPOINT)
+
+# Launch the process, and do not stop at the entry point.
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Frame #0 should be at our breakpoint.
+threads = lldbutil.get_threads_stopped_at_breakpoint(
+process, breakpoint)
+
+self.assertEquals(len(threads), 1)
+self.thread = threads[0]
+self.frame = self.thread.frames[0]
+self.assertTrue(self.frame, "Frame 0 is valid.")
+
+# Find "b" variables in frame
+b = self.frame.FindVariable("b")
+bp = self.frame.FindVariable("b_ptr")
+br = self.frame.FindVariable("b_ref")
+bpr = self.frame.FindVariable("b_ptr_ref")
+# Check expression paths
+self.assertEqual(self.path(b), "b")
+self.assertEqual(self.path(bp), "b_ptr")
+self.assertEqual(self.path(br), "b_ref")
+self.assertEqual(self.path(bpr), "b_ptr_ref")
+
+# Dereference "b" pointers
+bp_deref = bp.Dereference()
+bpr_deref = bpr.Dereference()  # a pointer
+bpr_deref2 = bpr_deref.Dereference()  # two Dereference() calls to get object
+# Check expression paths
+self.assertEqual(self.path(bp_deref), "*b_ptr")
+self.assertEqual(self.path(bpr_deref), "b_ptr_ref")
+self.assertEqual(self.path(bpr_deref2), "*b_ptr_ref")
+
+# Access "b" members and check expression paths
+self.assertEqual(self.path(b.GetChildMemberWithName("x")), "b.x")
+self.assertEqual(self.path(bp.GetChildMemberWithName("x")), "b_ptr->x")
+self.assertEqual(self.path(br.GetChildMemberWithName("x")), "b_ref.x")
+self.assertEqual(self.path(bp_deref.GetChildMemberWithName("x")), "(*b_ptr).x")
+self.assertEqual(self.path(bpr_deref.GetChildMemberWithName("x")), "b_ptr_ref->x")
+self.assertEqual(self.path(bpr_deref2.GetChildMemberWithName("x")), "(*b_ptr_ref).x")
+# TODO: Uncomment once accessing members on pointer 

[Lldb-commits] [lldb] d1b83b9 - [LLDB] Fix x86_64 build

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-12T09:19:29Z
New Revision: d1b83b984fc1715fb7540dbb03ac8d170775b395

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

LOG: [LLDB] Fix x86_64 build

28e65a6a63ab39be97d1a88fe7b4d0fa2f532643 changed the parameter
type of SetType but I forgot to build on x86 as well as arm64.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
index e23ec49ee6184..ba6be2357ff9f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -457,7 +457,7 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
   // then use the type specified by reg_info rather than the uint64_t
   // default
   if (reg_value.GetByteSize() > reg_info->byte_size)
-reg_value.SetType(reg_info);
+reg_value.SetType(*reg_info);
 }
 return error;
   }



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

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Technically yes, but then we have two ad-hoc solutions that support half the 
> log handlers. I think if we care about that we might as well go with the 
> T-based log handler behind the scenes.

Sure. More wondering whether the choice between circular buffer and file is a 
whole lot of code, but it isn't if we already have buffers.

Saving log files only sounds good to me.


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

https://reviews.llvm.org/D135631

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

[Lldb-commits] [lldb] 0577a9f - [LLDB] Change EmulateInstriction::ReadRegister to return Optional

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-12T08:37:36Z
New Revision: 0577a9f8d06b97bf8b4d9aa171096e0797e4f5d1

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

LOG: [LLDB] Change EmulateInstriction::ReadRegister to return Optional

Making it easier to understand and harder to misuse.
This only applies to the ReadRegister(const RegisterInfo _info) variant.

Depends on D135671

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/include/lldb/Core/EmulateInstruction.h
lldb/source/Core/EmulateInstruction.cpp
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/EmulateInstruction.h 
b/lldb/include/lldb/Core/EmulateInstruction.h
index 68b40673faab2..65b7982c36a0b 100644
--- a/lldb/include/lldb/Core/EmulateInstruction.h
+++ b/lldb/include/lldb/Core/EmulateInstruction.h
@@ -388,7 +388,7 @@ class EmulateInstruction : public PluginInterface {
uint32_t reg_num, std::string 
_name);
 
   // RegisterInfo variants
-  bool ReadRegister(const RegisterInfo _info, RegisterValue _value);
+  llvm::Optional ReadRegister(const RegisterInfo _info);
 
   uint64_t ReadRegisterUnsigned(const RegisterInfo _info,
 uint64_t fail_value, bool *success_ptr);

diff  --git a/lldb/source/Core/EmulateInstruction.cpp 
b/lldb/source/Core/EmulateInstruction.cpp
index 73cbcc722ef64..2901609ce7d73 100644
--- a/lldb/source/Core/EmulateInstruction.cpp
+++ b/lldb/source/Core/EmulateInstruction.cpp
@@ -72,20 +72,29 @@ EmulateInstruction::FindPlugin(const ArchSpec ,
 
 EmulateInstruction::EmulateInstruction(const ArchSpec ) : m_arch(arch) {}
 
-bool EmulateInstruction::ReadRegister(const RegisterInfo _info,
-  RegisterValue _value) {
-  if (m_read_reg_callback != nullptr)
-return m_read_reg_callback(this, m_baton, _info, reg_value);
-  return false;
+llvm::Optional
+EmulateInstruction::ReadRegister(const RegisterInfo _info) {
+  if (m_read_reg_callback == nullptr)
+return {};
+
+  RegisterValue reg_value;
+  bool success = m_read_reg_callback(this, m_baton, _info, reg_value);
+  if (success)
+return reg_value;
+  return {};
 }
 
 bool EmulateInstruction::ReadRegister(lldb::RegisterKind reg_kind,
   uint32_t reg_num,
   RegisterValue _value) {
   llvm::Optional reg_info = GetRegisterInfo(reg_kind, reg_num);
-  if (reg_info)
-return ReadRegister(*reg_info, reg_value);
-  return false;
+  if (!reg_info)
+return false;
+
+  llvm::Optional value = ReadRegister(*reg_info);
+  if (value)
+reg_value = *value;
+  return value.has_value();
 }
 
 uint64_t EmulateInstruction::ReadRegisterUnsigned(lldb::RegisterKind reg_kind,
@@ -103,12 +112,14 @@ uint64_t 
EmulateInstruction::ReadRegisterUnsigned(lldb::RegisterKind reg_kind,
 uint64_t EmulateInstruction::ReadRegisterUnsigned(const RegisterInfo _info,
   uint64_t fail_value,
   bool *success_ptr) {
-  RegisterValue reg_value;
-  if (ReadRegister(reg_info, reg_value))
-return reg_value.GetAsUInt64(fail_value, success_ptr);
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
+  llvm::Optional reg_value = ReadRegister(reg_info);
+  if (!reg_value) {
+if (success_ptr)
+  *success_ptr = false;
+return fail_value;
+  }
+
+  return reg_value->GetAsUInt64(fail_value, success_ptr);
 }
 
 bool EmulateInstruction::WriteRegister(const Context ,

diff  --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp 
b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index 4488b45ef8abe..3fc529ae20281 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -768,8 +768,6 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t 
opcode) {
   uint64_t address;
   uint64_t wb_address;
 
-  RegisterValue data_Rt;
-  RegisterValue data_Rt2;
   llvm::Optional reg_info_base =
   GetRegisterInfo(eRegisterKindLLDB, gpr_x0_arm64 + n);
   if (!reg_info_base)
@@ -824,21 +822,24 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const 
uint32_t opcode) {
 context_t2.SetRegisterToRegisterPlusOffset(*reg_info_Rt2, *reg_info_base,
size);
 
-if (!ReadRegister(*reg_info_Rt, data_Rt))
+llvm::Optional data_Rt = ReadRegister(*reg_info_Rt);
+if 

[Lldb-commits] [PATCH] D135672: [LLDB] Change EmulateInstriction::ReadRegister to return Optional

[David Spickett 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 rG0577a9f8d06b: [LLDB] Change EmulateInstriction::ReadRegister 
to return Optional (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135672

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/source/Core/EmulateInstruction.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp

Index: lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
===
--- lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
+++ lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
@@ -1153,18 +1153,19 @@
   /* We look for sp based non-volatile register stores */
   if (nonvolatile_reg_p(src)) {
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+  reg_info_src->byte_size, eByteOrderLittle,
+  error) == 0)
   return false;
 
 if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
Index: lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
===
--- lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
+++ lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
@@ -1259,18 +1259,19 @@
   return false;
 
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+  reg_info_src->byte_size, eByteOrderLittle,
+  error) == 0)
   return false;
 
 if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
@@ -1518,18 +1519,18 @@
   if (base == dwarf_sp_mips && nonvolatile_reg_p(src)) {
 RegisterInfo reg_info_src = {};
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
+  eByteOrderLittle, error) == 0)
   return false;
 
 if (!WriteMemory(context, address, buffer, reg_info_src.byte_size))
@@ -1600,18 +1601,19 @@
   return false;
 
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+  reg_info_src->byte_size, eByteOrderLittle,
+  error) == 0)
   return false;
 
 if (!WriteMemory(context, base_address, buffer, reg_info_src->byte_size))
Index: 

[Lldb-commits] [PATCH] D135672: [LLDB] Change EmulateInstriction::ReadRegister to return Optional

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 467057.
DavidSpickett added a comment.

Fix some undeclared vars, somehow I forgot to actualy build this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135672

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/source/Core/EmulateInstruction.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp

Index: lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
===
--- lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
+++ lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
@@ -1153,18 +1153,19 @@
   /* We look for sp based non-volatile register stores */
   if (nonvolatile_reg_p(src)) {
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+  reg_info_src->byte_size, eByteOrderLittle,
+  error) == 0)
   return false;
 
 if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
Index: lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
===
--- lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
+++ lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
@@ -1259,18 +1259,19 @@
   return false;
 
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+  reg_info_src->byte_size, eByteOrderLittle,
+  error) == 0)
   return false;
 
 if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
@@ -1518,18 +1519,18 @@
   if (base == dwarf_sp_mips && nonvolatile_reg_p(src)) {
 RegisterInfo reg_info_src = {};
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
+  eByteOrderLittle, error) == 0)
   return false;
 
 if (!WriteMemory(context, address, buffer, reg_info_src.byte_size))
@@ -1600,18 +1601,19 @@
   return false;
 
 Context context;
-RegisterValue data_src;
 context.type = eContextPushRegisterOnStack;
 context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
 uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
 Status error;
 
-if (!ReadRegister(*reg_info_base, data_src))
+llvm::Optional data_src = ReadRegister(*reg_info_base);
+if (!data_src)
   return false;
 
-if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
- eByteOrderLittle, error) == 0)
+if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+  reg_info_src->byte_size, eByteOrderLittle,
+  error) == 0)
   return false;
 
 if (!WriteMemory(context, base_address, buffer, reg_info_src->byte_size))
Index: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
===
--- 

[Lldb-commits] [lldb] 28e65a6 - [LLDB] Change RegisterValue::SetType param to const RegisterInfo

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-12T08:25:14Z
New Revision: 28e65a6a63ab39be97d1a88fe7b4d0fa2f532643

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

LOG: [LLDB] Change RegisterValue::SetType param to const RegisterInfo&

No one was pasing nullptr here.

Depends on D135670

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/include/lldb/Utility/RegisterValue.h
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/source/Utility/RegisterValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/RegisterValue.h 
b/lldb/include/lldb/Utility/RegisterValue.h
index 3a3c0a07f8f4c..ff3dc9b3dda42 100644
--- a/lldb/include/lldb/Utility/RegisterValue.h
+++ b/lldb/include/lldb/Utility/RegisterValue.h
@@ -84,7 +84,7 @@ class RegisterValue {
 
   void SetType(RegisterValue::Type type) { m_type = type; }
 
-  RegisterValue::Type SetType(const RegisterInfo *reg_info);
+  RegisterValue::Type SetType(const RegisterInfo _info);
 
   bool GetData(DataExtractor ) const;
 

diff  --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
index 146e33ff43b20..9af795f228290 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -75,7 +75,7 @@ NativeRegisterContextLinux::WriteRegisterRaw(uint32_t 
reg_index,
 memcpy(dst + (reg_info->byte_offset & 0x1), src, src_size);
 // Set this full register as the value to write.
 value_to_write.SetBytes(dst, full_value.GetByteSize(), byte_order);
-value_to_write.SetType(full_reg_info);
+value_to_write.SetType(*full_reg_info);
 reg_to_write = full_reg;
   }
 }

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index 8d4585a02a8c3..5ad2f7a8e9455 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -136,7 +136,7 @@ NativeRegisterContextLinux_arm::ReadRegister(const 
RegisterInfo *reg_info,
   // then use the type specified by reg_info rather than the uint64_t
   // default
   if (reg_value.GetByteSize() > reg_info->byte_size)
-reg_value.SetType(reg_info);
+reg_value.SetType(*reg_info);
 }
 return error;
   }

diff  --git a/lldb/source/Utility/RegisterValue.cpp 
b/lldb/source/Utility/RegisterValue.cpp
index f1d37b0e0cbe9..b9ce9fd18ac48 100644
--- a/lldb/source/Utility/RegisterValue.cpp
+++ b/lldb/source/Utility/RegisterValue.cpp
@@ -149,13 +149,12 @@ bool RegisterValue::GetScalarValue(Scalar ) const {
 
 void RegisterValue::Clear() { m_type = eTypeInvalid; }
 
-RegisterValue::Type RegisterValue::SetType(const RegisterInfo *reg_info) {
-  assert(reg_info && "Expected non null reg_info.");
+RegisterValue::Type RegisterValue::SetType(const RegisterInfo _info) {
   // To change the type, we simply copy the data in again, using the new format
   RegisterValue copy;
   DataExtractor copy_data;
   if (copy.CopyValue(*this) && copy.GetData(copy_data)) {
-Status error = SetValueFromData(*reg_info, copy_data, 0, true);
+Status error = SetValueFromData(reg_info, copy_data, 0, true);
 assert(error.Success() && "Expected SetValueFromData to succeed.");
 UNUSED_IF_ASSERT_DISABLED(error);
   }



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

[Lldb-commits] [PATCH] D135671: [LLDB] Change RegisterValue::SetType param to const RegisterInfo

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG28e65a6a63ab: [LLDB] Change RegisterValue::SetType param to 
const RegisterInfo (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135671

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
  lldb/source/Utility/RegisterValue.cpp


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -149,13 +149,12 @@
 
 void RegisterValue::Clear() { m_type = eTypeInvalid; }
 
-RegisterValue::Type RegisterValue::SetType(const RegisterInfo *reg_info) {
-  assert(reg_info && "Expected non null reg_info.");
+RegisterValue::Type RegisterValue::SetType(const RegisterInfo _info) {
   // To change the type, we simply copy the data in again, using the new format
   RegisterValue copy;
   DataExtractor copy_data;
   if (copy.CopyValue(*this) && copy.GetData(copy_data)) {
-Status error = SetValueFromData(*reg_info, copy_data, 0, true);
+Status error = SetValueFromData(reg_info, copy_data, 0, true);
 assert(error.Success() && "Expected SetValueFromData to succeed.");
 UNUSED_IF_ASSERT_DISABLED(error);
   }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -136,7 +136,7 @@
   // then use the type specified by reg_info rather than the uint64_t
   // default
   if (reg_value.GetByteSize() > reg_info->byte_size)
-reg_value.SetType(reg_info);
+reg_value.SetType(*reg_info);
 }
 return error;
   }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -75,7 +75,7 @@
 memcpy(dst + (reg_info->byte_offset & 0x1), src, src_size);
 // Set this full register as the value to write.
 value_to_write.SetBytes(dst, full_value.GetByteSize(), byte_order);
-value_to_write.SetType(full_reg_info);
+value_to_write.SetType(*full_reg_info);
 reg_to_write = full_reg;
   }
 }
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -84,7 +84,7 @@
 
   void SetType(RegisterValue::Type type) { m_type = type; }
 
-  RegisterValue::Type SetType(const RegisterInfo *reg_info);
+  RegisterValue::Type SetType(const RegisterInfo _info);
 
   bool GetData(DataExtractor ) const;
 


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -149,13 +149,12 @@
 
 void RegisterValue::Clear() { m_type = eTypeInvalid; }
 
-RegisterValue::Type RegisterValue::SetType(const RegisterInfo *reg_info) {
-  assert(reg_info && "Expected non null reg_info.");
+RegisterValue::Type RegisterValue::SetType(const RegisterInfo _info) {
   // To change the type, we simply copy the data in again, using the new format
   RegisterValue copy;
   DataExtractor copy_data;
   if (copy.CopyValue(*this) && copy.GetData(copy_data)) {
-Status error = SetValueFromData(*reg_info, copy_data, 0, true);
+Status error = SetValueFromData(reg_info, copy_data, 0, true);
 assert(error.Success() && "Expected SetValueFromData to succeed.");
 UNUSED_IF_ASSERT_DISABLED(error);
   }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -136,7 +136,7 @@
   // then use the type specified by reg_info rather than the uint64_t
   // default
   if (reg_value.GetByteSize() > reg_info->byte_size)
-reg_value.SetType(reg_info);
+reg_value.SetType(*reg_info);
 }
 return error;
   }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -75,7 +75,7 @@
 memcpy(dst + 

[Lldb-commits] [lldb] 6faa345 - [LLDB] Pass const RegisterInfo& to RegisterValue::SetValueFromData

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-12T08:19:30Z
New Revision: 6faa345da9d747c65c4d901c4ef26dfedf95da45

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

LOG: [LLDB] Pass const RegisterInfo& to RegisterValue::SetValueFromData

Familiar story, callers are either checking upfront that the pointer
wasn't null or not checking at all. SetValueFromData itself didn't
check either.

So make the parameter a ref and fixup the few places where a nullptr
check seems needed.

Depends on D135668

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/include/lldb/Utility/RegisterValue.h
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Utility/RegisterValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/RegisterValue.h 
b/lldb/include/lldb/Utility/RegisterValue.h
index 38decf4f9d883..3a3c0a07f8f4c 100644
--- a/lldb/include/lldb/Utility/RegisterValue.h
+++ b/lldb/include/lldb/Utility/RegisterValue.h
@@ -238,7 +238,7 @@ class RegisterValue {
   Status SetValueFromString(const RegisterInfo *reg_info,
 const char *value_str) = delete;
 
-  Status SetValueFromData(const RegisterInfo *reg_info, DataExtractor ,
+  Status SetValueFromData(const RegisterInfo _info, DataExtractor ,
   lldb::offset_t offset, bool partial_data_ok);
 
   const void *GetBytes() const;

diff  --git a/lldb/source/Core/ValueObjectRegister.cpp 
b/lldb/source/Core/ValueObjectRegister.cpp
index 2aa0c68ba40f5..98fbbd62c6a78 100644
--- a/lldb/source/Core/ValueObjectRegister.cpp
+++ b/lldb/source/Core/ValueObjectRegister.cpp
@@ -282,7 +282,7 @@ bool ValueObjectRegister::SetValueFromCString(const char 
*value_str,
 }
 
 bool ValueObjectRegister::SetData(DataExtractor , Status ) {
-  error = m_reg_value.SetValueFromData(_reg_info, data, 0, false);
+  error = m_reg_value.SetValueFromData(m_reg_info, data, 0, false);
   if (!error.Success())
 return false;
 

diff  --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 4e2bd12c10532..e98d117cb258b 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -398,7 +398,7 @@ bool ValueObjectVariable::SetData(DataExtractor , 
Status ) {
   error.SetErrorString("unable to retrieve register info");
   return false;
 }
-error = reg_value.SetValueFromData(reg_info, data, 0, true);
+error = reg_value.SetValueFromData(*reg_info, data, 0, true);
 if (error.Fail())
   return false;
 if (reg_ctx->WriteRegister(reg_info, reg_value)) {

diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index d942d53376e86..6739cedcbc8bd 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -304,7 +304,7 @@ ABIMacOSX_arm64::SetReturnValueObject(lldb::StackFrameSP 
_sp,
 if (byte_size <= 16) {
   if (byte_size <= RegisterValue::GetMaxByteSize()) {
 RegisterValue reg_value;
-error = reg_value.SetValueFromData(v0_info, data, 0, true);
+error = reg_value.SetValueFromData(*v0_info, data, 0, true);
 if (error.Success()) {
   if (!reg_ctx->WriteRegister(v0_info, reg_value))
 error.SetErrorString("failed to write register v0");
@@ -331,7 +331,7 @@ ABIMacOSX_arm64::SetReturnValueObject(lldb::StackFrameSP 
_sp,
 if (v0_info) {
   if (byte_size <= v0_info->byte_size) {
 RegisterValue reg_value;
-error = reg_value.SetValueFromData(v0_info, data, 0, true);
+error = reg_value.SetValueFromData(*v0_info, data, 0, true);
 if (error.Success()) {
   if (!reg_ctx->WriteRegister(v0_info, reg_value))
 error.SetErrorString("failed to write register v0");

diff  --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index c8130e704ecc0..de54ddee826e6 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -277,7 +277,7 @@ Status 
ABISysV_arm64::SetReturnValueObject(lldb::StackFrameSP _sp,
 if (byte_size <= 16) {
   if (byte_size <= RegisterValue::GetMaxByteSize()) {
 RegisterValue reg_value;
-  

[Lldb-commits] [PATCH] D135670: [LLDB] Pass const RegisterInfo& to RegisterValue::SetValueFromData

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6faa345da9d7: [LLDB] Pass const RegisterInfo to 
RegisterValue::SetValueFromData (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135670

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Utility/RegisterValue.cpp

Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -115,7 +115,7 @@
   // register value
   DataExtractor src_data(src, src_len, src_byte_order, 4);
 
-  error = SetValueFromData(_info, src_data, 0, true);
+  error = SetValueFromData(reg_info, src_data, 0, true);
   if (error.Fail())
 return 0;
 
@@ -150,16 +150,20 @@
 void RegisterValue::Clear() { m_type = eTypeInvalid; }
 
 RegisterValue::Type RegisterValue::SetType(const RegisterInfo *reg_info) {
+  assert(reg_info && "Expected non null reg_info.");
   // To change the type, we simply copy the data in again, using the new format
   RegisterValue copy;
   DataExtractor copy_data;
-  if (copy.CopyValue(*this) && copy.GetData(copy_data))
-SetValueFromData(reg_info, copy_data, 0, true);
+  if (copy.CopyValue(*this) && copy.GetData(copy_data)) {
+Status error = SetValueFromData(*reg_info, copy_data, 0, true);
+assert(error.Success() && "Expected SetValueFromData to succeed.");
+UNUSED_IF_ASSERT_DISABLED(error);
+  }
 
   return m_type;
 }
 
-Status RegisterValue::SetValueFromData(const RegisterInfo *reg_info,
+Status RegisterValue::SetValueFromData(const RegisterInfo _info,
DataExtractor ,
lldb::offset_t src_offset,
bool partial_data_ok) {
@@ -170,22 +174,22 @@
 return error;
   }
 
-  if (reg_info->byte_size == 0) {
+  if (reg_info.byte_size == 0) {
 error.SetErrorString("invalid register info.");
 return error;
   }
 
   uint32_t src_len = src.GetByteSize() - src_offset;
 
-  if (!partial_data_ok && (src_len < reg_info->byte_size)) {
+  if (!partial_data_ok && (src_len < reg_info.byte_size)) {
 error.SetErrorString("not enough data.");
 return error;
   }
 
   // Cap the data length if there is more than enough bytes for this register
   // value
-  if (src_len > reg_info->byte_size)
-src_len = reg_info->byte_size;
+  if (src_len > reg_info.byte_size)
+src_len = reg_info.byte_size;
 
   // Zero out the value in case we get partial data...
   memset(buffer.bytes, 0, sizeof(buffer.bytes));
@@ -193,20 +197,20 @@
   type128 int128;
 
   m_type = eTypeInvalid;
-  switch (reg_info->encoding) {
+  switch (reg_info.encoding) {
   case eEncodingInvalid:
 break;
   case eEncodingUint:
   case eEncodingSint:
-if (reg_info->byte_size == 1)
+if (reg_info.byte_size == 1)
   SetUInt8(src.GetMaxU32(_offset, src_len));
-else if (reg_info->byte_size <= 2)
+else if (reg_info.byte_size <= 2)
   SetUInt16(src.GetMaxU32(_offset, src_len));
-else if (reg_info->byte_size <= 4)
+else if (reg_info.byte_size <= 4)
   SetUInt32(src.GetMaxU32(_offset, src_len));
-else if (reg_info->byte_size <= 8)
+else if (reg_info.byte_size <= 8)
   SetUInt64(src.GetMaxU64(_offset, src_len));
-else if (reg_info->byte_size <= 16) {
+else if (reg_info.byte_size <= 16) {
   uint64_t data1 = src.GetU64(_offset);
   uint64_t data2 = src.GetU64(_offset);
   if (src.GetByteSize() == eByteOrderBig) {
@@ -220,16 +224,16 @@
 }
 break;
   case eEncodingIEEE754:
-if (reg_info->byte_size == sizeof(float))
+if (reg_info.byte_size == sizeof(float))
   SetFloat(src.GetFloat(_offset));
-else if (reg_info->byte_size == sizeof(double))
+else if (reg_info.byte_size == sizeof(double))
   SetDouble(src.GetDouble(_offset));
-else if (reg_info->byte_size == sizeof(long double))
+else if (reg_info.byte_size == sizeof(long double))
   SetLongDouble(src.GetLongDouble(_offset));
 break;
   case eEncodingVector: {
 m_type = eTypeBytes;
-buffer.length = reg_info->byte_size;
+buffer.length = reg_info.byte_size;
 buffer.byte_order = src.GetByteOrder();
 assert(buffer.length <= kMaxRegisterByteSize);
 if (buffer.length > kMaxRegisterByteSize)
@@ -242,7 +246,7 @@
 buffer.byte_order) == 0) // dst byte order
 {
   error.SetErrorStringWithFormat(
-  "failed to copy data for register write of %s", 

[Lldb-commits] [lldb] 812ad21 - [LLDB] Change RegisterValue::SetFromMemoryData to const RegisterInfo

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-12T08:10:24Z
New Revision: 812ad2167bd2e27f5d0dee07bb03a5910616e0b6

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

LOG: [LLDB] Change RegisterValue::SetFromMemoryData to const RegisterInfo&

All callers were either assuming their pointer was not null before calling
this, or checking beforehand.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/include/lldb/Utility/RegisterValue.h
lldb/source/Host/common/NativeRegisterContext.cpp
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
lldb/source/Target/RegisterContext.cpp
lldb/source/Utility/RegisterValue.cpp
lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/RegisterValue.h 
b/lldb/include/lldb/Utility/RegisterValue.h
index b39e2980abacc..38decf4f9d883 100644
--- a/lldb/include/lldb/Utility/RegisterValue.h
+++ b/lldb/include/lldb/Utility/RegisterValue.h
@@ -99,7 +99,7 @@ class RegisterValue {
uint32_t dst_len, lldb::ByteOrder dst_byte_order,
Status ) const;
 
-  uint32_t SetFromMemoryData(const RegisterInfo *reg_info, const void *src,
+  uint32_t SetFromMemoryData(const RegisterInfo _info, const void *src,
  uint32_t src_len, lldb::ByteOrder src_byte_order,
  Status );
 

diff  --git a/lldb/source/Host/common/NativeRegisterContext.cpp 
b/lldb/source/Host/common/NativeRegisterContext.cpp
index 0110a8ac9e2d4..1be519d129eef 100644
--- a/lldb/source/Host/common/NativeRegisterContext.cpp
+++ b/lldb/source/Host/common/NativeRegisterContext.cpp
@@ -376,7 +376,7 @@ Status NativeRegisterContext::ReadRegisterValueFromMemory(
   // TODO: we might need to add a parameter to this function in case the byte
   // order of the memory data doesn't match the process. For now we are
   // assuming they are the same.
-  reg_value.SetFromMemoryData(reg_info, src, src_len, process.GetByteOrder(),
+  reg_value.SetFromMemoryData(*reg_info, src, src_len, process.GetByteOrder(),
   error);
 
   return error;

diff  --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp 
b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index 070cc3112ab23..4488b45ef8abe 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -867,9 +867,8 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t 
opcode) {
 return false;
 }
 
-if (data_Rt.SetFromMemoryData(&(*reg_info_Rt), buffer,
-  reg_info_Rt->byte_size, eByteOrderLittle,
-  error) == 0)
+if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
+  eByteOrderLittle, error) == 0)
   return false;
 
 if (!vector && is_signed && !data_Rt.SignExtend(datasize))
@@ -884,7 +883,7 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t 
opcode) {
 return false;
 }
 
-if (data_Rt2.SetFromMemoryData(&(*reg_info_Rt2), buffer,
+if (data_Rt2.SetFromMemoryData(*reg_info_Rt2, buffer,
reg_info_Rt2->byte_size, eByteOrderLittle,
error) == 0)
   return false;
@@ -1017,9 +1016,8 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const 
uint32_t opcode) {
 if (!ReadMemory(context, address, buffer, reg_info_Rt->byte_size))
   return false;
 
-if (data_Rt.SetFromMemoryData(&(*reg_info_Rt), buffer,
-  reg_info_Rt->byte_size, eByteOrderLittle,
-  error) == 0)
+if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
+  eByteOrderLittle, error) == 0)
   return false;
 
 if (!WriteRegister(context, *reg_info_Rt, data_Rt))

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 61e784e28ca68..6c04f77dbe268 100644
--- 

[Lldb-commits] [PATCH] D135668: [LLDB] Change RegisterValue::SetFromMemoryData to const RegisterInfo

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG812ad2167bd2: [LLDB] Change RegisterValue::SetFromMemoryData 
to const RegisterInfo (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135668

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
  lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Utility/RegisterValue.cpp
  lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

Index: lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -215,7 +215,7 @@
   RegisterValue Value;
   Status ST;
   Value.SetFromMemoryData(
-  , Bytes.data(), Bytes.size(),
+  Info, Bytes.data(), Bytes.size(),
   Endian == support::little ? eByteOrderLittle : eByteOrderBig, ST);
   if (ST.Fail())
 return ST.ToError();
Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -76,15 +76,10 @@
   return bytes_copied;
 }
 
-uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo *reg_info,
+uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo _info,
   const void *src, uint32_t src_len,
   lldb::ByteOrder src_byte_order,
   Status ) {
-  if (reg_info == nullptr) {
-error.SetErrorString("invalid register info argument.");
-return 0;
-  }
-
   // Moving from addr into a register
   //
   // Case 1: src_len == dst_len
@@ -107,12 +102,12 @@
 return 0;
   }
 
-  const uint32_t dst_len = reg_info->byte_size;
+  const uint32_t dst_len = reg_info.byte_size;
 
   if (src_len > dst_len) {
 error.SetErrorStringWithFormat(
 "%u bytes is too big to store in register %s (%u bytes)", src_len,
-reg_info->name, dst_len);
+reg_info.name, dst_len);
 return 0;
   }
 
@@ -120,7 +115,7 @@
   // register value
   DataExtractor src_data(src, src_len, src_byte_order, 4);
 
-  error = SetValueFromData(reg_info, src_data, 0, true);
+  error = SetValueFromData(_info, src_data, 0, true);
   if (error.Fail())
 return 0;
 
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -357,7 +357,7 @@
 // TODO: we might need to add a parameter to this function in case the byte
 // order of the memory data doesn't match the process. For now we are
 // assuming they are the same.
-reg_value.SetFromMemoryData(reg_info, src, src_len,
+reg_value.SetFromMemoryData(*reg_info, src, src_len,
 process_sp->GetByteOrder(), error);
   } else
 error.SetErrorString("invalid process");
Index: lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
===
--- lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
+++ lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
@@ -806,7 +806,7 @@
  RegisterValue _value) {
   Status error;
   reg_value.SetFromMemoryData(
-  reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
+  *reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
   reg_info->byte_size, lldb::eByteOrderLittle, error);
   return error.Success();
 }
Index: lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
===
--- lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
+++ lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
@@ -521,7 +521,7 @@
RegisterValue _value) {
   Status error;
   reg_value.SetFromMemoryData(
-  reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
+  *reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
   reg_info->byte_size, lldb::eByteOrderLittle, error);
   return 

[Lldb-commits] [PATCH] D135668: [LLDB] Change RegisterValue::SetFromMemoryData to const RegisterInfo

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 467041.
DavidSpickett added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135668

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
  lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Utility/RegisterValue.cpp
  lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

Index: lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -215,7 +215,7 @@
   RegisterValue Value;
   Status ST;
   Value.SetFromMemoryData(
-  , Bytes.data(), Bytes.size(),
+  Info, Bytes.data(), Bytes.size(),
   Endian == support::little ? eByteOrderLittle : eByteOrderBig, ST);
   if (ST.Fail())
 return ST.ToError();
Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -76,15 +76,10 @@
   return bytes_copied;
 }
 
-uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo *reg_info,
+uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo _info,
   const void *src, uint32_t src_len,
   lldb::ByteOrder src_byte_order,
   Status ) {
-  if (reg_info == nullptr) {
-error.SetErrorString("invalid register info argument.");
-return 0;
-  }
-
   // Moving from addr into a register
   //
   // Case 1: src_len == dst_len
@@ -107,12 +102,12 @@
 return 0;
   }
 
-  const uint32_t dst_len = reg_info->byte_size;
+  const uint32_t dst_len = reg_info.byte_size;
 
   if (src_len > dst_len) {
 error.SetErrorStringWithFormat(
 "%u bytes is too big to store in register %s (%u bytes)", src_len,
-reg_info->name, dst_len);
+reg_info.name, dst_len);
 return 0;
   }
 
@@ -120,7 +115,7 @@
   // register value
   DataExtractor src_data(src, src_len, src_byte_order, 4);
 
-  error = SetValueFromData(reg_info, src_data, 0, true);
+  error = SetValueFromData(_info, src_data, 0, true);
   if (error.Fail())
 return 0;
 
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -357,7 +357,7 @@
 // TODO: we might need to add a parameter to this function in case the byte
 // order of the memory data doesn't match the process. For now we are
 // assuming they are the same.
-reg_value.SetFromMemoryData(reg_info, src, src_len,
+reg_value.SetFromMemoryData(*reg_info, src, src_len,
 process_sp->GetByteOrder(), error);
   } else
 error.SetErrorString("invalid process");
Index: lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
===
--- lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
+++ lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
@@ -806,7 +806,7 @@
  RegisterValue _value) {
   Status error;
   reg_value.SetFromMemoryData(
-  reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
+  *reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
   reg_info->byte_size, lldb::eByteOrderLittle, error);
   return error.Success();
 }
Index: lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
===
--- lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
+++ lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
@@ -521,7 +521,7 @@
RegisterValue _value) {
   Status error;
   reg_value.SetFromMemoryData(
-  reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
+  *reg_info, (const uint8_t *)_regs + reg_info->byte_offset,
   reg_info->byte_size, lldb::eByteOrderLittle, error);
   return error.Success();
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

[Lldb-commits] [PATCH] D135577: Summary:

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Oh and if you would like someone to land this on your behalf please provide a 
name and email address for the commit (of course waiting for access is fine 
too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [PATCH] D135577: Summary:

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Note: The reason the summary is so terse is that the ARC tool opens two 
> windows and I was very confused about where to write what.

Certainly would confuse me, maybe I've learned to autopilot through this 
though. Can you describe them? We can certainly add a note in the docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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