[Lldb-commits] [PATCH] D159315: [lldb] Add OperatingSystem base class to the lldb python module

2023-08-31 Thread Will Hawkins via Phabricator via lldb-commits
hawkinsw added a comment.

I hope that some comments are helpful! The documentation that you added is 
tremendously helpful -- I know how documentation is sometimes a thankless task, 
so I will say what everyone is thinking: "THANK YOU".




Comment at: lldb/examples/python/templates/operating_system.py:11
+"""
+Class that provides data for an instance of a LLDB 'OperatingSystemPython' 
plug-in class
+





Comment at: lldb/examples/python/templates/operating_system.py:27
+- queue : thread dispatch queue name (optional key/value pair)
+- state : thred state (mandatory, set to 'stopped' for now)
+- core : the index of the core (lldb) thread that this OS Thread should 
shadow





Comment at: lldb/examples/python/templates/operating_system.py:31
+Possible values include:
+'breakpoint' if the thread is stopped at a breakpoint
+'none' thread is just stopped because the process is stopped





Comment at: lldb/examples/python/templates/operating_system.py:32
+'breakpoint' if the thread is stopped at a breakpoint
+'none' thread is just stopped because the process is stopped
+'trace' the thread just single stepped





Comment at: lldb/examples/python/templates/operating_system.py:33
+'none' thread is just stopped because the process is stopped
+'trace' the thread just single stepped
+The usual value for this while threads are in memory is 'none'





Comment at: lldb/examples/python/templates/operating_system.py:73
+def get_thread_info(self):
+"""Get the list of operating system threads. This method get called
+automatically every time the process stops and it needs to update its





Comment at: lldb/examples/python/templates/operating_system.py:80
+containing at least for each entry, the thread id, it's name,
+queue, state, stop reason. It can also contain a 
`register_data_addr`
+The list can be empty.

I am sorry but I do not quite understand the meaning of this description. It is 
probably because I am dense, but maybe you can look at rewording it slightly?



Comment at: lldb/examples/python/templates/operating_system.py:89
+id. This method is called when unwinding the stack of one of the
+operating system thread.
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159315

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


[Lldb-commits] [PATCH] D159313: [lldb][NFCI] Remove use of ConstString in StructuredData

2023-08-31 Thread Alex Langford via Phabricator via lldb-commits
bulbazord marked an inline comment as done.
bulbazord added inline comments.



Comment at: lldb/include/lldb/Utility/StructuredData.h:448
-  if (!key.empty()) {
-ConstString key_cs(key);
-collection::const_iterator iter = m_dict.find(key_cs);

fdeazeve wrote:
> This line in particular also shows why I think this patch is appropriate: 
> even queries were introducing strings into the pool
Yes, this also was happening in `HasKey` as well. 



Comment at: lldb/include/lldb/Utility/StructuredData.h:537
 void AddItem(llvm::StringRef key, ObjectSP value_sp) {
-  ConstString key_cs(key);
-  m_dict[key_cs] = std::move(value_sp);
+  m_dict.insert({key, std::move(value_sp)});
 }

fdeazeve wrote:
> This is not equivalent to the old code, right? The previous code would 
> overwrite the contents if the key already existed.
Oh, good catch. I think what I want is `insert_or_assign`. Will update.



Comment at: lldb/source/Utility/StructuredData.cpp:173
+
+  llvm::sort(sorted_entries, [&](const Entry , const Entry ) -> bool {
+return lhs.first < rhs.first;

fdeazeve wrote:
> I thought pairs had `operator <` defined already?
Oh, I didn't even think to check. I'll try it and update if I don't need to 
define this lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159313

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


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

2023-08-31 Thread Alex Langford 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 rG764287f1ad69: [lldb] Add support for recognizing swift ast 
sections in object files (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159142

Files:
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Core/Section.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Symbol/ObjectFile.cpp


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -357,6 +357,7 @@
   case eSectionTypeDWARFAppleObjC:
   case eSectionTypeDWARFGNUDebugAltLink:
   case eSectionTypeCTF:
+  case eSectionTypeSwiftModules:
 return AddressClass::eDebug;
   case eSectionTypeEHFrame:
   case eSectionTypeARMexidx:
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1009,6 +1009,7 @@
   // .eh_frame can be truncated to 8 chars.
   .Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)
   .Case(".gosymtab", eSectionTypeGoSymtab)
+  .Case("swiftast", eSectionTypeSwiftModules)
   .Default(eSectionTypeInvalid);
   if (section_type != eSectionTypeInvalid)
 return section_type;
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1202,6 +1202,7 @@
 case eSectionTypeDWARFAppleObjC:
 case eSectionTypeDWARFGNUDebugAltLink:
 case eSectionTypeCTF:
+case eSectionTypeSwiftModules:
   return AddressClass::eDebug;
 
 case eSectionTypeEHFrame:
@@ -1476,6 +1477,7 @@
   static ConstString g_sect_name_data("__data");
   static ConstString g_sect_name_go_symtab("__gosymtab");
   static ConstString g_sect_name_ctf("__ctf");
+  static ConstString g_sect_name_swift_ast("__swift_ast");
 
   if (section_name == g_sect_name_dwarf_debug_abbrev)
 return eSectionTypeDWARFDebugAbbrev;
@@ -1555,6 +1557,8 @@
 return eSectionTypeGoSymtab;
   if (section_name == g_sect_name_ctf)
 return eSectionTypeCTF;
+  if (section_name == g_sect_name_swift_ast)
+return eSectionTypeSwiftModules;
   if (section_name == g_sect_name_objc_data ||
   section_name == g_sect_name_objc_classrefs ||
   section_name == g_sect_name_objc_superrefs ||
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1679,6 +1679,7 @@
   .Case(".gnu_debugaltlink", eSectionTypeDWARFGNUDebugAltLink)
   .Case(".gosymtab", eSectionTypeGoSymtab)
   .Case(".text", eSectionTypeCode)
+  .Case(".swift_ast", eSectionTypeSwiftModules)
   .Default(eSectionTypeOther);
 }
 
Index: lldb/source/Core/Section.cpp
===
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -149,6 +149,8 @@
 return "ctf";
   case eSectionTypeOther:
 return "regular";
+  case eSectionTypeSwiftModules:
+return "swift-modules";
   }
   return "unknown";
 }
@@ -455,6 +457,7 @@
   case eSectionTypeDWARFAppleObjC:
   case eSectionTypeDWARFGNUDebugAltLink:
   case eSectionTypeCTF:
+  case eSectionTypeSwiftModules:
 return true;
   }
   return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -739,6 +739,7 @@
   eSectionTypeDWARFDebugLocListsDwo,
   eSectionTypeDWARFDebugTuIndex,
   eSectionTypeCTF,
+  eSectionTypeSwiftModules,
 };
 
 FLAGS_ENUM(EmulateInstructionOptions){


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -357,6 +357,7 @@
   case eSectionTypeDWARFAppleObjC:
   case eSectionTypeDWARFGNUDebugAltLink:
   case eSectionTypeCTF:
+  case eSectionTypeSwiftModules:
 return AddressClass::eDebug;
   case eSectionTypeEHFrame:
   case eSectionTypeARMexidx:
Index: 

[Lldb-commits] [lldb] 764287f - [lldb] Add support for recognizing swift ast sections in object files

2023-08-31 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-31T15:16:12-07:00
New Revision: 764287f1ad69469cc264bb094e8fcdcfdd0fcdfb

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

LOG: [lldb] Add support for recognizing swift ast sections in object files

In Apple's downstream fork, there is support for understanding the swift
AST sections in various binaries. Even though the lldb on llvm.org does
not have support for debugging swift, I think it makes sense to move
support for recognizing swift ast sections upstream.

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

Added: 


Modified: 
lldb/include/lldb/lldb-enumerations.h
lldb/source/Core/Section.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Symbol/ObjectFile.cpp

Removed: 




diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index e2cda2a65eef5d..21e098481ce802 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -739,6 +739,7 @@ enum SectionType {
   eSectionTypeDWARFDebugLocListsDwo,
   eSectionTypeDWARFDebugTuIndex,
   eSectionTypeCTF,
+  eSectionTypeSwiftModules,
 };
 
 FLAGS_ENUM(EmulateInstructionOptions){

diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index 212b3119894fc9..9e98b59deb0357 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -149,6 +149,8 @@ const char *Section::GetTypeAsCString() const {
 return "ctf";
   case eSectionTypeOther:
 return "regular";
+  case eSectionTypeSwiftModules:
+return "swift-modules";
   }
   return "unknown";
 }
@@ -455,6 +457,7 @@ bool Section::ContainsOnlyDebugInfo() const {
   case eSectionTypeDWARFAppleObjC:
   case eSectionTypeDWARFGNUDebugAltLink:
   case eSectionTypeCTF:
+  case eSectionTypeSwiftModules:
 return true;
   }
   return false;

diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 700af84a14c061..2da971dff895b4 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1679,6 +1679,7 @@ static SectionType GetSectionTypeFromName(llvm::StringRef 
Name) {
   .Case(".gnu_debugaltlink", eSectionTypeDWARFGNUDebugAltLink)
   .Case(".gosymtab", eSectionTypeGoSymtab)
   .Case(".text", eSectionTypeCode)
+  .Case(".swift_ast", eSectionTypeSwiftModules)
   .Default(eSectionTypeOther);
 }
 

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3e52c9e3c04281..3ce2057d537227 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1202,6 +1202,7 @@ AddressClass 
ObjectFileMachO::GetAddressClass(lldb::addr_t file_addr) {
 case eSectionTypeDWARFAppleObjC:
 case eSectionTypeDWARFGNUDebugAltLink:
 case eSectionTypeCTF:
+case eSectionTypeSwiftModules:
   return AddressClass::eDebug;
 
 case eSectionTypeEHFrame:
@@ -1476,6 +1477,7 @@ static lldb::SectionType GetSectionType(uint32_t flags,
   static ConstString g_sect_name_data("__data");
   static ConstString g_sect_name_go_symtab("__gosymtab");
   static ConstString g_sect_name_ctf("__ctf");
+  static ConstString g_sect_name_swift_ast("__swift_ast");
 
   if (section_name == g_sect_name_dwarf_debug_abbrev)
 return eSectionTypeDWARFDebugAbbrev;
@@ -1555,6 +1557,8 @@ static lldb::SectionType GetSectionType(uint32_t flags,
 return eSectionTypeGoSymtab;
   if (section_name == g_sect_name_ctf)
 return eSectionTypeCTF;
+  if (section_name == g_sect_name_swift_ast)
+return eSectionTypeSwiftModules;
   if (section_name == g_sect_name_objc_data ||
   section_name == g_sect_name_objc_classrefs ||
   section_name == g_sect_name_objc_superrefs ||

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index e89906a02e1b96..7fb10a69391c56 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1009,6 +1009,7 @@ SectionType 
ObjectFilePECOFF::GetSectionType(llvm::StringRef sect_name,
   // .eh_frame can be truncated to 8 chars.
   .Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)
   .Case(".gosymtab", eSectionTypeGoSymtab)
+  .Case("swiftast", eSectionTypeSwiftModules)
   .Default(eSectionTypeInvalid);
   if (section_type != 

[Lldb-commits] [PATCH] D159313: [lldb][NFCI] Remove use of ConstString in StructuredData

2023-08-31 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve added inline comments.



Comment at: lldb/include/lldb/Utility/StructuredData.h:436
   auto array_sp = std::make_shared();
-  collection::const_iterator iter;
-  for (iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
+  for (auto iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
 auto key_object_sp = std::make_shared();

Since we are touching this line anyway, could you replace this with

```
for (StringRef key : llvm::make_first_range(m_dict))
```

This has a bit less cognitive burden IMO



Comment at: lldb/include/lldb/Utility/StructuredData.h:438
 auto key_object_sp = std::make_shared();
-key_object_sp->SetValue(iter->first.AsCString());
+key_object_sp->SetValue(iter->first());
 array_sp->Push(key_object_sp);

Also since you are touching this line, one could argue for it to be deleted and 
folded into `std::make_shared(Key);`

I don't feel strongly about it though, it could also be more appropriate for a 
separate commit



Comment at: lldb/include/lldb/Utility/StructuredData.h:448
-  if (!key.empty()) {
-ConstString key_cs(key);
-collection::const_iterator iter = m_dict.find(key_cs);

This line in particular also shows why I think this patch is appropriate: even 
queries were introducing strings into the pool



Comment at: lldb/include/lldb/Utility/StructuredData.h:537
 void AddItem(llvm::StringRef key, ObjectSP value_sp) {
-  ConstString key_cs(key);
-  m_dict[key_cs] = std::move(value_sp);
+  m_dict.insert({key, std::move(value_sp)});
 }

This is not equivalent to the old code, right? The previous code would 
overwrite the contents if the key already existed.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2035
   uint32_t reg;
-  if (llvm::to_integer(key.AsCString(), reg))
 expedited_register_map[reg] =

This is another win: the old code was calling `StringRef(const char*)`, which 
has to iterate over the entire string to find the null terminating character.



Comment at: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp:248
   if (do_truncate_remapping_names) {
-FileSpec build_path(key.AsCString());
 FileSpec source_path(DBGSourcePath.c_str());

same win here



Comment at: lldb/source/Target/Target.cpp:3866
-s.Printf("%s : %s\n", key.GetCString(),
-  object->GetStringValue().str().c_str());
 return true;

we talked about it in private, but I feel like stating this publicly: does 
anyone know if this was legal? `object->GetStringValue().str()` creates a 
temporary std::string. And then we call `c_str()` and pass that to the Printf 
function. Does the temporary std::string have its lifetime extended?



Comment at: lldb/source/Utility/StructuredData.cpp:173
+
+  llvm::sort(sorted_entries, [&](const Entry , const Entry ) -> bool {
+return lhs.first < rhs.first;

I thought pairs had `operator <` defined already?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159313

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


[Lldb-commits] [PATCH] D159234: [llvm-objdump] Enable assembly highlighting in llvm-objdump

2023-08-31 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere abandoned this revision.
JDevlieghere added a comment.

Merged with D159224 .


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

https://reviews.llvm.org/D159234

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


[Lldb-commits] [PATCH] D159315: [lldb] Add OperatingSystem base class to the lldb python module

2023-08-31 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch introduce an `OperatingSystem` base implementation in the `lldb`
python module to make it easier to lldb users to write their own
implementation.

The `OperatingSystem` base implementation is derived itself from the
`ScriptedThread` base implementation since they share some common grounds.

To achieve that, this patch makes changes to the `ScriptedThread`
initializer since it gets called by the `OperatingSystem` initializer.

I also took the opportunity to document the `OperatingSystem` base
class and methods.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159315

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/examples/python/templates/operating_system.py
  lldb/examples/python/templates/scripted_process.py
  lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py

Index: lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
===
--- lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
+++ lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
@@ -1,29 +1,14 @@
-#!/usr/bin/env python
-
 import lldb
 import struct
 
+from lldb.plugins.operating_system import OperatingSystem
+
 
-class OperatingSystemPlugIn(object):
+class OperatingSystemPlugIn(OperatingSystem):
 """Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class"""
 
 def __init__(self, process):
-"""Initialization needs a valid.SBProcess object.
-
-This plug-in will get created after a live process is valid and has stopped for the
-first time."""
-self.process = None
-self.registers = None
-self.threads = None
-if isinstance(process, lldb.SBProcess) and process.IsValid():
-self.process = process
-self.threads = None  # Will be an dictionary containing info for each thread
-
-def get_target(self):
-# NOTE: Don't use "lldb.target" when trying to get your target as the "lldb.target"
-# tracks the current target in the LLDB command interpreter which isn't the
-# correct thing to use for this plug-in.
-return self.process.target
+super().__init__(process)
 
 def create_thread(self, tid, context):
 if tid == 0x4:
@@ -40,23 +25,6 @@
 
 def get_thread_info(self):
 if not self.threads:
-# The sample dictionary below shows the values that can be returned for a thread
-# tid => thread ID (mandatory)
-# name => thread name (optional key/value pair)
-# queue => thread dispatch queue name (optional key/value pair)
-# state => thred state (mandatory, set to 'stopped' for now)
-# stop_reason => thread stop reason. (mandatory, usually set to 'none')
-#  Possible values include:
-#   'breakpoint' if the thread is stopped at a breakpoint
-#   'none' thread is just stopped because the process is stopped
-#   'trace' the thread just single stepped
-#   The usual value for this while threads are in memory is 'none'
-# register_data_addr => the address of the register data in memory (optional key/value pair)
-#   Specifying this key/value pair for a thread will avoid a call to get_register_data()
-#   and can be used when your registers are in a thread context structure that is contiguous
-#   in memory. Don't specify this if your register layout in memory doesn't match the layout
-# described by the dictionary returned from a call to the
-# get_register_info() method.
 self.threads = [
 {
 "tid": 0x1,
Index: lldb/examples/python/templates/scripted_process.py
===
--- lldb/examples/python/templates/scripted_process.py
+++ lldb/examples/python/templates/scripted_process.py
@@ -244,16 +244,16 @@
 """
 
 @abstractmethod
-def __init__(self, scripted_process, args):
+def __init__(self, process, args):
 """Construct a scripted thread.
 
 Args:
-process (ScriptedProcess): The scripted process owning this thread.
+process (ScriptedProcess/lldb.SBProcess): The process owning this thread.
 args (lldb.SBStructuredData): A Dictionary holding arbitrary
 key/value pairs used by the scripted thread.
 """
 self.target = None
-self.scripted_process = None
+self.originating_process = None
 self.process = None
 self.args = None
   

[Lldb-commits] [PATCH] D159314: [lldb] Introduce OperatingSystem{, Python}Interface and make us it

2023-08-31 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: JDevlieghere, jingham, bulbazord.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch aims to consolidate the OperatingSystem scripting affordance
by introducing a stable interface that conforms to the
Scripted{,Python}Interface.

This unify the way we call into python methods from lldb while
also improving its capabilities by allowing us to pass lldb_private
objects are arguments.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159314

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/bindings/python/python.swig
  lldb/include/lldb/Interpreter/Interfaces/OperatingSystemInterface.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
  lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -134,24 +134,9 @@
 
   lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() override;
 
-  StructuredData::GenericSP
-  OSPlugin_CreatePluginObject(const char *class_name,
-  lldb::ProcessSP process_sp) override;
-
-  StructuredData::DictionarySP
-  OSPlugin_RegisterInfo(StructuredData::ObjectSP os_plugin_object_sp) override;
   lldb::ScriptedThreadInterfaceSP CreateScriptedThreadInterface() override;
 
-  StructuredData::ArraySP
-  OSPlugin_ThreadsInfo(StructuredData::ObjectSP os_plugin_object_sp) override;
-
-  StructuredData::StringSP
-  OSPlugin_RegisterContextData(StructuredData::ObjectSP os_plugin_object_sp,
-   lldb::tid_t thread_id) override;
-
-  StructuredData::DictionarySP
-  OSPlugin_CreateThread(StructuredData::ObjectSP os_plugin_object_sp,
-lldb::tid_t tid, lldb::addr_t context) override;
+  lldb::OperatingSystemInterfaceSP CreateOperatingSystemInterface() override;
 
   StructuredData::ObjectSP
   LoadPluginModule(const FileSpec _spec,
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -14,6 +14,7 @@
 // LLDB Python header must be included first
 #include "lldb-python.h"
 
+#include "Interfaces/OperatingSystemPythonInterface.h"
 #include "Interfaces/ScriptedPlatformPythonInterface.h"
 #include "Interfaces/ScriptedProcessPythonInterface.h"
 #include "Interfaces/ScriptedThreadPythonInterface.h"
@@ -1521,6 +1522,11 @@
   return std::make_shared(*this);
 }
 
+OperatingSystemInterfaceSP
+ScriptInterpreterPythonImpl::CreateOperatingSystemInterface() {
+  return std::make_shared(*this);
+}
+
 StructuredData::ObjectSP
 ScriptInterpreterPythonImpl::CreateStructuredDataFromScriptObject(
 ScriptObject obj) {
@@ -1532,159 +1538,6 @@
   return py_obj.CreateStructuredObject();
 }
 
-StructuredData::GenericSP
-ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
-const char *class_name, lldb::ProcessSP process_sp) {
-  if (class_name == nullptr || class_name[0] == '\0')
-return StructuredData::GenericSP();
-
-  if (!process_sp)
-return StructuredData::GenericSP();
-
-  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
-  PythonObject ret_val = SWIGBridge::LLDBSWIGPythonCreateOSPlugin(
-  class_name, m_dictionary_name.c_str(), process_sp);
-
-  return StructuredData::GenericSP(
-  new StructuredPythonObject(std::move(ret_val)));
-}
-
-StructuredData::DictionarySP ScriptInterpreterPythonImpl::OSPlugin_RegisterInfo(
-StructuredData::ObjectSP os_plugin_object_sp) {
-  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
-
-  if (!os_plugin_object_sp)
-return {};
-
-  StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric();
-  if (!generic)
-return {};
-
-  PythonObject implementor(PyRefType::Borrowed,
-   (PyObject *)generic->GetValue());
-
-  if (!implementor.IsAllocated())
-return {};
-
-  llvm::Expected 

[Lldb-commits] [PATCH] D159310: [lldb] Move template python files to separate directory

2023-08-31 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

This looks fine to me. You'll probably want to wait for @JDevlieghere to take a 
look though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159310

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


[Lldb-commits] [PATCH] D159313: [lldb][NFCI] Remove use of ConstString in StructuredData

2023-08-31 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, fdeazeve.
Herald added a subscriber: mgrang.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The remaining use of ConstString in StructuredData is the Dictionary
class. Internally it's backed by a `std::map`.
I propose that we replace it with a `llvm::StringMap`.

Many StructuredData::Dictionary objects are ephemeral and only exist for
a short amount of time. Many of these Dictionaries are only produced
once and are never used again. That leaves us with a lot of string data
in the ConstString StringPool that is sitting there never to be used
again. Even if the same string is used many times for keys of different
Dictionary objects, that is something we can measure and adjust for
instead of assuming that every key may be reused at some point in the
future.

Quick comparisons of key data is likely not a concern with Dictionary,
but the use of `llvm::StringMap` means that lookups should be fast with
its hashing strategy.

Switching to a llvm::StringMap meant that the iteration order may be
different. To account for this when serializing/dumping the dictionary,
I added some code to sort the output by key before emitting anything.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159313

Files:
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/StructuredData.cpp

Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -162,8 +162,20 @@
 
 void StructuredData::Dictionary::Serialize(json::OStream ) const {
   s.objectBegin();
-  for (const auto  : m_dict) {
-s.attributeBegin(pair.first.GetStringRef());
+
+  // To ensure the output format is always stable, we sort the dictionary by key
+  // first.
+  using Entry = std::pair;
+  std::vector sorted_entries;
+  for (const auto  : m_dict)
+sorted_entries.push_back({pair.first(), pair.second});
+
+  llvm::sort(sorted_entries, [&](const Entry , const Entry ) -> bool {
+return lhs.first < rhs.first;
+  });
+
+  for (const auto  : sorted_entries) {
+s.attributeBegin(pair.first);
 pair.second->Serialize(s);
 s.attributeEnd();
   }
@@ -228,9 +240,22 @@
 
 void StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const {
   size_t indentation_level = s.GetIndentLevel();
-  for (auto iter = m_dict.begin(); iter != m_dict.end(); iter++) {
+
+  // To ensure the output format is always stable, we sort the dictionary by key
+  // first.
+  using Entry = std::pair;
+  std::vector sorted_entries;
+  for (const auto  : m_dict)
+sorted_entries.push_back({pair.first(), pair.second});
+
+  llvm::sort(sorted_entries, [&](const Entry , const Entry ) -> bool {
+return lhs.first < rhs.first;
+  });
+
+  for (auto iter = sorted_entries.begin(); iter != sorted_entries.end();
+   iter++) {
 // Sanitize.
-if (iter->first.IsNull() || iter->first.IsEmpty() || !iter->second)
+if (iter->first.empty() || !iter->second)
   continue;
 
 // Reset original indentation level.
@@ -238,7 +263,7 @@
 s.Indent();
 
 // Print key.
-s.Printf("%s:", iter->first.AsCString());
+s.Format("{0}:", iter->first);
 
 // Return to new line and increase indentation if value is record type.
 // Otherwise add spacing.
@@ -252,7 +277,7 @@
 
 // Print value and new line if now last pair.
 iter->second->GetDescription(s);
-if (std::next(iter) != m_dict.end())
+if (std::next(iter) != sorted_entries.end())
   s.EOL();
 
 // Reset indentation level if it was incremented previously.
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3859,11 +3859,10 @@
   s.Indent("Args:\n");
   s.SetIndentLevel(s.GetIndentLevel() + 4);
 
-  auto print_one_element = [](ConstString key,
+  auto print_one_element = [](llvm::StringRef key,
 StructuredData::Object *object) {
 s.Indent();
-s.Printf("%s : %s\n", key.GetCString(),
-  object->GetStringValue().str().c_str());
+s.Format("{0} : {1}\n", key, object->GetStringValue());
 return true;
   };
 
Index: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
===
--- lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -224,7 +224,7 @@
   [_sp, 

[Lldb-commits] [PATCH] D159310: [lldb] Move template python files to separate directory

2023-08-31 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch moves the template files for the various scripting
affordances to a separate directory.

This is a preparatory work for upcoming improvements and consolidations
to other scripting affordances.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159310

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/examples/python/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_platform.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/examples/python/templates/scripted_platform.py
  lldb/examples/python/templates/scripted_process.py


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -103,15 +103,15 @@
 ${lldb_python_target_dir}
 "plugins"
 FILES
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py"
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_platform.py")
+"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_process.py"
+"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_platform.py")
 
   if(APPLE)
 create_python_package(
   ${swig_target}
   ${lldb_python_target_dir} "macosx"
   FILES "${LLDB_SOURCE_DIR}/examples/python/crashlog.py"
-
"${LLDB_SOURCE_DIR}/examples/python/scripted_process/crashlog_scripted_process.py"
+"${LLDB_SOURCE_DIR}/examples/python/crashlog_scripted_process.py"
 "${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap.py")
 
 create_python_package(


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -103,15 +103,15 @@
 ${lldb_python_target_dir}
 "plugins"
 FILES
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py"
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_platform.py")
+"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_process.py"
+"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_platform.py")
 
   if(APPLE)
 create_python_package(
   ${swig_target}
   ${lldb_python_target_dir} "macosx"
   FILES "${LLDB_SOURCE_DIR}/examples/python/crashlog.py"
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/crashlog_scripted_process.py"
+"${LLDB_SOURCE_DIR}/examples/python/crashlog_scripted_process.py"
 "${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap.py")
 
 create_python_package(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] c9ecaf3 - [lldb] Fix test failure for breakpoint_function_callback.test

2023-08-31 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2023-08-31T22:19:02+01:00
New Revision: c9ecaf32f6aa92f79a054478df55b4b8c3e53697

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

LOG: [lldb] Fix test failure for breakpoint_function_callback.test

This should fix the test failure in breakpoint_function_callback.test
since SBStructuredData can now display the content of SBStructuredData.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

Removed: 




diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test 
b/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
index 125913f7e9bfce..4527b68ef41c09 100644
--- a/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
+++ b/lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -12,7 +12,7 @@ r
 # CHECK: nil
 breakpoint command add -s lua -F abc -k foo -v 123pizza!
 r
-# CHECK: 
+# CHECK: foo: 123pizza!
 # CHECK: 123pizza!
 breakpoint command add -s lua -o "abc(frame, bp_loc, ...)"
 r



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


[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

2023-08-31 Thread Ted Woodward via Phabricator via lldb-commits
ted updated this revision to Diff 555155.
ted marked an inline comment as done.
ted added a comment.

Implement Jason Molenda's request to use an ivar instead of static variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101

Files:
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
  lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h
  lldb/source/Plugins/ABI/RISCV/CMakeLists.txt
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp

Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1550,6 +1550,8 @@
 ArchSpec::eRISCV_float_abi_quad)
   features_str += "+f,+d,+q,";
 // FIXME: how do we detect features such as `+a`, `+m`?
+// Turn them on by default now, since everyone seems to use them
+features_str += "+a,+m,";
   }
 
   // We use m_disasm_up.get() to tell whether we are valid or not, so if this
Index: lldb/source/Plugins/ABI/RISCV/CMakeLists.txt
===
--- /dev/null
+++ lldb/source/Plugins/ABI/RISCV/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_library(lldbPluginABIRISCV PLUGIN
+  ABISysV_riscv.cpp
+
+  LINK_LIBS
+lldbCore
+lldbSymbol
+lldbTarget
+lldbPluginProcessUtility
+  LINK_COMPONENTS
+Support
+TargetParser
+  )
Index: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h
===
--- /dev/null
+++ lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h
@@ -0,0 +1,130 @@
+//===-- ABISysV_riscv.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_ABISysV_riscv_h_
+#define liblldb_ABISysV_riscv_h_
+
+// Other libraries and framework includes
+#include 
+
+#include "llvm/TargetParser/Triple.h"
+
+// Project includes
+#include "lldb/Target/ABI.h"
+#include "lldb/Target/Process.h"
+#include "lldb/lldb-private.h"
+
+class ABISysV_riscv : public lldb_private::RegInfoBasedABI {
+public:
+  ~ABISysV_riscv() override = default;
+
+  size_t GetRedZoneSize() const override { return 0; }
+
+  bool PrepareTrivialCall(lldb_private::Thread , lldb::addr_t sp,
+  lldb::addr_t functionAddress,
+  lldb::addr_t returnAddress,
+  llvm::ArrayRef args) const override;
+
+  // Special thread plan for GDB style non-jit function calls.
+  bool
+  PrepareTrivialCall(lldb_private::Thread , lldb::addr_t sp,
+ lldb::addr_t functionAddress, lldb::addr_t returnAddress,
+ llvm::Type ,
+ llvm::ArrayRef args) const override;
+
+  bool GetArgumentValues(lldb_private::Thread ,
+ lldb_private::ValueList ) const override;
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP _sp,
+   lldb::ValueObjectSP _value) override;
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread ,
+   lldb_private::CompilerType ) const override;
+
+  // Specialized to work with llvm IR types.
+  lldb::ValueObjectSP GetReturnValueObjectImpl(lldb_private::Thread ,
+   llvm::Type ) const override;
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan _plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan _plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+// The CFA must be 128 bit aligned, unless the E ABI is used
+lldb_private::ArchSpec arch = GetProcessSP()->GetTarget().GetArchitecture();
+uint32_t arch_flags = arch.GetFlags();
+if (arch_flags & lldb_private::ArchSpec::eRISCV_rve)
+  return (cfa & 0x3ull) == 0;
+return (cfa & 0xfull) == 0;
+  }
+
+  void SetIsRV64(bool is_rv64) { m_is_rv64 = is_rv64; }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+// Calls can use the least significant bit to store auxiliary information,
+// so no strict check is done for alignment.
+
+lldb_private::ArchSpec arch = GetProcessSP()->GetTarget().GetArchitecture();
+
+//  & 2 set is a fault if C extension is not used.
+uint32_t arch_flags = arch.GetFlags();
+if (!(arch_flags & lldb_private::ArchSpec::eRISCV_rvc) && (pc & 2))
+  return false;
+
+// Make sure 64 bit addr_t only has 

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

2023-08-31 Thread Owen Pan via Phabricator via lldb-commits
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`




Comment at: clang/lib/Format/TokenAnnotator.cpp:4899-4902
+  // Note that this allows the "{" to go over the column limit
   // when the column limit is just between ":" and "{", but that does
   // not happen too often and alternative formattings in this case are
   // not much better.

We should reflow the comment:
```
  // Note that this allows the "{" to go over the column limit when the
  // column limit is just between ":" and "{", but that does not happen too
  // often and alternative formattings in this case are not much better.
```



Comment at: clang/lib/Format/WhitespaceManager.h:202-203
 
-// Determine if every row in the the array
+// Determine if every row in the array
 // has the same number of columns.
 bool isRectangular() const {

We should merge the comment into a single line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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


[Lldb-commits] [lldb] 6813ef3 - Re-land "[lldb/docs] Silence warnings when generating website"

2023-08-31 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2023-08-31T20:35:10+01:00
New Revision: 6813ef37377e8d8fadf6efe01e1ed80cc53b9c86

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

LOG: Re-land "[lldb/docs] Silence warnings when generating website"

This patch re-lands f0731d5b61ba with more fixes and improvements.

First, this patch removes `__eq__` implementations from classes that
didn't implemented `operator!=` on the C++ implementation.

This patch removes sphinx document generation for special members such
as `__len__`, since there is no straightforward way to skip class that
don't implement them. We also don't want to introduce a change in
behavior by implementing artifical special members for classes that are
missing them.

Finally, this patch improve the ergonomics of some classes by
implementing special members where it makes sense, i.e. `hex(SBFrame)`
is equivalent to `SBFrame.GetPC()`.

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/bindings/interface/SBBreakpointListExtensions.i
lldb/bindings/interface/SBBroadcastExtensions.i
lldb/bindings/interface/SBFileSpecListExtensions.i
lldb/bindings/interface/SBMemoryRegionInfoListExtensions.i
lldb/bindings/interface/SBModuleSpecListExtensions.i
lldb/bindings/interface/SBQueueItemExtensions.i
lldb/bindings/interface/SBStructuredDataExtensions.i
lldb/bindings/interface/SBThreadCollectionExtensions.i

Modified: 
lldb/bindings/interface/SBAddressExtensions.i
lldb/bindings/interface/SBBreakpointLocationExtensions.i
lldb/bindings/interface/SBBreakpointNameExtensions.i
lldb/bindings/interface/SBCompileUnitExtensions.i
lldb/bindings/interface/SBDataExtensions.i
lldb/bindings/interface/SBDeclarationExtensions.i
lldb/bindings/interface/SBErrorExtensions.i
lldb/bindings/interface/SBFileSpecExtensions.i
lldb/bindings/interface/SBFrameExtensions.i
lldb/bindings/interface/SBFunctionExtensions.i
lldb/bindings/interface/SBInstructionExtensions.i
lldb/bindings/interface/SBLineEntryExtensions.i
lldb/bindings/interface/SBMemoryRegionInfoExtensions.i
lldb/bindings/interface/SBModuleExtensions.i
lldb/bindings/interface/SBProcessDocstrings.i
lldb/bindings/interface/SBProcessExtensions.i
lldb/bindings/interface/SBScriptObjectExtensions.i
lldb/bindings/interface/SBSectionExtensions.i
lldb/bindings/interface/SBStreamExtensions.i
lldb/bindings/interface/SBSymbolExtensions.i
lldb/bindings/interface/SBTargetExtensions.i
lldb/bindings/interface/SBThreadExtensions.i
lldb/bindings/interface/SBTypeCategoryExtensions.i
lldb/bindings/interface/SBTypeEnumMemberExtensions.i
lldb/bindings/interface/SBTypeExtensions.i
lldb/bindings/interface/SBTypeFilterExtensions.i
lldb/bindings/interface/SBTypeFormatExtensions.i
lldb/bindings/interface/SBTypeNameSpecifierExtensions.i
lldb/bindings/interface/SBTypeSummaryExtensions.i
lldb/bindings/interface/SBTypeSyntheticExtensions.i
lldb/bindings/interface/SBUnixSignalsExtensions.i
lldb/bindings/interface/SBValueExtensions.i
lldb/bindings/interface/SBWatchpointExtensions.i
lldb/bindings/interfaces.swig
lldb/bindings/python/python-extensions.swig
lldb/docs/conf.py
lldb/docs/python_api.rst

Removed: 




diff  --git a/lldb/bindings/interface/SBAddressExtensions.i 
b/lldb/bindings/interface/SBAddressExtensions.i
index 9aeba3ab45aca3..b94ceaddd6afd9 100644
--- a/lldb/bindings/interface/SBAddressExtensions.i
+++ b/lldb/bindings/interface/SBAddressExtensions.i
@@ -2,9 +2,9 @@ STRING_EXTENSION_OUTSIDE(SBAddress)
 
 %extend lldb::SBAddress {
 #ifdef SWIGPYTHON
-// operator== is a free function, which swig does not handle, so we inject
-// our own equality operator here
 %pythoncode%{
+# operator== is a free function, which swig does not handle, so we inject
+# our own equality operator here
 def __eq__(self, other):
   return not self.__ne__(other)
 %}

diff  --git a/lldb/bindings/interface/SBBreakpointListExtensions.i 
b/lldb/bindings/interface/SBBreakpointListExtensions.i
new file mode 100644
index 00..adde0c678f49f5
--- /dev/null
+++ b/lldb/bindings/interface/SBBreakpointListExtensions.i
@@ -0,0 +1,13 @@
+%extend lldb::SBBreakpointList {
+#ifdef SWIGPYTHON
+%pythoncode%{
+def __len__(self):
+'''Return the number of breakpoints in a lldb.SBBreakpointList 
object.'''
+return self.GetSize()
+
+def __iter__(self):
+'''Iterate over all breakpoints in a lldb.SBBreakpointList object.'''
+return lldb_iter(self, 'GetSize', 'GetBreakpointAtIndex')
+%}
+#endif
+}

diff  --git a/lldb/bindings/interface/SBBreakpointLocationExtensions.i 

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

2023-08-31 Thread Ted Woodward via Phabricator via lldb-commits
ted added a comment.

In D159101#4627705 , @DavidSpickett 
wrote:

> Great. Could you include that in the commit message? Seems like we get a lot 
> of questions about how mature risc-v support is, so we can point to this as a 
> milestone for that.

Will do.




Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:107
+static size_t word_size = 4U;
+static size_t reg_size = word_size;
+

jasonmolenda wrote:
> ted wrote:
> > DavidSpickett wrote:
> > > What's the reason to do this this way? It seems like adding an extra 
> > > argument to the constructor of ABISysV_riscv  would do the same thing 
> > > unless someone is calling the constructor directly but that's what 
> > > CreateInstance tries to prevent.
> > > 
> > > I see that mips has 2 ABI plugins, and MacOS on x86 has i386 and x86_64. 
> > > The former I don't think has massive differences between 32 and 64 but 
> > > the latter does.
> > > 
> > > So I agree with sharing the ABI here between riscv32 and riscv64 just 
> > > with the way it's implemented.
> > > 
> > > If it's because you use an array later, either make it a vector or better 
> > > an llvm::SmallVector which for rv32 will likely just use stack space for 
> > > 4 byte stuff.
> > I copied this from the ARC ABI plugin.
> > 
> > word_size and reg_size are used in computations, in PrepareTrivialCall and 
> > SetReturnValueObject.
> I'd prefer that the CreateInstance method initialize an ivar with the word 
> size based on the passed-in ArchSpec, or save the ArchSpec in an ivar.  
> risc-v seems to have many variants so I worry about saving the full ArchSpec 
> when ABI::CreateInstance is being called, possibly where we might not have 
> the most specific ArchSpec for this process?  But 32-bit v. 64-bit will 
> surely be constant.  When I was reading the patch later I'd see references to 
> `reg_size` and scratch my head where that variable was coming from.
> 
> This static will not work correctly if you have an lldb connected to an rv32 
> target and an rv64 target simultaneously.
> 
> Didn't the earlier ABI have a `isRV64` ivar (should have been m_is_rv64 but 
> whatever) to solve this?
This is a very good point - debugging both rv32 and rv64 programs at the same 
time could give incorrect results. I think setting an ivar with 32-bit vs 
64-bit in CreateInstance, and setting local variables based on that is a good 
idea.



Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:638
+bool ABISysV_riscv::CreateDefaultUnwindPlan(UnwindPlan _plan) {
+  return false;
+}

jasonmolenda wrote:
> We were emailing a couple of months ago and I think I suggested something 
> like this, based on the previous ABI patch
> 
> ```
> bool ABISysV_riscv::CreateDefaultUnwindPlan(UnwindPlan _plan) {
>   unwind_plan.Clear();
>   unwind_plan.SetRegisterKind(eRegisterKindGeneric);
> 
>   uint32_t pc_reg_num = LLDB_REGNUM_GENERIC_PC;
>   uint32_t fp_reg_num = LLDB_REGNUM_GENERIC_FP;
> 
>   UnwindPlan::RowSP row(new UnwindPlan::Row);
> 
>   // Define the CFA as the current frame pointer value.
>   row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 0);
>   row->SetOffset(0);
> 
>   int ptr_size = 4;
>   if (isRV64)
> ptr_size = 8;
> 
>   // Assume the ra reg (return pc) and caller's frame pointer 
>   // have been spilled to stack already.
>   row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
>   row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
> 
>   unwind_plan.AppendRow(row);
>   unwind_plan.SetSourceName("riscv default unwind plan");
>   unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
>   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
>   return true;
> }
> ```
The RISC-V ABI says the frame pointer is optional, and I haven't found a way to 
tell if the FP (register s0) is being used as an FP, or if it's a generic 
callee-saved register. 

Debugging an rv32 program that doesn't use the FP works with the current 
CreateDefaultUnwindPlan. I can step in, step over, step out and get a 
backtrace. Is there anything else I need to test?



Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h:79-80
+uint32_t arch_flags = arch.GetFlags();
+if (!(arch_flags & lldb_private::ArchSpec::eRISCV_rvc))
+  if (pc & 2)
+return false;

jasonmolenda wrote:
> DavidSpickett wrote:
> > Combine this into one if.
> Maybe more clear like
> ```
> if (arch_flags | ArchSpec::eRISCV_rvc && pc & 2) 
>   return false;
> ```
> 
> It's too bad ArchSpec doesn't have a `Flags()` method which returns a Flags 
> object, so this could be  `if (arch.Flags().Test(ArchSpec::eRISCV_rvc) && pc 
> & 2)`.
> 
> Suppose it would be just as easy to do 
> ```
>   Flags arch_flags(arch.GetFlags();
>   if (arch_flags.Test(ArchSpec::eRISCV_rvc) && pc & 2)
> return false;
> ```
I like this idea.

Adding a Flags() 

[Lldb-commits] [PATCH] D159237: [lldb][NFCI] Remove unneeded ConstString conversions

2023-08-31 Thread Alex Langford 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 rG14d95b26aee0: [lldb][NFCI] Remove unneeded ConstString 
conversions (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159237

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -76,7 +76,7 @@
 class ThreadOptionValueProperties
 : public Cloneable {
 public:
-  ThreadOptionValueProperties(ConstString name) : Cloneable(name) {}
+  ThreadOptionValueProperties(llvm::StringRef name) : Cloneable(name) {}
 
   const Property *
   GetPropertyAtIndex(size_t idx,
@@ -100,8 +100,7 @@
 
 ThreadProperties::ThreadProperties(bool is_global) : Properties() {
   if (is_global) {
-m_collection_sp =
-std::make_shared(ConstString("thread"));
+m_collection_sp = std::make_shared("thread");
 m_collection_sp->Initialize(g_thread_properties);
   } else
 m_collection_sp =
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4062,7 +4062,7 @@
 class TargetOptionValueProperties
 : public Cloneable {
 public:
-  TargetOptionValueProperties(ConstString name) : Cloneable(name) {}
+  TargetOptionValueProperties(llvm::StringRef name) : Cloneable(name) {}
 
   const Property *
   GetPropertyAtIndex(size_t idx,
@@ -4098,7 +4098,7 @@
OptionValueProperties> {
 public:
   TargetExperimentalOptionValueProperties()
-  : Cloneable(ConstString(Properties::GetExperimentalSettingsName())) {}
+  : Cloneable(Properties::GetExperimentalSettingsName()) {}
 };
 
 TargetExperimentalProperties::TargetExperimentalProperties()
@@ -4152,8 +4152,7 @@
 "errors if the setting is not present.",
 true, m_experimental_properties_up->GetValueProperties());
   } else {
-m_collection_sp =
-std::make_shared(ConstString("target"));
+m_collection_sp = std::make_shared("target");
 m_collection_sp->Initialize(g_target_properties);
 m_experimental_properties_up =
 std::make_unique();
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -89,7 +89,7 @@
 class ProcessOptionValueProperties
 : public Cloneable {
 public:
-  ProcessOptionValueProperties(ConstString name) : Cloneable(name) {}
+  ProcessOptionValueProperties(llvm::StringRef name) : Cloneable(name) {}
 
   const Property *
   GetPropertyAtIndex(size_t idx,
@@ -146,8 +146,7 @@
OptionValueProperties> {
 public:
   ProcessExperimentalOptionValueProperties()
-  : Cloneable(
-ConstString(Properties::GetExperimentalSettingsName())) {}
+  : Cloneable(Properties::GetExperimentalSettingsName()) {}
 };
 
 ProcessExperimentalProperties::ProcessExperimentalProperties()
@@ -162,8 +161,7 @@
 {
   if (process == nullptr) {
 // Global process properties, set them up one time
-m_collection_sp =
-std::make_shared(ConstString("process"));
+m_collection_sp = std::make_shared("process");
 m_collection_sp->Initialize(g_process_properties);
 m_collection_sp->AppendProperty(
 "thread", "Settings specific to threads.", true,
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -72,8 +72,8 @@
 
 } // namespace
 
-ConstString PlatformProperties::GetSettingName() {
-  static ConstString g_setting_name("platform");
+llvm::StringRef PlatformProperties::GetSettingName() 

[Lldb-commits] [lldb] 14d95b2 - [lldb][NFCI] Remove unneeded ConstString conversions

2023-08-31 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-31T11:27:59-07:00
New Revision: 14d95b26aee0ac0ac8a70252e8a3c7a986e0e812

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

LOG: [lldb][NFCI] Remove unneeded ConstString conversions

ConstString can be implicitly converted into a llvm::StringRef. This is
very useful in many places, but it also hides places where we are
creating a ConstString only to use it as a StringRef for the entire
lifespan of the ConstString object.

I locally removed the implicit conversion and found some of the places we
were doing this.

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

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Core/ModuleList.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Interpreter/CommandInterpreter.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/Thread.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 583e9a2e5a4c92..08a58c80ef8477 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -44,7 +44,7 @@ class PlatformProperties : public Properties {
 public:
   PlatformProperties();
 
-  static ConstString GetSettingName();
+  static llvm::StringRef GetSettingName();
 
   bool GetUseModuleCache() const;
   bool SetUseModuleCache(bool use_module_cache);

diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 17d4560f724b84..9842f56b1fe4b5 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -75,8 +75,7 @@ enum {
 } // namespace
 
 ModuleListProperties::ModuleListProperties() {
-  m_collection_sp =
-  std::make_shared(ConstString("symbols"));
+  m_collection_sp = std::make_shared("symbols");
   m_collection_sp->Initialize(g_modulelist_properties);
   m_collection_sp->SetValueChangedCallback(ePropertySymLinkPaths,
[this] { UpdateSymlinkMappings(); 
});

diff  --git a/lldb/source/Core/ValueObjectSyntheticFilter.cpp 
b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
index b91acbc053593d..59ed780b654f3a 100644
--- a/lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -311,7 +311,7 @@ 
ValueObjectSynthetic::GetChildMemberWithName(llvm::StringRef name,
  bool can_create) {
   UpdateValueIfNeeded();
 
-  uint32_t index = GetIndexOfChildWithName(ConstString(name));
+  uint32_t index = GetIndexOfChildWithName(name);
 
   if (index == UINT32_MAX)
 return lldb::ValueObjectSP();

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 1a8b691090ba2d..6d1ad799f2d10f 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -128,8 +128,8 @@ CommandInterpreter::CommandInterpreter(Debugger ,
bool synchronous_execution)
 : Broadcaster(debugger.GetBroadcasterManager(),
   CommandInterpreter::GetStaticBroadcasterClass().AsCString()),
-  Properties(OptionValuePropertiesSP(
-  new OptionValueProperties(ConstString("interpreter",
+  Properties(
+  OptionValuePropertiesSP(new OptionValueProperties("interpreter"))),
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
   m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),

diff  --git 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
index a4d56ef9f967a8..378b2472278605 100644
--- 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ 

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-31 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

We shouldn't need to pass in "bool use_color" to the Disassembler creation 
functions as the only reason to pass something to the creation function for any 
plug-in is if that data would exclude a disassembler if it didn't support 
color. But we always want to see some disassembly rather than none if a plug-in 
is the only one that handles a certain architecture and doesn't support color. 
We should either add accessors to enable colorization to the Disassembler 
virtual plug-in interface _or_ add "bool use_color" to the needed functions. 
Looks DisassembleBytes added a "bool use_color" argument to the call already, 
so we shouldn't need a "bool use_color" for the plug-in selection. Are there 
other places that need to know about the "use_color" setting that do not 
results from a direct function call that can have an argument added to it?

Even if colorization is enabled, It would still expect any APIs that return 
information in individual instructions, like:

  const char *SBInstruction::GetMnemonic(lldb::SBTarget target);
  const char *SBInstruction::GetOperands(lldb::SBTarget target);
  const char *SBInstruction::GetComment(lldb::SBTarget target);

To not have colorization attached to the returned string.




Comment at: lldb/include/lldb/Core/Disassembler.h:409
+  static lldb::DisassemblerSP FindPlugin(const ArchSpec ,
+ const char *flavor, bool use_color,
+ const char *plugin_name);

We shouldn't need to pass "use_color" in when finding the disassembler plug-in. 
We should add an accessor so the Disassembler virtual function list like:

```
bool Disassembler::SetUseColor(bool enable);
```
And the plug-in can return true if it supports colorization and false if not. 
The selection of the disassembler plug-in shouldn't be predicated on color 
support right?



Comment at: lldb/include/lldb/Core/Disassembler.h:416
   const char *flavor,
+  bool use_color,
   const char *plugin_name);

remove and use accessor suggested above?



Comment at: lldb/include/lldb/lldb-private-interfaces.h:33
+   const char *flavor,
+   bool use_color);
 typedef DynamicLoader *(*DynamicLoaderCreateInstance)(Process *process,

remove per above comments



Comment at: lldb/source/Commands/CommandObjectDisassemble.cpp:456-457
 
-  DisassemblerSP disassembler =
-  Disassembler::FindPlugin(m_options.arch, flavor_string, plugin_name);
+  DisassemblerSP disassembler = Disassembler::FindPlugin(
+  m_options.arch, flavor_string, GetDebugger().GetUseColor(), plugin_name);
 

revert



Comment at: lldb/source/Core/Disassembler.cpp:59
 DisassemblerSP Disassembler::FindPlugin(const ArchSpec ,
-const char *flavor,
+const char *flavor, bool use_color,
 const char *plugin_name) {

revert


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

https://reviews.llvm.org/D159164

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


[Lldb-commits] [PATCH] D159150: [lldb][NFCI] Replace bespoke iterator check with std::next

2023-08-31 Thread Alex Langford 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 rG9ed72d4d4d50: [lldb][NFCI] Replace bespoke iterator check 
with std::next (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159150

Files:
  lldb/source/Utility/StructuredData.cpp


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -228,9 +228,9 @@
 
 void StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const 
{
   size_t indentation_level = s.GetIndentLevel();
-  for (const auto  : m_dict) {
+  for (auto iter = m_dict.begin(); iter != m_dict.end(); iter++) {
 // Sanitize.
-if (pair.first.IsNull() || pair.first.IsEmpty() || !pair.second)
+if (iter->first.IsNull() || iter->first.IsEmpty() || !iter->second)
   continue;
 
 // Reset original indentation level.
@@ -238,11 +238,11 @@
 s.Indent();
 
 // Print key.
-s.Printf("%s:", pair.first.AsCString());
+s.Printf("%s:", iter->first.AsCString());
 
 // Return to new line and increase indentation if value is record type.
 // Otherwise add spacing.
-bool should_indent = IsRecordType(pair.second);
+bool should_indent = IsRecordType(iter->second);
 if (should_indent) {
   s.EOL();
   s.IndentMore();
@@ -251,8 +251,8 @@
 }
 
 // Print value and new line if now last pair.
-pair.second->GetDescription(s);
-if (pair != *(--m_dict.end()))
+iter->second->GetDescription(s);
+if (std::next(iter) != m_dict.end())
   s.EOL();
 
 // Reset indentation level if it was incremented previously.


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -228,9 +228,9 @@
 
 void StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const {
   size_t indentation_level = s.GetIndentLevel();
-  for (const auto  : m_dict) {
+  for (auto iter = m_dict.begin(); iter != m_dict.end(); iter++) {
 // Sanitize.
-if (pair.first.IsNull() || pair.first.IsEmpty() || !pair.second)
+if (iter->first.IsNull() || iter->first.IsEmpty() || !iter->second)
   continue;
 
 // Reset original indentation level.
@@ -238,11 +238,11 @@
 s.Indent();
 
 // Print key.
-s.Printf("%s:", pair.first.AsCString());
+s.Printf("%s:", iter->first.AsCString());
 
 // Return to new line and increase indentation if value is record type.
 // Otherwise add spacing.
-bool should_indent = IsRecordType(pair.second);
+bool should_indent = IsRecordType(iter->second);
 if (should_indent) {
   s.EOL();
   s.IndentMore();
@@ -251,8 +251,8 @@
 }
 
 // Print value and new line if now last pair.
-pair.second->GetDescription(s);
-if (pair != *(--m_dict.end()))
+iter->second->GetDescription(s);
+if (std::next(iter) != m_dict.end())
   s.EOL();
 
 // Reset indentation level if it was incremented previously.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 9ed72d4 - [lldb][NFCI] Replace bespoke iterator check with std::next

2023-08-31 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-31T10:55:00-07:00
New Revision: 9ed72d4d4d507b60a54f6b74e86433f837ded93c

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

LOG: [lldb][NFCI] Replace bespoke iterator check with std::next

The primary goal of this change is to change `if (pair != *(--m_dict.end()))`
to `if (std::next(iter) != m_dict.end())`. I was experimenting with
changing the underlying type of `m_dict` and found that this was an
issue. Specifically, it assumes that m_dict iterators are bidirectional.
This change should make it so we only need to assume m_dict iterators can move
forward.

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

Added: 


Modified: 
lldb/source/Utility/StructuredData.cpp

Removed: 




diff  --git a/lldb/source/Utility/StructuredData.cpp 
b/lldb/source/Utility/StructuredData.cpp
index c0ed1e5a5c73e4..c870a0eb82b27c 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -228,9 +228,9 @@ void 
StructuredData::Array::GetDescription(lldb_private::Stream ) const {
 
 void StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const 
{
   size_t indentation_level = s.GetIndentLevel();
-  for (const auto  : m_dict) {
+  for (auto iter = m_dict.begin(); iter != m_dict.end(); iter++) {
 // Sanitize.
-if (pair.first.IsNull() || pair.first.IsEmpty() || !pair.second)
+if (iter->first.IsNull() || iter->first.IsEmpty() || !iter->second)
   continue;
 
 // Reset original indentation level.
@@ -238,11 +238,11 @@ void 
StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const {
 s.Indent();
 
 // Print key.
-s.Printf("%s:", pair.first.AsCString());
+s.Printf("%s:", iter->first.AsCString());
 
 // Return to new line and increase indentation if value is record type.
 // Otherwise add spacing.
-bool should_indent = IsRecordType(pair.second);
+bool should_indent = IsRecordType(iter->second);
 if (should_indent) {
   s.EOL();
   s.IndentMore();
@@ -251,8 +251,8 @@ void 
StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const {
 }
 
 // Print value and new line if now last pair.
-pair.second->GetDescription(s);
-if (pair != *(--m_dict.end()))
+iter->second->GetDescription(s);
+if (std::next(iter) != m_dict.end())
   s.EOL();
 
 // Reset indentation level if it was incremented previously.



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


[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

2023-08-31 Thread Greg Clayton via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG07c215e8a8af: Fix shared library loading when users define 
duplicate _r_debug structure. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158583

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile
  
lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py
  lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
  lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
  lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp

Index: lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp
@@ -0,0 +1,32 @@
+#include "library_file.h"
+#include 
+#include 
+// Make a duplicate "_r_debug" symbol that is visible. This is the global
+// variable name that the dynamic loader uses to communicate changes in shared
+// libraries that get loaded and unloaded. LLDB finds the address of this
+// variable by reading the DT_DEBUG entry from the .dynamic section of the main
+// executable.
+// What will happen is the dynamic loader will use the "_r_debug" symbol from
+// itself until the a.out executable gets loaded. At this point the new
+// "_r_debug" symbol will take precedence over the orignal "_r_debug" symbol
+// from the dynamic loader and the copy below will get updated with shared
+// library state changes while the version that LLDB checks in the dynamic
+// loader stays the same for ever after this.
+//
+// When our DYLDRendezvous.cpp tries to check the state in the _r_debug
+// structure, it will continue to get the last eAdd as the state before the
+// switch in symbol resolution.
+//
+// Before a fix in LLDB, this would mean that we wouldn't ever load any shared
+// libraries since DYLDRendezvous was waiting to see a eAdd state followed by a
+// eConsistent state which would trigger the adding of shared libraries, but we
+// would never see this change because the local copy below is actually what
+// would get updated. Now if DYLDRendezvous detects two eAdd states in a row,
+// it will load the shared libraries instead of doing nothing and a log message
+// will be printed out if "log enable lldb dyld" is active.
+r_debug _r_debug;
+
+int main() {
+  library_function(); // Break here
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
@@ -0,0 +1 @@
+int library_function();
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
@@ -0,0 +1,7 @@
+#include "library_file.h"
+#include 
+
+int library_function(void) {
+  puts(__FUNCTION__); // Library break here
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py
@@ -0,0 +1,39 @@
+"""
+Test that LLDB can launch a linux executable through the dynamic loader where
+the main executable has an extra exported "_r_debug" symbol that used to mess
+up shared library loading with DYLDRendezvous and the POSIX dynamic loader
+plug-in. What used to happen is that any shared libraries other than the main
+executable and the dynamic loader and VSDO would not get loaded. This test
+checks to make sure that we still load libraries correctly when we have
+multiple "_r_debug" symbols. See comments in the main.cpp source file for full
+details on what the problem is.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestDyldWithMultipleRDebug(TestBase):
+@skipIf(oslist=no_match(["linux"]))
+@no_debug_info_test
+def test(self):
+self.build()
+# Run to a breakpoint in main.cpp to ensure we can hit breakpoints
+# in the main executable. Setting breakpoints by file and line ensures
+# that the main executable was loaded correctly by the dynamic loader
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// Break here", lldb.SBFileSpec("main.cpp"),
+extra_images=["testlib"]
+)
+# Set breakpoints both on shared library function to ensure that
+# 

[Lldb-commits] [lldb] 07c215e - Fix shared library loading when users define duplicate _r_debug structure.

2023-08-31 Thread Greg Clayton via lldb-commits

Author: Greg Clayton
Date: 2023-08-31T10:37:20-07:00
New Revision: 07c215e8a8af54d0084af7291ac29fef3672fcd8

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

LOG: Fix shared library loading when users define duplicate _r_debug structure.

We ran into a case where shared libraries would fail to load in some processes 
on linux. The issue turned out to be if the main executable or a shared library 
defined a symbol named "_r_debug", then it would cause problems once the 
executable that contained it was loaded into the process. The "_r_debug" 
structure is currently found by looking through the .dynamic section in the 
main executable and finding the DT_DEBUG entry which points to this structure. 
The dynamic loader will update this structure as shared libraries are loaded 
and LLDB watches the contents of this structure as the dyld breakpoint is hit. 
Currently we expect the "state" in this structure to change as things happen. 
An issue comes up if someone defines another "_r_debug" struct in their program:
```
r_debug _r_debug;
```
If this code is included, a new "_r_debug" structure is created and it causes 
problems once the executable is loaded. This is because of the way symbol 
lookups happen in linux: they use the shared library list in the order it 
created and the dynamic loader is always last. So at some point the dynamic 
loader will start updating this other copy of "_r_debug", yet LLDB is only 
watching the copy inside of the dynamic loader.

Steps that show the problem are:
- lldb finds the "_r_debug" structure via the DT_DEBUG entry in the .dynamic 
section and this points to the "_r_debug" in ld.so
- ld.so modifies its copy of "_r_debug" with "state = eAdd" before it loads the 
shared libraries and calls the dyld function that LLDB has set a breakpoint on 
and we find this state and do nothing (we are waiting for a state of 
eConsistent to tell us the shared libraries have been fully loaded)
- ld.so loads the main executable and any dependent shared libraries and wants 
to update the "_r_debug" structure, but it now finds "_r_debug" in the a.out 
program and updates the state in this other copy
- lldb hits the notification breakpoint and checks the ld.so copy of "_r_debug" 
which still has a state of "eAdd". LLDB wants the new "eConsistent" state which 
will trigger the shared libraries to load, but it gets stale data and doesn't 
do anyhing and library load is missed. The "_r_debug" in a.out has the state 
set correctly, but we don't know which "_r_debug" is the right one.

The new fix detects the two "eAdd" states and loads shared libraries and will 
emit a log message in the "log enable lldb dyld" log channel which states there 
might be multiple "_r_debug" structs.

The correct solution is that no one should be adding a duplicate "_r_debug" 
symbol to their binaries, but we have programs that are doing this already and 
since it can be done, we should be able to work with this and keep debug 
sessions working as expected. If a user #includes the  file, they can 
just use the existing "_r_debug" structure as it is defined in this header file 
as "extern struct r_debug _r_debug;" and no local copies need to be made.

If your ld.so has debug info, you can easily see the duplicate "_r_debug" 
structs by doing:
```
(lldb) target variable _r_debug --raw
(r_debug) _r_debug = {
  r_version = 1
  r_map = 0x77e30210
  r_brk = 140737349972416
  r_state = RT_CONSISTENT
  r_ldbase = 0
}
(r_debug) _r_debug = {
  r_version = 1
  r_map = 0x77e30210
  r_brk = 140737349972416
  r_state = RT_ADD
  r_ldbase = 140737349943296
}
(lldb) target variable &_r_debug
(r_debug *) &_r_debug = 0x55601040
(r_debug *) &_r_debug = 0x77e301e0
```
And if you do a "image lookup --address " in the addresses, you can see 
one is in the a.out and one in the ld.so.

Adding more logging to print out the m_previous and m_current Rendezvous 
structures to make things more clear. Also added a log when we detect multiple 
eAdd states in a row to detect this problem in logs.

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

Added: 
lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile

lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py
lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp

Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 

[Lldb-commits] [PATCH] D158833: [lldb] Move ScriptInterpreter Interfaces to subdirectory (NFC)

2023-08-31 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

LGTM thank you!


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

https://reviews.llvm.org/D158833

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


[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

2023-08-31 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

> add a way to make an SBTypeSynthetic from the appropriate C++ class

That's a great idea. Once we implement a couple of providers for our plugin 
I'll revisit the idea. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158010

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


[Lldb-commits] [PATCH] D137338: Fix dupe word typos

2023-08-31 Thread Louis Dionne via Phabricator via lldb-commits
ldionne added a comment.
Herald added subscribers: wangpc, gysit, Dinistro, hoy, bviyer, wlei, jplehr, 
PiotrZSL, luke, sunshaoce.
Herald added a reviewer: springerm.
Herald added a reviewer: kiranchandramohan.
Herald added a project: clang-format.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

The libcxx, libcxxabi and libunwind parts were committed in 
b397921fc7ef4e2882b3ae9c5f12fb40daca4078 
. Is it 
possible to close this or rebase it so we can remove the libc++ subscription to 
clear the review queue? We're trying to clean things up as part of the Github 
PR transition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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


[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

2023-08-31 Thread Louis Dionne via Phabricator via lldb-commits
ldionne commandeered this revision.
ldionne added a reviewer: t-tye.
ldionne added a comment.
Herald added a subscriber: jplehr.
Herald added a project: All.

[GitHub PR transition cleanup]

This has been open for 2 years with multiple subscribers able to see this 
discussion, but there doesn't seem to be a lot of interest for moving forward 
with this. I will commandeer and abandon to clear the review queue in the 
context of moving to Github PRs. If you'd still like to push for this to 
happen, feel free to re-open this as a Github PR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106339

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


[Lldb-commits] [lldb] 380c5da - Revert "Re-land "[lldb/docs] Silence warnings when generating website""

2023-08-31 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2023-08-31T16:31:10+01:00
New Revision: 380c5da98efa62920e9fbf3aa9a4fa8add236fb9

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

LOG: Revert "Re-land "[lldb/docs] Silence warnings when generating website""

This reverts 3 commit:
- f0731d5b61ba798e6d5a63a92d9228010e5a3b50.
- 8e0a087571a31057bb98939e3ada73227bed83c7.
- f2f5d6fb8d53bc4bd93a3d4e110134ed017b636f.

This changes were introduced to silence the warnings that are printed
when generating the lldb module documentation for the website but it
changed the python bindings and causes test failures on the macos bot:

https://green.lab.llvm.org/green/job/lldb-cmake/59438/

We will have to consider other options to silence these warnings.

Added: 


Modified: 
lldb/bindings/interface/SBAddressExtensions.i
lldb/bindings/interface/SBBlockExtensions.i
lldb/bindings/interface/SBBreakpointExtensions.i
lldb/bindings/interface/SBBreakpointLocationExtensions.i
lldb/bindings/interface/SBBreakpointNameExtensions.i
lldb/bindings/interface/SBCommandReturnObjectExtensions.i
lldb/bindings/interface/SBCompileUnitExtensions.i
lldb/bindings/interface/SBDataExtensions.i
lldb/bindings/interface/SBDebuggerExtensions.i
lldb/bindings/interface/SBDeclarationExtensions.i
lldb/bindings/interface/SBErrorExtensions.i
lldb/bindings/interface/SBExecutionContextExtensions.i
lldb/bindings/interface/SBFileExtensions.i
lldb/bindings/interface/SBFileSpecExtensions.i
lldb/bindings/interface/SBFrameExtensions.i
lldb/bindings/interface/SBFunctionExtensions.i
lldb/bindings/interface/SBInstructionExtensions.i
lldb/bindings/interface/SBInstructionListExtensions.i
lldb/bindings/interface/SBLineEntryExtensions.i
lldb/bindings/interface/SBMemoryRegionInfoExtensions.i
lldb/bindings/interface/SBModuleExtensions.i
lldb/bindings/interface/SBModuleSpecExtensions.i
lldb/bindings/interface/SBProcessDocstrings.i
lldb/bindings/interface/SBProcessExtensions.i
lldb/bindings/interface/SBScriptObjectExtensions.i
lldb/bindings/interface/SBSectionExtensions.i
lldb/bindings/interface/SBStreamExtensions.i
lldb/bindings/interface/SBStringListExtensions.i
lldb/bindings/interface/SBSymbolContextExtensions.i
lldb/bindings/interface/SBSymbolContextListExtensions.i
lldb/bindings/interface/SBSymbolExtensions.i
lldb/bindings/interface/SBTargetExtensions.i
lldb/bindings/interface/SBThreadExtensions.i
lldb/bindings/interface/SBTypeCategoryExtensions.i
lldb/bindings/interface/SBTypeEnumMemberExtensions.i
lldb/bindings/interface/SBTypeExtensions.i
lldb/bindings/interface/SBTypeFilterExtensions.i
lldb/bindings/interface/SBTypeFormatExtensions.i
lldb/bindings/interface/SBTypeNameSpecifierExtensions.i
lldb/bindings/interface/SBTypeSummaryExtensions.i
lldb/bindings/interface/SBTypeSyntheticExtensions.i
lldb/bindings/interface/SBUnixSignalsExtensions.i
lldb/bindings/interface/SBValueExtensions.i
lldb/bindings/interface/SBValueListExtensions.i
lldb/bindings/interface/SBWatchpointExtensions.i
lldb/bindings/interfaces.swig
lldb/bindings/python/python-extensions.swig
lldb/bindings/python/python.swig
lldb/docs/conf.py
lldb/docs/python_api.rst

Removed: 
lldb/bindings/interface/SBAttachInfoExtensions.i
lldb/bindings/interface/SBBreakpointListExtensions.i
lldb/bindings/interface/SBBroadcastExtensions.i
lldb/bindings/interface/SBCommandInterpreterExtensions.i
lldb/bindings/interface/SBCommandInterpreterRunOptionsExtensions.i
lldb/bindings/interface/SBCommunicationExtensions.i
lldb/bindings/interface/SBEnvironmentExtensions.i
lldb/bindings/interface/SBEventExtensions.i
lldb/bindings/interface/SBExpressionOptionsExtensions.i
lldb/bindings/interface/SBFileSpecListExtensions.i
lldb/bindings/interface/SBHostOSExtensions.i
lldb/bindings/interface/SBLanguageRuntimeExtensions.i
lldb/bindings/interface/SBLaunchInfoExtensions.i
lldb/bindings/interface/SBListenerExtensions.i
lldb/bindings/interface/SBMemoryRegionInfoListExtensions.i
lldb/bindings/interface/SBModuleSpecListExtensions.i
lldb/bindings/interface/SBPlatformConnectOptionsExtensions.i
lldb/bindings/interface/SBPlatformExtensions.i
lldb/bindings/interface/SBPlatformShellCommandExtensions.i
lldb/bindings/interface/SBProcessInfoExtensions.i
lldb/bindings/interface/SBQueueExtensions.i
lldb/bindings/interface/SBQueueItemExtensions.i
lldb/bindings/interface/SBReproducerExtensions.i
lldb/bindings/interface/SBSourceManagerExtensions.i
lldb/bindings/interface/SBStructuredDataExtensions.i
lldb/bindings/interface/SBThreadCollectionExtensions.i
lldb/bindings/interface/SBThreadPlanExtensions.i

[Lldb-commits] [PATCH] D159150: [lldb][NFCI] Replace bespoke iterator check with std::next

2023-08-31 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve accepted this revision.
fdeazeve added a comment.

Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159150

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


[Lldb-commits] [PATCH] D157883: [lldb][AArch64] Add SME's Array Storage (ZA) and streaming vector length (SVG) registers

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 554993.
DavidSpickett added a comment.
Herald added a subscriber: sunshaoce.

As for the SVE test, the sync variables don't need to be atomic. Each one has 
one writer and one reader only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157883

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/LinuxPTraceDefines_arm64sve.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp
  
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_za_reg/za_dynamic_resize/Makefile
  
lldb/test/API/commands/register/register/aarch64_za_reg/za_dynamic_resize/TestZAThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_za_reg/za_dynamic_resize/main.c
  
lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/Makefile
  
lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/TestZARegisterSaveRestore.py
  lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/main.c

Index: lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/main.c
@@ -0,0 +1,225 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Important details for this program:
+// * Making a syscall will disable streaming mode if it is active.
+// * Changing the vector length will make streaming mode and ZA inactive.
+// * ZA can be active independent of streaming mode.
+// * ZA's size is the streaming vector length squared.
+
+#ifndef PR_SME_SET_VL
+#define PR_SME_SET_VL 63
+#endif
+
+#ifndef PR_SME_GET_VL
+#define PR_SME_GET_VL 64
+#endif
+
+#ifndef PR_SME_VL_LEN_MASK
+#define PR_SME_VL_LEN_MASK 0x
+#endif
+
+#define SM_INST(c) asm volatile("msr s0_3_c4_c" #c "_3, xzr")
+#define SMSTART SM_INST(7)
+#define SMSTART_SM SM_INST(3)
+#define SMSTART_ZA SM_INST(5)
+#define SMSTOP SM_INST(6)
+#define SMSTOP_SM SM_INST(2)
+#define SMSTOP_ZA SM_INST(4)
+
+int start_vl = 0;
+int other_vl = 0;
+
+void write_sve_regs() {
+  // We assume the smefa64 feature is present, which allows ffr access
+  // in streaming mode.
+  asm volatile("setffr\n\t");
+  asm volatile("ptrue p0.b\n\t");
+  asm volatile("ptrue p1.h\n\t");
+  asm volatile("ptrue p2.s\n\t");
+  asm volatile("ptrue p3.d\n\t");
+  asm volatile("pfalse p4.b\n\t");
+  asm volatile("ptrue p5.b\n\t");
+  asm volatile("ptrue p6.h\n\t");
+  asm volatile("ptrue p7.s\n\t");
+  asm volatile("ptrue p8.d\n\t");
+  asm volatile("pfalse p9.b\n\t");
+  asm volatile("ptrue p10.b\n\t");
+  asm volatile("ptrue p11.h\n\t");
+  asm volatile("ptrue p12.s\n\t");
+  asm volatile("ptrue p13.d\n\t");
+  asm volatile("pfalse p14.b\n\t");
+  asm volatile("ptrue p15.b\n\t");
+
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+  asm volatile("cpy  z1.b, p5/z, #2\n\t");
+  asm volatile("cpy  z2.b, p10/z, #3\n\t");
+  asm volatile("cpy  z3.b, p15/z, #4\n\t");
+  asm volatile("cpy  z4.b, p0/z, #5\n\t");
+  asm volatile("cpy  z5.b, p5/z, #6\n\t");
+  asm volatile("cpy  z6.b, p10/z, #7\n\t");
+  asm volatile("cpy  z7.b, p15/z, #8\n\t");
+  asm volatile("cpy  z8.b, p0/z, #9\n\t");
+  asm volatile("cpy  z9.b, p5/z, #10\n\t");
+  asm volatile("cpy  z10.b, p10/z, #11\n\t");
+  asm volatile("cpy  z11.b, p15/z, #12\n\t");
+  asm volatile("cpy  z12.b, p0/z, #13\n\t");
+  asm volatile("cpy  z13.b, p5/z, #14\n\t");
+  asm volatile("cpy  z14.b, p10/z, #15\n\t");
+  asm volatile("cpy  z15.b, p15/z, #16\n\t");
+  asm volatile("cpy  z16.b, p0/z, #17\n\t");
+  asm volatile("cpy  z17.b, p5/z, #18\n\t");
+  asm volatile("cpy  z18.b, p10/z, #19\n\t");
+  asm volatile("cpy  z19.b, p15/z, #20\n\t");
+  asm volatile("cpy  z20.b, p0/z, #21\n\t");
+  asm volatile("cpy  z21.b, p5/z, #22\n\t");
+  asm volatile("cpy  z22.b, p10/z, #23\n\t");
+  asm volatile("cpy  z23.b, p15/z, #24\n\t");
+  asm volatile("cpy  z24.b, p0/z, #25\n\t");
+  asm volatile("cpy  z25.b, p5/z, #26\n\t");
+  asm volatile("cpy  z26.b, 

[Lldb-commits] [PATCH] D158833: [lldb] Move ScriptInterpreter Interfaces to subdirectory (NFC)

2023-08-31 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 554991.
mib marked an inline comment as done.
mib added a comment.

Address @bulbazord comments.


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

https://reviews.llvm.org/D158833

Files:
  lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
  lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
  lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
  lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedPlatformInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -140,6 +140,7 @@
 
   StructuredData::DictionarySP
   OSPlugin_RegisterInfo(StructuredData::ObjectSP os_plugin_object_sp) override;
+  lldb::ScriptedThreadInterfaceSP CreateScriptedThreadInterface() override;
 
   StructuredData::ArraySP
   OSPlugin_ThreadsInfo(StructuredData::ObjectSP os_plugin_object_sp) override;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -14,12 +14,13 @@
 // LLDB Python header must be included first
 #include "lldb-python.h"
 
+#include "Interfaces/ScriptedPlatformPythonInterface.h"
+#include "Interfaces/ScriptedProcessPythonInterface.h"
+#include "Interfaces/ScriptedThreadPythonInterface.h"
 #include "PythonDataObjects.h"
 #include "PythonReadline.h"
 #include "SWIGPythonBridge.h"
 #include "ScriptInterpreterPythonImpl.h"
-#include "ScriptedPlatformPythonInterface.h"
-#include "ScriptedProcessPythonInterface.h"
 
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFrame.h"
@@ -1515,6 +1516,11 @@
   return std::make_unique(*this);
 }
 
+ScriptedThreadInterfaceSP
+ScriptInterpreterPythonImpl::CreateScriptedThreadInterface() {
+  return std::make_shared(*this);
+}
+
 StructuredData::ObjectSP
 ScriptInterpreterPythonImpl::CreateStructuredDataFromScriptObject(
 ScriptObject obj) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
@@ -6,15 +6,15 @@
 //
 //===--===//
 
-#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
-#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_INTERFACES_SCRIPTEDTHREADPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_INTERFACES_SCRIPTEDTHREADPYTHONINTERFACE_H
 
 #include 

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22044f0bde01: [lldb][AArch64] Add testing of save/restore 
for Linux MTE control register (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157488

Files:
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
  
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,14 @@
+#include 
+
+// This is its own function so that lldb can call it.
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | 
PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = line_number("main.c", "// Set a break point here.")
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1
+)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"process status",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."],
+)
+
+# Bit 0 = tagged addressing enabled
+# Bit 1 = synchronous faults
+# Bit 2 = asynchronous faults
+# We start enabled with synchronous faults.
+self.expect("register read mte_ctrl", substrs=["0x0003"])
+
+# Change to asynchronous faults.
+self.runCmd("register write mte_ctrl 5")
+self.expect("register read mte_ctrl", substrs=["0x0005"])
+
+# This would return to synchronous faults if we did not restore the
+# previous value.
+self.expect("expression setup_mte()", substrs=["= 0"])
+self.expect("register read mte_ctrl", substrs=["0x0005"])
\ No newline at end of file
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,14 @@
+#include 
+
+// This is its own function so that lldb can call it.
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = 

[Lldb-commits] [lldb] 22044f0 - [lldb][AArch64] Add testing of save/restore for Linux MTE control register

2023-08-31 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2023-08-31T12:25:08+01:00
New Revision: 22044f0bde015f4bf53fca24831d030ff96efc51

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

LOG: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

This has always worked but had no coverage. Adding testing now so that
later I can refactor the save/restore code safely.

Reviewed By: omjavaid

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

Added: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile

lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c

Modified: 


Removed: 




diff  --git 
a/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile 
b/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
new file mode 100644
index 00..10495940055b63
--- /dev/null
+++ 
b/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
 
b/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
new file mode 100644
index 00..48e8bb872e3f02
--- /dev/null
+++ 
b/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = line_number("main.c", "// Set a break point here.")
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1
+)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"process status",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."],
+)
+
+# Bit 0 = tagged addressing enabled
+# Bit 1 = synchronous faults
+# Bit 2 = asynchronous faults
+# We start enabled with synchronous faults.
+self.expect("register read mte_ctrl", substrs=["0x0003"])
+
+# Change to asynchronous faults.
+self.runCmd("register write mte_ctrl 5")
+self.expect("register read mte_ctrl", substrs=["0x0005"])
+
+# This would return to synchronous faults if we did not restore the
+# previous value.
+self.expect("expression setup_mte()", substrs=["= 0"])
+self.expect("register read mte_ctrl", substrs=["0x0005"])
\ No newline at end of file

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c 
b/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
new file mode 100644
index 00..788093fd05038a
--- /dev/null
+++ b/lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,14 @@
+#include 
+
+// This is its own function so that lldb can call it.
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | 
PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}



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


[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
DavidSpickett marked an inline comment as done.
Closed by commit rG6697afe99fb6: [lldb][AArch64] Check SIMD save/restore in SVE 
SIMD test (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157000

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c


Index: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -1,10 +1,11 @@
 #include 
 #include 
 
-void write_simd_regs() {
+// base is added to each value. If base = 2, then v0 = 2, v1 = 3, etc.
+void write_simd_regs(unsigned base) {
 #define WRITE_SIMD(NUM)
\
   asm volatile("MOV v" #NUM ".d[0], %0\n\t"
\
-   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(base + NUM))
 
   WRITE_SIMD(0);
   WRITE_SIMD(1);
@@ -102,7 +103,7 @@
 #endif
   // else test plain SIMD access.
 
-  write_simd_regs();
+  write_simd_regs(0);
 
   return verify_simd_regs(); // Set a break point here.
 }
Index: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -1,6 +1,6 @@
 """
-Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
-streaming SVE and normal SIMD modes.
+Test that LLDB correctly reads and writes and restores AArch64 SIMD registers
+in SVE, streaming SVE and normal SIMD modes.
 
 There are a few operating modes and we use different strategies for each:
 * Without SVE, in SIMD mode - read the SIMD regset.
@@ -48,6 +48,13 @@
 pad = " ".join(["0x00"] * 7)
 return "{{0x{:02x} {} 0x{:02x} {}}}".format(n, pad, n, pad)
 
+def check_simd_values(self, value_offset):
+# These are 128 bit registers, so getting them from the API as unsigned
+# values doesn't work. Check the command output instead.
+for i in range(32):
+self.expect("register read v{}".format(i),
+substrs=[self.make_simd_value(i+value_offset)])
+
 def sve_simd_registers_impl(self, mode):
 self.skip_if_needed(mode)
 
@@ -68,12 +75,9 @@
 substrs=["stop reason = breakpoint 1."],
 )
 
-# These are 128 bit registers, so getting them from the API as unsigned
-# values doesn't work. Check the command output instead.
-for i in range(32):
-self.expect(
-"register read v{}".format(i), 
substrs=[self.make_simd_value(i)]
-)
+self.check_simd_values(0)
+self.runCmd("expression write_simd_regs(1)")
+self.check_simd_values(0)
 
 # Write a new set of values. The kernel will move the program back to
 # non-streaming mode here.
@@ -83,10 +87,7 @@
 )
 
 # Should be visible within lldb.
-for i in range(32):
-self.expect(
-"register read v{}".format(i), substrs=[self.make_simd_value(i 
+ 1)]
-)
+self.check_simd_values(1)
 
 # The program should agree with lldb.
 self.expect("continue", substrs=["exited with status = 0"])


Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -1,10 +1,11 @@
 #include 
 #include 
 
-void write_simd_regs() {
+// base is added to each value. If base = 2, then v0 = 2, v1 = 3, etc.
+void write_simd_regs(unsigned base) {
 #define WRITE_SIMD(NUM)\
   asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
-   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(base + NUM))
 
   WRITE_SIMD(0);
   WRITE_SIMD(1);
@@ -102,7 +103,7 @@
 #endif
   // else test plain SIMD access.
 
-  write_simd_regs();
+  write_simd_regs(0);
 
   return verify_simd_regs(); // Set a break point here.
 }
Index: 

[Lldb-commits] [lldb] 6697afe - [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

2023-08-31 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2023-08-31T10:21:53+01:00
New Revision: 6697afe99fb6df08d4c1903eb1df5446f2ec1993

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

LOG: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

While doing some refactoring I forgot to carry over the copying in of
SIMD data in normal mode, but no tests failed.

Turns out, it's very easy for us to get the restore wrong because
even if you forget the memcopy, setting the buffer to valid may
just read the data you had before the expression evaluation.

So I've extended the SVE SIMD testing (which includes the plain SIMD mode)
to check expression save/restore. This is the only test that fails
if you forget to do `m_fpu_is_valid = true` so I take from that, that
prior to this it wasn't tested at all.

As a bonus, we now have coverage of the same thing for SVE and SSVE modes.

Reviewed By: omjavaid

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

Added: 


Modified: 

lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c

Removed: 




diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
 
b/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
index 7b1fa8f9be41f7..c38a255aac6e29 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
+++ 
b/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -1,6 +1,6 @@
 """
-Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
-streaming SVE and normal SIMD modes.
+Test that LLDB correctly reads and writes and restores AArch64 SIMD registers
+in SVE, streaming SVE and normal SIMD modes.
 
 There are a few operating modes and we use 
diff erent strategies for each:
 * Without SVE, in SIMD mode - read the SIMD regset.
@@ -48,6 +48,13 @@ def make_simd_value(self, n):
 pad = " ".join(["0x00"] * 7)
 return "{{0x{:02x} {} 0x{:02x} {}}}".format(n, pad, n, pad)
 
+def check_simd_values(self, value_offset):
+# These are 128 bit registers, so getting them from the API as unsigned
+# values doesn't work. Check the command output instead.
+for i in range(32):
+self.expect("register read v{}".format(i),
+substrs=[self.make_simd_value(i+value_offset)])
+
 def sve_simd_registers_impl(self, mode):
 self.skip_if_needed(mode)
 
@@ -68,12 +75,9 @@ def sve_simd_registers_impl(self, mode):
 substrs=["stop reason = breakpoint 1."],
 )
 
-# These are 128 bit registers, so getting them from the API as unsigned
-# values doesn't work. Check the command output instead.
-for i in range(32):
-self.expect(
-"register read v{}".format(i), 
substrs=[self.make_simd_value(i)]
-)
+self.check_simd_values(0)
+self.runCmd("expression write_simd_regs(1)")
+self.check_simd_values(0)
 
 # Write a new set of values. The kernel will move the program back to
 # non-streaming mode here.
@@ -83,10 +87,7 @@ def sve_simd_registers_impl(self, mode):
 )
 
 # Should be visible within lldb.
-for i in range(32):
-self.expect(
-"register read v{}".format(i), substrs=[self.make_simd_value(i 
+ 1)]
-)
+self.check_simd_values(1)
 
 # The program should agree with lldb.
 self.expect("continue", substrs=["exited with status = 0"])

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c 
b/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
index 954d84039fb313..2156f094cab5c5 100644
--- a/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
+++ b/lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -1,10 +1,11 @@
 #include 
 #include 
 
-void write_simd_regs() {
+// base is added to each value. If base = 2, then v0 = 2, v1 = 3, etc.
+void write_simd_regs(unsigned base) {
 #define WRITE_SIMD(NUM)
\
   asm volatile("MOV v" #NUM ".d[0], %0\n\t"
\
-   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(base + NUM))
 
   WRITE_SIMD(0);
   WRITE_SIMD(1);
@@ -102,7 +103,7 @@ int main() {
 #endif
   // else test plain SIMD access.
 
-  write_simd_regs();
+  write_simd_regs(0);
 
   return verify_simd_regs(); // Set a break point here.
 }




[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

> The only area of concern could be a LLDB running on a resource constrained 
> container but we can ignore that for now.

Ironically we're the ones who are most likely to see that issue :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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


[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

2023-08-31 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D156687#4628743 , @DavidSpickett 
wrote:

> First thing to note is that WriteRegister also behaves this way, but there it 
> is more appropriate because it updates only part of the buffer before writing 
> it out in its entirety. Useful to know where the pattern came from though.
>
> You would need roughly the following per `WriteXYZ`:
>
>   -  error = WriteTLS();
>   +  error = WriteTLS(src);
>   
>   -Status NativeRegisterContextLinux_arm64::WriteTLS() {
>   +Status NativeRegisterContextLinux_arm64::WriteTLS(const void* 
> src/*=nullptr*/) {
>   
>   -  ioVec.iov_base = GetTLSBuffer();
>   +  ioVec.iov_base = src ? const_cast(src) : GetTLSBuffer();
>   
>   -  Status WriteTLS();
>   +  Status WriteTLS(const void* src=nullptr);
>
> We can assume that the buffer is the same as the data to be written back if 
> it's something static like TLS. For SVE/SME, we would have resized our buffer 
> first, so it holds there too.
>
> The added complexity isn't that much but I think it adds more thinking time 
> for a developer than it potentially saves in copying time. I already feel 
> like the separate `xyz_is_valid` is enough to think about and having a 
> potential second data source just adds to that load.
>
> From the header docs it seems I was right that this is used primarily for 
> expression evaluation:
>
>   // These two functions are used to implement "push" and "pop" of register
>   // states.  They are used primarily for expression evaluation, where we need
>   // to push a new state (storing the old one in data_sp) and then restoring
>   // the original state by passing the data_sp we got from ReadAllRegisters to
>   // WriteAllRegisterValues.
>
> Which you would be doing a lot of in a formatter for example, but you'd get 
> better savings implementing a more efficient packet format to do all that at 
> once, I guess.
>
> QSaveRegisterState / QRestoreRegisterState packets call it as part of 
> expression evaluation, though in theory it's not always that. That's an lldb 
> extension anyway so we're in control of it at least. In theory this could be 
> used to restore state that is not just the previous state but I don't know 
> how you'd trigger that from lldb.
>
> The other use is `NativeProcessLinux::Syscall` which is sufficiently rare we 
> can ignore that.
>
> I did do a very rough benchmark where I printed the same expression 2000 
> times, so each one is doing a save/restore. Once with the code in this review 
> right now, and again with this potential optimisation added to GPR/FPR/TLS 
> (I'm on a Mountain Jade machine without SVE). Caveat shared machine, made up 
> benchmark, etc. but all runs of both hovered between 16 and 17 seconds. 
> Neither seemed to be consistently lower or higher than the other. Doesn't 
> mean this isn't a speedup in isolation but if it is, it's dwarfed by the 
> syscalls and packets sent back and forth.

I mostly agree with what you have above. I was only thinking about ever 
increasing size of this buffer and thought if we can find a way around 
duplication. Most probably we ll never have SVE/SME enabled on a slow/small 
machine to bother us. The only area of concern could be a LLDB running on a 
resource constrained container but we can ignore that for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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


[Lldb-commits] [PATCH] D157967: [lldb][AArch64] Use atomics to sync threads in SVE threading test

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

The only way to know this truly works is to observe it on the bot for a while 
and I'm already piling too many reviews on Omair as it is. So I've gone ahead 
and landed this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157967

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


[Lldb-commits] [PATCH] D157967: [lldb][AArch64] Use atomics to sync threads in SVE threading test

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0407f681a7ef: [lldb][AArch64] Use atomics to sync threads in 
SVE threading test (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157967

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c


Index: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 
 #ifndef PR_SME_SET_VL
@@ -62,7 +63,18 @@
 
 int SET_VL_OPT = PR_SVE_SET_VL;
 
+// These ensure that when lldb stops in one of threadX / threadY, the other has
+// at least been created. That means we can continue the other onto the 
expected
+// breakpoint. Otherwise we could get to the breakpoint in one thread before 
the
+// other has started.
+volatile bool threadX_ready = false;
+volatile bool threadY_ready = false;
+
 void *threadX_func(void *x_arg) {
+  threadX_ready = true;
+  while (!threadY_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 4);
 #ifdef USE_SSVE
   SMSTART();
@@ -73,6 +85,10 @@
 }
 
 void *threadY_func(void *y_arg) {
+  threadY_ready = true;
+  while (!threadX_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 2);
 #ifdef USE_SSVE
   SMSTART();
Index: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -148,10 +148,6 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
-# If we start the checks too quickly, thread 3 may not have started.
-while process.GetNumThreads() < 3:
-pass
-
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:


Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 
 #ifndef PR_SME_SET_VL
@@ -62,7 +63,18 @@
 
 int SET_VL_OPT = PR_SVE_SET_VL;
 
+// These ensure that when lldb stops in one of threadX / threadY, the other has
+// at least been created. That means we can continue the other onto the expected
+// breakpoint. Otherwise we could get to the breakpoint in one thread before the
+// other has started.
+volatile bool threadX_ready = false;
+volatile bool threadY_ready = false;
+
 void *threadX_func(void *x_arg) {
+  threadX_ready = true;
+  while (!threadY_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 4);
 #ifdef USE_SSVE
   SMSTART();
@@ -73,6 +85,10 @@
 }
 
 void *threadY_func(void *y_arg) {
+  threadY_ready = true;
+  while (!threadX_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 2);
 #ifdef USE_SSVE
   SMSTART();
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -148,10 +148,6 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
-# If we start the checks too quickly, thread 3 may not have started.
-while process.GetNumThreads() < 3:
-pass
-
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 0407f68 - [lldb][AArch64] Use atomics to sync threads in SVE threading test

2023-08-31 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2023-08-31T10:06:16+01:00
New Revision: 0407f681a7efa61f81f885e186c25ed99aecb8d4

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

LOG: [lldb][AArch64] Use atomics to sync threads in SVE threading test

Previously we would "process continue" then wait for the number of
threads to be 3 before proceeding with the test.

Testing this on QEMU I saw it would sometimes get stuck at this check,
with one of the threads on a breakpoint before the other had started.
We do want it to be on a breakpoint, but we need the other thread to have
at least started so lldb can interact with both.

I've also seen it timeout on the Graviton buildbot, likely the same
cause.

To fix this add 2 variables to stall either thread until the other
has started up. Then it doesn't matter which one hits its breakpoint
first, the test will just continue the one that didn't, until both
are on the expected breakpoint.

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

Added: 


Modified: 

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c

Removed: 




diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
index 3ca20a1803b0fc..ecac3712674976 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -148,10 +148,6 @@ def run_sve_test(self, mode):
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
-# If we start the checks too quickly, thread 3 may not have started.
-while process.GetNumThreads() < 3:
-pass
-
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
index 419d2ddaa725fa..0bb6b3b57046f8 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
+++ 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 
 #ifndef PR_SME_SET_VL
@@ -62,7 +63,18 @@ static inline void write_sve_registers() {
 
 int SET_VL_OPT = PR_SVE_SET_VL;
 
+// These ensure that when lldb stops in one of threadX / threadY, the other has
+// at least been created. That means we can continue the other onto the 
expected
+// breakpoint. Otherwise we could get to the breakpoint in one thread before 
the
+// other has started.
+volatile bool threadX_ready = false;
+volatile bool threadY_ready = false;
+
 void *threadX_func(void *x_arg) {
+  threadX_ready = true;
+  while (!threadY_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 4);
 #ifdef USE_SSVE
   SMSTART();
@@ -73,6 +85,10 @@ void *threadX_func(void *x_arg) {
 }
 
 void *threadY_func(void *y_arg) {
+  threadY_ready = true;
+  while (!threadX_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 2);
 #ifdef USE_SSVE
   SMSTART();



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


[Lldb-commits] [PATCH] D157967: [lldb][AArch64] Use atomics to sync threads in SVE threading test

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 554938.
DavidSpickett added a comment.

Actually these don't need to be atomic. They aren't writing to the same 
locations
we just have one writer and one reader, and if the reader is delayed a bit it's
not a problem.

The volatile isn't needed at -O0 but might as well on the off chance something 
tries
to optimise it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157967

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c


Index: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 
 #ifndef PR_SME_SET_VL
@@ -62,7 +63,18 @@
 
 int SET_VL_OPT = PR_SVE_SET_VL;
 
+// These ensure that when lldb stops in one of threadX / threadY, the other has
+// at least been created. That means we can continue the other onto the 
expected
+// breakpoint. Otherwise we could get to the breakpoint in one thread before 
the
+// other has started.
+volatile bool threadX_ready = false;
+volatile bool threadY_ready = false;
+
 void *threadX_func(void *x_arg) {
+  threadX_ready = true;
+  while (!threadY_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 4);
 #ifdef USE_SSVE
   SMSTART();
@@ -73,6 +85,10 @@
 }
 
 void *threadY_func(void *y_arg) {
+  threadY_ready = true;
+  while (!threadX_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 2);
 #ifdef USE_SSVE
   SMSTART();
Index: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -148,10 +148,6 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
-# If we start the checks too quickly, thread 3 may not have started.
-while process.GetNumThreads() < 3:
-pass
-
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:


Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -1,4 +1,5 @@
 #include 
+#include 
 #include 
 
 #ifndef PR_SME_SET_VL
@@ -62,7 +63,18 @@
 
 int SET_VL_OPT = PR_SVE_SET_VL;
 
+// These ensure that when lldb stops in one of threadX / threadY, the other has
+// at least been created. That means we can continue the other onto the expected
+// breakpoint. Otherwise we could get to the breakpoint in one thread before the
+// other has started.
+volatile bool threadX_ready = false;
+volatile bool threadY_ready = false;
+
 void *threadX_func(void *x_arg) {
+  threadX_ready = true;
+  while (!threadY_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 4);
 #ifdef USE_SSVE
   SMSTART();
@@ -73,6 +85,10 @@
 }
 
 void *threadY_func(void *y_arg) {
+  threadY_ready = true;
+  while (!threadX_ready) {
+  }
+
   prctl(SET_VL_OPT, 8 * 2);
 #ifdef USE_SSVE
   SMSTART();
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -148,10 +148,6 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
-# If we start the checks too quickly, thread 3 may not have started.
-while process.GetNumThreads() < 3:
-pass
-
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

2023-08-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

I understand the mechanism now, so for lack of any other strong opinions, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158583

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