[Lldb-commits] [lldb] 35d7101 - Use Module's FileSpec for limiting binaries to set dyld breakpoint in

2021-10-14 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2021-10-14T23:58:23-07:00
New Revision: 35d710148b98e2ec52056271e7f9103620593b7a

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

LOG: Use Module's FileSpec for limiting binaries to set dyld breakpoint in

When DynamicLoaderMacOS::SetNotificationBreakpoint sets the breakpoint
for new binaries being loaded/unloaded, it limits the scope of that
breakpoint to just dyld, so we don't re-evaluate the breakpoint for
every new binary loaded.  I wrote this to get the module's ObjectFile
FileSpec in an earlier change, but this is not correct.  If lldb
is debugging a remote system, and it had to read dyld out of memory
from the remote system, it will have no FileSpec on the lldb debugger
host.  We need to grab the Module's FileSpec, which in this case is
actually falling back to the PlatformFileSpec, the binary path on the
target system.

rdar://84199646

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index 7a4294b43969..bedd100e147e 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -350,7 +350,7 @@ bool DynamicLoaderMacOS::SetNotificationBreakpoint() {
   LazyBool skip_prologue = eLazyBoolNo;
   FileSpecList *source_files = nullptr;
   FileSpecList dyld_filelist;
-  dyld_filelist.Append(dyld_sp->GetObjectFile()->GetFileSpec());
+  dyld_filelist.Append(dyld_sp->GetFileSpec());
 
   Breakpoint *breakpoint =
   m_process->GetTarget()



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


[Lldb-commits] [lldb] 7c704c0 - [NFC] fix a typo

2021-10-14 Thread Shao-Ce SUN via lldb-commits

Author: Shao-Ce SUN
Date: 2021-10-15T14:51:49+08:00
New Revision: 7c704c0f53bd7f785ec99fc6bedd71569816a28c

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

LOG: [NFC] fix a typo

Added: 


Modified: 
lldb/bindings/interface/SBSymbolContextList.i
lldb/test/API/python_api/target/TestTargetAPI.py
llvm/tools/llvm-mca/CodeRegionGenerator.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBSymbolContextList.i 
b/lldb/bindings/interface/SBSymbolContextList.i
index e9d4aa8d62db..14566b3e3720 100644
--- a/lldb/bindings/interface/SBSymbolContextList.i
+++ b/lldb/bindings/interface/SBSymbolContextList.i
@@ -14,7 +14,7 @@ namespace lldb {
 For example (from test/python_api/target/TestTargetAPI.py), ::
 
 def find_functions(self, exe_name):
-'''Exercise SBTaget.FindFunctions() API.'''
+'''Exercise SBTarget.FindFunctions() API.'''
 exe = os.path.join(os.getcwd(), exe_name)
 
 # Create a target by the debugger.

diff  --git a/lldb/test/API/python_api/target/TestTargetAPI.py 
b/lldb/test/API/python_api/target/TestTargetAPI.py
index 29e368224e81..0503aa8e4369 100644
--- a/lldb/test/API/python_api/target/TestTargetAPI.py
+++ b/lldb/test/API/python_api/target/TestTargetAPI.py
@@ -207,7 +207,7 @@ def find_data_section(self, target):
 return data_section
 
 def find_global_variables(self, exe_name):
-"""Exercise SBTaget.FindGlobalVariables() API."""
+"""Exercise SBTarget.FindGlobalVariables() API."""
 exe = self.getBuildArtifact(exe_name)
 
 # Create a target by the debugger.
@@ -272,7 +272,7 @@ def find_compile_units(self, exe):
 list[0].GetCompileUnit().GetFileSpec().GetFilename(), source_name)
 
 def find_functions(self, exe_name):
-"""Exercise SBTaget.FindFunctions() API."""
+"""Exercise SBTarget.FindFunctions() API."""
 exe = self.getBuildArtifact(exe_name)
 
 # Create a target by the debugger.
@@ -292,7 +292,7 @@ def find_functions(self, exe_name):
 self.assertEqual(sc.GetSymbol().GetName(), 'c')
 
 def get_description(self):
-"""Exercise SBTaget.GetDescription() API."""
+"""Exercise SBTarget.GetDescription() API."""
 exe = self.getBuildArtifact("a.out")
 
 # Create a target by the debugger.
@@ -321,7 +321,7 @@ def get_description(self):
 @skipIfRemote
 @no_debug_info_test
 def test_launch_new_process_and_redirect_stdout(self):
-"""Exercise SBTaget.Launch() API with redirected stdout."""
+"""Exercise SBTarget.Launch() API with redirected stdout."""
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -380,7 +380,7 @@ def test_launch_new_process_and_redirect_stdout(self):
 substrs=["a(1)", "b(2)", "a(3)"])
 
 def resolve_symbol_context_with_address(self):
-"""Exercise SBTaget.ResolveSymbolContextForAddress() API."""
+"""Exercise SBTarget.ResolveSymbolContextForAddress() API."""
 exe = self.getBuildArtifact("a.out")
 
 # Create a target by the debugger.

diff  --git a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp 
b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
index 6ad2a65592b9..6cdd0ba797aa 100644
--- a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
+++ b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp
@@ -114,7 +114,7 @@ Expected 
AsmCodeRegionGenerator::parseCodeRegions(
 
   // Need to initialize an MCTargetStreamer otherwise
   // certain asm directives will cause a segfault.
-  // Using nulls() so that anything emitted by the MCTagetStreamer
+  // Using nulls() so that anything emitted by the MCTargetStreamer
   // doesn't show up in the llvm-mca output.
   raw_ostream &OSRef = nulls();
   formatted_raw_ostream FOSRef(OSRef);



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


[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379921.
mgorny added a comment.

Use `ConstString::GetStringRef()` directly instead of converting through `const 
char *`. Not that it has any practical difference right now…


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa"])

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Right now if you run:
(lldb) statistics dump
It will dump stats for the current target only as one target dictionary. We can 
change this if desired.
(lldb) statistics dump --all-targets
will dump an dictionary that contains a top level "targets" key/value pair 
where the value is an array of the all target dictionaries statistics. Of 
course new key/value pairs can be added to this top level that total up stats 
from and of the TargetStats statistics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111686

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


[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Ok, take a look now. I added a GlobalStats class that contains the 
"enable/disable" state of the statistics gathering. This shows how things can 
be organized and dumped.

A few things about the current approach: Stats classes in Statistics.h are 
designed to be able to be fetched, like TargetStats, and then any class that 
owns a collection of these, like how GlobalStats could own a 
std::vector could then aggregate any sub stats into key/value 
pairs at this higher level. The original patch will end up gathering 
ModuleStats for each module in TargetStats, and then collecting total stats for 
debug info size, debug info parse and index time, symbol table parse and index 
time, and these total stats can be aggregated and displayed. So hopefully this 
shows the direction I was thinking. Let me know what you think and what changes 
you would like to see here.

-


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111686

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


[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 379860.
clayborg added a comment.
Herald added a subscriber: dang.

Fixed issues identified in first review round:

- typos in comments
- create GlobalStats class to contain the global statistics
- Make sure all code is using the new StatsClock, StatsDuration, and 
StatsTimepoint definitions
- added a "--all-targets" option to dump options to dump stats for all targets 
and open further discussion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111686

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Target/Target.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectStats.cpp
  lldb/source/Commands/Options.td
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py
  lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py

Index: lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
===
--- lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
+++ lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
@@ -10,6 +10,8 @@
 class TestStatsAPI(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
+NO_DEBUG_INFO_TESTCASE = True
+
 def test_stats_api(self):
 self.build()
 exe = self.getBuildArtifact("a.out")
@@ -26,9 +28,18 @@
 stats = target.GetStatistics()
 stream = lldb.SBStream()
 res = stats.GetAsJSON(stream)
-stats_json = sorted(json.loads(stream.GetData()))
-self.assertEqual(len(stats_json), 4)
-self.assertIn("Number of expr evaluation failures", stats_json)
-self.assertIn("Number of expr evaluation successes", stats_json)
-self.assertIn("Number of frame var failures", stats_json)
-self.assertIn("Number of frame var successes", stats_json)
+stats_json = json.loads(stream.GetData())
+self.assertEqual('expressionEvaluation' in stats_json, True,
+'Make sure the "expressionEvaluation" key in in target.GetStatistics()')
+self.assertEqual('frameVariable' in stats_json, True,
+'Make sure the "frameVariable" key in in target.GetStatistics()')
+expressionEvaluation = stats_json['expressionEvaluation']
+self.assertEqual('successes' in expressionEvaluation, True,
+'Make sure the "successes" key in in "expressionEvaluation" dictionary"')
+self.assertEqual('failures' in expressionEvaluation, True,
+'Make sure the "failures" key in in "expressionEvaluation" dictionary"')
+frameVariable = stats_json['frameVariable']
+self.assertEqual('successes' in frameVariable, True,
+'Make sure the "successes" key in in "frameVariable" dictionary"')
+self.assertEqual('failures' in frameVariable, True,
+'Make sure the "failures" key in in "frameVariable" dictionary"')
Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,4 +1,5 @@
 import lldb
+import json
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -7,22 +8,87 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-def test(self):
+def setUp(self):
+TestBase.setUp(self)
 self.build()
-lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
 
-self.expect("statistics disable", substrs=['need to enable statistics before disabling'], error=True)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_enable_disable(self):
+"""
+Test "statistics disable" and "statistics enable". These don't do
+anything anymore for cheap to gather statistics. In the future if
+statistics are expensive to gather, we can enable the feature inside
+of LLDB and test that enabling and disabling stops expesive information
+from being gathered.
+"""
+target = self.createTestTarget()
 
-# 'expression' should change the statistics.
+self.expect("statistics disable", substrs=['need to enable statistics before disabling'], error=True)
 self.expect("statistics enable")
 self.expect("statistics enable", substrs=['already enabled'], error=True)
-self.expect("expr patatino", substrs=['27'])
 self.expect("statistics disable")
-self.expect("statistics dump", s

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/include/lldb/Target/Statistics.h:23
+struct SuccessFailStats {
+  void Notify(bool success);
+

wallace wrote:
> JDevlieghere wrote:
> > I'd use an enum class with `Success` and `Failure` here instead of a bool 
> > that requires you to know what the argument is named to know if true means 
> > success of failure. 
> Or maybe just NotifySuccess(bool) to keep callers simple
I like the idea of adding:
```
void NotifySuccess();
void NotifyFailure();
```
JDevlieghere you ok with that?



Comment at: lldb/include/lldb/Target/Statistics.h:28
+  uint32_t successes = 0;
+  uint32_t failures = 0;
+};

JDevlieghere wrote:
> I would add a member for the name so that `ToJSON` can generate the whole 
> JSON object. I imagine something like
> 
> ```
> SuccessFailStats s("frameVariable");
> s.Notify(SuccessFailStats::Success);
> s.ToJSON()
> ```
> 
> which generates
> 
> ```
> "frameVariable": {
>   "failures": 0,
>   "successes": 1
> },
> ```
> 
Good idea!



Comment at: lldb/include/lldb/Target/Statistics.h:31
+
+class TargetStats {
+public:

JDevlieghere wrote:
> The direction I outlined in the other patch is to move away from tying the 
> statistics to the target. I think the class itself is fine, as long as we 
> agree that the target stats would be held by an object spanning all debuggers.
Yeah, my main patch did a little of both. The main idea behind the classes in 
this file are they don't need to be the storage for the stats, but they can be 
if that makes things easier. The main classes (Target, Module, Breakpoint, etc) 
can just have member variables in the main classes themselves, but anything 
inside of these classes can/should be used in case we want to aggregate stats 
from any contained Stats classes. For example, I want to have a ModuleStats 
class (see https://reviews.llvm.org/D110804), this class doesn't not live 
inside of Module itself, but can be constructed from a Module. Then the 
TargetStats class can have a std::vector classes that it gathers 
when TargetStats is asked to collect statistics. This same methodology can then 
be applied to the debugger where we have DebuggerStats.

What do you think of this approach:
- Any class can define a XXXStats class in this file which is used to collect 
statistics at any given point
- Classes can use the XXXStats class as a member variable if it makes sense, or 
just add member variables to store any objects needed to calculate the stats 
later when the object is asked to collect stats 
(target.GatherStatistics(target_stats);).

This would mean we could make a DebuggerStats class that can have a 
std::vector, and when the debugger.GatherStatistics(...) is 
called. Then either the DebuggerStats or TargetStats object can be asked to be 
converted to JSON when presenting to the user in the "statistics dump" command?



Comment at: lldb/include/lldb/Target/Statistics.h:35
+
+  void SetLaunchOrAttachTime();
+  void SetFirstPrivateStopTime();

JDevlieghere wrote:
> Why don't we have these return an `ElapsedTime`. Then you can do something 
> like 
> 
> ```
> ElapsedTime elapsed_time = m_stats.GetCreateTime(); 
> ```
> 
> RVO should ensure there are no copies, but you can always delete the copy 
> ctor to make sure. Maybe we should call it ScopedTime or something to make it 
> more obvious that this is a RAII object?
> 
These are setting the time at which things start. If we follow suggestions 
above, SetLaunchOrAttachTime(), SetFirstPrivateStopTime(), and 
SetFirstPublicStopTime() can be remove and all of the 
llvm::Optional member variables can be 
moved to Target class, and we can store only 
"llvm::Optional launch_or_attach_time;" member variables 
in the TargetStats class, which would get set when statistics are gathered?



Comment at: lldb/include/lldb/Target/Statistics.h:51
+  // and "statistics disable".
+  bool m_collecting_stats = false;
+  ElapsedTime::Duration create_time{0.0};

JDevlieghere wrote:
> In line with my earlier comment about not tying the stats tot he target, this 
> would be controlled by the global (singleton) statistic class. 
I put this here because of the existing SB API:
```
bool SBTarget::GetCollectingStats();
void SBTarget::SetCollectingStats(bool);
```
So you want debugger to have this? This m_collecting_stats can be moved to 
Debugger, or DebuggerStats? But that makes the above API useless or it becomes 
deprecated. We really don't need to set if we are collecting stats IMHO, since 
collection should be cheap. You ok with deprecating the "statistics 
enable"/"statistics disable" and the above APIs?





Comment at: lldb/include/lldb/Target/Statistics.h:53
+  ElapsedTime::Duration create_time{0.0};
+  llvm::Optional launch_or_attach_time;
+  llvm::Optional 
first_private_stop_time;

JDevlie

[Lldb-commits] [PATCH] D111827: [lldb/API] Add GetIndexInDebugger to SBTarget

2021-10-14 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib abandoned this revision.
mib added a comment.

In D111827#3065040 , @JDevlieghere 
wrote:

> We already have `SBDebugger::GetIndexOfTarget`, would that work?

Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111827

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


[Lldb-commits] [PATCH] D111816: [lldb] Remove logging from Platform::~Platform

2021-10-14 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Fixed, thanks!

In D111816#3064923 , @thakis wrote:

> http://45.33.8.238/mac/37050/step_4.txt
>
>   In file included from 
> ../../lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:9:
>   In file included from 
> ../../lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h:12:
>   In file included from 
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/map:495:
>   In file included from 
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__node_handle:63:
>   In file included from 
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/memory:682:
>   In file included from 
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/shared_ptr.h:25:
>   
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/unique_ptr.h:50:19:
>  error: invalid application of 'sizeof' to an incomplete type 
> 'lldb_private::ModuleCache'
>   static_assert(sizeof(_Tp) > 0,
> ^~~
>   
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/unique_ptr.h:315:7:
>  note: in instantiation of member function 
> 'std::default_delete::operator()' requested here
> __ptr_.second()(__tmp);
> ^
>   
> ../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/unique_ptr.h:269:19:
>  note: in instantiation of member function 
> 'std::unique_ptr::reset' requested here
> ~unique_ptr() { reset(); }
> ^
>   ../../lldb/include/lldb/Target/Platform.h:78:3: note: in instantiation of 
> member function 'std::unique_ptr::~unique_ptr' 
> requested here
> ~Platform() override = default;
> ^
>   ../../lldb/include/lldb/Target/Platform.h:39:7: note: forward declaration 
> of 'lldb_private::ModuleCache'
>   class ModuleCache;
> ^




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111816

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


[Lldb-commits] [lldb] 482c53f - [lldb] Move ~Platform to source file

2021-10-14 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-10-14T21:36:46+02:00
New Revision: 482c53fa0dce16c0edebb494117dcc8dd383427e

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

LOG: [lldb] Move ~Platform to source file

The called destructors of the members require the includes that are only
in the source file.

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Target/Platform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 81858bbfa702..7cb534afe6ec 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -75,7 +75,7 @@ class Platform : public PluginInterface {
 
   /// The destructor is virtual since this class is designed to be inherited
   /// from by the plug-in instance.
-  ~Platform() override = default;
+  ~Platform() override;
 
   static void Initialize();
 

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index a414bc746083..07cbc119740a 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -395,6 +395,8 @@ Platform::Platform(bool is_host)
   LLDB_LOGF(log, "%p Platform::Platform()", static_cast(this));
 }
 
+Platform::~Platform() = default;
+
 void Platform::GetStatus(Stream &strm) {
   std::string s;
   strm.Printf("  Platform: %s\n", GetPluginName().GetCString());



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


[Lldb-commits] [PATCH] D111827: [lldb/API] Add GetIndexInDebugger to SBTarget

2021-10-14 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

We already have `SBDebugger::GetIndexOfTarget`, would that work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111827

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


[Lldb-commits] [PATCH] D111827: [lldb/API] Add GetIndexInDebugger to SBTarget

2021-10-14 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: jingham, JDevlieghere.
mib added a project: LLDB.
Herald added a subscriber: arphaman.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch adds a new SBTarget::GetIndexInDebugger method to the SBAPI.

As its name suggests, this can be used to get the target's index in the
debugger's targets list.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111827

Files:
  lldb/bindings/interface/SBTarget.i
  lldb/include/lldb/API/SBTarget.h
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/target/TestTargetAPI.py


Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -497,3 +497,15 @@
 module = target.GetModuleAtIndex(i)
 self.assertTrue(target.IsLoaded(module), "Running the target 
should "
 "have loaded its modules.")
+
+def test_get_index_in_debugger(self):
+"""Exercise SBTarget.GetIndexInDebugger() API."""
+self.build()
+target = self.create_simple_target('a.out')
+
+self.assertEqual(target.GetIndexInDebugger(), 0)
+process = target.GetProcess()
+self.assertNotEqual(process.GetTarget().GetIndexInDebugger(), 0)
+process = target.LaunchSimple(None, None,
+  self.get_process_working_directory())
+self.assertEqual(process.GetTarget().GetIndexInDebugger(), 0)
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -183,6 +183,19 @@
   return LLDB_RECORD_RESULT(sb_process);
 }
 
+uint32_t SBTarget::GetIndexInDebugger() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(uint32_t, SBTarget, GetIndexInDebugger);
+
+  SBDebugger dbg = GetDebugger();
+
+  for (uint32_t i = 0; i < dbg.GetNumTargets(); i++) {
+if (dbg.GetTargetAtIndex(i) == *this)
+  return i;
+  }
+
+  return UINT32_MAX;
+}
+
 SBPlatform SBTarget::GetPlatform() {
   LLDB_RECORD_METHOD_NO_ARGS(lldb::SBPlatform, SBTarget, GetPlatform);
 
@@ -2510,6 +2523,7 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBTarget, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBTarget, operator bool, ());
   LLDB_REGISTER_METHOD(lldb::SBProcess, SBTarget, GetProcess, ());
+  LLDB_REGISTER_METHOD_CONST(uint32_t, SBTarget, GetIndexInDebugger, ());
   LLDB_REGISTER_METHOD(lldb::SBPlatform, SBTarget, GetPlatform, ());
   LLDB_REGISTER_METHOD_CONST(lldb::SBDebugger, SBTarget, GetDebugger, ());
   LLDB_REGISTER_METHOD(lldb::SBStructuredData, SBTarget, GetStatistics, ());
Index: lldb/include/lldb/API/SBTarget.h
===
--- lldb/include/lldb/API/SBTarget.h
+++ lldb/include/lldb/API/SBTarget.h
@@ -66,6 +66,8 @@
 
   lldb::SBProcess GetProcess();
 
+  uint32_t GetIndexInDebugger() const;
+
   /// Sets whether we should collect statistics on lldb or not.
   ///
   /// \param[in] v
Index: lldb/bindings/interface/SBTarget.i
===
--- lldb/bindings/interface/SBTarget.i
+++ lldb/bindings/interface/SBTarget.i
@@ -96,6 +96,9 @@
 lldb::SBProcess
 GetProcess ();
 
+uint32_t
+GetIndexInDebugger () const;
+
 
 %feature("docstring", "
 Return the platform object associated with the target.


Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -497,3 +497,15 @@
 module = target.GetModuleAtIndex(i)
 self.assertTrue(target.IsLoaded(module), "Running the target should "
 "have loaded its modules.")
+
+def test_get_index_in_debugger(self):
+"""Exercise SBTarget.GetIndexInDebugger() API."""
+self.build()
+target = self.create_simple_target('a.out')
+
+self.assertEqual(target.GetIndexInDebugger(), 0)
+process = target.GetProcess()
+self.assertNotEqual(process.GetTarget().GetIndexInDebugger(), 0)
+process = target.LaunchSimple(None, None,
+  self.get_process_working_directory())
+self.assertEqual(process.GetTarget().GetIndexInDebugger(), 0)
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -183,6 +183,19 @@
   return LLDB_RECORD_RESULT(sb_process);
 }
 
+uint32_t SBTarget::GetIndexInDebugger() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(uint32_t, SBTarget, GetIndexInDebugger);
+
+  SBDebugger dbg = GetDebugger();
+
+  for (ui

[Lldb-commits] [PATCH] D111816: [lldb] Remove logging from Platform::~Platform

2021-10-14 Thread Nico Weber via Phabricator via lldb-commits
thakis added a comment.

http://45.33.8.238/mac/37050/step_4.txt

  In file included from 
../../lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:9:
  In file included from 
../../lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h:12:
  In file included from 
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/map:495:
  In file included from 
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__node_handle:63:
  In file included from 
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/memory:682:
  In file included from 
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/shared_ptr.h:25:
  
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/unique_ptr.h:50:19:
 error: invalid application of 'sizeof' to an incomplete type 
'lldb_private::ModuleCache'
  static_assert(sizeof(_Tp) > 0,
^~~
  
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/unique_ptr.h:315:7:
 note: in instantiation of member function 
'std::default_delete::operator()' requested here
__ptr_.second()(__tmp);
^
  
../../../chrome/src/third_party/llvm-build/Release+Asserts/bin/../include/c++/v1/__memory/unique_ptr.h:269:19:
 note: in instantiation of member function 
'std::unique_ptr::reset' requested here
~unique_ptr() { reset(); }
^
  ../../lldb/include/lldb/Target/Platform.h:78:3: note: in instantiation of 
member function 'std::unique_ptr::~unique_ptr' 
requested here
~Platform() override = default;
^
  ../../lldb/include/lldb/Target/Platform.h:39:7: note: forward declaration of 
'lldb_private::ModuleCache'
  class ModuleCache;
^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111816

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


[Lldb-commits] [PATCH] D111816: [lldb] Remove logging from Platform::~Platform

2021-10-14 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe632e900ac10: [lldb] Remove logging from Platform::~Platform 
(authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111816

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Target/Platform.cpp


Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -395,15 +395,6 @@
   LLDB_LOGF(log, "%p Platform::Platform()", static_cast(this));
 }
 
-/// Destructor.
-///
-/// The destructor is virtual since this class is designed to be
-/// inherited from by the plug-in instance.
-Platform::~Platform() {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
-  LLDB_LOGF(log, "%p Platform::~Platform()", static_cast(this));
-}
-
 void Platform::GetStatus(Stream &strm) {
   std::string s;
   strm.Printf("  Platform: %s\n", GetPluginName().GetCString());
Index: lldb/include/lldb/Target/Platform.h
===
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -73,11 +73,9 @@
   /// Default Constructor
   Platform(bool is_host_platform);
 
-  /// Destructor.
-  ///
   /// The destructor is virtual since this class is designed to be inherited
   /// from by the plug-in instance.
-  ~Platform() override;
+  ~Platform() override = default;
 
   static void Initialize();
 


Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -395,15 +395,6 @@
   LLDB_LOGF(log, "%p Platform::Platform()", static_cast(this));
 }
 
-/// Destructor.
-///
-/// The destructor is virtual since this class is designed to be
-/// inherited from by the plug-in instance.
-Platform::~Platform() {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
-  LLDB_LOGF(log, "%p Platform::~Platform()", static_cast(this));
-}
-
 void Platform::GetStatus(Stream &strm) {
   std::string s;
   strm.Printf("  Platform: %s\n", GetPluginName().GetCString());
Index: lldb/include/lldb/Target/Platform.h
===
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -73,11 +73,9 @@
   /// Default Constructor
   Platform(bool is_host_platform);
 
-  /// Destructor.
-  ///
   /// The destructor is virtual since this class is designed to be inherited
   /// from by the plug-in instance.
-  ~Platform() override;
+  ~Platform() override = default;
 
   static void Initialize();
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] e632e90 - [lldb] Remove logging from Platform::~Platform

2021-10-14 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-10-14T20:42:45+02:00
New Revision: e632e900ac1092f581b42fb93662613db9977b5a

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

LOG: [lldb] Remove logging from Platform::~Platform

Platform instances are stored in a function-local static list. However, the
logging code involves locking a function-local static mutex. This only works on
some implementations where the Log mutex is by accident destroyed *after* the
Platform list is destroyed.

This fixes randomly failing tests due to `recursive_mutex lock failed: Invalid
argument`.

Reviewed By: kastiglione

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

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Target/Platform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 9aed70188097f..81858bbfa7023 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -73,11 +73,9 @@ class Platform : public PluginInterface {
   /// Default Constructor
   Platform(bool is_host_platform);
 
-  /// Destructor.
-  ///
   /// The destructor is virtual since this class is designed to be inherited
   /// from by the plug-in instance.
-  ~Platform() override;
+  ~Platform() override = default;
 
   static void Initialize();
 

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 7a40446261f73..a414bc7460837 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -395,15 +395,6 @@ Platform::Platform(bool is_host)
   LLDB_LOGF(log, "%p Platform::Platform()", static_cast(this));
 }
 
-/// Destructor.
-///
-/// The destructor is virtual since this class is designed to be
-/// inherited from by the plug-in instance.
-Platform::~Platform() {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
-  LLDB_LOGF(log, "%p Platform::~Platform()", static_cast(this));
-}
-
 void Platform::GetStatus(Stream &strm) {
   std::string s;
   strm.Printf("  Platform: %s\n", GetPluginName().GetCString());



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


[Lldb-commits] [lldb] 78e17e2 - [lldb] Rewrite TestDiamond and document some bugs.

2021-10-14 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-10-14T20:32:07+02:00
New Revision: 78e17e23aa0fe525d00a2e8f0c7469f9a6b94f40

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

LOG: [lldb] Rewrite TestDiamond and document some bugs.

Added: 
lldb/test/API/lang/cpp/diamond/TestCppDiamond.py

Modified: 
lldb/test/API/lang/cpp/diamond/main.cpp

Removed: 
lldb/test/API/lang/cpp/diamond/TestDiamond.py



diff  --git a/lldb/test/API/lang/cpp/diamond/TestCppDiamond.py 
b/lldb/test/API/lang/cpp/diamond/TestCppDiamond.py
new file mode 100644
index 0..4a561be425775
--- /dev/null
+++ b/lldb/test/API/lang/cpp/diamond/TestCppDiamond.py
@@ -0,0 +1,84 @@
+"""
+Test diamond inheritance.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test_with_sbvalue(self):
+"""
+Test that virtual base classes work in when SBValue objects are
+used to explore the class.
+"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// breakpoint 1", 
lldb.SBFileSpec("main.cpp"))
+
+j1 = self.frame().FindVariable("j1")
+j1_Derived1 = j1.GetChildAtIndex(0)
+j1_Derived2 = j1.GetChildAtIndex(1)
+j1_Derived1_VBase = j1_Derived1.GetChildAtIndex(0)
+j1_Derived2_VBase = j1_Derived2.GetChildAtIndex(0)
+j1_Derived1_VBase_m_value = j1_Derived1_VBase.GetChildAtIndex(0)
+j1_Derived2_VBase_m_value = j1_Derived2_VBase.GetChildAtIndex(0)
+
+self.assertEqual(
+j1_Derived1_VBase.GetLoadAddress(), 
j1_Derived2_VBase.GetLoadAddress(),
+"ensure virtual base class is the same between Derived1 and 
Derived2")
+self.assertEqual(j1_Derived1_VBase_m_value.GetValueAsUnsigned(
+1), j1_Derived2_VBase_m_value.GetValueAsUnsigned(2), "ensure 
m_value in VBase is the same")
+
self.assertEqual(self.frame().FindVariable("d").GetChildAtIndex(0).GetChildAtIndex(
+0).GetValueAsUnsigned(0), 12345, "ensure Derived2 from j1 is 
correct")
+
+# This reassigns 'd' to point to 'j2'.
+self.thread().StepOver()
+
+
self.assertEqual(self.frame().FindVariable("d").GetChildAtIndex(0).GetChildAtIndex(
+0).GetValueAsUnsigned(0), 12346, "ensure Derived2 from j2 is 
correct")
+
+@no_debug_info_test
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// breakpoint 1", 
lldb.SBFileSpec("main.cpp"))
+
+# All the children of j1.
+children = [
+ValueCheck(type="Derived1", children=[
+ValueCheck(type="VBase", children=[
+ValueCheck(type="int", name="m_value", value="12345")
+])
+]),
+ValueCheck(type="Derived2", children=[
+ValueCheck(type="VBase", children=[
+ValueCheck(type="int", name="m_value", value="12345")
+])
+]),
+ValueCheck(type="long", value="1"),
+]
+# Try using the class with expression evaluator/variable paths.
+self.expect_expr("j1", result_type="Joiner1", result_children=children)
+self.expect_var_path("j1", type="Joiner1", children=children)
+
+# Use the expression evaluator to access the members.
+self.expect_expr("j1.x", result_type="long", result_value="1")
+self.expect_expr("j1.m_value", result_type="int", result_value="12345")
+
+# Use variable paths to access the members.
+self.expect_var_path("j1.x", type="long", value="1")
+
+@expectedFailureAll
+@no_debug_info_test
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// breakpoint 1", 
lldb.SBFileSpec("main.cpp"))
+# FIXME: This is completely broken and 'succeeds' with an error that
+# there is noch such value/member in Joiner1. Move this up to the test
+# above when fixed.
+self.expect_var_path("j1.m_value", type="int", value="12345")

diff  --git a/lldb/test/API/lang/cpp/diamond/TestDiamond.py 
b/lldb/test/API/lang/cpp/diamond/TestDiamond.py
deleted file mode 100644
index a77864bd841ad..0
--- a/lldb/test/API/lang/cpp/diamond/TestDiamond.py
+++ /dev/null
@@ -1,51 +0,0 @@
-"""
-Tests that bool types work
-"""
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-
-
-class CPPTestDiamondInheritance(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def test_with_run_command(self):
-"""Test that virtual base classes work in when SBValue obje

[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379771.
mgorny marked an inline comment as done.
mgorny added a comment.

Use inline lists instead of `for` loops for registers. Use `llvm::SmallDense*` 
types with `StringRef`s.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa

[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379738.
mgorny added a comment.

Final round of refactoring. The code should be simpler and more readable now.

I don't have a strong opinion wrt generating names in a loop vs hardcoding the 
list (possibly using a macro). Please decide how I should proceed ;-).


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_dat

[Lldb-commits] [lldb] 722a2fb - [lldb] Fix 'frame diagnose' docstring typo

2021-10-14 Thread Dave Lee via lldb-commits

Author: Dave Lee
Date: 2021-10-14T08:32:20-07:00
New Revision: 722a2fb7f9a3f7deea81276213c6a2a48a0827cd

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

LOG: [lldb] Fix 'frame diagnose' docstring typo

Added: 


Modified: 
lldb/source/Commands/CommandObjectFrame.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp 
b/lldb/source/Commands/CommandObjectFrame.cpp
index 1fd36e65ae151..9e5d79ce6257a 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -105,7 +105,7 @@ class CommandObjectFrameDiagnose : public 
CommandObjectParsed {
 
   CommandObjectFrameDiagnose(CommandInterpreter &interpreter)
   : CommandObjectParsed(interpreter, "frame diagnose",
-"Try to determine what path path the current stop "
+"Try to determine what path the current stop "
 "location used to get to a register or address",
 nullptr,
 eCommandRequiresThread | eCommandTryTargetAPILock |



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


[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379732.
mgorny marked 3 inline comments as done.
mgorny added a comment.

Next iteration. I will try to simplify even more.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa"])
+
+self.runCmd("register write ch 0xf9")
+ 

[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:91
+  ArchSpec arch = process_sp->GetTarget().GetArchitecture();
+  bool is64bit;
+  switch (arch.GetMachine()) {

I'd just do `is64bit = arch.GetAddressByteSize() == 8`



Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:124-190
+  auto in_basenames = gpr_basenames.find(reg_name + 1);
+  if (in_basenames != gpr_basenames.end() &&
+  new_index < gpr_base_reg_indices.size()) {
+GPRReg ®_rec = gpr_base_reg_indices[new_index++];
+reg_rec.base_index = x.index();
+
+// construct derived register names

What if we introduced helper functions like:
```
// returns 0 for [re]ax, 1 for [re]bx, ...
Optional GetGPRNumber(llvm::StringRef name) { 
  // either a lookup table or the fancy name matching
}
GPRReg MakeGPRReg(size_t base_index, unsigned gpr_number) {
  // I'd probably implement this via lookup table(s)
}
```
.. or something similar?

The general idea is to split this function into smaller chunks and also try to 
abstract away the funky register names as much as possible.



Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:211-213
+const char *reg_name = x.name.AsCString();
+auto found = subreg_name_set.find(reg_name);
+if (found != subreg_name_set.end())

`llvm::is_contained(haystack, needle)`


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

https://reviews.llvm.org/D108831

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


[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379697.
mgorny added a comment.

Halfway through to a new approach for getting regnames.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[1] = "faf91314"
+  

[Lldb-commits] [PATCH] D111791: [lldb] Add --all option to "memory region"

2021-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision.
Herald added a subscriber: dang.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds an option to the memory region command
to print all regions at once. Like you can do by
starting at address 0 and repeating the command
manually.

memory region [-a] []

(lldb) memory region --all
[0x-0x0040) ---
[0x0040-0x00401000) r-x <...>/a.out PT_LOAD[0]
<...>
[0xfffdf000-0x0001) rw- [stack]
[0x0001-0x) ---

The output matches exactly what you'd get from
repeating the command. Including that it shows
unmapped areas between the mapped regions.

(this is why Process GetMemoryRegions is not
used, that skips unmapped areas)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111791

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

Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -41,24 +41,40 @@
 self.assertFalse(result.Succeeded())
 self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
 
+# We allow --all or an address argument, not both
+interp.HandleCommand("memory region --all 0", result)
+self.assertFalse(result.Succeeded())
+self.assertRegexpMatches(result.GetError(),
+"The --all argument cannot be used when an address argument is given")
+
 # Test that when the address fails to parse, we show an error and do not continue
 interp.HandleCommand("memory region not_an_address", result)
 self.assertFalse(result.Succeeded())
 self.assertEqual(result.GetError(),
 "error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")
 
+# Accumulate the results to compare with the --all output
+all_regions = ""
+
 # Now let's print the memory region starting at 0 which should always work.
 interp.HandleCommand("memory region 0x0", result)
 self.assertTrue(result.Succeeded())
 self.assertRegexpMatches(result.GetOutput(), "\\[0x0+-")
+all_regions += result.GetOutput()
 
 # Keep printing memory regions until we printed all of them.
 while True:
 interp.HandleCommand("memory region", result)
 if not result.Succeeded():
 break
+all_regions += result.GetOutput()
 
 # Now that we reached the end, 'memory region' should again print the usage.
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
 self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+
+# --all should match what repeating the command gives you
+interp.HandleCommand("memory region --all", result)
+self.assertTrue(result.Succeeded())
+self.assertEqual(result.GetOutput(), all_regions)
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -511,6 +511,11 @@
 Desc<"Start writing bytes from an offset within the input file.">;
 }
 
+let Command = "memory region" in {
+  def memory_region_all : Option<"all", "a">,
+Desc<"Show all memory regions.">;
+}
+
 let Command = "memory tag write" in {
   def memory_write_end_addr : Option<"end-addr", "e">, Group<1>,
   Arg<"AddressOrExpression">, Desc<
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1608,8 +1608,45 @@
 // CommandObjectMemoryRegion
 #pragma mark CommandObjectMemoryRegion
 
+#define LLDB_OPTIONS_memory_region
+#include "CommandOptions.inc"
+
 class CommandObjectMemoryRegion : public CommandObjectParsed {
 public:
+  class OptionGroupMemoryRegion : public OptionGroup {
+  public:
+OptionGroupMemoryRegion() : OptionGroup(), m_all(false, false) {}
+
+~OptionGroupMemoryRegion() override = default;
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_memory_region_options);
+}
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+  ExecutionContext *execution_context) override {
+  Status status;
+  const int short_option = g_memory_region_options[option_idx].short_option;
+
+  switch (short_option) {
+  case 'a':
+m_all.SetCurrentValue(true);
+

[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379653.
mgorny added a comment.

Move `LLDB_INVALID_REGNUM` check inside `addPartialRegister()`.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[1] = "faf91

[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 379652.
mgorny added a comment.

Use `ArchSpec` instead of passing hardcoded 64bit. Abort the whole process if 
at least one subreg is present, eliminating all the `have*` bools.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -163,6 +163,67 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+self.runCmd("register write cx 0xfbfa")
+reg_data[0] = "fafbfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0xfffefbfa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[0] = "faf9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9fa"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[0] = "f8f9feff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0xfffef9f8"])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffef9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 @skipIfLLVMTargetMissing("X86")
@@ -272,11 +333,25 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -289,6 +364,35 @@
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
 
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa

[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I don't think this is that critical over there, but I think you could do that.


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

https://reviews.llvm.org/D108831

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


[Lldb-commits] [PATCH] D111355: [lldb] Add serial:// protocol for connecting to serial port [WIP/PoC]

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

I don't really have a strong opinion about this. I think we should leave the 
option of leaving port settings unchanged. I'll check what gdb does, as I 
suppose interoperability with gdbserver is also useful. I guess people are more 
likely to connect to other servers than lldb over serial port after all.


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

https://reviews.llvm.org/D111355

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


[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing

2021-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D108831#3063282 , @labath wrote:

> We don't really want to support adding an arbitrary subset of sub-registers, 
> do we? I am thinking if this could be made simpler if it was all-or-nothing. 
> Like, during the initial pass you could check whether the list contains _any_ 
> subregister, and abort if it does. Then the subsequent pass could assume that 
> all subregisters need to be added...

Sure, that makes sense. Should I update AArch64 too?


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

https://reviews.llvm.org/D108831

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


[Lldb-commits] [lldb] fa639ed - [lldb] Fix TestStackCorefile.py for ca0ce99fc8

2021-10-14 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-10-14T10:38:48+02:00
New Revision: fa639eda6535732a5fd79c3f551d5a667b810963

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

LOG: [lldb] Fix TestStackCorefile.py for ca0ce99fc8

Added: 


Modified: 
lldb/test/API/macosx/stack-corefile/main.c

Removed: 




diff  --git a/lldb/test/API/macosx/stack-corefile/main.c 
b/lldb/test/API/macosx/stack-corefile/main.c
index a52fb3056caa4..afc206b6de847 100644
--- a/lldb/test/API/macosx/stack-corefile/main.c
+++ b/lldb/test/API/macosx/stack-corefile/main.c
@@ -6,8 +6,7 @@ int main() {
   int *heap_int = (int*) malloc(sizeof (int));
   *heap_int = 10;
 
-  char stack_str[80];
-  strcpy (stack_str, "stack string");
+  char stack_str[] = "stack string";
   char *heap_str = (char*) malloc(80);
   strcpy (heap_str, "heap string");
 



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


[Lldb-commits] [PATCH] D111779: [lldb] Make the thread_local g_global_boundary accessed from a single file

2021-10-14 Thread Martin Storsjö via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7106f588567b: [lldb] Make the thread_local g_global_boundary 
accessed from a single file (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111779

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


Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -16,6 +16,9 @@
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
+// Whether we're currently across the API boundary.
+static thread_local bool g_global_boundary = false;
+
 void *IndexToObject::GetObjectForIndexImpl(unsigned idx) {
   return m_mapping.lookup(idx);
 }
@@ -227,6 +230,13 @@
   return m_sequence;
 }
 
+void Recorder::PrivateThread() { g_global_boundary = true; }
+
+void Recorder::UpdateBoundary() {
+  if (m_local_boundary)
+g_global_boundary = false;
+}
+
 void InstrumentationData::Initialize(Serializer &serializer,
  Registry ®istry) {
   InstanceImpl().emplace(serializer, registry);
@@ -248,6 +258,5 @@
   return g_instrumentation_data;
 }
 
-thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
 std::atomic lldb_private::repro::Recorder::g_sequence;
 std::mutex lldb_private::repro::Recorder::g_mutex;
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -868,17 +868,14 @@
 
   /// Mark the current thread as a private thread and pretend that everything
   /// on this thread is behind happening behind the API boundary.
-  static void PrivateThread() { g_global_boundary = true; }
+  static void PrivateThread();
 
 private:
   static unsigned GetNextSequenceNumber() { return g_sequence++; }
   unsigned GetSequenceNumber() const;
 
   template  friend struct replay;
-  void UpdateBoundary() {
-if (m_local_boundary)
-  g_global_boundary = false;
-  }
+  void UpdateBoundary();
 
 #ifdef LLDB_REPRO_INSTR_TRACE
   void Log(unsigned id) {
@@ -902,9 +899,6 @@
   /// The sequence number for this pair of function and result.
   unsigned m_sequence;
 
-  /// Whether we're currently across the API boundary.
-  static thread_local bool g_global_boundary;
-
   /// Global mutex to protect concurrent access.
   static std::mutex g_mutex;
 


Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -16,6 +16,9 @@
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
+// Whether we're currently across the API boundary.
+static thread_local bool g_global_boundary = false;
+
 void *IndexToObject::GetObjectForIndexImpl(unsigned idx) {
   return m_mapping.lookup(idx);
 }
@@ -227,6 +230,13 @@
   return m_sequence;
 }
 
+void Recorder::PrivateThread() { g_global_boundary = true; }
+
+void Recorder::UpdateBoundary() {
+  if (m_local_boundary)
+g_global_boundary = false;
+}
+
 void InstrumentationData::Initialize(Serializer &serializer,
  Registry ®istry) {
   InstanceImpl().emplace(serializer, registry);
@@ -248,6 +258,5 @@
   return g_instrumentation_data;
 }
 
-thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
 std::atomic lldb_private::repro::Recorder::g_sequence;
 std::mutex lldb_private::repro::Recorder::g_mutex;
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -868,17 +868,14 @@
 
   /// Mark the current thread as a private thread and pretend that everything
   /// on this thread is behind happening behind the API boundary.
-  static void PrivateThread() { g_global_boundary = true; }
+  static void PrivateThread();
 
 private:
   static unsigned GetNextSequenceNumber() { return g_sequence++; }
   unsigned GetSequenceNumber() const;
 
   template  friend struct replay;
-  void UpdateBoundary() {
-if (m_local_boundary)
-  g_global_boundary = false;
-  }
+  void UpdateBoundary();
 
 #ifdef LLDB_REPRO_INSTR_TRACE
   void Log(unsigned id) {
@@ -902,9 +899,6 @@
   /// The sequence number for this pair of function and result.
   unsigned m_sequence;
 
-  /// Whether we're currently across the API boundary.
-  static thread_local bool g_global_boundary;
-
   /// Global mutex to protect concurr

[Lldb-commits] [lldb] 7106f58 - [lldb] Make the thread_local g_global_boundary accessed from a single file

2021-10-14 Thread Martin Storsjö via lldb-commits

Author: Martin Storsjö
Date: 2021-10-14T11:17:20+03:00
New Revision: 7106f588567b59acb17c77f6116ba433b6226333

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

LOG: [lldb] Make the thread_local g_global_boundary accessed from a single file

This makes the compiler generated code for accessing the thread local
variable much simpler (no need for wrapper functions and weak pointers
to potential init functions), and can avoid toolchain bugs regarding how
to access TLS variables.

In particular, this fixes LLDB when built with current GCC/binutils for
MinGW, see https://github.com/msys2/MINGW-packages/issues/8868.

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

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/Utility/ReproducerInstrumentation.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 2b2d273a17a8f..6c5d27879d362 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -868,17 +868,14 @@ class Recorder {
 
   /// Mark the current thread as a private thread and pretend that everything
   /// on this thread is behind happening behind the API boundary.
-  static void PrivateThread() { g_global_boundary = true; }
+  static void PrivateThread();
 
 private:
   static unsigned GetNextSequenceNumber() { return g_sequence++; }
   unsigned GetSequenceNumber() const;
 
   template  friend struct replay;
-  void UpdateBoundary() {
-if (m_local_boundary)
-  g_global_boundary = false;
-  }
+  void UpdateBoundary();
 
 #ifdef LLDB_REPRO_INSTR_TRACE
   void Log(unsigned id) {
@@ -902,9 +899,6 @@ class Recorder {
   /// The sequence number for this pair of function and result.
   unsigned m_sequence;
 
-  /// Whether we're currently across the API boundary.
-  static thread_local bool g_global_boundary;
-
   /// Global mutex to protect concurrent access.
   static std::mutex g_mutex;
 

diff  --git a/lldb/source/Utility/ReproducerInstrumentation.cpp 
b/lldb/source/Utility/ReproducerInstrumentation.cpp
index e5bd2ba4b6259..b3285f4b3776a 100644
--- a/lldb/source/Utility/ReproducerInstrumentation.cpp
+++ b/lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -16,6 +16,9 @@
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
+// Whether we're currently across the API boundary.
+static thread_local bool g_global_boundary = false;
+
 void *IndexToObject::GetObjectForIndexImpl(unsigned idx) {
   return m_mapping.lookup(idx);
 }
@@ -227,6 +230,13 @@ unsigned Recorder::GetSequenceNumber() const {
   return m_sequence;
 }
 
+void Recorder::PrivateThread() { g_global_boundary = true; }
+
+void Recorder::UpdateBoundary() {
+  if (m_local_boundary)
+g_global_boundary = false;
+}
+
 void InstrumentationData::Initialize(Serializer &serializer,
  Registry ®istry) {
   InstanceImpl().emplace(serializer, registry);
@@ -248,6 +258,5 @@ llvm::Optional 
&InstrumentationData::InstanceImpl() {
   return g_instrumentation_data;
 }
 
-thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
 std::atomic lldb_private::repro::Recorder::g_sequence;
 std::mutex lldb_private::repro::Recorder::g_mutex;



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


[Lldb-commits] [PATCH] D111634: [lldb] Print embedded nuls in char arrays (PR44649)

2021-10-14 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGca0ce99fc87c: [lldb] Print embedded nuls in char arrays 
(PR44649) (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D111634?vs=378988&id=379619#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111634

Files:
  lldb/source/Core/ValueObject.cpp
  
lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
  lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
@@ -17,7 +17,7 @@
 ## Variables specified using string forms. This behavior purely speculative -- I
 ## don't know of any compiler that would represent character strings this way.
 # CHECK: (char [7]) string = "string"
-# CHECK: (char [7]) strp = "strp"
+# CHECK: (char [7]) strp = "strp\0\0"
 ## Bogus attribute form. Let's make sure we don't crash at least.
 # CHECK: (char [7]) ref4 = 
 ## A variable of pointer type.
Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -8,11 +8,12 @@
 
 int main (int argc, char const *argv[])
 {
-A a, b;
+A a, b, c;
 // Deliberately write past the end of data to test that the formatter stops
 // at the end of array.
 memcpy(a.data, "FOOBAR", 7);
 memcpy(b.data, "FO\0BAR", 7);
+memcpy(c.data, "F\0O\0AR", 7);
 std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); //%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables true"))
 const char* constcharstar = stdstring.c_str();
 std::string longstring(
@@ -33,13 +34,15 @@
 return 0; //% if self.TraceOn(): self.runCmd('frame variable')
 //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
 //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+//% self.expect_var_path("a.data", summary='"FOOB"')
+//% self.expect_var_path("b.data", summary=r'"FO\0B"')
+//% self.expect_var_path("c.data", summary=r'"F\0O"')
+//%
 //% self.runCmd("setting set escape-non-printables false")
 //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
 //% self.expect_var_path('constcharstar', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
 //% self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
 //% self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))
-//% self.expect_var_path("a.data", summary='"FOOB"')
-// FIXME: Should this be "FO\0B" instead?
-//% self.expect_var_path("b.data", summary='"FO"')
+// FIXME: make "b.data" and "c.data" work sanely
 }
 
Index: lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
===
--- lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
+++ lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
@@ -90,8 +90,8 @@
 
 # Different character arrays.
 # FIXME: Passing a 'const char *' will ignore any given format,
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("character array", "cstring"))
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("c-string", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("character array", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("c-string", "cstring"))
 self.assertIn(' = " \\e\\a\\b\\f\\n\\r\\t\\vaA09" " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n',
   self.getFormatted("c-string", "(char *)cstring"))
 self.assertIn('=\n', self.getFormatted("c-string", "(__UINT64_TYPE__)0"))
@@ -132,10 +132,10 @@
 self.assertIn('= 0x2007080c0a0d090b415a617a30391b00\n', self.getFormatted("OSType", string_expr))
 
 # bytes
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("bytes", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("bytes", "cstring"))
 
 # bytes with ASCII
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFo

[Lldb-commits] [lldb] ca0ce99 - [lldb] Print embedded nuls in char arrays (PR44649)

2021-10-14 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-10-14T09:50:40+02:00
New Revision: ca0ce99fc87c9a49b4c4a69cc6e88594bf6eb13c

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

LOG: [lldb] Print embedded nuls in char arrays (PR44649)

When we know the bounds of the array, print any embedded nuls instead of
treating them as terminators. An exception to this rule is made for the
nul character at the very end of the string. We don't print that, as
otherwise 99% of the strings would end in \0. This way the strings
usually come out the same as how the user typed it into the compiler
(char foo[] = "with\0nuls"). It also matches how they come out in gdb.

This resolves a FIXME left from D111399, and leaves another FIXME for dealing
with nul characters in "escape-non-printables=false" mode. In this mode the
characters cause the entire summary string to be terminated prematurely.

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

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp

lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 9c1ba99da1d05..6794d0c7331d3 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -849,8 +849,10 @@ bool ValueObject::SetData(DataExtractor &data, Status 
&error) {
 
 static bool CopyStringDataToBufferSP(const StreamString &source,
  lldb::DataBufferSP &destination) {
-  destination = std::make_shared(source.GetSize() + 1, 0);
-  memcpy(destination->GetBytes(), source.GetString().data(), source.GetSize());
+  llvm::StringRef src = source.GetString();
+  src.consume_back(llvm::StringRef("\0", 1));
+  destination = std::make_shared(src.size(), 0);
+  memcpy(destination->GetBytes(), src.data(), src.size());
   return true;
 }
 
@@ -912,8 +914,8 @@ ValueObject::ReadPointedString(lldb::DataBufferSP 
&buffer_sp, Status &error,
   CopyStringDataToBufferSP(s, buffer_sp);
   return {0, was_capped};
 }
-buffer_sp = std::make_shared(cstr_len, 0);
-memcpy(buffer_sp->GetBytes(), cstr, cstr_len);
+s << llvm::StringRef(cstr, cstr_len);
+CopyStringDataToBufferSP(s, buffer_sp);
 return {cstr_len, was_capped};
   } else {
 s << "";
@@ -1196,6 +1198,7 @@ bool ValueObject::DumpPrintableRepresentation(
 options.SetQuote('"');
 options.SetSourceSize(buffer_sp->GetByteSize());
 options.SetIsTruncated(read_string.second);
+options.SetBinaryZeroIsTerminator(custom_format != 
eFormatVectorOfChar);
 formatters::StringPrinter::ReadBufferAndDumpToStream<
 lldb_private::formatters::StringPrinter::StringElementType::ASCII>(
 options);

diff  --git 
a/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
 
b/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
index 741acd0431bd0..c894b80228cfe 100644
--- 
a/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
+++ 
b/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
@@ -90,8 +90,8 @@ def test(self):
 
 # Different character arrays.
 # FIXME: Passing a 'const char *' will ignore any given format,
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("character array", "cstring"))
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("c-string", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("character array", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("c-string", "cstring"))
 self.assertIn(' = " \\e\\a\\b\\f\\n\\r\\t\\vaA09" " 
\\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n',
   self.getFormatted("c-string", "(char *)cstring"))
 self.assertIn('=\n', self.getFormatted("c-string", 
"(__UINT64_TYPE__)0"))
@@ -132,10 +132,10 @@ def test(self):
 self.assertIn('= 0x2007080c0a0d090b415a617a30391b00\n', 
self.getFormatted("OSType", string_expr))
 
 # bytes
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("bytes", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("bytes", "cstring"))
 
 # bytes with ASCII
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("bytes with ASCII", "cstring"))
+self.assertIn(r'=

[Lldb-commits] [PATCH] D111634: [lldb] Print embedded nuls in char arrays (PR44649)

2021-10-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:40
+//% self.expect_var_path("c.data", summary=r'"F\0O"')
 //% self.runCmd("setting set escape-non-printables false")
 //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')

teemperor wrote:
> I think the whole 'empty lines break inline tests' thing has been fixed at 
> some point, so could you drop a new line between this and the printable thing 
> when you land?
sounds good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111634

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