[Lldb-commits] [PATCH] D111899: LLDB tests modification for hardware breakpoints

2021-10-20 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py:269
 self.expect(side_effect.bp_loc, exe=False,
-patterns=["1.* where = .*main .* resolved, hit count = 1"])
+patterns=["1.* where = .*main .* resolved,( hardware,)? hit 
count = 1"])
 

jingham wrote:
> Why didn't you change this one to use your new function?
`check_breakpoint` checks the correctness of a breakpoint and its locations, 
whereas this test seems to be checking that `side_effect` is correctly updated 
inside the breakpoint command script.


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

https://reviews.llvm.org/D111899

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


[Lldb-commits] [PATCH] D112058: [lldb/DWARF] Ignore debug info pointing to the low addresses

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

In D112058#3073451 , @clayborg wrote:

> Looks good. Do we need a follow up patch to avoid creating functions that 
> should have been stripped?

I'm not entirely sure what you mean by that. Are you referring to the fact that 
`SymbolFileDWARF::ResolveFunction` creates an `lldb_private::Function`, which 
it does not add the to result SymbolContextList (instead of not creating the 
object in the first place?

If yes, then that sounds like a good idea. I don't know if this has more 
user-facing implications, but I was definitely puzzled by the fact that the 
stripped function still showed up in "image dump ast". I'll see what can be 
done about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112058

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


[Lldb-commits] [lldb] ffbff6c - [lldb/DWARF] Ignore debug info pointing to the low addresses

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

Author: Pavel Labath
Date: 2021-10-20T11:19:30+02:00
New Revision: ffbff6c511ba230954013eca8d824a66f6b4f9a5

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

LOG: [lldb/DWARF] Ignore debug info pointing to the low addresses

specifically, ignore addresses that point before the first code section.

This resurrects D87172 with several notable changes:
- it fixes a bug where the early exits in InitializeObject left
  m_first_code_address "initialized" to LLDB_INVALID_ADDRESS (0xfff..f),
  which caused _everything_ to be ignored.
- it extends the line table fix to function parsing as well, where it
  replaces a similar check which was checking the executable permissions
  of the section. This was insufficient because some
  position-independent elf executables can have an executable segment
  mapped at file address zero. (What makes this fix different is that it
  checks for the executable-ness of the sections contained within that
  segment, and those will not be at address zero.)
- It uses a different test case, with an elf file with near-zero
  addresses, and checks for both line table and function parsing.

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c396e591ddc4..9078fa3cdc26 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -468,6 +468,8 @@ SymbolFileDWARF::GetTypeSystemForLanguage(LanguageType 
language) {
 void SymbolFileDWARF::InitializeObject() {
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
 
+  InitializeFirstCodeAddress();
+
   if (!GetGlobalPluginProperties().IgnoreFileIndexes()) {
 StreamString module_desc;
 GetObjectFile()->GetModule()->GetDescription(module_desc.AsRawOstream(),
@@ -512,6 +514,25 @@ void SymbolFileDWARF::InitializeObject() {
   std::make_unique(*GetObjectFile()->GetModule(), *this);
 }
 
+void SymbolFileDWARF::InitializeFirstCodeAddress() {
+  InitializeFirstCodeAddressRecursive(
+  *m_objfile_sp->GetModule()->GetSectionList());
+  if (m_first_code_address == LLDB_INVALID_ADDRESS)
+m_first_code_address = 0;
+}
+
+void SymbolFileDWARF::InitializeFirstCodeAddressRecursive(
+const lldb_private::SectionList §ion_list) {
+  for (SectionSP section_sp : section_list) {
+if (section_sp->GetChildren().GetSize() > 0) {
+  InitializeFirstCodeAddressRecursive(section_sp->GetChildren());
+} else if (section_sp->GetType() == eSectionTypeCode) {
+  m_first_code_address =
+  std::min(m_first_code_address, section_sp->GetFileAddress());
+}
+  }
+}
+
 bool SymbolFileDWARF::SupportedVersion(uint16_t version) {
   return version >= 2 && version <= 5;
 }
@@ -,6 +1132,12 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit 
&comp_unit) {
   // The Sequences view contains only valid line sequences. Don't iterate over
   // the Rows directly.
   for (const llvm::DWARFDebugLine::Sequence &seq : line_table->Sequences) {
+// Ignore line sequences that do not start after the first code address.
+// All addresses generated in a sequence are incremental so we only need
+// to check the first one of the sequence. Check the comment at the
+// m_first_code_address declaration for more details on this.
+if (seq.LowPC < m_first_code_address)
+  continue;
 std::unique_ptr sequence =
 LineTable::CreateLineSequenceContainer();
 for (unsigned idx = seq.FirstRowIndex; idx < seq.LastRowIndex; ++idx) {
@@ -2236,12 +2263,9 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE 
&orig_die,
   addr = sc.function->GetAddressRange().GetBaseAddress();
 }
 
-
-if (auto section_sp = addr.GetSection()) {
-  if (section_sp->GetPermissions() & ePermissionsExecutable) {
-sc_list.Append(sc);
-return true;
-  }
+if (addr.IsValid() && addr.GetFileAddress() >= m_first_code_address) {
+  sc_list.Append(sc);
+  return true;
 }
   }
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index e02ae1347e84..6e58c8b6178e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -494,6 +494,11 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
 
   const lldb_private::FileSpecList &GetTypeUnitSupportFi

[Lldb-commits] [PATCH] D112058: [lldb/DWARF] Ignore debug info pointing to the low addresses

2021-10-20 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGffbff6c511ba: [lldb/DWARF] Ignore debug info pointing to the 
low addresses (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112058

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg
  lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml

Index: lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
@@ -0,0 +1,152 @@
+# RUN: yaml2obj %s > %t
+# RUN: %lldb %t -o "image dump line-table a.c" -o "image lookup -n _start" -o "image lookup -n f" -o exit | FileCheck %s
+
+# CHECK-LABEL: image dump line-table a.c
+# CHECK-NEXT: Line table for a.c
+# CHECK-NEXT: 0x0080: a.c:1
+# CHECK-NEXT: 0x0084: a.c:1
+# CHECK-NEXT: 0x0086: a.c:1
+# CHECK-EMPTY:
+# CHECK-NEXT: image lookup -n _start
+# CHECK-NEXT: 1 match found
+# CHECK-LABEL: image lookup -n f
+# CHECK-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+ProgramHeaders:
+  - Type:PT_LOAD
+Flags:   [ PF_X, PF_R ]
+Offset:  0x
+FirstSec:.text
+LastSec: .text
+Align:   0x1000
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x80
+AddressAlign:0x10
+Content: 554889E55DC3
+  - Name:.debug_abbrev
+Type:SHT_PROGBITS
+AddressAlign:0x1
+  - Name:.debug_info
+Type:SHT_PROGBITS
+AddressAlign:0x1
+  - Name:.debug_line
+Type:SHT_PROGBITS
+AddressAlign:0x1
+DWARF:
+  debug_ranges:
+- Offset:  0x0
+  AddrSize:0x8
+  Entries:
+- LowOffset:   0x0
+  HighOffset:  0x6
+- LowOffset:   0x80
+  HighOffset:  0x86
+  debug_abbrev:
+- ID:  0
+  Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_yes
+  Attributes:
+- Attribute:   DW_AT_producer
+  Form:DW_FORM_string
+- Attribute:   DW_AT_name
+  Form:DW_FORM_string
+- Attribute:   DW_AT_stmt_list
+  Form:DW_FORM_sec_offset
+- Attribute:   DW_AT_low_pc
+  Form:DW_FORM_addr
+- Attribute:   DW_AT_ranges
+  Form:DW_FORM_sec_offset
+- Code:0x0002
+  Tag: DW_TAG_subprogram
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_low_pc
+  Form:DW_FORM_addr
+- Attribute:   DW_AT_high_pc
+  Form:DW_FORM_data4
+- Attribute:   DW_AT_name
+  Form:DW_FORM_string
+  debug_info:
+- Version: 4
+  AbbrevTableID:   0
+  AbbrOffset:  0x
+  AddrSize:8
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- CStr:Hand-written DWARF
+- CStr:a.c
+- Value:   0x
+- Value:   0x
+- Value:   0x
+- AbbrCode:0x0002
+  Values:
+- Value:   0x
+- Value:   0x0006
+- CStr:f
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0080
+- Value:   0x0006
+- CStr:_start
+- AbbrCode:0x
+  debug_line:
+- Version: 4
+  MinInstLength:   1
+  MaxOpsPerInst:   1
+  DefaultIsStmt:   1
+  LineBase:251
+  LineRange:   14
+  OpcodeBase:  13
+  StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ]
+  IncludeDirs: []
+  Files:
+- Name:a.c
+  DirIdx:  0
+  ModTime: 0
+  Length:  0
+  Opcodes:
+- Opcode:  DW_LNS_extended_op
+  ExtLen:  9
+  SubOpcode:   DW_LNE_set_address
+  Data:0
+- Opcode:  DW_LNS_cop

[Lldb-commits] [lldb] 551d118 - [lldb/test] Remove quote/unquote steps from the make invocations

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

Author: Pavel Labath
Date: 2021-10-20T11:35:28+02:00
New Revision: 551d118805c808936956e464dc21e05acb478f78

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

LOG: [lldb/test] Remove quote/unquote steps from the make invocations

None of the commands we run really rely on shell features. Running them
with shell=False, simplifies the code as there is no need for elaborate
quoting.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/builders/darwin.py
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index b49c6324c8ca..38e64220ed63 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -2,6 +2,7 @@
 import platform
 import subprocess
 import sys
+import itertools
 
 import lldbsuite.test.lldbtest as lldbtest
 import lldbsuite.test.lldbutil as lldbutil
@@ -25,11 +26,11 @@ def getExtraMakeArgs(self):
 Helper function to return extra argumentsfor the make system. This
 method is meant to be overridden by platform specific builders.
 """
-return ""
+return []
 
 def getArchCFlags(self, architecture):
 """Returns the ARCH_CFLAGS for the make system."""
-return ""
+return []
 
 def getMake(self, test_subdir, test_name):
 """Returns the invocation for GNU make.
@@ -59,29 +60,27 @@ def getMake(self, test_subdir, test_name):
 
 def getCmdLine(self, d):
 """
-Helper function to return a properly formatted command line argument(s)
-string used for the make system.
+Helper function to return a command line argument string used for the
+make system.
 """
 
-# If d is None or an empty mapping, just return an empty string.
+# If d is None or an empty mapping, just return an empty list.
 if not d:
-return ""
-pattern = '%s="%s"' if "win32" in sys.platform else "%s='%s'"
+return []
 
 def setOrAppendVariable(k, v):
 append_vars = ["CFLAGS", "CFLAGS_EXTRAS", "LD_EXTRAS"]
 if k in append_vars and k in os.environ:
 v = os.environ[k] + " " + v
-return pattern % (k, v)
+return '%s=%s' % (k, v)
 
-cmdline = " ".join(
-[setOrAppendVariable(k, v) for k, v in list(d.items())])
+cmdline = [setOrAppendVariable(k, v) for k, v in list(d.items())]
 
 return cmdline
 
-def runBuildCommands(self, commands, sender):
+def runBuildCommands(self, commands):
 try:
-lldbtest.system(commands, sender=sender)
+lldbtest.system(commands)
 except subprocess.CalledProcessError as called_process_error:
 # Convert to a build-specific error.
 # We don't do that in lldbtest.system() since that
@@ -93,7 +92,7 @@ def getArchSpec(self, architecture):
 Helper function to return the key-value string to specify the 
architecture
 used for the make system.
 """
-return ("ARCH=" + architecture) if architecture else ""
+return ["ARCH=" + architecture] if architecture else []
 
 def getCCSpec(self, compiler):
 """
@@ -104,9 +103,8 @@ def getCCSpec(self, compiler):
 if not cc and configuration.compiler:
 cc = configuration.compiler
 if cc:
-return "CC=\"%s\"" % cc
-else:
-return ""
+return ["CC=\"%s\"" % cc]
+return []
 
 def getSDKRootSpec(self):
 """
@@ -114,8 +112,8 @@ def getSDKRootSpec(self):
 used for the make system.
 """
 if configuration.sdkroot:
-return "SDKROOT={}".format(configuration.sdkroot)
-return ""
+return ["SDKROOT={}".format(configuration.sdkroot)]
+return []
 
 def getModuleCacheSpec(self):
 """
@@ -123,9 +121,9 @@ def getModuleCacheSpec(self):
 module cache used for the make system.
 """
 if configuration.clang_module_cache_dir:
-return "CLANG_MODULE_CACHE_DIR={}".format(
-configuration.clang_module_cache_dir)
-return ""
+return ["CLANG_MODULE_CACHE_DIR={}".format(
+configuration.clang_module_cache_dir)]
+return []
 
 def _getDebugInfoArgs(self, debug_info):
 if debug_info is None:
@@ -138,28 +136,23 @@ def _getDebugInfoArgs(self, debug_info):
 return ["MAKE_DSYM=NO", "MAKE_GMODULES

[Lldb-commits] [PATCH] D111990: [lldb/test] Remove quote/unquote steps from the make invocations

2021-10-20 Thread Pavel Labath via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG551d118805c8: [lldb/test] Remove quote/unquote steps from 
the make invocations (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111990

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/builders/darwin.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py

Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -494,24 +494,16 @@
 'ls: non_existent_file: No such file or directory\n'
 """
 
-# Assign the sender object to variable 'test' and remove it from kwargs.
-test = kwargs.pop('sender', None)
-
-# [['make', 'clean', 'foo'], ['make', 'foo']] -> ['make clean foo', 'make foo']
-commandList = [' '.join(x) for x in commands]
 output = ""
 error = ""
-for shellCommand in commandList:
+for shellCommand in commands:
 if 'stdout' in kwargs:
 raise ValueError(
 'stdout argument not allowed, it will be overridden.')
-if 'shell' in kwargs and kwargs['shell'] == False:
-raise ValueError('shell=False not allowed')
 process = Popen(
 shellCommand,
 stdout=PIPE,
 stderr=STDOUT,
-shell=True,
 **kwargs)
 pid = process.pid
 this_output, this_error = process.communicate()
@@ -1473,8 +1465,8 @@
 testname = self.getBuildDirBasename()
 
 module = builder_module()
-if not module.build(debug_info, self, architecture, compiler,
-   dictionary, testdir, testname):
+if not module.build(debug_info, architecture, compiler, dictionary,
+testdir, testname):
 raise Exception("Don't know how to build binary")
 
 # ==
@@ -1673,7 +1665,7 @@
 def cleanup(self, dictionary=None):
 """Platform specific way to do cleanup after build."""
 module = builder_module()
-if not module.cleanup(self, dictionary):
+if not module.cleanup(dictionary):
 raise Exception(
 "Don't know how to do cleanup with dictionary: " +
 dictionary)
Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -80,16 +80,14 @@
 args['CODESIGN'] = 'codesign'
 
 # Return extra args as a formatted string.
-return ' '.join(
-{'{}="{}"'.format(key, value)
- for key, value in args.items()})
+return ['{}={}'.format(key, value) for key, value in args.items()]
 
 def getArchCFlags(self, arch):
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
 if vendor is None or os is None or version is None or env is None:
-return ""
+return []
 
 # Construct the triple from its components.
 triple = '-'.join([arch, vendor, os, version, env])
@@ -101,7 +99,7 @@
 elif os == "macosx":
 version_min = "-m{}-version-min={}".format(os, version)
 
-return "ARCH_CFLAGS=\"-target {} {}\"".format(triple, version_min)
+return ["ARCH_CFLAGS=-target {} {}".format(triple, version_min)]
 
 def _getDebugInfoArgs(self, debug_info):
 if debug_info == "dsym":
Index: lldb/packages/Python/lldbsuite/test/builders/builder.py
===
--- lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -2,6 +2,7 @@
 import platform
 import subprocess
 import sys
+import itertools
 
 import lldbsuite.test.lldbtest as lldbtest
 import lldbsuite.test.lldbutil as lldbutil
@@ -25,11 +26,11 @@
 Helper function to return extra argumentsfor the make system. This
 method is meant to be overridden by platform specific builders.
 """
-return ""
+return []
 
 def getArchCFlags(self, architecture):
 """Returns the ARCH_CFLAGS for the make system."""
-return ""
+return []
 
 def getMake(self, test_subdir, test_name):
 """Returns the invocation for GNU make.
@@ -59,29 +60,27 @@
 
 def getCmdLine(self, d):
 """
-Helper function to return a properly formatted command line argument(s)
-string used for the make s

[Lldb-commits] [PATCH] D112061: [lldb] Remove ConstString from GetPluginNameStatic of some plugins

2021-10-20 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112061

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


[Lldb-commits] [lldb] 956df6f - [lldb] Improve assert message in TestCPPAccelerator

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

Author: Raphael Isemann
Date: 2021-10-20T12:04:28+02:00
New Revision: 956df6fa620a0ca75fd6e62b5318fb4d14304a4f

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

LOG: [lldb] Improve assert message in TestCPPAccelerator

`log` is just some IO object that gets printed as `<_io.TextIOWrapper = 
filename`
but the intention here was to print the actual found log contents.

Added: 


Modified: 
lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py 
b/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
index 3600de758ff3..83499d8f95f6 100644
--- a/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ b/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -23,11 +23,12 @@ def test(self):
 # of it not being in the first file looked at.
 self.expect('frame variable inner_d')
 
-log = open(logfile, 'r')
+with open(logfile) as f:
+log = f.readlines()
 n = 0
 for line in log:
 if re.findall(r'[abcdefg]\.o: FindByNameAndTag\(\)', line):
 self.assertIn("d.o", line)
 n += 1
 
-self.assertEqual(n, 1, log)
+self.assertEqual(n, 1, "".join(log))



___
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

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



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774
+  llvm::Error error = llvm::Error::success();
+  m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite,
+ serial_options.get(), true, error);

It would be better to hide this by making the constructor private and have a 
static factory function returning `Expected>`. Bonus 
points if you can move all the fallible operations into the factory function so 
that the (now private) constructor does not need the Error argument at 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] [lldb] 6561c07 - [lldb] [Process/Utility] Define qN regs on ARM via helper macro

2021-10-20 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-10-20T13:08:17+02:00
New Revision: 6561c074c072beb6c8e400a62bd5943a1f26a72a

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

LOG: [lldb] [Process/Utility] Define qN regs on ARM via helper macro

Add a FPU_QREG macro to define qN registers.  This is a piece-wise
attempt of reconstructing D112066 with the goal of figuring out which
part of the larger change breaks the buildbot.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h
index 2eabe1659c2b..a0bae19b1fbc 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h
@@ -320,6 +320,15 @@ static uint32_t g_q15_contains[] = {fpu_d30, fpu_d31, 
LLDB_INVALID_REGNUM};
  nullptr, g_##name##_invalidates,  
\
   }
 
+#define FPU_QREG(name, offset) 
\
+  {
\
+#name, nullptr, 16, FPU_OFFSET(offset), eEncodingVector,   
\
+eFormatVectorOfUInt8,  
\
+{LLDB_INVALID_REGNUM, dwarf_##name, LLDB_INVALID_REGNUM,   
\
+ LLDB_INVALID_REGNUM, fpu_##name },
\
+ g_##name##_contains, nullptr, 
\
+  }
+
 static RegisterInfo g_register_infos_arm[] = {
 //  NAME ALT SZ   OFFSET  ENCODING  FORMAT
 //  EH_FRAME DWARFGENERIC
@@ -612,198 +621,22 @@ static RegisterInfo g_register_infos_arm[] = {
 FPU_REG(d30, 8, 60, q15),
 FPU_REG(d31, 8, 62, q15),
 
-{
-"q0",
-nullptr,
-16,
-FPU_OFFSET(0),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q0, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q0},
-g_q0_contains,
-nullptr,
-},
-{
-"q1",
-nullptr,
-16,
-FPU_OFFSET(4),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q1, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q1},
-g_q1_contains,
-nullptr,
-},
-{
-"q2",
-nullptr,
-16,
-FPU_OFFSET(8),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q2, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q2},
-g_q2_contains,
-nullptr,
-},
-{
-"q3",
-nullptr,
-16,
-FPU_OFFSET(12),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q3, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q3},
-g_q3_contains,
-nullptr,
-},
-{
-"q4",
-nullptr,
-16,
-FPU_OFFSET(16),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q4, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q4},
-g_q4_contains,
-nullptr,
-},
-{
-"q5",
-nullptr,
-16,
-FPU_OFFSET(20),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q5, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q5},
-g_q5_contains,
-nullptr,
-},
-{
-"q6",
-nullptr,
-16,
-FPU_OFFSET(24),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q6, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q6},
-g_q6_contains,
-nullptr,
-},
-{
-"q7",
-nullptr,
-16,
-FPU_OFFSET(28),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q7, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q7},
-g_q7_contains,
-nullptr,
-},
-{
-"q8",
-nullptr,
-16,
-FPU_OFFSET(32),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q8, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, fpu_q8},
-g_q8_contains,
-nullptr,
-},
-{
-"q9",
-nullptr,
-16,
-FPU_OFFSET(36),
-eEncodingVector,
-eFormatVectorOfUInt8,
-{LLDB_INVALID_REGNUM, dwarf_q9, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM

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

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774
+  llvm::Error error = llvm::Error::success();
+  m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite,
+ serial_options.get(), true, error);

labath wrote:
> It would be better to hide this by making the constructor private and have a 
> static factory function returning `Expected>`. Bonus 
> points if you can move all the fallible operations into the factory function 
> so that the (now private) constructor does not need the Error argument at all
I know but I couldn't make this work — the compiler just won't find a way to 
convert the new `SerialPort` instance into `Expected`. I suspect I 
would have to add move ctors and assignment operators all the way up the class 
hierarchy.


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] D111355: [lldb] Add serial:// protocol for connecting to serial port

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



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774
+  llvm::Error error = llvm::Error::success();
+  m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite,
+ serial_options.get(), true, error);

mgorny wrote:
> labath wrote:
> > It would be better to hide this by making the constructor private and have 
> > a static factory function returning `Expected>`. 
> > Bonus points if you can move all the fallible operations into the factory 
> > function so that the (now private) constructor does not need the Error 
> > argument at all
> I know but I couldn't make this work — the compiler just won't find a way to 
> convert the new `SerialPort` instance into `Expected`. I suspect 
> I would have to add move ctors and assignment operators all the way up the 
> class hierarchy.
That's where the `unique_ptr` part comes in. :)


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] D111030: [lldb] [Host] Add setters for common teletype properties to Terminal

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



Comment at: lldb/source/Host/common/Terminal.cpp:316-347
+  bool parity_bit = true;
+  bool odd_bit = false;
+  bool stick_bit = false;
+
+  switch (parity) {
+  case Parity::No:
+parity_bit = false;

I am wondering if we can avoid the double conversion (enum->bools->flags). 
Would something like this be shorter/cleaner:
```
unsigned(?) GetParityFlagMask() {
  return PARENB | PARODD
#ifdef CMSPAR
| CMSPAR
#endif
  ;
}
Expected GetParityFlags(Parity) // error if parity not supported

SetParity(parity) {
getParityFlags(parity); // and check result
GetData(); // and check result
data.m_termios.c_cflag &= ~GetParityFlagMask();
data.m_termios.c_cflag |= flags;
SetData();
```



Comment at: lldb/unittests/Host/posix/TerminalTest.cpp:187-189
+llvm::Failed(testing::Property(
+&llvm::ErrorInfoBase::message,
+"space/mark parity is not supported by the 
platform")));

btw, we have `FailedWithMessage` for this these days.


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

https://reviews.llvm.org/D111030

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


[Lldb-commits] [PATCH] D112131: [lldb] [Process/Linux] Support arbitrarily-sized FPR writes on ARM

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, teemperor.
Herald added a subscriber: kristof.beyls.
mgorny requested review of this revision.

Support arbitrarily-sized FPR writes on ARM in order to fix writing qN
registers directly.  Currently, writing them works only by accident
due to value_regs splitting them into smaller writes via dN and sN
registers.


https://reviews.llvm.org/D112131

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -182,27 +182,9 @@
 uint32_t fpr_offset = CalculateFprOffset(reg_info);
 assert(fpr_offset < sizeof m_fpr);
 uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-switch (reg_info->byte_size) {
-case 2:
-  *(uint16_t *)dst = reg_value.GetAsUInt16();
-  break;
-case 4:
-  *(uint32_t *)dst = reg_value.GetAsUInt32();
-  break;
-case 8:
-  *(uint64_t *)dst = reg_value.GetAsUInt64();
-  break;
-default:
-  assert(false && "Unhandled data size.");
-  return Status("unhandled register data size %" PRIu32,
-reg_info->byte_size);
-}
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
-Status error = WriteFPR();
-if (error.Fail())
-  return error;
-
-return Status();
+return WriteFPR();
   }
 
   return Status("failed - register wasn't recognized to be a GPR or an FPR, "


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -182,27 +182,9 @@
 uint32_t fpr_offset = CalculateFprOffset(reg_info);
 assert(fpr_offset < sizeof m_fpr);
 uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-switch (reg_info->byte_size) {
-case 2:
-  *(uint16_t *)dst = reg_value.GetAsUInt16();
-  break;
-case 4:
-  *(uint32_t *)dst = reg_value.GetAsUInt32();
-  break;
-case 8:
-  *(uint64_t *)dst = reg_value.GetAsUInt64();
-  break;
-default:
-  assert(false && "Unhandled data size.");
-  return Status("unhandled register data size %" PRIu32,
-reg_info->byte_size);
-}
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
-Status error = WriteFPR();
-if (error.Fail())
-  return error;
-
-return Status();
+return WriteFPR();
   }
 
   return Status("failed - register wasn't recognized to be a GPR or an FPR, "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D111899: LLDB tests modification for hardware breakpoints

2021-10-20 Thread Nikolay Chokoev via Phabricator via lldb-commits
georgiev updated this revision to Diff 380919.
georgiev added a comment.

Added breakpoint resolved check.
Addressed some of the comments.


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

https://reviews.llvm.org/D111899

Files:
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
  lldb/test/API/functionalities/dead-strip/TestDeadStrip.py
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/memory/cache/TestMemoryCache.py
  lldb/test/API/functionalities/memory/find/TestMemoryFind.py
  lldb/test/API/functionalities/memory/read/TestMemoryRead.py
  lldb/test/API/lang/c/anonymous/TestAnonymous.py
  lldb/test/API/lang/c/array_types/TestArrayTypes.py
  lldb/test/API/lang/c/bitfields/TestBitfields.py
  lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
  lldb/test/API/lang/c/const_variables/TestConstVariables.py
  lldb/test/API/lang/c/enum_types/TestEnumTypes.py
  lldb/test/API/lang/c/forward/TestForwardDeclaration.py
  lldb/test/API/lang/c/function_types/TestFunctionTypes.py
  lldb/test/API/lang/c/global_variables/TestGlobalVariables.py
  lldb/test/API/lang/c/local_variables/TestLocalVariables.py
  lldb/test/API/lang/c/modules/TestCModules.py
  lldb/test/API/lang/c/register_variables/TestRegisterVariables.py
  lldb/test/API/lang/c/set_values/TestSetValues.py
  lldb/test/API/lang/c/shared_lib/TestSharedLib.py
  
lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/class_types/TestClassTypes.py
  lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
  lldb/test/API/lang/cpp/inlines/TestInlines.py
  lldb/test/API/lang/cpp/namespace_definitions/TestNamespaceDefinitions.py
  lldb/test/API/lang/cpp/signed_types/TestSignedTypes.py
  lldb/test/API/lang/objc/conflicting-definition/TestConflictingDefinition.py
  lldb/test/API/lang/objc/forward-decl/TestForwardDecl.py
  lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py
  lldb/test/API/lang/objc/modules-auto-import/TestModulesAutoImport.py
  lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
  lldb/test/API/lang/objc/modules/TestObjCModules.py
  lldb/test/API/lang/objc/objc-new-syntax/ObjCNewSyntaxTest.py
  lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
  
lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py

Index: lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py
===
--- lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py
+++ lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py
@@ -40,8 +40,7 @@
  'stop reason = breakpoint'])
 
 # The breakpoint should have a hit count of 1.
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 d1 = self.frame().FindVariable("d1")
 d1.SetPreferSyntheticValue(True)
Index: lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
===
--- lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
+++ lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
@@ -33,14 +33,12 @@
  'stop reason = breakpoint'])
 
 # Run and stop at Foo
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 self.runCmd("continue", RUN_SUCCEEDED)
 
 # Run at stop at main
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 # This should display correctly.
 self.expect(
@@ -71,14 +69,12 @@
  'stop reason = breakpoint'])
 
 # Run and stop at Foo
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 self.runCmd("continue", RUN_SUCCEEDED)
 
 # Run at stop at main
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hi

[Lldb-commits] [PATCH] D111899: LLDB tests modification for hardware breakpoints

2021-10-20 Thread Nikolay Chokoev via Phabricator via lldb-commits
georgiev updated this revision to Diff 380922.
georgiev added a comment.

Diff update


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

https://reviews.llvm.org/D111899

Files:
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
  lldb/test/API/functionalities/dead-strip/TestDeadStrip.py
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/memory/cache/TestMemoryCache.py
  lldb/test/API/functionalities/memory/find/TestMemoryFind.py
  lldb/test/API/functionalities/memory/read/TestMemoryRead.py
  lldb/test/API/lang/c/anonymous/TestAnonymous.py
  lldb/test/API/lang/c/array_types/TestArrayTypes.py
  lldb/test/API/lang/c/bitfields/TestBitfields.py
  lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
  lldb/test/API/lang/c/const_variables/TestConstVariables.py
  lldb/test/API/lang/c/enum_types/TestEnumTypes.py
  lldb/test/API/lang/c/forward/TestForwardDeclaration.py
  lldb/test/API/lang/c/function_types/TestFunctionTypes.py
  lldb/test/API/lang/c/global_variables/TestGlobalVariables.py
  lldb/test/API/lang/c/local_variables/TestLocalVariables.py
  lldb/test/API/lang/c/modules/TestCModules.py
  lldb/test/API/lang/c/register_variables/TestRegisterVariables.py
  lldb/test/API/lang/c/set_values/TestSetValues.py
  lldb/test/API/lang/c/shared_lib/TestSharedLib.py
  
lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/class_types/TestClassTypes.py
  lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
  lldb/test/API/lang/cpp/inlines/TestInlines.py
  lldb/test/API/lang/cpp/namespace_definitions/TestNamespaceDefinitions.py
  lldb/test/API/lang/cpp/signed_types/TestSignedTypes.py
  lldb/test/API/lang/objc/conflicting-definition/TestConflictingDefinition.py
  lldb/test/API/lang/objc/forward-decl/TestForwardDecl.py
  lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py
  lldb/test/API/lang/objc/modules-auto-import/TestModulesAutoImport.py
  lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
  lldb/test/API/lang/objc/modules/TestObjCModules.py
  lldb/test/API/lang/objc/objc-new-syntax/ObjCNewSyntaxTest.py
  lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
  
lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py

Index: lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py
===
--- lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py
+++ lldb/test/API/lang/objc/single-entry-dictionary/TestObjCSingleEntryDictionary.py
@@ -40,8 +40,7 @@
  'stop reason = breakpoint'])
 
 # The breakpoint should have a hit count of 1.
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 d1 = self.frame().FindVariable("d1")
 d1.SetPreferSyntheticValue(True)
Index: lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
===
--- lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
+++ lldb/test/API/lang/objc/real-definition/TestRealDefinition.py
@@ -33,14 +33,12 @@
  'stop reason = breakpoint'])
 
 # Run and stop at Foo
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 self.runCmd("continue", RUN_SUCCEEDED)
 
 # Run at stop at main
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 # This should display correctly.
 self.expect(
@@ -71,14 +69,12 @@
  'stop reason = breakpoint'])
 
 # Run and stop at Foo
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 self.runCmd("continue", RUN_SUCCEEDED)
 
 # Run at stop at main
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
 
 # This should display correct

[Lldb-commits] [lldb] 192331b - [lldb] [Process/Linux] Support arbitrarily-sized FPR writes on ARM

2021-10-20 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-10-20T15:06:44+02:00
New Revision: 192331b890e238a8ede4e61e6c5294e7eaa365fd

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

LOG: [lldb] [Process/Linux] Support arbitrarily-sized FPR writes on ARM

Support arbitrarily-sized FPR writes on ARM in order to fix writing qN
registers directly.  Currently, writing them works only by accident
due to value_regs splitting them into smaller writes via dN and sN
registers.

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index dcd68ddd279d..7e798ac3cb56 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -182,27 +182,9 @@ NativeRegisterContextLinux_arm::WriteRegister(const 
RegisterInfo *reg_info,
 uint32_t fpr_offset = CalculateFprOffset(reg_info);
 assert(fpr_offset < sizeof m_fpr);
 uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-switch (reg_info->byte_size) {
-case 2:
-  *(uint16_t *)dst = reg_value.GetAsUInt16();
-  break;
-case 4:
-  *(uint32_t *)dst = reg_value.GetAsUInt32();
-  break;
-case 8:
-  *(uint64_t *)dst = reg_value.GetAsUInt64();
-  break;
-default:
-  assert(false && "Unhandled data size.");
-  return Status("unhandled register data size %" PRIu32,
-reg_info->byte_size);
-}
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
-Status error = WriteFPR();
-if (error.Fail())
-  return error;
-
-return Status();
+return WriteFPR();
   }
 
   return Status("failed - register wasn't recognized to be a GPR or an FPR, "



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


[Lldb-commits] [lldb] f290efc - [lldb] [ABI/X86] Support combining xmm* and ymm*h regs into ymm*

2021-10-20 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-10-20T15:06:45+02:00
New Revision: f290efc32622cc59566ec7ac13e74a039b6047c2

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

LOG: [lldb] [ABI/X86] Support combining xmm* and ymm*h regs into ymm*

gdbserver does not expose combined ymm* registers but rather XSAVE-style
split xmm* and ymm*h portions.  Extend value_regs to support combining
multiple registers and use it to create user-friendly ymm* registers
that are combined from split xmm* and ymm*h portions.

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

Added: 


Modified: 
lldb/include/lldb/lldb-private-types.h
lldb/source/Plugins/ABI/X86/ABIX86.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/DynamicRegisterInfo.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Removed: 




diff  --git a/lldb/include/lldb/lldb-private-types.h 
b/lldb/include/lldb/lldb-private-types.h
index 5e71b68630a9..3be7003cd0fb 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -51,8 +51,10 @@ struct RegisterInfo {
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be read first, at which point
   /// the value for this register will be valid. For example, the value list
-  /// for ah would be eax (x86) or rax (x64).
-  uint32_t *value_regs; //
+  /// for ah would be eax (x86) or rax (x64). Register numbers are
+  /// of eRegisterKindLLDB. If multiple registers are listed, the final
+  /// value will be the concatenation of them.
+  uint32_t *value_regs;
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be invalidated when the value 
of
   /// this register changes. For example, the invalidate list for eax would be

diff  --git a/lldb/source/Plugins/ABI/X86/ABIX86.cpp 
b/lldb/source/Plugins/ABI/X86/ABIX86.cpp
index 2615235cdd8f..7cdba0c5fe57 100644
--- a/lldb/source/Plugins/ABI/X86/ABIX86.cpp
+++ b/lldb/source/Plugins/ABI/X86/ABIX86.cpp
@@ -85,6 +85,46 @@ 
addPartialRegisters(std::vector ®s,
   }
 }
 
+static void
+addCombinedRegisters(std::vector ®s,
+ llvm::ArrayRef subregs1,
+ llvm::ArrayRef subregs2, uint32_t base_size,
+ lldb::Encoding encoding, lldb::Format format) {
+  for (auto it : llvm::zip(subregs1, subregs2)) {
+RegData *regdata1, *regdata2;
+std::tie(regdata1, regdata2) = it;
+assert(regdata1);
+assert(regdata2);
+
+// verify that we've got matching target registers
+if (regdata1->subreg_name != regdata2->subreg_name)
+  continue;
+
+uint32_t base_index1 = regdata1->base_index.getValue();
+uint32_t base_index2 = regdata2->base_index.getValue();
+if (regs[base_index1].byte_size != base_size ||
+regs[base_index2].byte_size != base_size)
+  continue;
+
+lldb_private::DynamicRegisterInfo::Register new_reg{
+lldb_private::ConstString(regdata1->subreg_name),
+lldb_private::ConstString(),
+lldb_private::ConstString("supplementary registers"),
+base_size * 2,
+LLDB_INVALID_INDEX32,
+encoding,
+format,
+LLDB_INVALID_REGNUM,
+LLDB_INVALID_REGNUM,
+LLDB_INVALID_REGNUM,
+LLDB_INVALID_REGNUM,
+{base_index1, base_index2},
+{}};
+
+addSupplementaryRegister(regs, new_reg);
+  }
+}
+
 typedef llvm::SmallDenseMap, 64>
 BaseRegToRegsMap;
 
@@ -121,19 +161,33 @@ typedef llvm::SmallDenseMap, 64>
 #define STMM(n)
\
   { BaseRegToRegsMap::value_type("st" #n, {{MM, "mm" #n, llvm::None}}) }
 
+#define YMM(n) 
\
+  {BaseRegToRegsMap::value_type("ymm" #n "h",  
\
+{{YMM_YMMh, "ymm" #n, llvm::None}})},  
\
+  {
\
+BaseRegToRegsMap::value_type("xmm" #n, {{YMM_XMM, "ymm" #n, llvm::None}})  
\
+  }
+
 BaseRegToRegsMap makeBaseRegMap(bool is64bit) {
-  BaseRegToRegsMap out{{// GPRs common to amd64 & i386
-GPRh("a"), GPRh("b"), GPRh("c"), GPRh("d"), GPR("si"),
-GPR("di"), GPR("bp"), GPR("sp"),
+  BaseRegToRegsMap out{
+  {// GPRs common to amd64 & i386
+   GPRh("a"), GPRh("b"), GPRh("c"), GPRh("d"), GPR("si"), GPR("di"),
+   GPR("bp"), GPR("sp"),
 
-// ST/MM registers
-STM

[Lldb-commits] [lldb] 99277a8 - [lldb] [Process/Utility] Fix value_regs/invalidate_regs for ARM

2021-10-20 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-10-20T15:06:45+02:00
New Revision: 99277a81f807e6f4c63ececdb6974d6d5f1f3562

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

LOG: [lldb] [Process/Utility] Fix value_regs/invalidate_regs for ARM

Fix incorrect values for value_regs, and incomplete values for
invalidate_regs in RegisterInfos_arm.  The value_regs entry needs
to list only one base (i.e. larger) register that needs to be read
to get the value for this register, while invalidate_regs needs to list
all other registers (including pseudo-register) whose values would
change when this register is written to.

7a8ba4ffbeecb5070926b80bb839a4d80539f1ac fixed a similar problem
for ARM64.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h
index a0bae19b1fbc..ace2e5a9f68b 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm.h
@@ -254,22 +254,38 @@ static uint32_t g_s29_invalidates[] = {fpu_d14, fpu_q7, 
LLDB_INVALID_REGNUM};
 static uint32_t g_s30_invalidates[] = {fpu_d15, fpu_q7, LLDB_INVALID_REGNUM};
 static uint32_t g_s31_invalidates[] = {fpu_d15, fpu_q7, LLDB_INVALID_REGNUM};
 
-static uint32_t g_d0_invalidates[] = {fpu_q0, LLDB_INVALID_REGNUM};
-static uint32_t g_d1_invalidates[] = {fpu_q0, LLDB_INVALID_REGNUM};
-static uint32_t g_d2_invalidates[] = {fpu_q1, LLDB_INVALID_REGNUM};
-static uint32_t g_d3_invalidates[] = {fpu_q1, LLDB_INVALID_REGNUM};
-static uint32_t g_d4_invalidates[] = {fpu_q2, LLDB_INVALID_REGNUM};
-static uint32_t g_d5_invalidates[] = {fpu_q2, LLDB_INVALID_REGNUM};
-static uint32_t g_d6_invalidates[] = {fpu_q3, LLDB_INVALID_REGNUM};
-static uint32_t g_d7_invalidates[] = {fpu_q3, LLDB_INVALID_REGNUM};
-static uint32_t g_d8_invalidates[] = {fpu_q4, LLDB_INVALID_REGNUM};
-static uint32_t g_d9_invalidates[] = {fpu_q4, LLDB_INVALID_REGNUM};
-static uint32_t g_d10_invalidates[] = {fpu_q5, LLDB_INVALID_REGNUM};
-static uint32_t g_d11_invalidates[] = {fpu_q5, LLDB_INVALID_REGNUM};
-static uint32_t g_d12_invalidates[] = {fpu_q6, LLDB_INVALID_REGNUM};
-static uint32_t g_d13_invalidates[] = {fpu_q6, LLDB_INVALID_REGNUM};
-static uint32_t g_d14_invalidates[] = {fpu_q7, LLDB_INVALID_REGNUM};
-static uint32_t g_d15_invalidates[] = {fpu_q7, LLDB_INVALID_REGNUM};
+static uint32_t g_d0_invalidates[] = {fpu_q0, fpu_s0, fpu_s1,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d1_invalidates[] = {fpu_q0, fpu_s2, fpu_s3,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d2_invalidates[] = {fpu_q1, fpu_s4, fpu_s5,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d3_invalidates[] = {fpu_q1, fpu_s6, fpu_s7,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d4_invalidates[] = {fpu_q2, fpu_s8, fpu_s9,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d5_invalidates[] = {fpu_q2, fpu_s10, fpu_s11,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d6_invalidates[] = {fpu_q3, fpu_s12, fpu_s13,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d7_invalidates[] = {fpu_q3, fpu_s14, fpu_s15,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d8_invalidates[] = {fpu_q4, fpu_s16, fpu_s17,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d9_invalidates[] = {fpu_q4, fpu_s18, fpu_s19,
+  LLDB_INVALID_REGNUM};
+static uint32_t g_d10_invalidates[] = {fpu_q5, fpu_s20, fpu_s21,
+   LLDB_INVALID_REGNUM};
+static uint32_t g_d11_invalidates[] = {fpu_q5, fpu_s22, fpu_s23,
+   LLDB_INVALID_REGNUM};
+static uint32_t g_d12_invalidates[] = {fpu_q6, fpu_s24, fpu_s25,
+   LLDB_INVALID_REGNUM};
+static uint32_t g_d13_invalidates[] = {fpu_q6, fpu_s26, fpu_s27,
+   LLDB_INVALID_REGNUM};
+static uint32_t g_d14_invalidates[] = {fpu_q7, fpu_s28, fpu_s29,
+   LLDB_INVALID_REGNUM};
+static uint32_t g_d15_invalidates[] = {fpu_q7, fpu_s30, fpu_s31,
+   LLDB_INVALID_REGNUM};
 static uint32_t g_d16_invalidates[] = {fpu_q8, LLDB_INVALID_REGNUM};
 static uint32_t g_d17_invalidates[] = {fpu_q8, LLDB_INVALID_REGNUM};
 static uint32_t g_d18_invalidates[] = {fpu_q9, LLDB_INVALID_REGNUM};
@@ -287,37 +303,54 @@ static uint32_t g_d29

[Lldb-commits] [PATCH] D112131: [lldb] [Process/Linux] Support arbitrarily-sized FPR writes on ARM

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG192331b890e2: [lldb] [Process/Linux] Support 
arbitrarily-sized FPR writes on ARM (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112131

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -182,27 +182,9 @@
 uint32_t fpr_offset = CalculateFprOffset(reg_info);
 assert(fpr_offset < sizeof m_fpr);
 uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-switch (reg_info->byte_size) {
-case 2:
-  *(uint16_t *)dst = reg_value.GetAsUInt16();
-  break;
-case 4:
-  *(uint32_t *)dst = reg_value.GetAsUInt32();
-  break;
-case 8:
-  *(uint64_t *)dst = reg_value.GetAsUInt64();
-  break;
-default:
-  assert(false && "Unhandled data size.");
-  return Status("unhandled register data size %" PRIu32,
-reg_info->byte_size);
-}
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
-Status error = WriteFPR();
-if (error.Fail())
-  return error;
-
-return Status();
+return WriteFPR();
   }
 
   return Status("failed - register wasn't recognized to be a GPR or an FPR, "


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -182,27 +182,9 @@
 uint32_t fpr_offset = CalculateFprOffset(reg_info);
 assert(fpr_offset < sizeof m_fpr);
 uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-switch (reg_info->byte_size) {
-case 2:
-  *(uint16_t *)dst = reg_value.GetAsUInt16();
-  break;
-case 4:
-  *(uint32_t *)dst = reg_value.GetAsUInt32();
-  break;
-case 8:
-  *(uint64_t *)dst = reg_value.GetAsUInt64();
-  break;
-default:
-  assert(false && "Unhandled data size.");
-  return Status("unhandled register data size %" PRIu32,
-reg_info->byte_size);
-}
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
-Status error = WriteFPR();
-if (error.Fail())
-  return error;
-
-return Status();
+return WriteFPR();
   }
 
   return Status("failed - register wasn't recognized to be a GPR or an FPR, "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

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

So how should I proceed?


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

https://reviews.llvm.org/D110535

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


[Lldb-commits] [PATCH] D111030: [lldb] [Host] Add setters for common teletype properties to Terminal

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

Make `SetParity()` simpler.


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

https://reviews.llvm.org/D111030

Files:
  lldb/include/lldb/Host/Terminal.h
  lldb/source/Host/common/Terminal.cpp
  lldb/unittests/Host/posix/TerminalTest.cpp

Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -72,6 +72,149 @@
   EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
 }
 
+TEST_F(TerminalTest, SetRaw) {
+  struct termios terminfo;
+
+  ASSERT_THAT_ERROR(m_term.SetRaw(), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  // NB: cfmakeraw() on glibc disables IGNBRK, on FreeBSD sets it
+  EXPECT_EQ(terminfo.c_iflag &
+(BRKINT | PARMRK | ISTRIP | INLCR | IGNCR | ICRNL | IXON),
+0U);
+  EXPECT_EQ(terminfo.c_oflag & OPOST, 0U);
+  EXPECT_EQ(terminfo.c_lflag & (ICANON | ECHO | ISIG | IEXTEN), 0U);
+  EXPECT_EQ(terminfo.c_cflag & (CSIZE | PARENB), 0U | CS8);
+  EXPECT_EQ(terminfo.c_cc[VMIN], 1);
+  EXPECT_EQ(terminfo.c_cc[VTIME], 0);
+}
+
+TEST_F(TerminalTest, SetBaudRate) {
+  struct termios terminfo;
+
+  ASSERT_THAT_ERROR(m_term.SetBaudRate(38400), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  EXPECT_EQ(cfgetispeed(&terminfo), static_cast(B38400));
+  EXPECT_EQ(cfgetospeed(&terminfo), static_cast(B38400));
+
+  ASSERT_THAT_ERROR(m_term.SetBaudRate(115200), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  EXPECT_EQ(cfgetispeed(&terminfo), static_cast(B115200));
+  EXPECT_EQ(cfgetospeed(&terminfo), static_cast(B115200));
+
+  // uncommon value
+#if defined(B153600)
+  ASSERT_THAT_ERROR(m_term.SetBaudRate(153600), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  EXPECT_EQ(cfgetispeed(&terminfo), static_cast(B153600));
+  EXPECT_EQ(cfgetospeed(&terminfo), static_cast(B153600));
+#else
+  ASSERT_THAT_ERROR(m_term.SetBaudRate(153600),
+llvm::Failed(testing::Property(
+&llvm::ErrorInfoBase::message,
+"baud rate 153600 unsupported by the platform")));
+#endif
+}
+
+TEST_F(TerminalTest, SetStopBits) {
+  struct termios terminfo;
+
+  ASSERT_THAT_ERROR(m_term.SetStopBits(1), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  EXPECT_EQ(terminfo.c_cflag & CSTOPB, 0U);
+
+  ASSERT_THAT_ERROR(m_term.SetStopBits(2), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  EXPECT_NE(terminfo.c_cflag & CSTOPB, 0U);
+
+  ASSERT_THAT_ERROR(m_term.SetStopBits(0),
+llvm::Failed(testing::Property(
+&llvm::ErrorInfoBase::message,
+"invalid stop bit count: 0 (must be 1 or 2)")));
+  ASSERT_THAT_ERROR(m_term.SetStopBits(3),
+llvm::Failed(testing::Property(
+&llvm::ErrorInfoBase::message,
+"invalid stop bit count: 3 (must be 1 or 2)")));
+}
+
+TEST_F(TerminalTest, SetParity) {
+  struct termios terminfo;
+
+  ASSERT_THAT_ERROR(m_term.SetParity(Terminal::Parity::No), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+  EXPECT_EQ(terminfo.c_cflag & PARENB, 0U);
+
+  ASSERT_THAT_ERROR(m_term.SetParity(Terminal::Parity::Even),
+llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_EQ(terminfo.c_cflag & PARODD, 0U);
+#if defined(CMSPAR)
+  EXPECT_EQ(terminfo.c_cflag & CMSPAR, 0U);
+#endif
+
+  ASSERT_THAT_ERROR(m_term.SetParity(Terminal::Parity::Odd), llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_NE(terminfo.c_cflag & PARODD, 0U);
+#if defined(CMSPAR)
+  EXPECT_EQ(terminfo.c_cflag & CMSPAR, 0U);
+#endif
+
+#if defined(CMSPAR)
+  ASSERT_THAT_ERROR(m_term.SetParity(Terminal::Parity::Space),
+llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_EQ(terminfo.c_cflag & PARODD, 0U);
+  EXPECT_NE(terminfo.c_cflag & CMSPAR, 0U);
+
+  ASSERT_THAT_ERROR(m_term.SetParity(Terminal::Parity::Mark),
+llvm::Succeeded());
+  ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_NE(terminfo.c_cflag & PARODD, 0U);
+  EXPECT_NE(terminfo.c_cflag & CMSPAR, 0U);
+#else
+  ASSERT_THAT_ERROR(m_term.SetParity(Terminal::Parity::Space),
+llvm::Failed(testing::Property(
+&llvm::ErrorI

[Lldb-commits] [PATCH] D111030: [lldb] [Host] Add setters for common teletype properties to Terminal

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/source/Host/common/Terminal.cpp:316-347
+  bool parity_bit = true;
+  bool odd_bit = false;
+  bool stick_bit = false;
+
+  switch (parity) {
+  case Parity::No:
+parity_bit = false;

labath wrote:
> I am wondering if we can avoid the double conversion (enum->bools->flags). 
> Would something like this be shorter/cleaner:
> ```
> unsigned(?) GetParityFlagMask() {
>   return PARENB | PARODD
> #ifdef CMSPAR
> | CMSPAR
> #endif
>   ;
> }
> Expected GetParityFlags(Parity) // error if parity not supported
> 
> SetParity(parity) {
> getParityFlags(parity); // and check result
> GetData(); // and check result
> data.m_termios.c_cflag &= ~GetParityFlagMask();
> data.m_termios.c_cflag |= flags;
> SetData();
> ```
TBH, I don't like the extra functions because they mean more `#ifdef`s. How do 
you feel about my new version? I think it might be less optimal but I think 
it's as readable as it gets.


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

https://reviews.llvm.org/D111030

___
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

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774
+  llvm::Error error = llvm::Error::success();
+  m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite,
+ serial_options.get(), true, error);

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > It would be better to hide this by making the constructor private and 
> > > have a static factory function returning 
> > > `Expected>`. Bonus points if you can move all the 
> > > fallible operations into the factory function so that the (now private) 
> > > constructor does not need the Error argument at all
> > I know but I couldn't make this work — the compiler just won't find a way 
> > to convert the new `SerialPort` instance into `Expected`. I 
> > suspect I would have to add move ctors and assignment operators all the way 
> > up the class hierarchy.
> That's where the `unique_ptr` part comes in. :)
I presume you mean `shared_ptr` here, or know some dirty trick.


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] D111355: [lldb] Add serial:// protocol for connecting to serial port

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

Use a fallible ctor approach.


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

https://reviews.llvm.org/D111355

Files:
  lldb/include/lldb/Host/File.h
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/common/File.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -427,7 +427,7 @@
 return libc.ptsname(self._master.fileno()).decode()
 
 def get_connect_url(self):
-return "file://" + self.get_connect_address()
+return "serial://" + self.get_connect_address()
 
 def close_server(self):
 self._slave.close()
Index: lldb/test/API/functionalities/gdb_remote_client/TestPty.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestPty.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestPty.py
@@ -9,6 +9,38 @@
 mydir = TestBase.compute_mydir(__file__)
 server_socket_class = PtyServerSocket
 
+def get_term_attrs(self):
+import termios
+return termios.tcgetattr(self.server._socket._slave)
+
+def setUp(self):
+super().setUp()
+self.orig_attr = self.get_term_attrs()
+
+def assert_raw_mode(self, current_attr):
+import termios
+self.assertEqual(current_attr[0] & (termios.BRKINT |
+termios.PARMRK |
+termios.ISTRIP | termios.INLCR |
+termios.IGNCR | termios.ICRNL |
+termios.IXON),
+ 0)
+self.assertEqual(current_attr[1] & termios.OPOST, 0)
+self.assertEqual(current_attr[2] & termios.CSIZE, termios.CS8)
+self.assertEqual(current_attr[3] & (termios.ICANON | termios.ECHO |
+termios.ISIG | termios.IEXTEN),
+ 0)
+self.assertEqual(current_attr[6][termios.VMIN], 1)
+self.assertEqual(current_attr[6][termios.VTIME], 0)
+
+def get_parity_flags(self, attr):
+import termios
+return attr[2] & (termios.PARENB | termios.PARODD)
+
+def get_stop_bit_flags(self, attr):
+import termios
+return attr[2] & termios.CSTOPB
+
 def test_process_connect_sync(self):
 """Test the process connect command in synchronous mode"""
 try:
@@ -17,8 +49,20 @@
 substrs=['Platform: remote-gdb-server', 'Connected: no'])
 self.expect("process connect " + self.server.get_connect_url(),
 substrs=['Process', 'stopped'])
+
+current_attr = self.get_term_attrs()
+# serial:// should set raw mode
+self.assert_raw_mode(current_attr)
+# other parameters should be unmodified
+self.assertEqual(current_attr[4:6], self.orig_attr[4:6])
+self.assertEqual(self.get_parity_flags(current_attr),
+ self.get_parity_flags(self.orig_attr))
+self.assertEqual(self.get_stop_bit_flags(current_attr),
+ self.get_stop_bit_flags(self.orig_attr))
 finally:
 self.dbg.GetSelectedTarget().GetProcess().Kill()
+# original mode should be restored on exit
+self.assertEqual(self.get_term_attrs(), self.orig_attr)
 
 def test_process_connect_async(self):
 """Test the process connect command in asynchronous mode"""
@@ -31,7 +75,63 @@
 substrs=['Process', 'stopped'])
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateStopped])
+
+current_attr = self.get_term_attrs()
+# serial:// should set raw mode
+self.assert_raw_mode(current_attr)
+# other parameters should be unmodified
+self.assertEqual(current_attr[4:6], self.orig_attr[4:6])
+self.assertEqual(self.get_parity_flags(current_attr),
+ self.get_parity_flags(self.orig_attr))
+self.assertEqual(self.get_stop_bit_flags(current_attr),
+ self.get_stop_bit_flags(self.orig_attr))
 finally:
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateEx

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

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



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774
+  llvm::Error error = llvm::Error::success();
+  m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite,
+ serial_options.get(), true, error);

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > It would be better to hide this by making the constructor private and 
> > > > have a static factory function returning 
> > > > `Expected>`. Bonus points if you can move all 
> > > > the fallible operations into the factory function so that the (now 
> > > > private) constructor does not need the Error argument at all
> > > I know but I couldn't make this work — the compiler just won't find a way 
> > > to convert the new `SerialPort` instance into `Expected`. I 
> > > suspect I would have to add move ctors and assignment operators all the 
> > > way up the class hierarchy.
> > That's where the `unique_ptr` part comes in. :)
> I presume you mean `shared_ptr` here, or know some dirty trick.
a unique_ptr is convertible to a shared_ptr, but not the other way around, so 
it's better to return the former


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] D112147: [lldb] Fix lookup for global constants in namespaces

2021-10-20 Thread Tonko Sabolčec via Phabricator via lldb-commits
tonkosi created this revision.
tonkosi added reviewers: werat, teemperor.
tonkosi requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

LLDB uses mangled name to construct a fully qualified name for global
variables. Sometimes DW_TAG_linkage_name attribute is missing from
debug info, so LLDB has to rely on parent entries to construct the
fully qualified name.

Currently, the fallback is handled when the parent DW_TAG is either
DW_TAG_compiled_unit or DW_TAG_partial_unit, which may not work well
for global constants in namespaces. For example:

  namespace ns {
const int x = 10;
  }

may produce the following debug info:

  <1><2a>: Abbrev Number: 2 (DW_TAG_namespace)
 <2b>   DW_AT_name: (indirect string, offset: 0x5e): ns
  <2><2f>: Abbrev Number: 3 (DW_TAG_variable)
 <30>   DW_AT_name: (indirect string, offset: 0x61): x
 <34>   DW_AT_type: <0x3c>
 <38>   DW_AT_decl_file   : 1
 <39>   DW_AT_decl_line   : 2
 <3a>   DW_AT_const_value : 10

Since the fallback didn't handle the case when parent tag is
DW_TAG_namespace, LLDB wasn't able to match the variable by its fully
qualified name "ns::x". This change fixes this by additional check
if the parent is a DW_TAG_namespace.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112147

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/lang/cpp/global_variables/TestCPPGlobalVariables.py
  lldb/test/API/lang/cpp/global_variables/main.cpp


Index: lldb/test/API/lang/cpp/global_variables/main.cpp
===
--- lldb/test/API/lang/cpp/global_variables/main.cpp
+++ lldb/test/API/lang/cpp/global_variables/main.cpp
@@ -1,10 +1,12 @@
 #include 
 
 namespace abc {
-   int g_file_global_int = 42;
+int g_file_global_int = 42;
+const int g_file_global_const_int = 1337;
 }
 
 int main (int argc, char const *argv[])
 {
-return abc::g_file_global_int; // Set break point at this line.
+  int unused = abc::g_file_global_const_int;
+  return abc::g_file_global_int; // Set break point at this line.
 }
Index: lldb/test/API/lang/cpp/global_variables/TestCPPGlobalVariables.py
===
--- lldb/test/API/lang/cpp/global_variables/TestCPPGlobalVariables.py
+++ lldb/test/API/lang/cpp/global_variables/TestCPPGlobalVariables.py
@@ -15,12 +15,11 @@
 TestBase.setUp(self)
 self.source = lldb.SBFileSpec('main.cpp')
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
 def test(self):
 self.build()
 
 (target, _, _, _) = lldbutil.run_to_source_breakpoint(self, "// Set 
break point at this line.", self.source)
-
+
 # Check that we can access g_file_global_int by its name
 self.expect("target variable g_file_global_int", 
VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['42'])
@@ -29,6 +28,20 @@
 self.expect("target variable xyz::g_file_global_int", 
VARIABLES_DISPLAYED_CORRECTLY,
 error=True, substrs=['can\'t find global variable'])
 
+# Check that we can access g_file_global_const_int by its name
+self.expect("target variable g_file_global_const_int", 
VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['1337'])
+self.expect("target variable abc::g_file_global_const_int", 
VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['1337'])
+self.expect("target variable xyz::g_file_global_const_int", 
VARIABLES_DISPLAYED_CORRECTLY,
+error=True, substrs=['can\'t find global variable'])
+
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
+def test_access_by_mangled_name(self):
+self.build()
+
+(target, _, _, _) = lldbutil.run_to_source_breakpoint(self, "// Set 
break point at this line.", self.source)
+
 # Check that we can access g_file_global_int by its mangled name
 addr = 
target.EvaluateExpression("&abc::g_file_global_int").GetValueAsUnsigned()
 self.assertNotEqual(addr, 0)
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3278,7 +3278,8 @@
 // able to generate a fully qualified name from the
 // declaration context.
 if ((parent_tag == DW_TAG_compile_unit ||
- parent_tag == DW_TAG_partial_unit) &&
+ parent_tag == DW_TAG_partial_unit ||
+ parent_tag == DW_TAG_namespace) &&
 Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
   mangled =
   
GetDWARFDeclContext(die).GetQualifiedNameAsConstString().GetCString();


Index: lldb/test/API/lang/cpp/global_variables/main.cpp
==

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

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

Use unique_ptr.


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

https://reviews.llvm.org/D111355

Files:
  lldb/include/lldb/Host/File.h
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/common/File.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -427,7 +427,7 @@
 return libc.ptsname(self._master.fileno()).decode()
 
 def get_connect_url(self):
-return "file://" + self.get_connect_address()
+return "serial://" + self.get_connect_address()
 
 def close_server(self):
 self._slave.close()
Index: lldb/test/API/functionalities/gdb_remote_client/TestPty.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestPty.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestPty.py
@@ -9,6 +9,38 @@
 mydir = TestBase.compute_mydir(__file__)
 server_socket_class = PtyServerSocket
 
+def get_term_attrs(self):
+import termios
+return termios.tcgetattr(self.server._socket._slave)
+
+def setUp(self):
+super().setUp()
+self.orig_attr = self.get_term_attrs()
+
+def assert_raw_mode(self, current_attr):
+import termios
+self.assertEqual(current_attr[0] & (termios.BRKINT |
+termios.PARMRK |
+termios.ISTRIP | termios.INLCR |
+termios.IGNCR | termios.ICRNL |
+termios.IXON),
+ 0)
+self.assertEqual(current_attr[1] & termios.OPOST, 0)
+self.assertEqual(current_attr[2] & termios.CSIZE, termios.CS8)
+self.assertEqual(current_attr[3] & (termios.ICANON | termios.ECHO |
+termios.ISIG | termios.IEXTEN),
+ 0)
+self.assertEqual(current_attr[6][termios.VMIN], 1)
+self.assertEqual(current_attr[6][termios.VTIME], 0)
+
+def get_parity_flags(self, attr):
+import termios
+return attr[2] & (termios.PARENB | termios.PARODD)
+
+def get_stop_bit_flags(self, attr):
+import termios
+return attr[2] & termios.CSTOPB
+
 def test_process_connect_sync(self):
 """Test the process connect command in synchronous mode"""
 try:
@@ -17,8 +49,20 @@
 substrs=['Platform: remote-gdb-server', 'Connected: no'])
 self.expect("process connect " + self.server.get_connect_url(),
 substrs=['Process', 'stopped'])
+
+current_attr = self.get_term_attrs()
+# serial:// should set raw mode
+self.assert_raw_mode(current_attr)
+# other parameters should be unmodified
+self.assertEqual(current_attr[4:6], self.orig_attr[4:6])
+self.assertEqual(self.get_parity_flags(current_attr),
+ self.get_parity_flags(self.orig_attr))
+self.assertEqual(self.get_stop_bit_flags(current_attr),
+ self.get_stop_bit_flags(self.orig_attr))
 finally:
 self.dbg.GetSelectedTarget().GetProcess().Kill()
+# original mode should be restored on exit
+self.assertEqual(self.get_term_attrs(), self.orig_attr)
 
 def test_process_connect_async(self):
 """Test the process connect command in asynchronous mode"""
@@ -31,7 +75,63 @@
 substrs=['Process', 'stopped'])
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateStopped])
+
+current_attr = self.get_term_attrs()
+# serial:// should set raw mode
+self.assert_raw_mode(current_attr)
+# other parameters should be unmodified
+self.assertEqual(current_attr[4:6], self.orig_attr[4:6])
+self.assertEqual(self.get_parity_flags(current_attr),
+ self.get_parity_flags(self.orig_attr))
+self.assertEqual(self.get_stop_bit_flags(current_attr),
+ self.get_stop_bit_flags(self.orig_attr))
 finally:
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateExited])
+  

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

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked 3 inline comments as done.
mgorny added inline comments.



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774
+  llvm::Error error = llvm::Error::success();
+  m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite,
+ serial_options.get(), true, error);

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > mgorny wrote:
> > > > labath wrote:
> > > > > It would be better to hide this by making the constructor private and 
> > > > > have a static factory function returning 
> > > > > `Expected>`. Bonus points if you can move all 
> > > > > the fallible operations into the factory function so that the (now 
> > > > > private) constructor does not need the Error argument at all
> > > > I know but I couldn't make this work — the compiler just won't find a 
> > > > way to convert the new `SerialPort` instance into 
> > > > `Expected`. I suspect I would have to add move ctors and 
> > > > assignment operators all the way up the class hierarchy.
> > > That's where the `unique_ptr` part comes in. :)
> > I presume you mean `shared_ptr` here, or know some dirty trick.
> a unique_ptr is convertible to a shared_ptr, but not the other way around, so 
> it's better to return the former
Ah, indeed. I was missing std::move.


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] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

2021-10-20 Thread Chris Lattner via Phabricator via lldb-commits
lattner added a comment.

Do you have commit access?  If not, please read the llvm developer policy and 
follow the instructions, thanks!


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

https://reviews.llvm.org/D110535

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


[Lldb-commits] [PATCH] D111964: [lldb] [lldb-server] Allow any protocol supported by lldb

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

Add a PoC for testing lldb-server over a pty.


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

https://reviews.llvm.org/D111964

Files:
  lldb/test/API/tools/lldb-server/TestPtyServer.py
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -230,24 +230,27 @@
   final_host_and_port.append("localhost");
 final_host_and_port.append(host_and_port.str());
 
-if (reverse_connect) {
+bool is_url = host_and_port.contains("://");
+if (reverse_connect || is_url) {
   // llgs will connect to the gdb-remote client.
 
-  // Ensure we have a port number for the connection.
-  // Note: use rfind, because the host/port may look like "[::1]:12345".
-  uint32_t connection_portno = 0;
-  const std::string::size_type colon_pos = final_host_and_port.rfind(':');
-  if (colon_pos != std::string::npos)
-llvm::to_integer(final_host_and_port.substr(colon_pos + 1),
- connection_portno);
-  if (connection_portno == 0) {
-llvm::errs() << "error: port number must be specified on when using "
-"reverse connect\n";
-exit(1);
-  }
+  if (!is_url) {
+// Ensure we have a port number for the connection.
+// Note: use rfind, because the host/port may look like "[::1]:12345".
+uint32_t connection_portno = 0;
+const std::string::size_type colon_pos = final_host_and_port.rfind(':');
+if (colon_pos != std::string::npos)
+  llvm::to_integer(final_host_and_port.substr(colon_pos + 1),
+   connection_portno);
+if (connection_portno == 0) {
+  llvm::errs() << "error: port number must be specified on when using "
+  "reverse connect\n";
+  exit(1);
+}
 
-  // Build the connection string.
-  final_host_and_port.insert(0, "connect://");
+// Build the connection string.
+final_host_and_port.insert(0, "connect://");
+  }
 
   // Create the connection.
   connection_up.reset(new ConnectionFileDescriptor);
Index: lldb/test/API/tools/lldb-server/TestPtyServer.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestPtyServer.py
@@ -0,0 +1,64 @@
+import gdbremote_testcase
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbgdbserverutils import *
+
+
+@skipIfWindows
+class PtyServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+super().setUp()
+import pty
+import tty
+master, slave = pty.openpty()
+tty.setraw(master)
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+
+def get_debug_monitor_command_line_args(self, attach_pid=None):
+commandline_args = self.debug_monitor_extra_args
+if attach_pid:
+commandline_args += ["--attach=%d" % attach_pid]
+
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+pty_path = libc.ptsname(self._master.fileno()).decode()
+commandline_args += ["serial://%s" % (pty_path,)]
+return commandline_args
+
+def connect_to_debug_monitor(self, attach_pid=None):
+self.reverse_connect = False
+server = self.launch_debug_monitor(attach_pid=attach_pid)
+self.assertIsNotNone(server)
+
+# TODO: make it into proper abstraction
+class FakeSocket:
+def __init__(self, fd):
+self.fd = fd
+
+def sendall(self, frame):
+self.fd.write(frame)
+
+def recv(self, count):
+return self.fd.read(count)
+
+self.sock = FakeSocket(self._master)
+self._server = Server(self.sock, server)
+return server
+
+@add_test_categories(["llgs"])
+def test_pty_server(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["lldb-server <  26> read packet: $QThreadSuffixSupported#e4",
+ "lldb-server <   6> send packet: $OK#9a"],
+True)
+
+self.expect_gdbremote_sequence()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

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

In D110535#3075563 , @lattner wrote:

> Do you have commit access?  If not, please read the llvm developer policy and 
> follow the instructions, thanks!

That's not the problem. The problem is that @labath and @joerg seem not to 
agree on how it should be done, and I don't know whether to hack support for 
`char` delimiter to preserve the old syntax as @joerg wanted, or to keep it as 
is (i.e. without support for `char` delimiter) as @labath originally suggested.


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

https://reviews.llvm.org/D110535

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


[Lldb-commits] [PATCH] D112058: [lldb/DWARF] Ignore debug info pointing to the low addresses

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

In D112058#3074618 , @labath wrote:

> In D112058#3073451 , @clayborg 
> wrote:
>
>> Looks good. Do we need a follow up patch to avoid creating functions that 
>> should have been stripped?
>
> I'm not entirely sure what you mean by that. Are you referring to the fact 
> that `SymbolFileDWARF::ResolveFunction` creates an `lldb_private::Function`, 
> which it does not add the to result SymbolContextList (instead of not 
> creating the object in the first place?
>
> If yes, then that sounds like a good idea. I don't know if this has more 
> user-facing implications, but I was definitely puzzled by the fact that the 
> stripped function still showed up in "image dump symfile". I'll see what can 
> be done about that.

Yeah, that is what I was talking about. I think if we modify 
SymbolFileDWARF::ParseFunction() that would be the easiest to quickly get the 
ranges for the DW_TAG_subprogram and avoid calling 
DWARFASTParser::ParseFunctionFromDWARF() if the low PC is less than 
m_first_code_address we can avoid bloat in the ASTs and keep our memory 
footprint down in the LLDB process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112058

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


[Lldb-commits] [PATCH] D112047: [lldb/test] Update TestScriptedProcess to use skinny corefiles

2021-10-20 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 381020.
mib added a comment.

Add `self` argument to scripted_process base class methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112047

Files:
  lldb/examples/python/scripted_process/main.stack-dump
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/examples/python/scripted_process/stack_core_scripted_process.py
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -2,7 +2,7 @@
 Test python scripted process in lldb
 """
 
-import os
+import os, json, tempfile
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -10,14 +10,15 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test import lldbtest
 
-
 class ScriptedProcesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
 def setUp(self):
 TestBase.setUp(self)
-self.source = "main.c"
+self.scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process']
+self.scripted_process_example_abspath = os.path.join(self.getSourceDir(),
+*self.scripted_process_example_relpath)
 
 def tearDown(self):
 TestBase.tearDown(self)
@@ -78,18 +79,31 @@
 self.assertGreater(thread.GetNumFrames(), 0)
 
 frame = thread.GetFrameAtIndex(0)
+GPRs = None
 register_set = frame.registers # Returns an SBValueList.
 for regs in register_set:
-if 'GPR' in regs.name:
-registers  = regs
+if 'general purpose' in regs.name.lower():
+GPRs = regs
 break
 
-self.assertTrue(registers, "Invalid General Purpose Registers Set")
-self.assertEqual(registers.GetNumChildren(), 21)
-for idx, reg in enumerate(registers, start=1):
+self.assertTrue(GPRs, "Invalid General Purpose Registers Set")
+self.assertEqual(GPRs.GetNumChildren(), 21)
+for idx, reg in enumerate(GPRs, start=1):
 self.assertEqual(idx, int(reg.value, 16))
 
-@skipIfDarwin
+def create_stack_skinny_corefile(self):
+self.build()
+target, process, thread, _ = lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+self.assertTrue(process.IsValid(), "Process is invalid.")
+# FIXME: Use SBAPI to save the process corefile.
+stack_core = tempfile.NamedTemporaryFile()
+self.runCmd("process save-core -s stack  " + stack_core.name)
+self.assertTrue(os.path.exists(stack_core.name), "No stack-only corefile found.")
+self.assertTrue(self.dbg.DeleteTarget(target), "Couldn't delete target")
+print(stack_core.name)
+target = self.dbg.CreateTarget(None)
+return target.LoadCore(self.getBuildArtifact(stack_core.name))
+
 @skipUnlessDarwin
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
@@ -101,26 +115,40 @@
 for module in target.modules:
 if 'a.out' in module.GetFileSpec().GetFilename():
 main_module = module
+break
 
 self.assertTrue(main_module, "Invalid main module.")
 error = target.SetModuleLoadAddress(main_module, 0)
 self.assertTrue(error.Success(), "Reloading main module at offset 0 failed.")
 
-scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
-self.runCmd("command script import " + os.path.join(self.getSourceDir(),
-*scripted_process_example_relpath))
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+self.runCmd("command script import " + os.path.join(self.scripted_process_example_abspath,
+"stack_core_scripted_process.py"))
 
-process = target.GetProcess()
+corefile_process = self.create_stack_skinny_corefile()
+self.assertTrue(corefile_process, PROCESS_IS_VALID)
+
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"backing_target_idx" : self.dbg.GetIndexOfTarget(corefile_process.GetTarget())
+}))
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetProcessPluginName("ScriptedProcess")
+launch_info.SetScriptedProcessClassName("stack_core_scripted_process.StackCoreScriptedPro

[Lldb-commits] [PATCH] D112163: Enable libc++ in the build for libcxx initializerlist pretty printers

2021-10-20 Thread David Blaikie via Phabricator via lldb-commits
dblaikie created this revision.
dblaikie added reviewers: teemperor, labath.
dblaikie requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112163

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
@@ -14,11 +14,7 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIfWindows  # libc++ not ported to Windows yet
-@skipIf(compiler="gcc")
-@expectedFailureAll(
-oslist=["linux"],
-bugnumber="fails on clang 3.5 and tot")
+@add_test_categories(["libc++"])
 def test(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
@@ -1,4 +1,6 @@
 CXX_SOURCES := main.cpp
 CXXFLAGS_EXTRAS := -std=c++11
 
+USE_LIBCPP := 1
+
 include Makefile.rules


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
@@ -14,11 +14,7 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIfWindows  # libc++ not ported to Windows yet
-@skipIf(compiler="gcc")
-@expectedFailureAll(
-oslist=["linux"],
-bugnumber="fails on clang 3.5 and tot")
+@add_test_categories(["libc++"])
 def test(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile
@@ -1,4 +1,6 @@
 CXX_SOURCES := main.cpp
 CXXFLAGS_EXTRAS := -std=c++11
 
+USE_LIBCPP := 1
+
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112163: Enable libc++ in the build for libcxx initializerlist pretty printers

2021-10-20 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.
Herald added a subscriber: JDevlieghere.

Looks like this test, when it was introduced/substantially refactored in D9426 
 - what was missing compared to all the other 
tests updated to run on linux, was the Makefile change to use libc++, so it was 
failing in places where libc++ wasn't the default. Fix that and it seems to 
pass for me, at least. Not sure if we need to keep some of the other expected 
failures (on some particular subset of clang, etc?)

Also there's a few other of these pretty printer tests that are still 
classified as skip-on-gcc, though at least some have been reclassified as 
passing-on-gcc in changes since D9426 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112163

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


[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

2021-10-20 Thread David Blaikie via Phabricator via lldb-commits
dblaikie created this revision.
dblaikie added a reviewer: aprantl.
dblaikie requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Hopefully these were also fixed bi r343545 /
7fd4513920d2fed533ad420976529ef43eb42a35


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112165

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
@@ -27,8 +27,6 @@
  '// Set fourth break point at this line.')
 
 @add_test_categories(["libc++"])
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
@@ -20,8 +20,6 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -20,8 +20,6 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
@@ -27,8 +27,6 @@
  '// Set fourth break point at this line.')
 
 @add_test_categories(["libc++"])
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
@@ -20,8 +20,6 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -20,8 +20,6 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.'

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

2021-10-20 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/Statistics.h:96
+};
+
+class GlobalStats {

Do we expect there to be something like `DebuggerStats`? I think it would be 
nice from a hierarchy perspective that Global Stats have a map of debugger -> 
to debuggerstat who then in turn hold on to a map of target -> target stats. 
That hierarchy would work really well for JSON (except they would be lists 
instead of maps). 



Comment at: lldb/include/lldb/Target/Statistics.h:97-114
+class GlobalStats {
+public:
+  static void SetCollectingStats(bool enable) { g_collecting_stats = enable; }
+  static bool GetCollectingStats() { return g_collecting_stats; }
+
+  /// Get metrics associated with all targets in a debugger in JSON format.
+  ///

For things like how many lldb_asserts we've hit, this actually will contain its 
own stats, not just things per debugger and target. I think it makes more sense 
to make this a Singleton as I did in D110895.


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] D112107: [lldb/Plugins] Make `ScriptedInterface::CreatePluginObject` more generic

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

This LGTM but it would be nice if the commit message could be a bit more 
specific about the motivation and explain the relationship between an ObjectSP 
and DictionarySP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112107

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


[Lldb-commits] [PATCH] D112167: [lldb/Plugins] Refactor ScriptedThread register context creation

2021-10-20 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch changes the ScriptedThread class to create the register
context when Process::RefreshStateAfterStop is called rather than
doing it in the thread constructor.

This is required to update the thread state for execution control
support.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112167

Files:
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -64,32 +64,6 @@
   m_script_object_sp = object_sp;
 
   SetID(scripted_thread_interface->GetThreadID());
-
-  llvm::Optional reg_data =
-  scripted_thread_interface->GetRegisterContext();
-  if (!reg_data) {
-error.SetErrorString("Failed to get scripted thread registers data.");
-return;
-  }
-
-  DataBufferSP data_sp(
-  std::make_shared(reg_data->c_str(), reg_data->size()));
-
-  if (!data_sp->GetByteSize()) {
-error.SetErrorString("Failed to copy raw registers data.");
-return;
-  }
-
-  std::shared_ptr reg_ctx_memory =
-  std::make_shared(
-  *this, 0, *GetDynamicRegisterInfo(), LLDB_INVALID_ADDRESS);
-  if (!reg_ctx_memory) {
-error.SetErrorString("Failed to create a register context.");
-return;
-  }
-
-  reg_ctx_memory->SetAllRegisterData(data_sp);
-  m_reg_context_sp = reg_ctx_memory;
 }
 
 ScriptedThread::~ScriptedThread() { DestroyThread(); }
@@ -115,11 +89,8 @@
 void ScriptedThread::ClearStackFrames() { Thread::ClearStackFrames(); }
 
 RegisterContextSP ScriptedThread::GetRegisterContext() {
-  if (!m_reg_context_sp) {
-m_reg_context_sp = std::make_shared(
-*this, LLDB_INVALID_ADDRESS);
-GetInterface()->GetRegisterContext();
-  }
+  if (!m_reg_context_sp)
+m_reg_context_sp = CreateRegisterContextForFrame(nullptr);
   return m_reg_context_sp;
 }
 
@@ -127,9 +98,38 @@
 ScriptedThread::CreateRegisterContextForFrame(StackFrame *frame) {
   uint32_t concrete_frame_idx = frame ? frame->GetConcreteFrameIndex() : 0;
 
-  if (concrete_frame_idx == 0)
-return GetRegisterContext();
-  return GetUnwinder().CreateRegisterContextForFrame(frame);
+  if (concrete_frame_idx)
+return GetUnwinder().CreateRegisterContextForFrame(frame);
+
+  lldb::RegisterContextSP reg_ctx_sp;
+  Status error;
+
+  llvm::Optional reg_data = GetInterface()->GetRegisterContext();
+  if (!reg_data) {
+error.SetErrorString("Failed to get scripted thread registers data.");
+return nullptr;
+  }
+
+  DataBufferSP data_sp(
+  std::make_shared(reg_data->c_str(), reg_data->size()));
+
+  if (!data_sp->GetByteSize()) {
+error.SetErrorString("Failed to copy raw registers data.");
+return nullptr;
+  }
+
+  std::shared_ptr reg_ctx_memory =
+  std::make_shared(
+  *this, 0, *GetDynamicRegisterInfo(), LLDB_INVALID_ADDRESS);
+  if (!reg_ctx_memory) {
+error.SetErrorString("Failed to create a register context.");
+return nullptr;
+  }
+
+  reg_ctx_memory->SetAllRegisterData(data_sp);
+  m_reg_context_sp = reg_ctx_memory;
+
+  return m_reg_context_sp;
 }
 
 bool ScriptedThread::CalculateStopInfo() {
@@ -183,9 +183,7 @@
 }
 
 void ScriptedThread::RefreshStateAfterStop() {
-  // TODO: Implement
-  if (m_reg_context_sp)
-m_reg_context_sp->InvalidateAllRegisters();
+  GetRegisterContext()->InvalidateIfNeeded(false /*=force*/);
 }
 
 lldb::ScriptedThreadInterfaceSP ScriptedThread::GetInterface() const {
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -78,7 +78,7 @@
 
   Status DoDestroy() override;
 
-  void RefreshStateAfterStop() override{};
+  void RefreshStateAfterStop() override;
 
   bool IsAlive() override;
 
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -20,7 +20,6 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/RegisterContext.h"
-
 #include "lldb/Utility/State.h"
 
 #include 
@@ -296,6 +295,7 @@
   // actually new threads will get added to new_thread_list.
 
   CheckInterpreterAndScriptObject();
+  m_thread_plans.ClearThreadCache();
 
   Status error;
   ScriptLanguage language = m_interpreter->GetLanguage();
@

[Lldb-commits] [lldb] 207998c - [lldb] Remove variable "any" which is set but not used

2021-10-20 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-10-20T12:08:35-07:00
New Revision: 207998c242c8c8a270ff22a5136da87338546725

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

LOG: [lldb] Remove variable "any" which is set but not used

Added: 


Modified: 

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
 
b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
index 3069214e47b84..beb2cbd1507b4 100644
--- 
a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -110,12 +110,10 @@ - (BOOL)spawnWithPath:(NSString *)path
 
 CoreSimulatorSupport::ModelIdentifier::ModelIdentifier(const std::string &mi)
 : m_family(), m_versions() {
-  bool any = false;
   bool first_digit = false;
   unsigned int val = 0;
 
   for (char c : mi) {
-any = true;
 if (::isdigit(c)) {
   if (!first_digit)
 first_digit = true;



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


[Lldb-commits] [PATCH] D112169: [lldb] [Communication] Add a WriteAll() method that resumes writing

2021-10-20 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, teemperor.
mgorny requested review of this revision.

Add a Communication::WriteAll() that resumes Write() if the initial call
did not write all data.  Use it in GDBRemoteCommunication when sending
packets in order to fix handling partial writes (i.e. just resume/retry
them rather than erring out).  This fixes LLDB failures when writing
large packets to a pty.

The solution used utilizes a short sleep in order to wait for the write
buffer to be freed.  Using select() would be better but it would require
much deeper changes to the code and does not seem justified at the time.


https://reviews.llvm.org/D112169

Files:
  lldb/include/lldb/Core/Communication.h
  lldb/source/Core/Communication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -101,7 +101,7 @@
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS));
   ConnectionStatus status = eConnectionStatusSuccess;
   char ch = '+';
-  const size_t bytes_written = Write(&ch, 1, status, nullptr);
+  const size_t bytes_written = WriteAll(&ch, 1, status, nullptr);
   LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, 
ch);
   m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written);
   return bytes_written;
@@ -111,7 +111,7 @@
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS));
   ConnectionStatus status = eConnectionStatusSuccess;
   char ch = '-';
-  const size_t bytes_written = Write(&ch, 1, status, nullptr);
+  const size_t bytes_written = WriteAll(&ch, 1, status, nullptr);
   LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, 
ch);
   m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written);
   return bytes_written;
@@ -137,7 +137,7 @@
 ConnectionStatus status = eConnectionStatusSuccess;
 const char *packet_data = packet.data();
 const size_t packet_length = packet.size();
-size_t bytes_written = Write(packet_data, packet_length, status, nullptr);
+size_t bytes_written = WriteAll(packet_data, packet_length, status, 
nullptr);
 if (log) {
   size_t binary_start_offset = 0;
   if (strncmp(packet_data, "$vFile:pwrite:", strlen("$vFile:pwrite:")) ==
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -189,6 +190,22 @@
   return 0;
 }
 
+size_t Communication::WriteAll(const void *src, size_t src_len,
+   ConnectionStatus &status, Status *error_ptr) {
+  size_t total_written = 0;
+  while (1) {
+size_t written = Write(static_cast(src) + total_written, 
src_len - total_written, status, error_ptr);
+total_written += written;
+if (status != eConnectionStatusSuccess || total_written >= src_len)
+  break;
+// sleep a short while to avoid wasting CPU cycles while we are unlikely
+// to be able to write again immediately
+// FIXME: improve this to use select()
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  }
+  return total_written;
+}
+
 bool Communication::StartReadThread(Status *error_ptr) {
   if (error_ptr)
 error_ptr->Clear();
Index: lldb/include/lldb/Core/Communication.h
===
--- lldb/include/lldb/Core/Communication.h
+++ lldb/include/lldb/Core/Communication.h
@@ -209,6 +209,22 @@
   size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status,
Status *error_ptr);
 
+  /// Repeatedly attempt writing until either \a src_len bytes are written
+  /// or a permanent failure occurs.
+  ///
+  /// \param[in] src
+  /// A source buffer that must be at least \a src_len bytes
+  /// long.
+  ///
+  /// \param[in] src_len
+  /// The number of bytes to attempt to write, and also the
+  /// number of bytes are currently available in \a src.
+  ///
+  /// \return
+  /// The number of bytes actually Written.
+  size_t WriteAll(const void *src, size_t src_len,
+  lldb::ConnectionStatus &status, Status *error_ptr);
+
   /// Sets the connection that it to be used by this class.
   ///
   /// By making a communication class that uses different connections it


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/G

[Lldb-commits] [PATCH] D112169: [lldb] [Communication] Add a WriteAll() method that resumes writing

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

clang-format


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

https://reviews.llvm.org/D112169

Files:
  lldb/include/lldb/Core/Communication.h
  lldb/source/Core/Communication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -101,7 +101,7 @@
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS));
   ConnectionStatus status = eConnectionStatusSuccess;
   char ch = '+';
-  const size_t bytes_written = Write(&ch, 1, status, nullptr);
+  const size_t bytes_written = WriteAll(&ch, 1, status, nullptr);
   LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, 
ch);
   m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written);
   return bytes_written;
@@ -111,7 +111,7 @@
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS));
   ConnectionStatus status = eConnectionStatusSuccess;
   char ch = '-';
-  const size_t bytes_written = Write(&ch, 1, status, nullptr);
+  const size_t bytes_written = WriteAll(&ch, 1, status, nullptr);
   LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, 
ch);
   m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written);
   return bytes_written;
@@ -137,7 +137,7 @@
 ConnectionStatus status = eConnectionStatusSuccess;
 const char *packet_data = packet.data();
 const size_t packet_length = packet.size();
-size_t bytes_written = Write(packet_data, packet_length, status, nullptr);
+size_t bytes_written = WriteAll(packet_data, packet_length, status, 
nullptr);
 if (log) {
   size_t binary_start_offset = 0;
   if (strncmp(packet_data, "$vFile:pwrite:", strlen("$vFile:pwrite:")) ==
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -189,6 +190,23 @@
   return 0;
 }
 
+size_t Communication::WriteAll(const void *src, size_t src_len,
+   ConnectionStatus &status, Status *error_ptr) {
+  size_t total_written = 0;
+  while (1) {
+size_t written = Write(static_cast(src) + total_written,
+   src_len - total_written, status, error_ptr);
+total_written += written;
+if (status != eConnectionStatusSuccess || total_written >= src_len)
+  break;
+// sleep a short while to avoid wasting CPU cycles while we are unlikely
+// to be able to write again immediately
+// FIXME: improve this to use select()
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  }
+  return total_written;
+}
+
 bool Communication::StartReadThread(Status *error_ptr) {
   if (error_ptr)
 error_ptr->Clear();
Index: lldb/include/lldb/Core/Communication.h
===
--- lldb/include/lldb/Core/Communication.h
+++ lldb/include/lldb/Core/Communication.h
@@ -209,6 +209,22 @@
   size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status,
Status *error_ptr);
 
+  /// Repeatedly attempt writing until either \a src_len bytes are written
+  /// or a permanent failure occurs.
+  ///
+  /// \param[in] src
+  /// A source buffer that must be at least \a src_len bytes
+  /// long.
+  ///
+  /// \param[in] src_len
+  /// The number of bytes to attempt to write, and also the
+  /// number of bytes are currently available in \a src.
+  ///
+  /// \return
+  /// The number of bytes actually Written.
+  size_t WriteAll(const void *src, size_t src_len,
+  lldb::ConnectionStatus &status, Status *error_ptr);
+
   /// Sets the connection that it to be used by this class.
   ///
   /// By making a communication class that uses different connections it


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -101,7 +101,7 @@
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS));
   ConnectionStatus status = eConnectionStatusSuccess;
   char ch = '+';
-  const size_t bytes_written = Write(&ch, 1, status, nullptr);
+  const size_t bytes_written = WriteAll(&ch, 1, status, nullptr);
   LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, ch);
   m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written);
 

[Lldb-commits] [PATCH] D112167: [lldb/Plugins] Refactor ScriptedThread register context creation

2021-10-20 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:186
 void ScriptedThread::RefreshStateAfterStop() {
-  // TODO: Implement
-  if (m_reg_context_sp)
-m_reg_context_sp->InvalidateAllRegisters();
+  GetRegisterContext()->InvalidateIfNeeded(false /*=force*/);
 }

It should be `/*force=*/false`


See [clang-tidy - 
bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone-argument-comment.html)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112167

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


[Lldb-commits] [PATCH] D112180: Libcpp bitset syntethic children and tests

2021-10-20 Thread Danil Stefaniuc via Phabricator via lldb-commits
danilashtefan created this revision.
Herald added a subscriber: mgorny.
danilashtefan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112180

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppBitset.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/TestDataFormatterStdBitset.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+
+template
+void fill(std::bitset &b) {
+  b.set();
+  b[0] = b[1] = false;
+  for (std::size_t i = 2; i < N; ++i) {
+for (std::size_t j = 2*i; j < N; j+=i)
+  b[j] = false;
+  }
+}
+
+template
+void by_ref_and_ptr(std::bitset &ref, std::bitset *ptr) {
+// Check ref and ptr
+return;
+}
+
+int main() {
+  std::bitset<0> empty;
+  std::bitset<13> small;
+  fill(small);
+  std::bitset<70> large;
+  fill(large);
+  by_ref_and_ptr(small, &small); // break here
+  by_ref_and_ptr(large, &large);
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/TestDataFormatterStdBitset.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/bitset/TestDataFormatterStdBitset.py
@@ -0,0 +1,232 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class StdBitsetDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+primes = [1]*300
+primes[0] = primes[1] = 0
+for i in range(2, len(primes)):
+for j in range(2*i, len(primes), i):
+primes[j] = 0
+self.primes = primes
+
+def check(self, name, size):
+var = self.frame().FindVariable(name)
+self.assertTrue(var.IsValid())
+self.assertEqual(var.GetNumChildren(), size)
+for i in range(size):
+child = var.GetChildAtIndex(i)
+self.assertEqual(child.GetValueAsUnsigned(), self.primes[i],
+"variable: %s, index: %d"%(name, size))
+
+@add_test_categories(["libstdcxx"])
+def test_value(self):
+"""Test that std::bitset is displayed correctly"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.check("empty", 0)
+self.check("small", 13)
+self.check("large", 70)
+
+@add_test_categories(["libstdcxx"])
+def test_ptr_and_ref(self):
+"""Test that ref and ptr to std::bitset is displayed correctly"""
+self.build()
+(_, process, _, bkpt) = lldbutil.run_to_source_breakpoint(self,
+'Check ref and ptr',
+lldb.SBFileSpec("main.cpp", False))
+
+#self.check("ref", 13)
+#self.check("ptr", 13)
+self.expect("p ref",
+substrs=['[0] = false',
+'[1] = false',
+'[2] = true',
+'[3] = true', 
+'[4] = false', 
+'[5] = true', 
+'[6] = false', 
+'[7] = true', 
+'[8] = false',
+'[9] = false', 
+'[10] = false', 
+'[11] = true', 
+'[12] = false'])
+
+self.expect("p *ptr",
+substrs=['[0] = false',
+'[1] = false',
+'[2] = true',
+'[3] = true', 
+'[4] = false', 
+'[5] = true', 
+'[6] = false', 
+'[7] = true', 
+'[8] = false',
+'[9] = false', 
+'[10] = false', 
+'[11] = true', 
+'[12] = false'])
+
+ 
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("p ref",
+substrs=['[0] = false',
+'[1] = false',
+'[2] = true',
+'[3] = true',
+'[4] =

[Lldb-commits] [PATCH] D112167: [lldb/Plugins] Refactor ScriptedThread register context creation

2021-10-20 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:99
 ScriptedThread::CreateRegisterContextForFrame(StackFrame *frame) {
   uint32_t concrete_frame_idx = frame ? frame->GetConcreteFrameIndex() : 0;
 

`const`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112167

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


[Lldb-commits] [PATCH] D112167: [lldb/Plugins] Refactor ScriptedThread register context creation

2021-10-20 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 381091.
mib added a comment.

Address @shafik comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112167

Files:
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -64,32 +64,6 @@
   m_script_object_sp = object_sp;
 
   SetID(scripted_thread_interface->GetThreadID());
-
-  llvm::Optional reg_data =
-  scripted_thread_interface->GetRegisterContext();
-  if (!reg_data) {
-error.SetErrorString("Failed to get scripted thread registers data.");
-return;
-  }
-
-  DataBufferSP data_sp(
-  std::make_shared(reg_data->c_str(), reg_data->size()));
-
-  if (!data_sp->GetByteSize()) {
-error.SetErrorString("Failed to copy raw registers data.");
-return;
-  }
-
-  std::shared_ptr reg_ctx_memory =
-  std::make_shared(
-  *this, 0, *GetDynamicRegisterInfo(), LLDB_INVALID_ADDRESS);
-  if (!reg_ctx_memory) {
-error.SetErrorString("Failed to create a register context.");
-return;
-  }
-
-  reg_ctx_memory->SetAllRegisterData(data_sp);
-  m_reg_context_sp = reg_ctx_memory;
 }
 
 ScriptedThread::~ScriptedThread() { DestroyThread(); }
@@ -115,11 +89,8 @@
 void ScriptedThread::ClearStackFrames() { Thread::ClearStackFrames(); }
 
 RegisterContextSP ScriptedThread::GetRegisterContext() {
-  if (!m_reg_context_sp) {
-m_reg_context_sp = std::make_shared(
-*this, LLDB_INVALID_ADDRESS);
-GetInterface()->GetRegisterContext();
-  }
+  if (!m_reg_context_sp)
+m_reg_context_sp = CreateRegisterContextForFrame(nullptr);
   return m_reg_context_sp;
 }
 
@@ -127,9 +98,38 @@
 ScriptedThread::CreateRegisterContextForFrame(StackFrame *frame) {
   uint32_t concrete_frame_idx = frame ? frame->GetConcreteFrameIndex() : 0;
 
-  if (concrete_frame_idx == 0)
-return GetRegisterContext();
-  return GetUnwinder().CreateRegisterContextForFrame(frame);
+  if (concrete_frame_idx)
+return GetUnwinder().CreateRegisterContextForFrame(frame);
+
+  lldb::RegisterContextSP reg_ctx_sp;
+  Status error;
+
+  llvm::Optional reg_data = GetInterface()->GetRegisterContext();
+  if (!reg_data) {
+error.SetErrorString("Failed to get scripted thread registers data.");
+return nullptr;
+  }
+
+  DataBufferSP data_sp(
+  std::make_shared(reg_data->c_str(), reg_data->size()));
+
+  if (!data_sp->GetByteSize()) {
+error.SetErrorString("Failed to copy raw registers data.");
+return nullptr;
+  }
+
+  std::shared_ptr reg_ctx_memory =
+  std::make_shared(
+  *this, 0, *GetDynamicRegisterInfo(), LLDB_INVALID_ADDRESS);
+  if (!reg_ctx_memory) {
+error.SetErrorString("Failed to create a register context.");
+return nullptr;
+  }
+
+  reg_ctx_memory->SetAllRegisterData(data_sp);
+  m_reg_context_sp = reg_ctx_memory;
+
+  return m_reg_context_sp;
 }
 
 bool ScriptedThread::CalculateStopInfo() {
@@ -183,9 +183,7 @@
 }
 
 void ScriptedThread::RefreshStateAfterStop() {
-  // TODO: Implement
-  if (m_reg_context_sp)
-m_reg_context_sp->InvalidateAllRegisters();
+  GetRegisterContext()->InvalidateIfNeeded(/*force=*/false);
 }
 
 lldb::ScriptedThreadInterfaceSP ScriptedThread::GetInterface() const {
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -78,7 +78,7 @@
 
   Status DoDestroy() override;
 
-  void RefreshStateAfterStop() override{};
+  void RefreshStateAfterStop() override;
 
   bool IsAlive() override;
 
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -20,7 +20,6 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/RegisterContext.h"
-
 #include "lldb/Utility/State.h"
 
 #include 
@@ -296,6 +295,7 @@
   // actually new threads will get added to new_thread_list.
 
   CheckInterpreterAndScriptObject();
+  m_thread_plans.ClearThreadCache();
 
   Status error;
   ScriptLanguage language = m_interpreter->GetLanguage();
@@ -321,6 +321,12 @@
   return new_thread_list.GetSize(false) > 0;
 }
 
+void ScriptedProcess::RefreshStateAfterStop() {
+  // Let all threads recover from stopping and do any clean up based on the
+  // previous thread state (if any).
+  m_thread_list.RefreshStateAfterStop();
+}
+
 bool ScriptedP

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

2021-10-20 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid accepted this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

This look great. Thanks for doing the legit fix.

@mgorny This is something you might wanna implement/test on BSD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

___
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-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/include/lldb/Target/Statistics.h:96
+};
+
+class GlobalStats {

JDevlieghere wrote:
> Do we expect there to be something like `DebuggerStats`? I think it would be 
> nice from a hierarchy perspective that Global Stats have a map of debugger -> 
> to debuggerstat who then in turn hold on to a map of target -> target stats. 
> That hierarchy would work really well for JSON (except they would be lists 
> instead of maps). 
I can change this to DebuggerStats as this is essentially what this was.



Comment at: lldb/include/lldb/Target/Statistics.h:114
+  static bool g_collecting_stats;
+};
+

This can be expensive if you start locking a mutex just to increment a stat 
that is a counter and will make statistics slow down the debugger. I would 
rather rely on std::atomic or the locks already built into Target or Debugger 
if possible. If lldb_asserts are firing off at a high rate, we can't be 
spending thousands of instructions locking and unlocking mutexes.


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-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 381106.
clayborg added a comment.

Rename GlobalStats to DebuggerStats.


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", substrs=['expr evaluation successes : 1\n',
-'expr evaluation failures : 0\n'])
+self.expect("statistics disable", substrs=['need to enable statistics before disabling'], error=True)
 
-self.expect("statistics enable")
-# Doesn't parse.
+def verify_key_i

[Lldb-commits] [PATCH] D112199: [LLDB] [NFC] Typo fix in usage text for "type filter" command

2021-10-20 Thread Shivam Gupta via Phabricator via lldb-commits
xgupta created this revision.
xgupta added reviewers: labath, teemperor.
xgupta requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When you invoke "help type filter" the resulting help shows:

Syntax: type synthetic []

This patch fixes the help so it says "type filter" instead of "type synthetic".

patch by: "Daniel Jalkut "


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112199

Files:
  lldb/source/Commands/CommandObjectType.cpp


Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2978,7 +2978,7 @@
   CommandObjectTypeFilter(CommandInterpreter &interpreter)
   : CommandObjectMultiword(interpreter, "type filter",
"Commands for operating on type filters.",
-   "type synthetic [] ") {
+   "type filter [] ") {
 LoadSubCommand(
 "add", CommandObjectSP(new CommandObjectTypeFilterAdd(interpreter)));
 LoadSubCommand("clear", CommandObjectSP(


Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2978,7 +2978,7 @@
   CommandObjectTypeFilter(CommandInterpreter &interpreter)
   : CommandObjectMultiword(interpreter, "type filter",
"Commands for operating on type filters.",
-   "type synthetic [] ") {
+   "type filter [] ") {
 LoadSubCommand(
 "add", CommandObjectSP(new CommandObjectTypeFilterAdd(interpreter)));
 LoadSubCommand("clear", CommandObjectSP(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112199: [LLDB] [NFC] Typo fix in usage text for "type filter" command

2021-10-20 Thread Shivam Gupta via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd531e5cf5841: [LLDB] [NFC] Typo fix in usage text for 
"type filter" command (authored by Daniel Jalkut 
, committed by xgupta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112199

Files:
  lldb/source/Commands/CommandObjectType.cpp


Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2978,7 +2978,7 @@
   CommandObjectTypeFilter(CommandInterpreter &interpreter)
   : CommandObjectMultiword(interpreter, "type filter",
"Commands for operating on type filters.",
-   "type synthetic [] ") {
+   "type filter [] ") {
 LoadSubCommand(
 "add", CommandObjectSP(new CommandObjectTypeFilterAdd(interpreter)));
 LoadSubCommand("clear", CommandObjectSP(


Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2978,7 +2978,7 @@
   CommandObjectTypeFilter(CommandInterpreter &interpreter)
   : CommandObjectMultiword(interpreter, "type filter",
"Commands for operating on type filters.",
-   "type synthetic [] ") {
+   "type filter [] ") {
 LoadSubCommand(
 "add", CommandObjectSP(new CommandObjectTypeFilterAdd(interpreter)));
 LoadSubCommand("clear", CommandObjectSP(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] d531e5c - [LLDB] [NFC] Typo fix in usage text for "type filter" command

2021-10-20 Thread Shivam Gupta via lldb-commits

Author: Daniel Jalkut
Date: 2021-10-21T12:22:52+05:30
New Revision: d531e5cf58413e34dc006a580d2c109863bddaa1

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

LOG: [LLDB] [NFC] Typo fix in usage text for "type filter" command

When you invoke "help type filter" the resulting help shows:

Syntax: type synthetic []

This patch fixes the help so it says "type filter" instead of "type synthetic".

patch by: "Daniel Jalkut "

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectType.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectType.cpp 
b/lldb/source/Commands/CommandObjectType.cpp
index 90e224867e2a4..3bff844786f4e 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -2978,7 +2978,7 @@ class CommandObjectTypeFilter : public 
CommandObjectMultiword {
   CommandObjectTypeFilter(CommandInterpreter &interpreter)
   : CommandObjectMultiword(interpreter, "type filter",
"Commands for operating on type filters.",
-   "type synthetic [] ") {
+   "type filter [] ") {
 LoadSubCommand(
 "add", CommandObjectSP(new CommandObjectTypeFilterAdd(interpreter)));
 LoadSubCommand("clear", CommandObjectSP(



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